diff mbox

[v3] Fix arm-netbsd build error: convert from FPA to VFP

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

Commit Message

Doug Evans via gdb-patches Feb. 28, 2020, 7:19 p.m. UTC
Hi Alan,

there is already a num_vfp_registers field in tdep, so I thought it easiest
to just use that. Does that look good? See below.

Thanks,
Christian

----

The floating point register interface has changed to this:
https://github.com/NetBSD/src/blob/trunk/sys/arch/arm/include/reg.h

It now uses VFP instead of FPA registers. This patch updates
arm-nbsd-nat.c accordingly.

Also implements read_description so that these registers are correctly
printed by "info registers" et al.

Tested by compiling & running on arm-netbsd on qemu.

gdb/ChangeLog:

2020-02-11  Christian Biesinger  <cbiesinger@google.com>

	* arm-nbsd-nat.c (arm_supply_fparegset): Rename to...
	(arm_supply_vfpregset): ...this, and update to use VFP registers.
	(fetch_fp_register): Update.
	(fetch_fp_regs): Update.
	(store_fp_register): Update.
	(store_fp_regs): Update.
	(arm_netbsd_nat_target::read_description): New function.
	(fetch_elfcore_registers): Update.
---
 gdb/arm-nbsd-nat.c | 121 +++++++++++++++++++++++++--------------------
 1 file changed, 68 insertions(+), 53 deletions(-)

Comments

Alan Hayward March 2, 2020, 11:42 a.m. UTC | #1
> On 28 Feb 2020, at 19:19, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:

> 

> Hi Alan,

> 

> there is already a num_vfp_registers field in tdep, so I thought it easiest

> to just use that. Does that look good? See below.

> 


That works for the ARM_FP_TYPE_NONE case.

I had to convince myself it works for the aarch32_read_description case.
And after thinking it through, I’m happy because both have the same size
FP register banks.

So, ok to push.

> Thanks,

> Christian

> 

> ----

> 

> The floating point register interface has changed to this:

> https://github.com/NetBSD/src/blob/trunk/sys/arch/arm/include/reg.h

> 

> It now uses VFP instead of FPA registers. This patch updates

> arm-nbsd-nat.c accordingly.

> 

> Also implements read_description so that these registers are correctly

> printed by "info registers" et al.

> 

> Tested by compiling & running on arm-netbsd on qemu.

> 

> gdb/ChangeLog:

> 

> 2020-02-11  Christian Biesinger  <cbiesinger@google.com>

> 

> 	* arm-nbsd-nat.c (arm_supply_fparegset): Rename to...

> 	(arm_supply_vfpregset): ...this, and update to use VFP registers.

> 	(fetch_fp_register): Update.

> 	(fetch_fp_regs): Update.

> 	(store_fp_register): Update.

> 	(store_fp_regs): Update.

> 	(arm_netbsd_nat_target::read_description): New function.

> 	(fetch_elfcore_registers): Update.

> ---

> gdb/arm-nbsd-nat.c | 121 +++++++++++++++++++++++++--------------------

> 1 file changed, 68 insertions(+), 53 deletions(-)

> 

> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c

> index 11afc289c3..cee7917db2 100644

> --- a/gdb/arm-nbsd-nat.c

> +++ b/gdb/arm-nbsd-nat.c

> @@ -26,10 +26,12 @@

> #include "target.h"

> #include <sys/types.h>

> #include <sys/ptrace.h>

> +#include <sys/sysctl.h>

> #include <machine/reg.h>

> #include <machine/frame.h>

> 

> #include "arm-tdep.h"

> +#include "aarch32-tdep.h"

> #include "inf-ptrace.h"

> 

> class arm_netbsd_nat_target final : public inf_ptrace_target

> @@ -38,6 +40,7 @@ public:

>   /* Add our register access methods.  */

>   void fetch_registers (struct regcache *, int) override;

>   void store_registers (struct regcache *, int) override;

> +  const struct target_desc *read_description () override;

> };

> 

> static arm_netbsd_nat_target the_arm_netbsd_nat_target;

> @@ -65,15 +68,17 @@ arm_supply_gregset (struct regcache *regcache, struct reg *gregset)

> }

> 

> static void

> -arm_supply_fparegset (struct regcache *regcache, struct fpreg *fparegset)

> +arm_supply_vfpregset (struct regcache *regcache, struct fpreg *fpregset)

> {

> -  int regno;

> +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());

> +  if (tdep->vfp_register_count == 0)

> +    return;

> 

> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)

> -    regcache->raw_supply (regno,

> -			  (char *) &fparegset->fpr[regno - ARM_F0_REGNUM]);

> +  struct vfpreg &vfp = fpregset->fpr_vfp;

> +  for (int regno = 0; regno <= tdep->vfp_register_count; regno++)

> +    regcache->raw_supply (regno + ARM_D0_REGNUM, (char *) &vfp.vfp_regs[regno]);

> 

> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);

> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);

> }

> 

> static void

> @@ -147,10 +152,10 @@ static void

> fetch_fp_register (struct regcache *regcache, int regno)

> {

>   struct fpreg inferior_fp_registers;

> -  int ret;

> +  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),

> +		    (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);

> 

> -  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),

> -		(PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);

> +  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;

> 

>   if (ret < 0)

>     {

> @@ -158,18 +163,17 @@ fetch_fp_register (struct regcache *regcache, int regno)

>       return;

>     }

> 

> -  switch (regno)

> +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());

> +  if (regno == ARM_FPSCR_REGNUM)

> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);

> +  else if (regno >= ARM_D0_REGNUM

> +	   && regno <= ARM_D0_REGNUM + tdep->vfp_register_count)

>     {

> -    case ARM_FPS_REGNUM:

> -      regcache->raw_supply (ARM_FPS_REGNUM,

> -			    (char *) &inferior_fp_registers.fpr_fpsr);

> -      break;

> -

> -    default:

> -      regcache->raw_supply

> -	(regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);

> -      break;

> +      regcache->raw_supply (regno,

> +			    (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);

>     }

> +  else

> +    warning (_("Invalid register number."));

> }

> 

> static void

> @@ -188,7 +192,7 @@ fetch_fp_regs (struct regcache *regcache)

>       return;

>     }

> 

> -  arm_supply_fparegset (regcache, &inferior_fp_registers);

> +  arm_supply_vfpregset (regcache, &inferior_fp_registers);

> }

> 

> void

> @@ -327,10 +331,9 @@ static void

> store_fp_register (const struct regcache *regcache, int regno)

> {

>   struct fpreg inferior_fp_registers;

> -  int ret;

> -

> -  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),

> -		(PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);

> +  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),

> +		    (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);

> +  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;

> 

>   if (ret < 0)

>     {

> @@ -338,18 +341,17 @@ store_fp_register (const struct regcache *regcache, int regno)

>       return;

>     }

> 

> -  switch (regno)

> +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());

> +  if (regno == ARM_FPSCR_REGNUM)

> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);

> +  else if (regno >= ARM_D0_REGNUM

> +	   && regno <= ARM_D0_REGNUM + tdep->vfp_register_count)

>     {

> -    case ARM_FPS_REGNUM:

> -      regcache->raw_collect (ARM_FPS_REGNUM,

> -			     (char *) &inferior_fp_registers.fpr_fpsr);

> -      break;

> -

> -    default:

> -      regcache->raw_collect

> -	(regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);

> -      break;

> +      regcache->raw_collect (regno,

> +			     (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);

>     }

> +  else

> +    warning (_("Invalid register number."));

> 

>   ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),

> 		(PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);

> @@ -361,20 +363,20 @@ store_fp_register (const struct regcache *regcache, int regno)

> static void

> store_fp_regs (const struct regcache *regcache)

> {

> -  struct fpreg inferior_fp_registers;

> -  int ret;

> -  int regno;

> -

> +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());

> +  if (tdep->vfp_register_count == 0)

> +    return;

> 

> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)

> +  struct fpreg fpregs;

> +  for (int regno = 0; regno <= tdep->vfp_register_count; regno++)

>     regcache->raw_collect

> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);

> +      (regno + ARM_D0_REGNUM, (char *) &fpregs.fpr_vfp.vfp_regs[regno]);

> 

> -  regcache->raw_collect (ARM_FPS_REGNUM,

> -			 (char *) &inferior_fp_registers.fpr_fpsr);

> +  regcache->raw_collect (ARM_FPSCR_REGNUM,

> +			 (char *) &fpregs.fpr_vfp.vfp_fpscr);

> 

> -  ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),

> -		(PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);

> +  int ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),

> +		    (PTRACE_TYPE_ARG3) &fpregs, 0);

> 

>   if (ret < 0)

>     warning (_("unable to store floating-point registers"));

> @@ -397,6 +399,23 @@ arm_netbsd_nat_target::store_registers (struct regcache *regcache, int regno)

>     }

> }

