Re: [PATCHv3 1/9] gdb: unify parts of the Linux and FreeBSD core dumping code

[Lancelot SIX <lsix@lancelotsix.com>]

Le Mon, Feb 15, 2021 at 05:29:04PM +0000, Andrew Burgess a écrit :
> While reviewing the Linux and FreeBSD core dumping code within GDB for
> another patch series, I noticed that the code that collects the
> registers for each thread and writes these into ELF note format is
> basically identical between Linux and FreeBSD.
> 
> This commit merges this code and moves it into a new file gcore-elf.c.
> 
> The function find_signalled_thread is moved from linux-tdep.c to
> gcore.c despite not being shared.  A later commit will make use of
> this function.
> 
> I did merge, and then revert a previous version of this patch (commit
> 82a1fd3a4935 for the original patch and 03642b7189bc for the revert).
> The problem with the original patch is that it introduced a
> unconditional dependency between GDB and some ELF specific functions
> in the BFD library, e.g. elfcore_write_prstatus and
> elfcore_write_register_note.  It was pointed out in this mailing list
> post:
> 
>   https://sourceware.org/pipermail/gdb-patches/2021-February/175750.html
> 
> that this change was breaking any build of GDB for non-ELF targets.
> To confirm this breakage, and to test this new version of GDB I
> configured and built for the target x86_64-apple-darwin20.3.0.
> 
> Where the previous version of this patch placed all of the common code
> into gcore.c, which is included in all builds of GDB, this new patch
> only places non-ELF specific generic code (i.e. find_signalled_thread)
> into gcore.c, the ELF specific code is put into the new gcore-elf.c
> file, which is only included in GDB if BFD has ELF support.
> 
> The contents of gcore-elf.c are referenced unconditionally from
> linux-tdep.c and fbsd-tdep.c, this is fine, we previously always
> assumed that these two targets required ELF support, and we continue
> to make that assumption after this patch; nothing has changed there.
> 
> With my previous version of this patch the darwin target mentioned
> above failed to build, but with the new version, the target builds
> fine.
> 
> There are a couple of minor changes to the FreeBSD target after this
> commit, but I believe that these are changes for the better:
> 
> (1) For FreeBSD we always used to record the thread-id in the core
> file by using ptid_t.lwp ().  In contrast the Linux code did this:
> 
>     /* For remote targets the LWP may not be available, so use the TID.  */
>     long lwp = ptid.lwp ();
>     if (lwp == 0)
>       lwp = ptid.tid ();
> 
> Both target now do this:
> 
>     /* The LWP is often not available for bare metal target, in which case
>        use the tid instead.  */
>     if (ptid.lwp_p ())
>       lwp = ptid.lwp ();
>     else
>       lwp = ptid.tid ();
> 
> Which is equivalent for Linux, but is a change for FreeBSD.  I think
> that all this means is that in some cases where GDB might have
> previously recorded a thread-id of 0 for each thread, we might now get
> something more useful.
> 
> (2) When collecting the registers for Linux we collected into a zero
> initialised buffer.  By contrast on FreeBSD the buffer is left
> uninitialised.  In the new code the buffer is always zero initialised.
> I suspect once the registers are copied into the buffer there's
> probably no gaps left so this makes no difference, but if it does then
> using zeros rather than random bits of GDB's memory is probably a good
> thing.
> 
> Otherwise, there should be no other user visible changes after this
> commit.
> 
> Tested this on x86-64/GNU-Linux and x86-64/FreeBSD-12.2 with no
> regressions.
> 
> gdb/ChangeLog:
> 
> 	* Makefile.in (SFILES): Add gcore-elf.c.
> 	(HFILES_NO_SRCDIR): Add gcore-elf.h
> 	* configure: Regenerate.
> 	* configure.ac: Add gcore-elf.o to CONFIG_OBS if we have ELF
> 	support.
> 	* fbsd-tdep.c: Add 'gcore-elf.h' include.
> 	(struct fbsd_collect_regset_section_cb_data): Delete.
> 	(fbsd_collect_regset_section_cb): Delete.
> 	(fbsd_collect_thread_registers): Delete.
> 	(struct fbsd_corefile_thread_data): Delete.
> 	(fbsd_corefile_thread): Delete.
> 	(fbsd_make_corefile_notes): Call
> 	gcore_elf_build_thread_register_notes instead of the now deleted
> 	FreeBSD code.
> 	* gcore-elf.c: New file, the content was moved here from
> 	linux-tdep.c, functions were renamed and given minor cleanup.
> 	* gcore-elf.h: New file.
> 	* gcore.c (gcore_find_signalled_thread): Moved here from
> 	linux-tdep.c and given a new name.  Minor cleanups.
> 	* gcore.h (gcore_find_signalled_thread): Declare.
> 	* linux-tdep.c: Add 'gcore.h' and 'gcore-elf.h' includes.
> 	(struct linux_collect_regset_section_cb_data): Delete.
> 	(linux_collect_regset_section_cb): Delete.
> 	(linux_collect_thread_registers): Delete.
> 	(linux_corefile_thread): Call
> 	gcore_elf_build_thread_register_notes.
> 	(find_signalled_thread): Delete.
> 	(linux_make_corefile_notes): Call gcore_find_signalled_thread.
> ---
>  gdb/ChangeLog    |  31 ++++++++++
>  gdb/Makefile.in  |   2 +
>  gdb/configure    |   2 +-
>  gdb/configure.ac |   2 +-
>  gdb/fbsd-tdep.c  | 134 ++------------------------------------------
>  gdb/gcore-elf.c  | 136 ++++++++++++++++++++++++++++++++++++++++++++
>  gdb/gcore-elf.h  |  39 +++++++++++++
>  gdb/gcore.c      |  21 +++++++
>  gdb/gcore.h      |   9 +++
>  gdb/linux-tdep.c | 143 +++--------------------------------------------
>  10 files changed, 255 insertions(+), 264 deletions(-)
>  create mode 100644 gdb/gcore-elf.c
>  create mode 100644 gdb/gcore-elf.h
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index ae89b85eb56..cf5017e7f66 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1191,6 +1191,7 @@ SFILES = \
>  	dtrace-probe.c \
>  	elfread.c \
>  	f-exp.y \
> +	gcore-elf.c \
>  	gdb.c \
>  	go-exp.y \
>  	m2-exp.y \
> @@ -1292,6 +1293,7 @@ HFILES_NO_SRCDIR = \
>  	frame-unwind.h \
>  	frv-tdep.h \
>  	ft32-tdep.h \
> +	gcore-elf.h \
>  	gcore.h \
>  	gdb_bfd.h \
>  	gdb_curses.h \
> diff --git a/gdb/configure b/gdb/configure
> index 51b4d1921c5..4707fd01174 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -17264,7 +17264,7 @@ $as_echo "$gdb_cv_var_elf" >&6; }
>    LDFLAGS=$OLD_LDFLAGS
>    LIBS=$OLD_LIBS
>  if test "$gdb_cv_var_elf" = yes; then
> -  CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o"
> +  CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o gcore-elf.o"
>  
>  $as_echo "#define HAVE_ELF 1" >>confdefs.h
>  
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 618c59166e4..db765af0577 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -1882,7 +1882,7 @@ WIN32LIBS="$WIN32LIBS $WIN32APILIBS"
>  GDB_AC_CHECK_BFD([for ELF support in BFD], gdb_cv_var_elf,
>                   [bfd_get_elf_phdr_upper_bound (NULL)], elf-bfd.h)
>  if test "$gdb_cv_var_elf" = yes; then
> -  CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o"
> +  CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o gcore-elf.o"
>    AC_DEFINE(HAVE_ELF, 1,
>  	    [Define if ELF support should be included.])
>    # -ldl is provided by bfd/Makfile.am (LIBDL) <PLUGINS>.
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index cc51e921ae2..dc4278cd644 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> @@ -32,6 +32,7 @@
>  
>  #include "elf-bfd.h"
>  #include "fbsd-tdep.h"
> +#include "gcore-elf.h"
>  
>  /* This enum is derived from FreeBSD's <sys/signal.h>.  */
>  
> @@ -583,129 +584,6 @@ find_signalled_thread (struct thread_info *info, void *data)
>    return 0;
>  }
>  
> -/* Structure for passing information from
> -   fbsd_collect_thread_registers via an iterator to
> -   fbsd_collect_regset_section_cb. */
> -
> -struct fbsd_collect_regset_section_cb_data
> -{
> -  fbsd_collect_regset_section_cb_data (const struct regcache *regcache,
> -				       bfd *obfd,
> -				       gdb::unique_xmalloc_ptr<char> &note_data,
> -				       int *note_size,
> -				       unsigned long lwp,
> -				       gdb_signal stop_signal)
> -    : regcache (regcache),
> -      obfd (obfd),
> -      note_data (note_data),
> -      note_size (note_size),
> -      lwp (lwp),
> -      stop_signal (stop_signal)
> -  {}
> -
> -  const struct regcache *regcache;
> -  bfd *obfd;
> -  gdb::unique_xmalloc_ptr<char> &note_data;
> -  int *note_size;
> -  unsigned long lwp;
> -  enum gdb_signal stop_signal;
> -  bool abort_iteration = false;
> -};
> -
> -static void
> -fbsd_collect_regset_section_cb (const char *sect_name, int supply_size,
> -				int collect_size, const struct regset *regset,
> -				const char *human_name, void *cb_data)
> -{
> -  char *buf;
> -  struct fbsd_collect_regset_section_cb_data *data
> -    = (struct fbsd_collect_regset_section_cb_data *) cb_data;
> -
> -  if (data->abort_iteration)
> -    return;
> -
> -  gdb_assert (regset->collect_regset);
> -
> -  buf = (char *) xmalloc (collect_size);
> -  regset->collect_regset (regset, data->regcache, -1, buf, collect_size);
> -
> -  /* PRSTATUS still needs to be treated specially.  */
> -  if (strcmp (sect_name, ".reg") == 0)
> -    data->note_data.reset (elfcore_write_prstatus
> -			     (data->obfd, data->note_data.release (),
> -			      data->note_size, data->lwp,
> -			      gdb_signal_to_host (data->stop_signal),
> -			      buf));
> -  else
> -    data->note_data.reset (elfcore_write_register_note
> -			     (data->obfd, data->note_data.release (),
> -			      data->note_size, sect_name, buf,
> -			      collect_size));
> -  xfree (buf);
> -
> -  if (data->note_data == NULL)
> -    data->abort_iteration = true;
> -}
> -
> -/* Records the thread's register state for the corefile note
> -   section.  */
> -
> -static void
> -fbsd_collect_thread_registers (const struct regcache *regcache,
> -			       ptid_t ptid, bfd *obfd,
> -			       gdb::unique_xmalloc_ptr<char> &note_data,
> -			       int *note_size,
> -			       enum gdb_signal stop_signal)
> -{
> -  fbsd_collect_regset_section_cb_data data (regcache, obfd, note_data,
> -					    note_size, ptid.lwp (),
> -					    stop_signal);
> -
> -  gdbarch_iterate_over_regset_sections (regcache->arch (),
> -					fbsd_collect_regset_section_cb,
> -					&data, regcache);
> -}
> -
> -struct fbsd_corefile_thread_data
> -{
> -  fbsd_corefile_thread_data (struct gdbarch *gdbarch,
> -			     bfd *obfd,
> -			     gdb::unique_xmalloc_ptr<char> &note_data,
> -			     int *note_size,
> -			     gdb_signal stop_signal)
> -    : gdbarch (gdbarch),
> -      obfd (obfd),
> -      note_data (note_data),
> -      note_size (note_size),
> -      stop_signal (stop_signal)
> -  {}
> -
> -  struct gdbarch *gdbarch;
> -  bfd *obfd;
> -  gdb::unique_xmalloc_ptr<char> &note_data;
> -  int *note_size;
> -  enum gdb_signal stop_signal;
> -};
> -
> -/* Records the thread's register state for the corefile note
> -   section.  */
> -
> -static void
> -fbsd_corefile_thread (struct thread_info *info,
> -		      struct fbsd_corefile_thread_data *args)
> -{
> -  struct regcache *regcache;
> -
> -  regcache = get_thread_arch_regcache (info->inf->process_target (),
> -				       info->ptid, args->gdbarch);
> -
> -  target_fetch_registers (regcache, -1);
> -
> -  fbsd_collect_thread_registers (regcache, info->ptid, args->obfd,
> -				 args->note_data, args->note_size,
> -				 args->stop_signal);
> -}
> -
>  /* Return a byte_vector containing the contents of a core dump note
>     for the target object of type OBJECT.  If STRUCTSIZE is non-zero,
>     the data is prefixed with a 32-bit integer size to match the format
> @@ -782,16 +660,16 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
>  	signalled_thr = curr_thr;
>      }
>  
> -  fbsd_corefile_thread_data thread_args (gdbarch, obfd, note_data, note_size,
> -					 signalled_thr->suspend.stop_signal);
> -
> -  fbsd_corefile_thread (signalled_thr, &thread_args);
> +  enum gdb_signal stop_signal = signalled_thr->suspend.stop_signal;
> +  gcore_elf_build_thread_register_notes (gdbarch, signalled_thr, stop_signal,
> +					 obfd, &note_data, note_size);
>    for (thread_info *thr : current_inferior ()->non_exited_threads ())
>      {
>        if (thr == signalled_thr)
>  	continue;
>  
> -      fbsd_corefile_thread (thr, &thread_args);
> +      gcore_elf_build_thread_register_notes (gdbarch, thr, stop_signal,
> +					     obfd, &note_data, note_size);
>      }
>  
>    /* Auxiliary vector.  */
> diff --git a/gdb/gcore-elf.c b/gdb/gcore-elf.c
> new file mode 100644
> index 00000000000..ebc94277d35
> --- /dev/null
> +++ b/gdb/gcore-elf.c
> @@ -0,0 +1,136 @@
> +/* Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "gcore-elf.h"
> +#include "elf-bfd.h"
> +#include "target.h"
> +#include "regcache.h"
> +#include "gdbarch.h"
> +#include "gdbthread.h"
> +#include "inferior.h"
> +#include "regset.h"
> +
> +/* Structure for passing information from GCORE_COLLECT_THREAD_REGISTERS
> +   via an iterator to GCORE_COLLECT_REGSET_SECTION_CB. */
> +
> +struct gcore_elf_collect_regset_section_cb_data
> +{
> +  gcore_elf_collect_regset_section_cb_data
> +	(struct gdbarch *gdbarch, const struct regcache *regcache,
> +	 bfd *obfd, ptid_t ptid, gdb_signal stop_signal,
> +	 gdb::unique_xmalloc_ptr<char> *note_data, int *note_size)
> +    : gdbarch (gdbarch), regcache (regcache), obfd (obfd),
> +      note_data (note_data), note_size (note_size),
> +      stop_signal (stop_signal)
> +  {
> +    /* The LWP is often not available for bare metal target, in which case
> +       use the tid instead.  */
> +    if (ptid.lwp_p ())
> +      lwp = ptid.lwp ();
> +    else
> +      lwp = ptid.tid ();
> +  }
> +
> +  struct gdbarch *gdbarch;
> +  const struct regcache *regcache;
> +  bfd *obfd;
> +  gdb::unique_xmalloc_ptr<char> *note_data;
> +  int *note_size;
> +  unsigned long lwp;
> +  enum gdb_signal stop_signal;
> +  bool abort_iteration = false;
> +};
> +
> +/* Callback for ITERATE_OVER_REGSET_SECTIONS that records a single
> +   regset in the core file note section.  */
> +
> +static void
> +gcore_elf_collect_regset_section_cb (const char *sect_name,
> +				     int supply_size, int collect_size,
> +				     const struct regset *regset,
> +				     const char *human_name, void *cb_data)
> +{
> +  struct gcore_elf_collect_regset_section_cb_data *data
> +    = (struct gcore_elf_collect_regset_section_cb_data *) cb_data;
> +  bool variable_size_section = (regset != NULL
> +				&& regset->flags & REGSET_VARIABLE_SIZE);
> +

Hi,

Maybe 'regset != nullptr'?

> +  gdb_assert (variable_size_section || supply_size == collect_size);
> +
> +  if (data->abort_iteration)
> +    return;
> +
> +  gdb_assert (regset != nullptr && regset->collect_regset != nullptr);
> +
> +  /* This is intentionally zero-initialized by using std::vector, so
> +     that any padding bytes in the core file will show as 0.  */
> +  std::vector<gdb_byte> buf (collect_size);
> +
> +  regset->collect_regset (regset, data->regcache, -1, buf.data (),
> +			  collect_size);
> +
> +  /* PRSTATUS still needs to be treated specially.  */
> +  if (strcmp (sect_name, ".reg") == 0)
> +    data->note_data->reset (elfcore_write_prstatus
> +			    (data->obfd, data->note_data->release (),
> +			     data->note_size, data->lwp,
> +			     gdb_signal_to_host (data->stop_signal),
> +			     buf.data ()));
> +  else
> +    data->note_data->reset (elfcore_write_register_note
> +			    (data->obfd, data->note_data->release (),
> +			     data->note_size, sect_name, buf.data (),
> +			     collect_size));
> +
> +  if (data->note_data == nullptr)
> +    data->abort_iteration = true;

I think you want '*data->note_data == nullptr' here.

data->note_data cannot be null (otherwise one of the
data->note_data->reset would have caused problem). It is the value
within the unique_ptr you are interested in.

Looks like this comes from a refactoring issue. In the original code,
note_data was a ref instead of a pointer (in both fbsd and linux side).
May ask why you changed it to a pointer? Is that to handle that param
similarly to other which are referred to by pointer?

Lancelot.

> +}
> +
> +/* Records the register state of thread PTID out of REGCACHE into the note
> +   buffer represented by *NOTE_DATA and NOTE_SIZE.  OBFD is the bfd into
> +   which the core file is being created, and STOP_SIGNAL is the signal that
> +   cause thread PTID to stop.  */
> +
> +static void
> +gcore_elf_collect_thread_registers
> +	(const struct regcache *regcache, ptid_t ptid, bfd *obfd,
> +	 gdb::unique_xmalloc_ptr<char> *note_data, int *note_size,
> +	 enum gdb_signal stop_signal)
> +{
> +  struct gdbarch *gdbarch = regcache->arch ();
> +  gcore_elf_collect_regset_section_cb_data data (gdbarch, regcache, obfd,
> +						 ptid, stop_signal,
> +						 note_data, note_size);
> +  gdbarch_iterate_over_regset_sections
> +    (gdbarch, gcore_elf_collect_regset_section_cb, &data, regcache);
> +}
> +
> +/* See gcore-elf.h.  */
> +
> +void
> +gcore_elf_build_thread_register_notes
> +  (struct gdbarch *gdbarch, struct thread_info *info, gdb_signal stop_signal,
> +   bfd *obfd, gdb::unique_xmalloc_ptr<char> *note_data, int *note_size)
> +{
> +  struct regcache *regcache
> +    = get_thread_arch_regcache (info->inf->process_target (),
> +				info->ptid, gdbarch);
> +  target_fetch_registers (regcache, -1);
> +  gcore_elf_collect_thread_registers (regcache, info->ptid, obfd,
> +				      note_data, note_size, stop_signal);
> +}
> diff --git a/gdb/gcore-elf.h b/gdb/gcore-elf.h
> new file mode 100644
> index 00000000000..d667686adc7
> --- /dev/null
> +++ b/gdb/gcore-elf.h
> @@ -0,0 +1,39 @@
> +/* Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +/* This file contains generic functions for writing ELF based core files.  */
> +
> +#if !defined (GCORE_ELF_H)
> +#define GCORE_ELF_H 1
> +
> +#include "gdb_bfd.h"
> +#include "gdbsupport/gdb_signals.h"
> +#include "gcore.h"
> +
> +struct gdbarch;
> +struct thread_info;
> +
> +/* Add content to *NOTE_DATA (and update *NOTE_SIZE) to describe the
> +   registers of thread INFO.  Report the thread as having stopped with
> +   STOP_SIGNAL.  The core file is being written to OBFD, and GDBARCH is the
> +   architecture for which the core file is being generated.  */
> +
> +extern void gcore_elf_build_thread_register_notes
> +  (struct gdbarch *gdbarch, struct thread_info *info, gdb_signal stop_signal,
> +   bfd *obfd, gdb::unique_xmalloc_ptr<char> *note_data, int *note_size);
> +
> +#endif /* GCORE_ELF_H */
> diff --git a/gdb/gcore.c b/gdb/gcore.c
> index 73ac6b09c70..3b9025322f3 100644
> --- a/gdb/gcore.c
> +++ b/gdb/gcore.c
> @@ -579,6 +579,27 @@ gcore_memory_sections (bfd *obfd)
>    return 1;
>  }
>  
> +/* See gcore.h.  */
> +
> +thread_info *
> +gcore_find_signalled_thread ()
> +{
> +  thread_info *curr_thr = inferior_thread ();
> +  if (curr_thr->state != THREAD_EXITED
> +      && curr_thr->suspend.stop_signal != GDB_SIGNAL_0)
> +    return curr_thr;
> +
> +  for (thread_info *thr : current_inferior ()->non_exited_threads ())
> +    if (thr->suspend.stop_signal != GDB_SIGNAL_0)
> +      return thr;
> +
> +  /* Default to the current thread, unless it has exited.  */
> +  if (curr_thr->state != THREAD_EXITED)
> +    return curr_thr;
> +
> +  return nullptr;
> +}
> +
>  void _initialize_gcore ();
>  void
>  _initialize_gcore ()
> diff --git a/gdb/gcore.h b/gdb/gcore.h
> index af37ff39b41..7e8e316926b 100644
> --- a/gdb/gcore.h
> +++ b/gdb/gcore.h
> @@ -22,10 +22,19 @@
>  
>  #include "gdb_bfd.h"
>  
> +struct thread_info;
> +
>  extern gdb_bfd_ref_ptr create_gcore_bfd (const char *filename);
>  extern void write_gcore_file (bfd *obfd);
>  extern int objfile_find_memory_regions (struct target_ops *self,
>  					find_memory_region_ftype func,
>  					void *obfd);
>  
> +/* Find the signalled thread.  In case there's more than one signalled
> +   thread, prefer the current thread, if it is signalled.  If no thread was
> +   signalled, default to the current thread, unless it has exited, in which
> +   case return NULL.  */
> +
> +extern thread_info *gcore_find_signalled_thread ();
> +
>  #endif /* GCORE_H */
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index e9f8e1b6133..5bfd82d1673 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -39,6 +39,8 @@
>  #include "gdb_regex.h"
>  #include "gdbsupport/enum-flags.h"
>  #include "gdbsupport/gdb_optional.h"
> +#include "gcore.h"
> +#include "gcore-elf.h"
>  
>  #include <ctype.h>
>  
> @@ -1597,104 +1599,6 @@ linux_make_mappings_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
>      }
>  }
>  
> -/* Structure for passing information from
> -   linux_collect_thread_registers via an iterator to
> -   linux_collect_regset_section_cb. */
> -
> -struct linux_collect_regset_section_cb_data
> -{
> -  linux_collect_regset_section_cb_data (struct gdbarch *gdbarch,
> -					const struct regcache *regcache,
> -					bfd *obfd,
> -					gdb::unique_xmalloc_ptr<char> &note_data,
> -					int *note_size,
> -					unsigned long lwp,
> -					gdb_signal stop_signal)
> -    : gdbarch (gdbarch), regcache (regcache), obfd (obfd),
> -      note_data (note_data), note_size (note_size), lwp (lwp),
> -      stop_signal (stop_signal)
> -  {}
> -
> -  struct gdbarch *gdbarch;
> -  const struct regcache *regcache;
> -  bfd *obfd;
> -  gdb::unique_xmalloc_ptr<char> &note_data;
> -  int *note_size;
> -  unsigned long lwp;
> -  enum gdb_signal stop_signal;
> -  bool abort_iteration = false;
> -};
> -
> -/* Callback for iterate_over_regset_sections that records a single
> -   regset in the corefile note section.  */
> -
> -static void
> -linux_collect_regset_section_cb (const char *sect_name, int supply_size,
> -				 int collect_size, const struct regset *regset,
> -				 const char *human_name, void *cb_data)
> -{
> -  struct linux_collect_regset_section_cb_data *data
> -    = (struct linux_collect_regset_section_cb_data *) cb_data;
> -  bool variable_size_section = (regset != NULL
> -				&& regset->flags & REGSET_VARIABLE_SIZE);
> -
> -  if (!variable_size_section)
> -    gdb_assert (supply_size == collect_size);
> -
> -  if (data->abort_iteration)
> -    return;
> -
> -  gdb_assert (regset && regset->collect_regset);
> -
> -  /* This is intentionally zero-initialized by using std::vector, so
> -     that any padding bytes in the core file will show as 0.  */
> -  std::vector<gdb_byte> buf (collect_size);
> -
> -  regset->collect_regset (regset, data->regcache, -1, buf.data (),
> -			  collect_size);
> -
> -  /* PRSTATUS still needs to be treated specially.  */
> -  if (strcmp (sect_name, ".reg") == 0)
> -    data->note_data.reset (elfcore_write_prstatus
> -			     (data->obfd, data->note_data.release (),
> -			      data->note_size, data->lwp,
> -			      gdb_signal_to_host (data->stop_signal),
> -			      buf.data ()));
> -  else
> -    data->note_data.reset (elfcore_write_register_note
> -			   (data->obfd, data->note_data.release (),
> -			    data->note_size, sect_name, buf.data (),
> -			    collect_size));
> -
> -  if (data->note_data == NULL)
> -    data->abort_iteration = true;
> -}
> -
> -/* Records the thread's register state for the corefile note
> -   section.  */
> -
> -static void
> -linux_collect_thread_registers (const struct regcache *regcache,
> -				ptid_t ptid, bfd *obfd,
> -				gdb::unique_xmalloc_ptr<char> &note_data,
> -				int *note_size,
> -				enum gdb_signal stop_signal)
> -{
> -  struct gdbarch *gdbarch = regcache->arch ();
> -
> -  /* For remote targets the LWP may not be available, so use the TID.  */
> -  long lwp = ptid.lwp ();
> -  if (lwp == 0)
> -    lwp = ptid.tid ();
> -
> -  linux_collect_regset_section_cb_data data (gdbarch, regcache, obfd, note_data,
> -					     note_size, lwp, stop_signal);
> -
> -  gdbarch_iterate_over_regset_sections (gdbarch,
> -					linux_collect_regset_section_cb,
> -					&data, regcache);
> -}
> -
>  /* Fetch the siginfo data for the specified thread, if it exists.  If
>     there is no data, or we could not read it, return an empty
>     buffer.  */
> @@ -1746,22 +1650,17 @@ static void
>  linux_corefile_thread (struct thread_info *info,
>  		       struct linux_corefile_thread_data *args)
>  {
> -  struct regcache *regcache;
> -
> -  regcache = get_thread_arch_regcache (info->inf->process_target (),
> -				       info->ptid, args->gdbarch);
> -
> -  target_fetch_registers (regcache, -1);
> -  gdb::byte_vector siginfo_data = linux_get_siginfo_data (info, args->gdbarch);
> -
> -  linux_collect_thread_registers (regcache, info->ptid, args->obfd,
> -				  args->note_data, args->note_size,
> -				  args->stop_signal);
> +  gcore_elf_build_thread_register_notes (args->gdbarch, info,
> +					 args->stop_signal,
> +					 args->obfd, &args->note_data,
> +					 args->note_size);
>  
>    /* Don't return anything if we got no register information above,
>       such a core file is useless.  */
>    if (args->note_data != NULL)
>      {
> +      gdb::byte_vector siginfo_data
> +	= linux_get_siginfo_data (info, args->gdbarch);
>        if (!siginfo_data.empty ())
>  	args->note_data.reset (elfcore_write_note (args->obfd,
>  						   args->note_data.release (),
> @@ -1960,30 +1859,6 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
>    return 1;
>  }
>  
> -/* Find the signalled thread.  In case there's more than one signalled
> -   thread, prefer the current thread, if it is signalled.  If no
> -   thread was signalled, default to the current thread, unless it has
> -   exited, in which case return NULL.  */
> -
> -static thread_info *
> -find_signalled_thread ()
> -{
> -  thread_info *curr_thr = inferior_thread ();
> -  if (curr_thr->state != THREAD_EXITED
> -      && curr_thr->suspend.stop_signal != GDB_SIGNAL_0)
> -    return curr_thr;
> -
> -  for (thread_info *thr : current_inferior ()->non_exited_threads ())
> -    if (thr->suspend.stop_signal != GDB_SIGNAL_0)
> -      return thr;
> -
> -  /* Default to the current thread, unless it has exited.  */
> -  if (curr_thr->state != THREAD_EXITED)
> -    return curr_thr;
> -
> -  return nullptr;
> -}
> -
>  /* Build the note section for a corefile, and return it in a malloc
>     buffer.  */
>  
> @@ -2021,7 +1896,7 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
>    /* Like the kernel, prefer dumping the signalled thread first.
>       "First thread" is what tools use to infer the signalled
>       thread.  */
> -  thread_info *signalled_thr = find_signalled_thread ();
> +  thread_info *signalled_thr = gcore_find_signalled_thread ();
>    gdb_signal stop_signal;
>    if (signalled_thr != nullptr)
>      stop_signal = signalled_thr->suspend.stop_signal;
> -- 
> 2.25.4
>