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

Message ID 675727df761218e1d70b46c119671d4c2573c0b8.1613410057.git.andrew.burgess@embecosm.com
State New
Headers show
Series
  • Bare-metal core dumps for RISC-V
Related show

Commit Message

Andrew Burgess Feb. 15, 2021, 5:29 p.m.
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

-- 
2.25.4

Comments

Nick Clifton via Binutils Feb. 15, 2021, 10:56 p.m. | #1
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

>
Andrew Burgess Feb. 16, 2021, 4:55 p.m. | #2
* Lancelot SIX <lsix@lancelotsix.com> [2021-02-15 22:56:14 +0000]:

> 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.


Thanks for pointing out both of these issues.  I've addressed them in
the revised patch below.

> 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?


I was pretty sure that the coding standard said references are OK for
read-only values, but parameters that can be modified should be passed
as pointer.

I went to look this up on the coding standard wiki and couldn't find
any mention of this rule.  But I'm sure I remember this being a thing
at one point.

Thanks,
Andrew

---

commit 17e785148f34bb0fef9ec87390a9d779a0efae1a
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Mon Jan 18 16:00:38 2021 +0000

    gdb: unify parts of the Linux and FreeBSD core dumping code
    
    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.

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..b0fb808fae0
--- /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 != nullptr
+				&& regset->flags & REGSET_VARIABLE_SIZE);
+
+  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;
+}
+
+/* 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;

Patch

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);
+
+  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;
+}
+
+/* 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;