public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Re: [PATCH] Remove use of deprecated core functions (in NetBSD/ARM)
       [not found]   ` <CAPTJ0XFOMHsVb2CNuowXmTcB-SgM65-FO8DURqP-5C4hXUFT6w@mail.gmail.com>
@ 2020-03-09 20:53     ` John Baldwin
  0 siblings, 0 replies; 4+ messages in thread
From: John Baldwin @ 2020-03-09 20:53 UTC (permalink / raw)
  To: Christian Biesinger; +Cc: gdb-patches, Kamil Rytarowski

On 3/6/20 2:48 PM, Christian Biesinger via gdb-patches wrote:
> 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?

Fair enough, that does make it harder to handle as simply.  The current patch
is certainly an improvement.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Remove use of deprecated core functions (in NetBSD/ARM)
       [not found] ` <f732d439-95bc-e427-b1d6-b9fcfaabbd8a@simark.ca>
@ 2020-03-11 20:48   ` Christian Biesinger
  0 siblings, 0 replies; 4+ messages in thread
From: Christian Biesinger @ 2020-03-11 20:48 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Kamil Rytarowski

Ping -- Kamil, any thoughts on the below patch?

On Fri, Mar 6, 2020 at 3:12 PM Simon Marchi <simark@simark.ca> wrote:
>
> 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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Remove use of deprecated core functions (in NetBSD/ARM)
       [not found] <20200306205900.239755-1-cbiesinger@google.com>
       [not found] ` <fda20e67-3cdd-89ee-4c3a-7ab334a30793@FreeBSD.org>
       [not found] ` <f732d439-95bc-e427-b1d6-b9fcfaabbd8a@simark.ca>
@ 2020-03-12 13:44 ` Kamil Rytarowski
  2020-03-12 17:23   ` Christian Biesinger
  2 siblings, 1 reply; 4+ messages in thread
From: Kamil Rytarowski @ 2020-03-12 13:44 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches


[-- Attachment #1.1: Type: text/plain, Size: 12538 bytes --]

I don't see anything wrong with this patch. Please go ahead.

On 06.03.2020 21:59, 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.
> ---
>  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
> 
> 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
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Remove use of deprecated core functions (in NetBSD/ARM)
  2020-03-12 13:44 ` Kamil Rytarowski
@ 2020-03-12 17:23   ` Christian Biesinger
  0 siblings, 0 replies; 4+ messages in thread
From: Christian Biesinger @ 2020-03-12 17:23 UTC (permalink / raw)
  To: Kamil Rytarowski; +Cc: gdb-patches

On Thu, Mar 12, 2020 at 8:45 AM Kamil Rytarowski <n54@gmx.com> wrote:
>
> I don't see anything wrong with this patch. Please go ahead.

Thanks, I pushed this with Simon's nits fixed.

Christian

> On 06.03.2020 21:59, 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.
> > ---
> >  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
> >
> > 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
> >
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-03-12 17:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200306205900.239755-1-cbiesinger@google.com>
     [not found] ` <fda20e67-3cdd-89ee-4c3a-7ab334a30793@FreeBSD.org>
     [not found]   ` <CAPTJ0XFOMHsVb2CNuowXmTcB-SgM65-FO8DURqP-5C4hXUFT6w@mail.gmail.com>
2020-03-09 20:53     ` [PATCH] Remove use of deprecated core functions (in NetBSD/ARM) John Baldwin
     [not found] ` <f732d439-95bc-e427-b1d6-b9fcfaabbd8a@simark.ca>
2020-03-11 20:48   ` Christian Biesinger
2020-03-12 13:44 ` Kamil Rytarowski
2020-03-12 17:23   ` Christian Biesinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).