diff mbox

Remove use of deprecated core functions (in NetBSD/ARM)

Message ID 20200306205900.239755-1-cbiesinger@google.com
State New
Headers show

Commit Message

Doug Evans via gdb-patches March 6, 2020, 8:59 p.m. UTC
This is in preparation for deleting deprecated_add_core_fns and
related code.

As a side-effect, this makes it possible to read NetBSD/ARM
core files on non-NetBSD/ARM platforms, subject to PR corefiles/25638.

I have removed this comment:
-  /* This is ok: we're running native...  */
Since we are using the gdbarch from the regcache, we should be
guaranteed to be calling the right function here, so it shouldn't
matter whether we are running native.

Tested by reading a NetBSD/ARM core file on Linux/x86-64 and NetBSD/ARM;
the "info registers" output matches the one from the system GDB.

gdb/ChangeLog:

2020-03-05  Christian Biesinger  <cbiesinger@google.com>

	* Makefile.in (HFILES_NO_SRCDIR): Add new arm-nbsd-tdep.h file.
	* arm-nbsd-nat.c (arm_supply_gregset): Moved to arm-nbsd-tdep and
	renamed to arm_nbsd_supply_gregset.
	(fetch_register): Update to call arm_nbsd_supply_gregset.
	(fetch_regs): Remove in favor of fetch_register with a -1 regno.
	(arm_netbsd_nat_target::fetch_registers): Update.
	(fetch_elfcore_registers): Removed.
	(_initialize_arm_netbsd_nat): Removed call to deprecated_add_core_fns.
	* arm-nbsd-tdep.c (struct arm_nbsd_reg): New struct.
	(arm_nbsd_supply_gregset): Moved from arm-nbsd-nat.c and updated to
	not require NetBSD system headers.
	(arm_nbsd_regset): New struct.
	(arm_nbsd_iterate_over_regset_sections): New function.
	(arm_netbsd_init_abi_common): Updated to call
	set_gdbarch_iterate_over_regset_sections.
	* arm-nbsd-tdep.h: New file.
---
 gdb/Makefile.in     |   1 +
 gdb/arm-nbsd-nat.c  | 125 ++------------------------------------------
 gdb/arm-nbsd-tdep.c |  70 ++++++++++++++++++++++++-
 gdb/arm-nbsd-tdep.h |  27 ++++++++++
 4 files changed, 101 insertions(+), 122 deletions(-)
 create mode 100644 gdb/arm-nbsd-tdep.h

Comments

Simon Marchi March 6, 2020, 9:12 p.m. UTC | #1
On 2020-03-06 3:59 p.m., Christian Biesinger via gdb-patches wrote:
> This is in preparation for deleting deprecated_add_core_fns and
> related code.
> 
> As a side-effect, this makes it possible to read NetBSD/ARM
> core files on non-NetBSD/ARM platforms, subject to PR corefiles/25638.
> 
> I have removed this comment:
> -  /* This is ok: we're running native...  */
> Since we are using the gdbarch from the regcache, we should be
> guaranteed to be calling the right function here, so it shouldn't
> matter whether we are running native.
> 
> Tested by reading a NetBSD/ARM core file on Linux/x86-64 and NetBSD/ARM;
> the "info registers" output matches the one from the system GDB.
> 
> gdb/ChangeLog:
> 
> 2020-03-05  Christian Biesinger  <cbiesinger@google.com>
> 
> 	* Makefile.in (HFILES_NO_SRCDIR): Add new arm-nbsd-tdep.h file.
> 	* arm-nbsd-nat.c (arm_supply_gregset): Moved to arm-nbsd-tdep and
> 	renamed to arm_nbsd_supply_gregset.
> 	(fetch_register): Update to call arm_nbsd_supply_gregset.
> 	(fetch_regs): Remove in favor of fetch_register with a -1 regno.
> 	(arm_netbsd_nat_target::fetch_registers): Update.
> 	(fetch_elfcore_registers): Removed.
> 	(_initialize_arm_netbsd_nat): Removed call to deprecated_add_core_fns.
> 	* arm-nbsd-tdep.c (struct arm_nbsd_reg): New struct.
> 	(arm_nbsd_supply_gregset): Moved from arm-nbsd-nat.c and updated to
> 	not require NetBSD system headers.
> 	(arm_nbsd_regset): New struct.
> 	(arm_nbsd_iterate_over_regset_sections): New function.
> 	(arm_netbsd_init_abi_common): Updated to call
> 	set_gdbarch_iterate_over_regset_sections.
> 	* arm-nbsd-tdep.h: New file.

Thanks, this looks good to me, but I would appreciate if Kamil to take a quick
look as well.

I noted some nits below (no need to send another version just for them).

> @@ -35,6 +37,70 @@ static const gdb_byte arm_nbsd_arm_be_breakpoint[] = {0xe6, 0x00, 0x00, 0x11};
>  static const gdb_byte arm_nbsd_thumb_le_breakpoint[] = {0xfe, 0xde};
>  static const gdb_byte arm_nbsd_thumb_be_breakpoint[] = {0xde, 0xfe};
>  
> +/* This matches struct reg from NetBSD's sys/arch/arm/include/reg.h */

I think it would be nice to provide the link in the comment (a permalink,
in case the file ever moves):

https://github.com/NetBSD/src/blob/7c13e6e6773bb171f4ed3ed53013e9d24b3c1eac/sys/arch/arm/include/reg.h#L39

> +struct arm_nbsd_reg {

Curly bracket on next line.

> +  uint32_t reg[13];
> +  uint32_t sp;
> +  uint32_t lr;
> +  uint32_t pc;
> +  uint32_t cpsr;
> +};
> +
> +void
> +arm_nbsd_supply_gregset (const struct regset *regset, struct regcache *regcache,
> +			 int regnum, const void *gregs, size_t len)
> +{
> +  const arm_nbsd_reg *gregset = static_cast<const arm_nbsd_reg *>(gregs);
> +  gdb_assert (len >= sizeof (arm_nbsd_reg));
> +
> +  /* Integer registers.  */
> +  for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++)
> +    if (regnum == -1 || regnum == i)
> +      regcache->raw_supply (i, (char *) &gregset->reg[i]);
> +
> +  if (regnum == -1 || regnum == ARM_SP_REGNUM)
> +    regcache->raw_supply (ARM_SP_REGNUM, (char *) &gregset->sp);
> +  if (regnum == -1 || regnum == ARM_LR_REGNUM)
> +    regcache->raw_supply (ARM_LR_REGNUM, (char *) &gregset->lr);
> +  if (regnum == -1 || regnum == ARM_PC_REGNUM)
> +    {
> +      CORE_ADDR r_pc = gdbarch_addr_bits_remove (regcache->arch (), gregset->pc);
> +      regcache->raw_supply (ARM_PC_REGNUM, (char *) &r_pc);
> +    }

Can you add some empty lines between each separate condition?  I find this
looks too much like if/else if.

Simon
John Baldwin March 6, 2020, 10:41 p.m. UTC | #2
On 3/6/20 12:59 PM, Christian Biesinger via gdb-patches wrote:
> This is in preparation for deleting deprecated_add_core_fns and
> related code.
> 
> As a side-effect, this makes it possible to read NetBSD/ARM
> core files on non-NetBSD/ARM platforms, subject to PR corefiles/25638.
> 
> I have removed this comment:
> -  /* This is ok: we're running native...  */
> Since we are using the gdbarch from the regcache, we should be
> guaranteed to be calling the right function here, so it shouldn't
> matter whether we are running native.
> 
> Tested by reading a NetBSD/ARM core file on Linux/x86-64 and NetBSD/ARM;
> the "info registers" output matches the one from the system GDB.
> 
> diff --git a/gdb/arm-nbsd-tdep.c b/gdb/arm-nbsd-tdep.c
> index 2642024022..689c4b6219 100644
> --- a/gdb/arm-nbsd-tdep.c
> +++ b/gdb/arm-nbsd-tdep.c
> @@ -35,6 +37,70 @@ static const gdb_byte arm_nbsd_arm_be_breakpoint[] = {0xe6, 0x00, 0x00, 0x11};
>  static const gdb_byte arm_nbsd_thumb_le_breakpoint[] = {0xfe, 0xde};
>  static const gdb_byte arm_nbsd_thumb_be_breakpoint[] = {0xde, 0xfe};
>  
> +/* This matches struct reg from NetBSD's sys/arch/arm/include/reg.h */
> +struct arm_nbsd_reg {
> +  uint32_t reg[13];
> +  uint32_t sp;
> +  uint32_t lr;
> +  uint32_t pc;
> +  uint32_t cpsr;
> +};

I would encourage you to use a regcache_map instead.  It's a bit simpler to
work with, and if NetBSD copies the same structure out as part of the
sigcontext on the signal stack, you can reuse the regcache_map in your
signal frame unwinder.  I've updated the arm, aarch64, and risc-v FreeBSD
architectures to use this if you want to see a reference on how to use a
map, e.g. from arm-fbsd-tdep.c:

/* Register maps.  */

static const struct regcache_map_entry arm_fbsd_gregmap[] =
  {
    { 13, ARM_A1_REGNUM, 4 }, /* r0 ... r12 */
    { 1, ARM_SP_REGNUM, 4 },
    { 1, ARM_LR_REGNUM, 4 },
    { 1, ARM_PC_REGNUM, 4 },
    { 1, ARM_PS_REGNUM, 4 },
    { 0 }
  };

...

/* Register set definitions.  */

const struct regset arm_fbsd_gregset =
  {
    arm_fbsd_gregmap,
    regcache_supply_regset, regcache_collect_regset
  };

...

/* Implement the "iterate_over_regset_sections" gdbarch method.  */

static void
arm_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
				       iterate_over_regset_sections_cb *cb,
				       void *cb_data,
				       const struct regcache *regcache)
{
  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

  cb (".reg", ARM_FBSD_SIZEOF_GREGSET, ARM_FBSD_SIZEOF_GREGSET,
      &arm_fbsd_gregset, NULL, cb_data);


The native target is also able to use the regcache_map, e.g. from
arm-fbsd-nat.c:

/* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
   for all registers.  */

void
arm_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
{
  pid_t pid = get_ptrace_pid (regcache->ptid ());

  if (regnum == -1 || getregs_supplies (regnum))
    {
      struct reg regs;

      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
	perror_with_name (_("Couldn't get registers"));

      regcache->supply_regset (&arm_fbsd_gregset, regnum, &regs,
			       sizeof (regs));
    }

...

And for the signal frame unwinder in arm-fbsd-tdep.c, the single
call to trad_frame_set_reg_regmap() lets you pull in all the register
from the sigcontext in one go:

/* Implement the "init" method of struct tramp_frame.  */

static void
arm_fbsd_sigframe_init (const struct tramp_frame *self,
			struct frame_info *this_frame,
			struct trad_frame_cache *this_cache,
			CORE_ADDR func)
{
  struct gdbarch *gdbarch = get_frame_arch (this_frame);
  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
  CORE_ADDR sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
  CORE_ADDR mcontext_addr = (sp
			     + ARM_SIGFRAME_UCONTEXT_OFFSET
			     + ARM_UCONTEXT_MCONTEXT_OFFSET);
  ULONGEST mcontext_vfp_addr;

  trad_frame_set_reg_regmap (this_cache, arm_fbsd_gregmap, mcontext_addr,
			     regcache_map_entry_size (arm_fbsd_gregmap));
Doug Evans via gdb-patches March 6, 2020, 10:48 p.m. UTC | #3
On Fri, Mar 6, 2020 at 4:41 PM John Baldwin <jhb@freebsd.org> wrote:
>
> On 3/6/20 12:59 PM, Christian Biesinger via gdb-patches wrote:
> > This is in preparation for deleting deprecated_add_core_fns and
> > related code.
> >
> > As a side-effect, this makes it possible to read NetBSD/ARM
> > core files on non-NetBSD/ARM platforms, subject to PR corefiles/25638.
> >
> > I have removed this comment:
> > -  /* This is ok: we're running native...  */
> > Since we are using the gdbarch from the regcache, we should be
> > guaranteed to be calling the right function here, so it shouldn't
> > matter whether we are running native.
> >
> > Tested by reading a NetBSD/ARM core file on Linux/x86-64 and NetBSD/ARM;
> > the "info registers" output matches the one from the system GDB.
> >
> > diff --git a/gdb/arm-nbsd-tdep.c b/gdb/arm-nbsd-tdep.c
> > index 2642024022..689c4b6219 100644
> > --- a/gdb/arm-nbsd-tdep.c
> > +++ b/gdb/arm-nbsd-tdep.c
> > @@ -35,6 +37,70 @@ static const gdb_byte arm_nbsd_arm_be_breakpoint[] = {0xe6, 0x00, 0x00, 0x11};
> >  static const gdb_byte arm_nbsd_thumb_le_breakpoint[] = {0xfe, 0xde};
> >  static const gdb_byte arm_nbsd_thumb_be_breakpoint[] = {0xde, 0xfe};
> >
> > +/* This matches struct reg from NetBSD's sys/arch/arm/include/reg.h */
> > +struct arm_nbsd_reg {
> > +  uint32_t reg[13];
> > +  uint32_t sp;
> > +  uint32_t lr;
> > +  uint32_t pc;
> > +  uint32_t cpsr;
> > +};
>
> I would encourage you to use a regcache_map instead.  It's a bit simpler to
> work with, and if NetBSD copies the same structure out as part of the
> sigcontext on the signal stack, you can reuse the regcache_map in your
> signal frame unwinder.  I've updated the arm, aarch64, and risc-v FreeBSD
> architectures to use this if you want to see a reference on how to use agdbarch_addr_bits_remove
> map, e.g. from arm-fbsd-tdep.c:

Thanks for the feedback; I thought about that but there are two places
in this function that make this harder -- there is an arm_apcs_32
check that affects what gets supplied for ARM_PS_REGNUM and there's a
call to gdbarch_addr_bits_remove on the PC, perhaps for the thumb bit.
So I think I cannot convert it while keeping the same semantics?

The fetch_registers implementation does share this code with my patch.

Any thoughts?

Christian

>
> /* Register maps.  */
>
> static const struct regcache_map_entry arm_fbsd_gregmap[] =
>   {
>     { 13, ARM_A1_REGNUM, 4 }, /* r0 ... r12 */
>     { 1, ARM_SP_REGNUM, 4 },
>     { 1, ARM_LR_REGNUM, 4 },
>     { 1, ARM_PC_REGNUM, 4 },
>     { 1, ARM_PS_REGNUM, 4 },
>     { 0 }
>   };
>
> ...
>
> /* Register set definitions.  */
>
> const struct regset arm_fbsd_gregset =
>   {
>     arm_fbsd_gregmap,
>     regcache_supply_regset, regcache_collect_regset
>   };
>
> ...
>
> /* Implement the "iterate_over_regset_sections" gdbarch method.  */
>
> static void
> arm_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
>                                        iterate_over_regset_sections_cb *cb,
>                                        void *cb_data,
>                                        const struct regcache *regcache)
> {
>   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>
>   cb (".reg", ARM_FBSD_SIZEOF_GREGSET, ARM_FBSD_SIZEOF_GREGSET,
>       &arm_fbsd_gregset, NULL, cb_data);
>
>
> The native target is also able to use the regcache_map, e.g. from
> arm-fbsd-nat.c:
>
> /* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
>    for all registers.  */
>
> void
> arm_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
> {
>   pid_t pid = get_ptrace_pid (regcache->ptid ());
>
>   if (regnum == -1 || getregs_supplies (regnum))
>     {
>       struct reg regs;
>
>       if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
>         perror_with_name (_("Couldn't get registers"));
>
>       regcache->supply_regset (&arm_fbsd_gregset, regnum, &regs,
>                                sizeof (regs));
>     }
>
> ...
>
> And for the signal frame unwinder in arm-fbsd-tdep.c, the single
> call to trad_frame_set_reg_regmap() lets you pull in all the register
> from the sigcontext in one go:
>
> /* Implement the "init" method of struct tramp_frame.  */
>
> static void
> arm_fbsd_sigframe_init (const struct tramp_frame *self,
>                         struct frame_info *this_frame,
>                         struct trad_frame_cache *this_cache,
>                         CORE_ADDR func)
> {
>   struct gdbarch *gdbarch = get_frame_arch (this_frame);
>   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>   CORE_ADDR sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
>   CORE_ADDR mcontext_addr = (sp
>                              + ARM_SIGFRAME_UCONTEXT_OFFSET
>                              + ARM_UCONTEXT_MCONTEXT_OFFSET);
>   ULONGEST mcontext_vfp_addr;
>
>   trad_frame_set_reg_regmap (this_cache, arm_fbsd_gregmap, mcontext_addr,
>                              regcache_map_entry_size (arm_fbsd_gregmap));
>
>
> --
> John Baldwin
diff mbox

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 7c0a0aefbc..1a837eda8d 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1205,6 +1205,7 @@  HFILES_NO_SRCDIR = \
 	arc-tdep.h \
 	arch-utils.h \
 	arm-linux-tdep.h \
+	arm-nbsd-tdep.h \
 	arm-tdep.h \
 	auto-load.h \
 	auxv.h \
diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
index 04fedd4be3..cb31462737 100644
--- a/gdb/arm-nbsd-nat.c
+++ b/gdb/arm-nbsd-nat.c
@@ -31,6 +31,7 @@ 
 #include <machine/frame.h>
 
 #include "arm-tdep.h"
+#include "arm-nbsd-tdep.h"
 #include "aarch32-tdep.h"
 #include "inf-ptrace.h"
 
@@ -45,28 +46,6 @@  public:
 
 static arm_netbsd_nat_target the_arm_netbsd_nat_target;
 
-static void
-arm_supply_gregset (struct regcache *regcache, struct reg *gregset)
-{
-  int regno;
-  CORE_ADDR r_pc;
-
-  /* Integer registers.  */
-  for (regno = ARM_A1_REGNUM; regno < ARM_SP_REGNUM; regno++)
-    regcache->raw_supply (regno, (char *) &gregset->r[regno]);
-
-  regcache->raw_supply (ARM_SP_REGNUM, (char *) &gregset->r_sp);
-  regcache->raw_supply (ARM_LR_REGNUM, (char *) &gregset->r_lr);
-  /* This is ok: we're running native...  */
-  r_pc = gdbarch_addr_bits_remove (regcache->arch (), gregset->r_pc);
-  regcache->raw_supply (ARM_PC_REGNUM, (char *) &r_pc);
-
-  if (arm_apcs_32)
-    regcache->raw_supply (ARM_PS_REGNUM, (char *) &gregset->r_cpsr);
-  else
-    regcache->raw_supply (ARM_PS_REGNUM, (char *) &gregset->r_pc);
-}
-
 static void
 arm_supply_vfpregset (struct regcache *regcache, struct fpreg *fpregset)
 {
@@ -95,57 +74,8 @@  fetch_register (struct regcache *regcache, int regno)
       warning (_("unable to fetch general register"));
       return;
     }
-
-  switch (regno)
-    {
-    case ARM_SP_REGNUM:
-      regcache->raw_supply (ARM_SP_REGNUM, (char *) &inferior_registers.r_sp);
-      break;
-
-    case ARM_LR_REGNUM:
-      regcache->raw_supply (ARM_LR_REGNUM, (char *) &inferior_registers.r_lr);
-      break;
-
-    case ARM_PC_REGNUM:
-      /* This is ok: we're running native...  */
-      inferior_registers.r_pc = gdbarch_addr_bits_remove
-				  (regcache->arch (),
-				   inferior_registers.r_pc);
-      regcache->raw_supply (ARM_PC_REGNUM, (char *) &inferior_registers.r_pc);
-      break;
-
-    case ARM_PS_REGNUM:
-      if (arm_apcs_32)
-	regcache->raw_supply (ARM_PS_REGNUM,
-			      (char *) &inferior_registers.r_cpsr);
-      else
-	regcache->raw_supply (ARM_PS_REGNUM,
-			      (char *) &inferior_registers.r_pc);
-      break;
-
-    default:
-      regcache->raw_supply (regno, (char *) &inferior_registers.r[regno]);
-      break;
-    }
-}
-
-static void
-fetch_regs (struct regcache *regcache)
-{
-  struct reg inferior_registers;
-  int ret;
-  int regno;
-
-  ret = ptrace (PT_GETREGS, regcache->ptid ().pid (),
-		(PTRACE_TYPE_ARG3) &inferior_registers, 0);
-
-  if (ret < 0)
-    {
-      warning (_("unable to fetch general registers"));
-      return;
-    }
-
-  arm_supply_gregset (regcache, &inferior_registers);
+  arm_nbsd_supply_gregset (nullptr, regcache, regno, &inferior_registers,
+			   sizeof (inferior_registers));
 }
 
 static void
@@ -207,7 +137,7 @@  arm_netbsd_nat_target::fetch_registers (struct regcache *regcache, int regno)
     }
   else
     {
-      fetch_regs (regcache);
+      fetch_register (regcache, -1);
       fetch_fp_regs (regcache);
     }
 }
@@ -416,56 +346,9 @@  arm_netbsd_nat_target::read_description ()
   return arm_read_description (ARM_FP_TYPE_VFPV3);
 }
 
-static void
-fetch_elfcore_registers (struct regcache *regcache,
-			 gdb_byte *core_reg_sect, unsigned core_reg_size,
-			 int which, CORE_ADDR ignore)
-{
-  struct reg gregset;
-  struct fpreg fparegset;
-
-  switch (which)
-    {
-    case 0:	/* Integer registers.  */
-      if (core_reg_size != sizeof (struct reg))
-	warning (_("wrong size of register set in core file"));
-      else
-	{
-	  /* The memcpy may be unnecessary, but we can't really be sure
-	     of the alignment of the data in the core file.  */
-	  memcpy (&gregset, core_reg_sect, sizeof (gregset));
-	  arm_supply_gregset (regcache, &gregset);
-	}
-      break;
-
-    case 2:
-      /* cbiesinger/2020-02-12 -- as far as I can tell, ARM/NetBSD does
-         not write any floating point registers into the core file (tested
-	 with NetBSD 9.1_RC1).  When it does, this block will need to read them,
-	 and the arm-netbsd gdbarch will need a core_read_description function
-	 to return the right description for them.  */
-      break;
-
-    default:
-      /* Don't know what kind of register request this is; just ignore it.  */
-      break;
-    }
-}
-
-static struct core_fns arm_netbsd_elfcore_fns =
-{
-  bfd_target_elf_flavour,		/* core_flavour.  */
-  default_check_format,			/* check_format.  */
-  default_core_sniffer,			/* core_sniffer.  */
-  fetch_elfcore_registers,		/* core_read_registers.  */
-  NULL
-};
-
 void _initialize_arm_netbsd_nat ();
 void
 _initialize_arm_netbsd_nat ()
 {
   add_inf_child_target (&the_arm_netbsd_nat_target);
-
-  deprecated_add_core_fns (&arm_netbsd_elfcore_fns);
 }
diff --git a/gdb/arm-nbsd-tdep.c b/gdb/arm-nbsd-tdep.c
index 2642024022..689c4b6219 100644
--- a/gdb/arm-nbsd-tdep.c
+++ b/gdb/arm-nbsd-tdep.c
@@ -21,7 +21,9 @@ 
 #include "osabi.h"
 
 #include "arch/arm.h"
+#include "arm-nbsd-tdep.h"
 #include "arm-tdep.h"
+#include "regset.h"
 #include "solib-svr4.h"
 
 /* Description of the longjmp buffer.  */
@@ -35,6 +37,70 @@  static const gdb_byte arm_nbsd_arm_be_breakpoint[] = {0xe6, 0x00, 0x00, 0x11};
 static const gdb_byte arm_nbsd_thumb_le_breakpoint[] = {0xfe, 0xde};
 static const gdb_byte arm_nbsd_thumb_be_breakpoint[] = {0xde, 0xfe};
 
+/* This matches struct reg from NetBSD's sys/arch/arm/include/reg.h */
+struct arm_nbsd_reg {
+  uint32_t reg[13];
+  uint32_t sp;
+  uint32_t lr;
+  uint32_t pc;
+  uint32_t cpsr;
+};
+
+void
+arm_nbsd_supply_gregset (const struct regset *regset, struct regcache *regcache,
+			 int regnum, const void *gregs, size_t len)
+{
+  const arm_nbsd_reg *gregset = static_cast<const arm_nbsd_reg *>(gregs);
+  gdb_assert (len >= sizeof (arm_nbsd_reg));
+
+  /* Integer registers.  */
+  for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++)
+    if (regnum == -1 || regnum == i)
+      regcache->raw_supply (i, (char *) &gregset->reg[i]);
+
+  if (regnum == -1 || regnum == ARM_SP_REGNUM)
+    regcache->raw_supply (ARM_SP_REGNUM, (char *) &gregset->sp);
+  if (regnum == -1 || regnum == ARM_LR_REGNUM)
+    regcache->raw_supply (ARM_LR_REGNUM, (char *) &gregset->lr);
+  if (regnum == -1 || regnum == ARM_PC_REGNUM)
+    {
+      CORE_ADDR r_pc = gdbarch_addr_bits_remove (regcache->arch (), gregset->pc);
+      regcache->raw_supply (ARM_PC_REGNUM, (char *) &r_pc);
+    }
+
+  if (regnum == -1 || regnum == ARM_PS_REGNUM)
+    {
+      if (arm_apcs_32)
+	regcache->raw_supply (ARM_PS_REGNUM, (char *) &gregset->cpsr);
+      else
+	regcache->raw_supply (ARM_PS_REGNUM, (char *) &gregset->pc);
+    }
+}
+
+static const struct regset arm_nbsd_regset = {
+  nullptr,
+  arm_nbsd_supply_gregset,
+  /* We don't need a collect function because we only use this reading registers
+     (via iterate_over_regset_sections and fetch_regs/fetch_register).  */
+  nullptr,
+  0
+};
+
+static void
+arm_nbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
+				       iterate_over_regset_sections_cb *cb,
+				       void *cb_data,
+				       const struct regcache *regcache)
+{
+  cb (".reg", sizeof (arm_nbsd_reg), sizeof (arm_nbsd_reg), &arm_nbsd_regset,
+      NULL, cb_data);
+  /* cbiesinger/2020-02-12 -- as far as I can tell, ARM/NetBSD does
+     not write any floating point registers into the core file (tested
+     with NetBSD 9.1_RC1).  When it does, this function will need to read them,
+     and the arm-netbsd gdbarch will need a core_read_description function
+     to return the right description for them.  */
+}
+
 static void
 arm_netbsd_init_abi_common (struct gdbarch_info info,
 			    struct gdbarch *gdbarch)
@@ -66,10 +132,12 @@  arm_netbsd_init_abi_common (struct gdbarch_info info,
   tdep->jb_pc = ARM_NBSD_JB_PC;
   tdep->jb_elt_size = ARM_NBSD_JB_ELEMENT_SIZE;
 
+  set_gdbarch_iterate_over_regset_sections
+    (gdbarch, arm_nbsd_iterate_over_regset_sections);
   /* Single stepping.  */
   set_gdbarch_software_single_step (gdbarch, arm_software_single_step);
 }
-  
+
 static void
 arm_netbsd_elf_init_abi (struct gdbarch_info info,
 			 struct gdbarch *gdbarch)
diff --git a/gdb/arm-nbsd-tdep.h b/gdb/arm-nbsd-tdep.h
new file mode 100644
index 0000000000..1b35395031
--- /dev/null
+++ b/gdb/arm-nbsd-tdep.h
@@ -0,0 +1,27 @@ 
+/* NetBSD/ARM support.
+
+   Copyright (C) 2020 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/>.  */
+
+#ifndef ARM_NBSD_TDEP_H
+#define ARM_NBSD_TDEP_H
+
+void arm_nbsd_supply_gregset
+  (const struct regset *regset, struct regcache *regcache,
+   int regnum, const void *gregs, size_t len);
+
+#endif