> 

> +const struct target_desc *

> +arm_netbsd_nat_target::read_description ()

> +{

> +  int flag;

> +  size_t len = sizeof (flag);

> +

> +  if (sysctlbyname("machdep.fpu_present", &flag, &len, NULL, 0) != 0

> +      || !flag)

> +    return arm_read_description (ARM_FP_TYPE_NONE);

> +

> +  len = sizeof(flag);

> +  if (sysctlbyname("machdep.neon_present", &flag, &len, NULL, 0) == 0 && flag)

> +    return aarch32_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,

> @@ -420,15 +439,11 @@ fetch_elfcore_registers (struct regcache *regcache,

>       break;

> 

>     case 2:

> -      if (core_reg_size != sizeof (struct fpreg))

> -	warning (_("wrong size of FPA 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 (&fparegset, core_reg_sect, sizeof (fparegset));

> -	  arm_supply_fparegset (regcache, &fparegset);

> -	}

> +      /* 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:

> -- 

> 2.25.0.265.gbab2e86ba0-goog

>
Doug Evans via gdb-patches March 2, 2020, 5:28 p.m. UTC | #2
On Mon, Mar 2, 2020 at 5:42 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
> > On 28 Feb 2020, at 19:19, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
> >
> > Hi Alan,
> >
> > there is already a num_vfp_registers field in tdep, so I thought it easiest
> > to just use that. Does that look good? See below.
> >
>
> That works for the ARM_FP_TYPE_NONE case.
>
> I had to convince myself it works for the aarch32_read_description case.
> And after thinking it through, I’m happy because both have the same size
> FP register banks.
>
> So, ok to push.

Thanks! Pushing now with a minor change in {fetch,store}_fp_register
to not fetch/store ARM_FPSCR_REGNUM if tdep->vfp_register_count == 0.

Christian

> > The floating point register interface has changed to this:
> > https://github.com/NetBSD/src/blob/trunk/sys/arch/arm/include/reg.h
> >
> > It now uses VFP instead of FPA registers. This patch updates
> > arm-nbsd-nat.c accordingly.
> >
> > Also implements read_description so that these registers are correctly
> > printed by "info registers" et al.
> >
> > Tested by compiling & running on arm-netbsd on qemu.
> >
> > gdb/ChangeLog:
> >
> > 2020-02-11  Christian Biesinger  <cbiesinger@google.com>
> >
> >       * arm-nbsd-nat.c (arm_supply_fparegset): Rename to...
> >       (arm_supply_vfpregset): ...this, and update to use VFP registers.
> >       (fetch_fp_register): Update.
> >       (fetch_fp_regs): Update.
> >       (store_fp_register): Update.
> >       (store_fp_regs): Update.
> >       (arm_netbsd_nat_target::read_description): New function.
> >       (fetch_elfcore_registers): Update.
> > ---
> > gdb/arm-nbsd-nat.c | 121 +++++++++++++++++++++++++--------------------
> > 1 file changed, 68 insertions(+), 53 deletions(-)
> >
> > diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> > index 11afc289c3..cee7917db2 100644
> > --- a/gdb/arm-nbsd-nat.c
> > +++ b/gdb/arm-nbsd-nat.c
> > @@ -26,10 +26,12 @@
> > #include "target.h"
> > #include <sys/types.h>
> > #include <sys/ptrace.h>
> > +#include <sys/sysctl.h>
> > #include <machine/reg.h>
> > #include <machine/frame.h>
> >
> > #include "arm-tdep.h"
> > +#include "aarch32-tdep.h"
> > #include "inf-ptrace.h"
> >
> > class arm_netbsd_nat_target final : public inf_ptrace_target
> > @@ -38,6 +40,7 @@ public:
> >   /* Add our register access methods.  */
> >   void fetch_registers (struct regcache *, int) override;
> >   void store_registers (struct regcache *, int) override;
> > +  const struct target_desc *read_description () override;
> > };
> >
> > static arm_netbsd_nat_target the_arm_netbsd_nat_target;
> > @@ -65,15 +68,17 @@ arm_supply_gregset (struct regcache *regcache, struct reg *gregset)
> > }
> >
> > static void
> > -arm_supply_fparegset (struct regcache *regcache, struct fpreg *fparegset)
> > +arm_supply_vfpregset (struct regcache *regcache, struct fpreg *fpregset)
> > {
> > -  int regno;
> > +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
> > +  if (tdep->vfp_register_count == 0)
> > +    return;
> >
> > -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
> > -    regcache->raw_supply (regno,
> > -                       (char *) &fparegset->fpr[regno - ARM_F0_REGNUM]);
> > +  struct vfpreg &vfp = fpregset->fpr_vfp;
> > +  for (int regno = 0; regno <= tdep->vfp_register_count; regno++)
> > +    regcache->raw_supply (regno + ARM_D0_REGNUM, (char *) &vfp.vfp_regs[regno]);
> >
> > -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
> > +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> > }
> >
> > static void
> > @@ -147,10 +152,10 @@ static void
> > fetch_fp_register (struct regcache *regcache, int regno)
> > {
> >   struct fpreg inferior_fp_registers;
> > -  int ret;
> > +  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
> > +                 (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> >
> > -  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
> > -             (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > +  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
> >
> >   if (ret < 0)
> >     {
> > @@ -158,18 +163,17 @@ fetch_fp_register (struct regcache *regcache, int regno)
> >       return;
> >     }
> >
> > -  switch (regno)
> > +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
> > +  if (regno == ARM_FPSCR_REGNUM)
> > +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> > +  else if (regno >= ARM_D0_REGNUM
> > +        && regno <= ARM_D0_REGNUM + tdep->vfp_register_count)
> >     {
> > -    case ARM_FPS_REGNUM:
> > -      regcache->raw_supply (ARM_FPS_REGNUM,
> > -                         (char *) &inferior_fp_registers.fpr_fpsr);
> > -      break;
> > -
> > -    default:
> > -      regcache->raw_supply
> > -     (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> > -      break;
> > +      regcache->raw_supply (regno,
> > +                         (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> >     }
> > +  else
> > +    warning (_("Invalid register number."));
> > }
> >
> > static void
> > @@ -188,7 +192,7 @@ fetch_fp_regs (struct regcache *regcache)
> >       return;
> >     }
> >
> > -  arm_supply_fparegset (regcache, &inferior_fp_registers);
> > +  arm_supply_vfpregset (regcache, &inferior_fp_registers);
> > }
> >
> > void
> > @@ -327,10 +331,9 @@ static void
> > store_fp_register (const struct regcache *regcache, int regno)
> > {
> >   struct fpreg inferior_fp_registers;
> > -  int ret;
> > -
> > -  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
> > -             (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > +  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
> > +                 (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > +  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
> >
> >   if (ret < 0)
> >     {
> > @@ -338,18 +341,17 @@ store_fp_register (const struct regcache *regcache, int regno)
> >       return;
> >     }
> >
> > -  switch (regno)
> > +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
> > +  if (regno == ARM_FPSCR_REGNUM)
> > +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> > +  else if (regno >= ARM_D0_REGNUM
> > +        && regno <= ARM_D0_REGNUM + tdep->vfp_register_count)
> >     {
> > -    case ARM_FPS_REGNUM:
> > -      regcache->raw_collect (ARM_FPS_REGNUM,
> > -                          (char *) &inferior_fp_registers.fpr_fpsr);
> > -      break;
> > -
> > -    default:
> > -      regcache->raw_collect
> > -     (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> > -      break;
> > +      regcache->raw_collect (regno,
> > +                          (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> >     }
> > +  else
> > +    warning (_("Invalid register number."));
> >
> >   ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
> >               (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > @@ -361,20 +363,20 @@ store_fp_register (const struct regcache *regcache, int regno)
> > static void
> > store_fp_regs (const struct regcache *regcache)
> > {
> > -  struct fpreg inferior_fp_registers;
> > -  int ret;
> > -  int regno;
> > -
> > +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
> > +  if (tdep->vfp_register_count == 0)
> > +    return;
> >
> > -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
> > +  struct fpreg fpregs;
> > +  for (int regno = 0; regno <= tdep->vfp_register_count; regno++)
> >     regcache->raw_collect
> > -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> > +      (regno + ARM_D0_REGNUM, (char *) &fpregs.fpr_vfp.vfp_regs[regno]);
> >
> > -  regcache->raw_collect (ARM_FPS_REGNUM,
> > -                      (char *) &inferior_fp_registers.fpr_fpsr);
> > +  regcache->raw_collect (ARM_FPSCR_REGNUM,
> > +                      (char *) &fpregs.fpr_vfp.vfp_fpscr);
> >
> > -  ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
> > -             (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
> > +  int ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
> > +                 (PTRACE_TYPE_ARG3) &fpregs, 0);
> >
> >   if (ret < 0)
> >     warning (_("unable to store floating-point registers"));
> > @@ -397,6 +399,23 @@ arm_netbsd_nat_target::store_registers (struct regcache *regcache, int regno)
> >     }
> > }
> >
> > +const struct target_desc *
> > +arm_netbsd_nat_target::read_description ()
> > +{
> > +  int flag;
> > +  size_t len = sizeof (flag);
> > +
> > +  if (sysctlbyname("machdep.fpu_present", &flag, &len, NULL, 0) != 0
> > +      || !flag)
> > +    return arm_read_description (ARM_FP_TYPE_NONE);
> > +
> > +  len = sizeof(flag);
> > +  if (sysctlbyname("machdep.neon_present", &flag, &len, NULL, 0) == 0 && flag)
> > +    return aarch32_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,
> > @@ -420,15 +439,11 @@ fetch_elfcore_registers (struct regcache *regcache,
> >       break;
> >
> >     case 2:
> > -      if (core_reg_size != sizeof (struct fpreg))
> > -     warning (_("wrong size of FPA 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 (&fparegset, core_reg_sect, sizeof (fparegset));
> > -       arm_supply_fparegset (regcache, &fparegset);
> > -     }
> > +      /* 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:
> > --
> > 2.25.0.265.gbab2e86ba0-goog
> >
>
Kamil Rytarowski March 2, 2020, 5:30 p.m. UTC | #3
On 02.03.2020 18:28, Christian Biesinger via gdb-patches wrote:
> On Mon, Mar 2, 2020 at 5:42 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
>>> On 28 Feb 2020, at 19:19, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
>>>
>>> Hi Alan,
>>>
>>> there is already a num_vfp_registers field in tdep, so I thought it easiest
>>> to just use that. Does that look good? See below.
>>>
>>
>> That works for the ARM_FP_TYPE_NONE case.
>>
>> I had to convince myself it works for the aarch32_read_description case.
>> And after thinking it through, I’m happy because both have the same size
>> FP register banks.
>>
>> So, ok to push.
> 
> Thanks! Pushing now with a minor change in {fetch,store}_fp_register
> to not fetch/store ARM_FPSCR_REGNUM if tdep->vfp_register_count == 0.
> 

Thank you! Great work.

> Christian
> 
>>> The floating point register interface has changed to this:
>>> https://github.com/NetBSD/src/blob/trunk/sys/arch/arm/include/reg.h
>>>
>>> It now uses VFP instead of FPA registers. This patch updates
>>> arm-nbsd-nat.c accordingly.
>>>
>>> Also implements read_description so that these registers are correctly
>>> printed by "info registers" et al.
>>>
>>> Tested by compiling & running on arm-netbsd on qemu.
>>>
>>> gdb/ChangeLog:
>>>
>>> 2020-02-11  Christian Biesinger  <cbiesinger@google.com>
>>>
>>>       * arm-nbsd-nat.c (arm_supply_fparegset): Rename to...
>>>       (arm_supply_vfpregset): ...this, and update to use VFP registers.
>>>       (fetch_fp_register): Update.
>>>       (fetch_fp_regs): Update.
>>>       (store_fp_register): Update.
>>>       (store_fp_regs): Update.
>>>       (arm_netbsd_nat_target::read_description): New function.
>>>       (fetch_elfcore_registers): Update.
>>> ---
>>> gdb/arm-nbsd-nat.c | 121 +++++++++++++++++++++++++--------------------
>>> 1 file changed, 68 insertions(+), 53 deletions(-)
>>>
>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
>>> index 11afc289c3..cee7917db2 100644
>>> --- a/gdb/arm-nbsd-nat.c
>>> +++ b/gdb/arm-nbsd-nat.c
>>> @@ -26,10 +26,12 @@
>>> #include "target.h"
>>> #include <sys/types.h>
>>> #include <sys/ptrace.h>
>>> +#include <sys/sysctl.h>
>>> #include <machine/reg.h>
>>> #include <machine/frame.h>
>>>
>>> #include "arm-tdep.h"
>>> +#include "aarch32-tdep.h"
>>> #include "inf-ptrace.h"
>>>
>>> class arm_netbsd_nat_target final : public inf_ptrace_target
>>> @@ -38,6 +40,7 @@ public:
>>>   /* Add our register access methods.  */
>>>   void fetch_registers (struct regcache *, int) override;
>>>   void store_registers (struct regcache *, int) override;
>>> +  const struct target_desc *read_description () override;
>>> };
>>>
>>> static arm_netbsd_nat_target the_arm_netbsd_nat_target;
>>> @@ -65,15 +68,17 @@ arm_supply_gregset (struct regcache *regcache, struct reg *gregset)
>>> }
>>>
>>> static void
>>> -arm_supply_fparegset (struct regcache *regcache, struct fpreg *fparegset)
>>> +arm_supply_vfpregset (struct regcache *regcache, struct fpreg *fpregset)
>>> {
>>> -  int regno;
>>> +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
>>> +  if (tdep->vfp_register_count == 0)
>>> +    return;
>>>
>>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
>>> -    regcache->raw_supply (regno,
>>> -                       (char *) &fparegset->fpr[regno - ARM_F0_REGNUM]);
>>> +  struct vfpreg &vfp = fpregset->fpr_vfp;
>>> +  for (int regno = 0; regno <= tdep->vfp_register_count; regno++)
>>> +    regcache->raw_supply (regno + ARM_D0_REGNUM, (char *) &vfp.vfp_regs[regno]);
>>>
>>> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
>>> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>> }
>>>
>>> static void
>>> @@ -147,10 +152,10 @@ static void
>>> fetch_fp_register (struct regcache *regcache, int regno)
>>> {
>>>   struct fpreg inferior_fp_registers;
>>> -  int ret;
>>> +  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
>>> +                 (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>>
>>> -  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
>>> -             (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>> +  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
>>>
>>>   if (ret < 0)
>>>     {
>>> @@ -158,18 +163,17 @@ fetch_fp_register (struct regcache *regcache, int regno)
>>>       return;
>>>     }
>>>
>>> -  switch (regno)
>>> +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
>>> +  if (regno == ARM_FPSCR_REGNUM)
>>> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>> +  else if (regno >= ARM_D0_REGNUM
>>> +        && regno <= ARM_D0_REGNUM + tdep->vfp_register_count)
>>>     {
>>> -    case ARM_FPS_REGNUM:
>>> -      regcache->raw_supply (ARM_FPS_REGNUM,
>>> -                         (char *) &inferior_fp_registers.fpr_fpsr);
>>> -      break;
>>> -
>>> -    default:
>>> -      regcache->raw_supply
>>> -     (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>>> -      break;
>>> +      regcache->raw_supply (regno,
>>> +                         (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>     }
>>> +  else
>>> +    warning (_("Invalid register number."));
>>> }
>>>
>>> static void
>>> @@ -188,7 +192,7 @@ fetch_fp_regs (struct regcache *regcache)
>>>       return;
>>>     }
>>>
>>> -  arm_supply_fparegset (regcache, &inferior_fp_registers);
>>> +  arm_supply_vfpregset (regcache, &inferior_fp_registers);
>>> }
>>>
>>> void
>>> @@ -327,10 +331,9 @@ static void
>>> store_fp_register (const struct regcache *regcache, int regno)
>>> {
>>>   struct fpreg inferior_fp_registers;
>>> -  int ret;
>>> -
>>> -  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
>>> -             (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>> +  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
>>> +                 (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>> +  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
>>>
>>>   if (ret < 0)
>>>     {
>>> @@ -338,18 +341,17 @@ store_fp_register (const struct regcache *regcache, int regno)
>>>       return;
>>>     }
>>>
>>> -  switch (regno)
>>> +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
>>> +  if (regno == ARM_FPSCR_REGNUM)
>>> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>> +  else if (regno >= ARM_D0_REGNUM
>>> +        && regno <= ARM_D0_REGNUM + tdep->vfp_register_count)
>>>     {
>>> -    case ARM_FPS_REGNUM:
>>> -      regcache->raw_collect (ARM_FPS_REGNUM,
>>> -                          (char *) &inferior_fp_registers.fpr_fpsr);
>>> -      break;
>>> -
>>> -    default:
>>> -      regcache->raw_collect
>>> -     (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>>> -      break;
>>> +      regcache->raw_collect (regno,
>>> +                          (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>     }
>>> +  else
>>> +    warning (_("Invalid register number."));
>>>
>>>   ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
>>>               (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>> @@ -361,20 +363,20 @@ store_fp_register (const struct regcache *regcache, int regno)
>>> static void
>>> store_fp_regs (const struct regcache *regcache)
>>> {
>>> -  struct fpreg inferior_fp_registers;
>>> -  int ret;
>>> -  int regno;
>>> -
>>> +  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
>>> +  if (tdep->vfp_register_count == 0)
>>> +    return;
>>>
>>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
>>> +  struct fpreg fpregs;
>>> +  for (int regno = 0; regno <= tdep->vfp_register_count; regno++)
>>>     regcache->raw_collect
>>> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>>> +      (regno + ARM_D0_REGNUM, (char *) &fpregs.fpr_vfp.vfp_regs[regno]);
>>>
>>> -  regcache->raw_collect (ARM_FPS_REGNUM,
>>> -                      (char *) &inferior_fp_registers.fpr_fpsr);
>>> +  regcache->raw_collect (ARM_FPSCR_REGNUM,
>>> +                      (char *) &fpregs.fpr_vfp.vfp_fpscr);
>>>
>>> -  ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
>>> -             (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
>>> +  int ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
>>> +                 (PTRACE_TYPE_ARG3) &fpregs, 0);
>>>
>>>   if (ret < 0)
>>>     warning (_("unable to store floating-point registers"));
>>> @@ -397,6 +399,23 @@ arm_netbsd_nat_target::store_registers (struct regcache *regcache, int regno)
>>>     }
>>> }
>>>
>>> +const struct target_desc *
>>> +arm_netbsd_nat_target::read_description ()
>>> +{
>>> +  int flag;
>>> +  size_t len = sizeof (flag);
>>> +
>>> +  if (sysctlbyname("machdep.fpu_present", &flag, &len, NULL, 0) != 0
>>> +      || !flag)
>>> +    return arm_read_description (ARM_FP_TYPE_NONE);
>>> +
>>> +  len = sizeof(flag);
>>> +  if (sysctlbyname("machdep.neon_present", &flag, &len, NULL, 0) == 0 && flag)
>>> +    return aarch32_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,
>>> @@ -420,15 +439,11 @@ fetch_elfcore_registers (struct regcache *regcache,
>>>       break;
>>>
>>>     case 2:
>>> -      if (core_reg_size != sizeof (struct fpreg))
>>> -     warning (_("wrong size of FPA 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 (&fparegset, core_reg_sect, sizeof (fparegset));
>>> -       arm_supply_fparegset (regcache, &fparegset);
>>> -     }
>>> +      /* 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:
>>> --
>>> 2.25.0.265.gbab2e86ba0-goog
>>>
>>
diff mbox

Patch

diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
index 11afc289c3..cee7917db2 100644
--- a/gdb/arm-nbsd-nat.c
+++ b/gdb/arm-nbsd-nat.c
@@ -26,10 +26,12 @@ 
 #include "target.h"
 #include <sys/types.h>
 #include <sys/ptrace.h>
+#include <sys/sysctl.h>
 #include <machine/reg.h>
 #include <machine/frame.h>
 
 #include "arm-tdep.h"
+#include "aarch32-tdep.h"
 #include "inf-ptrace.h"
 
 class arm_netbsd_nat_target final : public inf_ptrace_target
@@ -38,6 +40,7 @@  public:
   /* Add our register access methods.  */
   void fetch_registers (struct regcache *, int) override;
   void store_registers (struct regcache *, int) override;
+  const struct target_desc *read_description () override;
 };
 
 static arm_netbsd_nat_target the_arm_netbsd_nat_target;
@@ -65,15 +68,17 @@  arm_supply_gregset (struct regcache *regcache, struct reg *gregset)
 }
 
 static void
-arm_supply_fparegset (struct regcache *regcache, struct fpreg *fparegset)
+arm_supply_vfpregset (struct regcache *regcache, struct fpreg *fpregset)
 {
-  int regno;
+  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
+  if (tdep->vfp_register_count == 0)
+    return;
 
-  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
-    regcache->raw_supply (regno,
-			  (char *) &fparegset->fpr[regno - ARM_F0_REGNUM]);
+  struct vfpreg &vfp = fpregset->fpr_vfp;
+  for (int regno = 0; regno <= tdep->vfp_register_count; regno++)
+    regcache->raw_supply (regno + ARM_D0_REGNUM, (char *) &vfp.vfp_regs[regno]);
 
-  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
+  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
 }
 
 static void
@@ -147,10 +152,10 @@  static void
 fetch_fp_register (struct regcache *regcache, int regno)
 {
   struct fpreg inferior_fp_registers;
-  int ret;
+  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
+		    (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
 
-  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
-		(PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
+  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
 
   if (ret < 0)
     {
@@ -158,18 +163,17 @@  fetch_fp_register (struct regcache *regcache, int regno)
       return;
     }
 
-  switch (regno)
+  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
+  if (regno == ARM_FPSCR_REGNUM)
+    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
+  else if (regno >= ARM_D0_REGNUM
+	   && regno <= ARM_D0_REGNUM + tdep->vfp_register_count)
     {
-    case ARM_FPS_REGNUM:
-      regcache->raw_supply (ARM_FPS_REGNUM,
-			    (char *) &inferior_fp_registers.fpr_fpsr);
-      break;
-
-    default:
-      regcache->raw_supply
-	(regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
-      break;
+      regcache->raw_supply (regno,
+			    (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
     }
+  else
+    warning (_("Invalid register number."));
 }
 
 static void
@@ -188,7 +192,7 @@  fetch_fp_regs (struct regcache *regcache)
       return;
     }
 
-  arm_supply_fparegset (regcache, &inferior_fp_registers);
+  arm_supply_vfpregset (regcache, &inferior_fp_registers);
 }
 
 void
@@ -327,10 +331,9 @@  static void
 store_fp_register (const struct regcache *regcache, int regno)
 {
   struct fpreg inferior_fp_registers;
-  int ret;
-
-  ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
-		(PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
+  int ret = ptrace (PT_GETFPREGS, regcache->ptid ().pid (),
+		    (PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
+  struct vfpreg &vfp = inferior_fp_registers.fpr_vfp;
 
   if (ret < 0)
     {
@@ -338,18 +341,17 @@  store_fp_register (const struct regcache *regcache, int regno)
       return;
     }
 
-  switch (regno)
+  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
+  if (regno == ARM_FPSCR_REGNUM)
+    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
+  else if (regno >= ARM_D0_REGNUM
+	   && regno <= ARM_D0_REGNUM + tdep->vfp_register_count)
     {
-    case ARM_FPS_REGNUM:
-      regcache->raw_collect (ARM_FPS_REGNUM,
-			     (char *) &inferior_fp_registers.fpr_fpsr);
-      break;
-
-    default:
-      regcache->raw_collect
-	(regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
-      break;
+      regcache->raw_collect (regno,
+			     (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
     }
+  else
+    warning (_("Invalid register number."));
 
   ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
 		(PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
@@ -361,20 +363,20 @@  store_fp_register (const struct regcache *regcache, int regno)
 static void
 store_fp_regs (const struct regcache *regcache)
 {
-  struct fpreg inferior_fp_registers;
-  int ret;
-  int regno;
-
+  struct gdbarch_tdep *tdep = gdbarch_tdep (regcache->arch ());
+  if (tdep->vfp_register_count == 0)
+    return;
 
-  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
+  struct fpreg fpregs;
+  for (int regno = 0; regno <= tdep->vfp_register_count; regno++)
     regcache->raw_collect
-      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
+      (regno + ARM_D0_REGNUM, (char *) &fpregs.fpr_vfp.vfp_regs[regno]);
 
-  regcache->raw_collect (ARM_FPS_REGNUM,
-			 (char *) &inferior_fp_registers.fpr_fpsr);
+  regcache->raw_collect (ARM_FPSCR_REGNUM,
+			 (char *) &fpregs.fpr_vfp.vfp_fpscr);
 
-  ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
-		(PTRACE_TYPE_ARG3) &inferior_fp_registers, 0);
+  int ret = ptrace (PT_SETFPREGS, regcache->ptid ().pid (),
+		    (PTRACE_TYPE_ARG3) &fpregs, 0);
 
   if (ret < 0)
     warning (_("unable to store floating-point registers"));
@@ -397,6 +399,23 @@  arm_netbsd_nat_target::store_registers (struct regcache *regcache, int regno)
     }
 }
 
+const struct target_desc *
+arm_netbsd_nat_target::read_description ()
+{
+  int flag;
+  size_t len = sizeof (flag);
+
+  if (sysctlbyname("machdep.fpu_present", &flag, &len, NULL, 0) != 0
+      || !flag)
+    return arm_read_description (ARM_FP_TYPE_NONE);
+
+  len = sizeof(flag);
+  if (sysctlbyname("machdep.neon_present", &flag, &len, NULL, 0) == 0 && flag)
+    return aarch32_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,
@@ -420,15 +439,11 @@  fetch_elfcore_registers (struct regcache *regcache,
       break;
 
     case 2:
-      if (core_reg_size != sizeof (struct fpreg))
-	warning (_("wrong size of FPA 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 (&fparegset, core_reg_sect, sizeof (fparegset));
-	  arm_supply_fparegset (regcache, &fparegset);
-	}
+      /* 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: