public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix arm-netbsd build error
@ 2020-02-11 22:55 Christian Biesinger via gdb-patches
  2020-02-11 23:35 ` Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-02-11 22:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger

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.

Tested by compiling on arm-netbsd on qemu. For actually testing, there
seems to be something missing as "info registers" only shows FPA
registers and no VFP ones. I am still investigating why this is;
please let me know if you know. However, I think this is still good
to check in as-is.

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.
	(fetch_elfcore_registers): Update.
---
 gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 46 deletions(-)

diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
index 11afc289c3..8027f54dfe 100644
--- a/gdb/arm-nbsd-nat.c
+++ b/gdb/arm-nbsd-nat.c
@@ -65,15 +65,13 @@ 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;
-
-  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
+    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
 
-  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
+  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
 }
 
 static void
@@ -147,10 +145,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 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
       return;
     }
 
-  switch (regno)
+  if (regno == ARM_FPSCR_REGNUM)
+    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
+  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
     {
-    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 +183,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 +322,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 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
       return;
     }
 
-  switch (regno)
+  if (regno == ARM_FPSCR_REGNUM)
+    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
+  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
     {
-    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 +352,17 @@ 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 fpreg fpregs;
 
-
-  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
+  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
     regcache->raw_collect
-      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
+      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
 
-  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"));
@@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
 	  /* 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);
+	  arm_supply_vfpregset (regcache, &fparegset);
 	}
       break;
 
-- 
2.25.0.225.g125e21ebc7-goog

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

* Re: [PATCH] Fix arm-netbsd build error
  2020-02-11 22:55 [PATCH] Fix arm-netbsd build error Christian Biesinger via gdb-patches
@ 2020-02-11 23:35 ` Christian Biesinger via gdb-patches
  2020-02-12 13:30   ` Alan Hayward
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-02-11 23:35 UTC (permalink / raw)
  To: gdb-patches

On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> 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.
>
> Tested by compiling on arm-netbsd on qemu. For actually testing, there
> seems to be something missing as "info registers" only shows FPA
> registers and no VFP ones. I am still investigating why this is;
> please let me know if you know. However, I think this is still good
> to check in as-is.

Hm... this is perhaps because arm_netbsd_nat_target does not implement
read_description; if it returned arm_read_description
(ARM_FP_TYPE_VFPV2) this may work?

>
> 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.
>         (fetch_elfcore_registers): Update.
> ---
>  gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
>  1 file changed, 34 insertions(+), 46 deletions(-)
>
> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> index 11afc289c3..8027f54dfe 100644
> --- a/gdb/arm-nbsd-nat.c
> +++ b/gdb/arm-nbsd-nat.c
> @@ -65,15 +65,13 @@ 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;
> -
> -  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>
> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>  }
>
>  static void
> @@ -147,10 +145,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 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
>        return;
>      }
>
> -  switch (regno)
> +  if (regno == ARM_FPSCR_REGNUM)
> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>      {
> -    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 +183,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 +322,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 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
>        return;
>      }
>
> -  switch (regno)
> +  if (regno == ARM_FPSCR_REGNUM)
> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>      {
> -    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 +352,17 @@ 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 fpreg fpregs;
>
> -
> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>      regcache->raw_collect
> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>
> -  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"));
> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
>           /* 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);
> +         arm_supply_vfpregset (regcache, &fparegset);
>         }
>        break;
>
> --
> 2.25.0.225.g125e21ebc7-goog
>

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

* Re: [PATCH] Fix arm-netbsd build error
  2020-02-11 23:35 ` Christian Biesinger via gdb-patches
@ 2020-02-12 13:30   ` Alan Hayward
  2020-02-12 16:29     ` Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Hayward @ 2020-02-12 13:30 UTC (permalink / raw)
  To: Christian Biesinger; +Cc: gdb-patches, nd



> On 11 Feb 2020, at 23:34, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
> 
> On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
> <cbiesinger@google.com> wrote:
>> 
>> 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.
>> 
>> Tested by compiling on arm-netbsd on qemu. For actually testing, there
>> seems to be something missing as "info registers" only shows FPA
>> registers and no VFP ones. I am still investigating why this is;
>> please let me know if you know. However, I think this is still good
>> to check in as-is.
> 
> Hm... this is perhaps because arm_netbsd_nat_target does not implement
> read_description; if it returned arm_read_description
> (ARM_FP_TYPE_VFPV2) this may work?
> 

Yes, looks like netbsd isn’t using any target description functionality.

I suspect the code is getting into arm_gdbarch_init() with a null tdesc,
and then using the AUTO setting. But that’s just a guess.

Implementing read_description as you suggest should help. However,
read_description should probably do HWCAP checking similar to the
arm_linux_nat_target and arm_fbsd_nat_target versions.

Without that, I’d worry that your patch below might start writing off the
end of the regcache that had been allocated for a fewer number of registers.

>> 
>> 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.
>>        (fetch_elfcore_registers): Update.
>> ---
>> gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
>> 1 file changed, 34 insertions(+), 46 deletions(-)
>> 
>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
>> index 11afc289c3..8027f54dfe 100644
>> --- a/gdb/arm-nbsd-nat.c
>> +++ b/gdb/arm-nbsd-nat.c
>> @@ -65,15 +65,13 @@ 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;
>> -
>> -  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>> 
>> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
>> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>> }
>> 
>> static void
>> @@ -147,10 +145,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 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
>>       return;
>>     }
>> 
>> -  switch (regno)
>> +  if (regno == ARM_FPSCR_REGNUM)
>> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>     {
>> -    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 +183,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 +322,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 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
>>       return;
>>     }
>> 
>> -  switch (regno)
>> +  if (regno == ARM_FPSCR_REGNUM)
>> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>     {
>> -    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 +352,17 @@ 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 fpreg fpregs;
>> 
>> -
>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>>     regcache->raw_collect
>> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>> 
>> -  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"));
>> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
>>          /* 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);
>> +         arm_supply_vfpregset (regcache, &fparegset);
>>        }
>>       break;
>> 
>> --
>> 2.25.0.225.g125e21ebc7-goog
>> 


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

* Re: [PATCH] Fix arm-netbsd build error
  2020-02-12 13:30   ` Alan Hayward
@ 2020-02-12 16:29     ` Christian Biesinger via gdb-patches
  2020-02-12 17:10       ` Kamil Rytarowski
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-02-12 16:29 UTC (permalink / raw)
  To: Alan Hayward, Kamil Rytarowski; +Cc: gdb-patches, nd

On Wed, Feb 12, 2020 at 7:29 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
>
>
>
> > On 11 Feb 2020, at 23:34, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
> >
> > On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
> > <cbiesinger@google.com> wrote:
> >>
> >> 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.
> >>
> >> Tested by compiling on arm-netbsd on qemu. For actually testing, there
> >> seems to be something missing as "info registers" only shows FPA
> >> registers and no VFP ones. I am still investigating why this is;
> >> please let me know if you know. However, I think this is still good
> >> to check in as-is.
> >
> > Hm... this is perhaps because arm_netbsd_nat_target does not implement
> > read_description; if it returned arm_read_description
> > (ARM_FP_TYPE_VFPV2) this may work?
> >
>
> Yes, looks like netbsd isn’t using any target description functionality.
>
> I suspect the code is getting into arm_gdbarch_init() with a null tdesc,
> and then using the AUTO setting. But that’s just a guess.
>
> Implementing read_description as you suggest should help. However,
> read_description should probably do HWCAP checking similar to the
> arm_linux_nat_target and arm_fbsd_nat_target versions.
>
> Without that, I’d worry that your patch below might start writing off the
> end of the regcache that had been allocated for a fewer number of registers.

Hm... well, I've probably spent entirely too much time on this
already, but just in case there's an easy fix -- Kamil, does NetBSD
provide an API similar to Linux's HWCAP API that would let me check
which VFP version the current CPU is using? It seems AUXV does not
contain HWCAP data on NetBSD, unlike Linux/FreeBSD, though I may be
missing something.

(Compare arm_fbsd_read_description_auxv /
arm_linux_nat_target::read_description)

Christian

>
> >>
> >> 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.
> >>        (fetch_elfcore_registers): Update.
> >> ---
> >> gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
> >> 1 file changed, 34 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> >> index 11afc289c3..8027f54dfe 100644
> >> --- a/gdb/arm-nbsd-nat.c
> >> +++ b/gdb/arm-nbsd-nat.c
> >> @@ -65,15 +65,13 @@ 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;
> >> -
> >> -  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> >> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> >>
> >> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
> >> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> >> }
> >>
> >> static void
> >> @@ -147,10 +145,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 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
> >>       return;
> >>     }
> >>
> >> -  switch (regno)
> >> +  if (regno == ARM_FPSCR_REGNUM)
> >> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> >> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
> >>     {
> >> -    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 +183,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 +322,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 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
> >>       return;
> >>     }
> >>
> >> -  switch (regno)
> >> +  if (regno == ARM_FPSCR_REGNUM)
> >> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> >> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
> >>     {
> >> -    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 +352,17 @@ 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 fpreg fpregs;
> >>
> >> -
> >> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
> >> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> >>     regcache->raw_collect
> >> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> >> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> >>
> >> -  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"));
> >> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
> >>          /* 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);
> >> +         arm_supply_vfpregset (regcache, &fparegset);
> >>        }
> >>       break;
> >>
> >> --
> >> 2.25.0.225.g125e21ebc7-goog
> >>
>

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

* Re: [PATCH] Fix arm-netbsd build error
  2020-02-12 16:29     ` Christian Biesinger via gdb-patches
@ 2020-02-12 17:10       ` Kamil Rytarowski
  2020-02-12 17:17         ` Kamil Rytarowski
  0 siblings, 1 reply; 15+ messages in thread
From: Kamil Rytarowski @ 2020-02-12 17:10 UTC (permalink / raw)
  To: Christian Biesinger, Alan Hayward; +Cc: gdb-patches, nd


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

On 12.02.2020 17:28, Christian Biesinger wrote:
> On Wed, Feb 12, 2020 at 7:29 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
>>
>>
>>
>>> On 11 Feb 2020, at 23:34, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
>>>
>>> On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
>>> <cbiesinger@google.com> wrote:
>>>>
>>>> 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.
>>>>
>>>> Tested by compiling on arm-netbsd on qemu. For actually testing, there
>>>> seems to be something missing as "info registers" only shows FPA
>>>> registers and no VFP ones. I am still investigating why this is;
>>>> please let me know if you know. However, I think this is still good
>>>> to check in as-is.
>>>
>>> Hm... this is perhaps because arm_netbsd_nat_target does not implement
>>> read_description; if it returned arm_read_description
>>> (ARM_FP_TYPE_VFPV2) this may work?
>>>
>>
>> Yes, looks like netbsd isn’t using any target description functionality.
>>
>> I suspect the code is getting into arm_gdbarch_init() with a null tdesc,
>> and then using the AUTO setting. But that’s just a guess.
>>
>> Implementing read_description as you suggest should help. However,
>> read_description should probably do HWCAP checking similar to the
>> arm_linux_nat_target and arm_fbsd_nat_target versions.
>>
>> Without that, I’d worry that your patch below might start writing off the
>> end of the regcache that had been allocated for a fewer number of registers.
> 
> Hm... well, I've probably spent entirely too much time on this
> already, but just in case there's an easy fix -- Kamil, does NetBSD
> provide an API similar to Linux's HWCAP API that would let me check
> which VFP version the current CPU is using? It seems AUXV does not
> contain HWCAP data on NetBSD, unlike Linux/FreeBSD, though I may be
> missing something.
> 
> (Compare arm_fbsd_read_description_auxv /
> arm_linux_nat_target::read_description)
> 
> Christian
> 

Thank you for your work.

HWCAP in auxv is not supported (although there is a stub for it).

Here is a complete algorithm to detect FPU on ARM 32-bit.

http://netbsd.org/~kamil/arm-fpu.c

>>
>>>>
>>>> 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.
>>>>        (fetch_elfcore_registers): Update.
>>>> ---
>>>> gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
>>>> 1 file changed, 34 insertions(+), 46 deletions(-)
>>>>
>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
>>>> index 11afc289c3..8027f54dfe 100644
>>>> --- a/gdb/arm-nbsd-nat.c
>>>> +++ b/gdb/arm-nbsd-nat.c
>>>> @@ -65,15 +65,13 @@ 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;
>>>> -
>>>> -  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>>>> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>
>>>> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
>>>> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>> }
>>>>
>>>> static void
>>>> @@ -147,10 +145,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 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
>>>>       return;
>>>>     }
>>>>
>>>> -  switch (regno)
>>>> +  if (regno == ARM_FPSCR_REGNUM)
>>>> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>>>     {
>>>> -    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 +183,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 +322,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 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
>>>>       return;
>>>>     }
>>>>
>>>> -  switch (regno)
>>>> +  if (regno == ARM_FPSCR_REGNUM)
>>>> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>>>     {
>>>> -    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 +352,17 @@ 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 fpreg fpregs;
>>>>
>>>> -
>>>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
>>>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>>>>     regcache->raw_collect
>>>> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>>>> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>
>>>> -  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"));
>>>> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
>>>>          /* 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);
>>>> +         arm_supply_vfpregset (regcache, &fparegset);
>>>>        }
>>>>       break;
>>>>
>>>> --
>>>> 2.25.0.225.g125e21ebc7-goog
>>>>
>>



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

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

* Re: [PATCH] Fix arm-netbsd build error
  2020-02-12 17:10       ` Kamil Rytarowski
@ 2020-02-12 17:17         ` Kamil Rytarowski
  2020-02-12 23:28           ` Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Kamil Rytarowski @ 2020-02-12 17:17 UTC (permalink / raw)
  To: Christian Biesinger, Alan Hayward; +Cc: gdb-patches, nd


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

On 12.02.2020 18:09, Kamil Rytarowski wrote:
> On 12.02.2020 17:28, Christian Biesinger wrote:
>> On Wed, Feb 12, 2020 at 7:29 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
>>>
>>>
>>>
>>>> On 11 Feb 2020, at 23:34, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
>>>>
>>>> On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
>>>> <cbiesinger@google.com> wrote:
>>>>>
>>>>> 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.
>>>>>
>>>>> Tested by compiling on arm-netbsd on qemu. For actually testing, there
>>>>> seems to be something missing as "info registers" only shows FPA
>>>>> registers and no VFP ones. I am still investigating why this is;
>>>>> please let me know if you know. However, I think this is still good
>>>>> to check in as-is.
>>>>
>>>> Hm... this is perhaps because arm_netbsd_nat_target does not implement
>>>> read_description; if it returned arm_read_description
>>>> (ARM_FP_TYPE_VFPV2) this may work?
>>>>
>>>
>>> Yes, looks like netbsd isn’t using any target description functionality.
>>>
>>> I suspect the code is getting into arm_gdbarch_init() with a null tdesc,
>>> and then using the AUTO setting. But that’s just a guess.
>>>
>>> Implementing read_description as you suggest should help. However,
>>> read_description should probably do HWCAP checking similar to the
>>> arm_linux_nat_target and arm_fbsd_nat_target versions.
>>>
>>> Without that, I’d worry that your patch below might start writing off the
>>> end of the regcache that had been allocated for a fewer number of registers.
>>
>> Hm... well, I've probably spent entirely too much time on this
>> already, but just in case there's an easy fix -- Kamil, does NetBSD
>> provide an API similar to Linux's HWCAP API that would let me check
>> which VFP version the current CPU is using? It seems AUXV does not
>> contain HWCAP data on NetBSD, unlike Linux/FreeBSD, though I may be
>> missing something.
>>
>> (Compare arm_fbsd_read_description_auxv /
>> arm_linux_nat_target::read_description)
>>
>> Christian
>>
> 
> Thank you for your work.
> 
> HWCAP in auxv is not supported (although there is a stub for it).
> 
> Here is a complete algorithm to detect FPU on ARM 32-bit.
> 
> http://netbsd.org/~kamil/arm-fpu.c
> 

Here is another example with SIMD:

http://cvsweb.netbsd.org/bsdweb.cgi/xsrc/external/mit/pixman/dist/pixman/pixman-arm.c.diff?r1=1.1&r2=1.2&f=h

>>>
>>>>>
>>>>> 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.
>>>>>        (fetch_elfcore_registers): Update.
>>>>> ---
>>>>> gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
>>>>> 1 file changed, 34 insertions(+), 46 deletions(-)
>>>>>
>>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
>>>>> index 11afc289c3..8027f54dfe 100644
>>>>> --- a/gdb/arm-nbsd-nat.c
>>>>> +++ b/gdb/arm-nbsd-nat.c
>>>>> @@ -65,15 +65,13 @@ 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;
>>>>> -
>>>>> -  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>>>>> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>>
>>>>> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
>>>>> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>>> }
>>>>>
>>>>> static void
>>>>> @@ -147,10 +145,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 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
>>>>>       return;
>>>>>     }
>>>>>
>>>>> -  switch (regno)
>>>>> +  if (regno == ARM_FPSCR_REGNUM)
>>>>> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>>>>     {
>>>>> -    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 +183,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 +322,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 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
>>>>>       return;
>>>>>     }
>>>>>
>>>>> -  switch (regno)
>>>>> +  if (regno == ARM_FPSCR_REGNUM)
>>>>> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>>>>     {
>>>>> -    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 +352,17 @@ 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 fpreg fpregs;
>>>>>
>>>>> -
>>>>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
>>>>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>>>>>     regcache->raw_collect
>>>>> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>>>>> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>>
>>>>> -  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"));
>>>>> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
>>>>>          /* 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);
>>>>> +         arm_supply_vfpregset (regcache, &fparegset);
>>>>>        }
>>>>>       break;
>>>>>
>>>>> --
>>>>> 2.25.0.225.g125e21ebc7-goog
>>>>>
>>>
> 
> 



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

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

* Re: [PATCH] Fix arm-netbsd build error
  2020-02-12 17:17         ` Kamil Rytarowski
@ 2020-02-12 23:28           ` Christian Biesinger via gdb-patches
  2020-02-12 23:43             ` Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-02-12 23:28 UTC (permalink / raw)
  To: Kamil Rytarowski; +Cc: Alan Hayward, gdb-patches, nd

Thanks, Kamil!

Please forgive the stupid question, but is SIMD in this context the
same as IWMMXT?

Christian

On Wed, Feb 12, 2020 at 11:16 AM Kamil Rytarowski <n54@gmx.com> wrote:
>
> On 12.02.2020 18:09, Kamil Rytarowski wrote:
> > On 12.02.2020 17:28, Christian Biesinger wrote:
> >> On Wed, Feb 12, 2020 at 7:29 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
> >>>
> >>>
> >>>
> >>>> On 11 Feb 2020, at 23:34, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
> >>>>
> >>>> On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
> >>>> <cbiesinger@google.com> wrote:
> >>>>>
> >>>>> 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.
> >>>>>
> >>>>> Tested by compiling on arm-netbsd on qemu. For actually testing, there
> >>>>> seems to be something missing as "info registers" only shows FPA
> >>>>> registers and no VFP ones. I am still investigating why this is;
> >>>>> please let me know if you know. However, I think this is still good
> >>>>> to check in as-is.
> >>>>
> >>>> Hm... this is perhaps because arm_netbsd_nat_target does not implement
> >>>> read_description; if it returned arm_read_description
> >>>> (ARM_FP_TYPE_VFPV2) this may work?
> >>>>
> >>>
> >>> Yes, looks like netbsd isn’t using any target description functionality.
> >>>
> >>> I suspect the code is getting into arm_gdbarch_init() with a null tdesc,
> >>> and then using the AUTO setting. But that’s just a guess.
> >>>
> >>> Implementing read_description as you suggest should help. However,
> >>> read_description should probably do HWCAP checking similar to the
> >>> arm_linux_nat_target and arm_fbsd_nat_target versions.
> >>>
> >>> Without that, I’d worry that your patch below might start writing off the
> >>> end of the regcache that had been allocated for a fewer number of registers.
> >>
> >> Hm... well, I've probably spent entirely too much time on this
> >> already, but just in case there's an easy fix -- Kamil, does NetBSD
> >> provide an API similar to Linux's HWCAP API that would let me check
> >> which VFP version the current CPU is using? It seems AUXV does not
> >> contain HWCAP data on NetBSD, unlike Linux/FreeBSD, though I may be
> >> missing something.
> >>
> >> (Compare arm_fbsd_read_description_auxv /
> >> arm_linux_nat_target::read_description)
> >>
> >> Christian
> >>
> >
> > Thank you for your work.
> >
> > HWCAP in auxv is not supported (although there is a stub for it).
> >
> > Here is a complete algorithm to detect FPU on ARM 32-bit.
> >
> > http://netbsd.org/~kamil/arm-fpu.c
> >
>
> Here is another example with SIMD:
>
> http://cvsweb.netbsd.org/bsdweb.cgi/xsrc/external/mit/pixman/dist/pixman/pixman-arm.c.diff?r1=1.1&r2=1.2&f=h
>
> >>>
> >>>>>
> >>>>> 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.
> >>>>>        (fetch_elfcore_registers): Update.
> >>>>> ---
> >>>>> gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
> >>>>> 1 file changed, 34 insertions(+), 46 deletions(-)
> >>>>>
> >>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> >>>>> index 11afc289c3..8027f54dfe 100644
> >>>>> --- a/gdb/arm-nbsd-nat.c
> >>>>> +++ b/gdb/arm-nbsd-nat.c
> >>>>> @@ -65,15 +65,13 @@ 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;
> >>>>> -
> >>>>> -  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> >>>>> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> >>>>>
> >>>>> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
> >>>>> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> >>>>> }
> >>>>>
> >>>>> static void
> >>>>> @@ -147,10 +145,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 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
> >>>>>       return;
> >>>>>     }
> >>>>>
> >>>>> -  switch (regno)
> >>>>> +  if (regno == ARM_FPSCR_REGNUM)
> >>>>> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> >>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
> >>>>>     {
> >>>>> -    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 +183,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 +322,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 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
> >>>>>       return;
> >>>>>     }
> >>>>>
> >>>>> -  switch (regno)
> >>>>> +  if (regno == ARM_FPSCR_REGNUM)
> >>>>> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> >>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
> >>>>>     {
> >>>>> -    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 +352,17 @@ 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 fpreg fpregs;
> >>>>>
> >>>>> -
> >>>>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
> >>>>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> >>>>>     regcache->raw_collect
> >>>>> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> >>>>> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> >>>>>
> >>>>> -  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"));
> >>>>> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
> >>>>>          /* 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);
> >>>>> +         arm_supply_vfpregset (regcache, &fparegset);
> >>>>>        }
> >>>>>       break;
> >>>>>
> >>>>> --
> >>>>> 2.25.0.225.g125e21ebc7-goog
> >>>>>
> >>>
> >
> >
>
>

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

* Re: [PATCH] Fix arm-netbsd build error
  2020-02-12 23:28           ` Christian Biesinger via gdb-patches
@ 2020-02-12 23:43             ` Christian Biesinger via gdb-patches
  2020-02-13  0:01               ` Kamil Rytarowski
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-02-12 23:43 UTC (permalink / raw)
  To: Kamil Rytarowski; +Cc: Alan Hayward, gdb-patches, nd

And one more question -- since VFP 1 is deprecated (not even mentioned
on http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0472j/chr1359124231926.html)
and your example codes don't include VFP 2, should I assume that
fpu_present implies VFP 3 and that VFPv3-D16 doesn't happen?

Thanks again,
Christian

On Wed, Feb 12, 2020 at 5:28 PM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> Thanks, Kamil!
>
> Please forgive the stupid question, but is SIMD in this context the
> same as IWMMXT?
>
> Christian
>
> On Wed, Feb 12, 2020 at 11:16 AM Kamil Rytarowski <n54@gmx.com> wrote:
> >
> > On 12.02.2020 18:09, Kamil Rytarowski wrote:
> > > On 12.02.2020 17:28, Christian Biesinger wrote:
> > >> On Wed, Feb 12, 2020 at 7:29 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
> > >>>
> > >>>
> > >>>
> > >>>> On 11 Feb 2020, at 23:34, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
> > >>>>
> > >>>> On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
> > >>>> <cbiesinger@google.com> wrote:
> > >>>>>
> > >>>>> 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.
> > >>>>>
> > >>>>> Tested by compiling on arm-netbsd on qemu. For actually testing, there
> > >>>>> seems to be something missing as "info registers" only shows FPA
> > >>>>> registers and no VFP ones. I am still investigating why this is;
> > >>>>> please let me know if you know. However, I think this is still good
> > >>>>> to check in as-is.
> > >>>>
> > >>>> Hm... this is perhaps because arm_netbsd_nat_target does not implement
> > >>>> read_description; if it returned arm_read_description
> > >>>> (ARM_FP_TYPE_VFPV2) this may work?
> > >>>>
> > >>>
> > >>> Yes, looks like netbsd isn’t using any target description functionality.
> > >>>
> > >>> I suspect the code is getting into arm_gdbarch_init() with a null tdesc,
> > >>> and then using the AUTO setting. But that’s just a guess.
> > >>>
> > >>> Implementing read_description as you suggest should help. However,
> > >>> read_description should probably do HWCAP checking similar to the
> > >>> arm_linux_nat_target and arm_fbsd_nat_target versions.
> > >>>
> > >>> Without that, I’d worry that your patch below might start writing off the
> > >>> end of the regcache that had been allocated for a fewer number of registers.
> > >>
> > >> Hm... well, I've probably spent entirely too much time on this
> > >> already, but just in case there's an easy fix -- Kamil, does NetBSD
> > >> provide an API similar to Linux's HWCAP API that would let me check
> > >> which VFP version the current CPU is using? It seems AUXV does not
> > >> contain HWCAP data on NetBSD, unlike Linux/FreeBSD, though I may be
> > >> missing something.
> > >>
> > >> (Compare arm_fbsd_read_description_auxv /
> > >> arm_linux_nat_target::read_description)
> > >>
> > >> Christian
> > >>
> > >
> > > Thank you for your work.
> > >
> > > HWCAP in auxv is not supported (although there is a stub for it).
> > >
> > > Here is a complete algorithm to detect FPU on ARM 32-bit.
> > >
> > > http://netbsd.org/~kamil/arm-fpu.c
> > >
> >
> > Here is another example with SIMD:
> >
> > http://cvsweb.netbsd.org/bsdweb.cgi/xsrc/external/mit/pixman/dist/pixman/pixman-arm.c.diff?r1=1.1&r2=1.2&f=h
> >
> > >>>
> > >>>>>
> > >>>>> 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.
> > >>>>>        (fetch_elfcore_registers): Update.
> > >>>>> ---
> > >>>>> gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
> > >>>>> 1 file changed, 34 insertions(+), 46 deletions(-)
> > >>>>>
> > >>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> > >>>>> index 11afc289c3..8027f54dfe 100644
> > >>>>> --- a/gdb/arm-nbsd-nat.c
> > >>>>> +++ b/gdb/arm-nbsd-nat.c
> > >>>>> @@ -65,15 +65,13 @@ 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;
> > >>>>> -
> > >>>>> -  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> > >>>>> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> > >>>>>
> > >>>>> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
> > >>>>> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> > >>>>> }
> > >>>>>
> > >>>>> static void
> > >>>>> @@ -147,10 +145,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 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
> > >>>>>       return;
> > >>>>>     }
> > >>>>>
> > >>>>> -  switch (regno)
> > >>>>> +  if (regno == ARM_FPSCR_REGNUM)
> > >>>>> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> > >>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
> > >>>>>     {
> > >>>>> -    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 +183,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 +322,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 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
> > >>>>>       return;
> > >>>>>     }
> > >>>>>
> > >>>>> -  switch (regno)
> > >>>>> +  if (regno == ARM_FPSCR_REGNUM)
> > >>>>> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> > >>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
> > >>>>>     {
> > >>>>> -    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 +352,17 @@ 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 fpreg fpregs;
> > >>>>>
> > >>>>> -
> > >>>>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
> > >>>>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> > >>>>>     regcache->raw_collect
> > >>>>> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> > >>>>> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> > >>>>>
> > >>>>> -  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"));
> > >>>>> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
> > >>>>>          /* 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);
> > >>>>> +         arm_supply_vfpregset (regcache, &fparegset);
> > >>>>>        }
> > >>>>>       break;
> > >>>>>
> > >>>>> --
> > >>>>> 2.25.0.225.g125e21ebc7-goog
> > >>>>>
> > >>>
> > >
> > >
> >
> >

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

* Re: [PATCH] Fix arm-netbsd build error
  2020-02-12 23:43             ` Christian Biesinger via gdb-patches
@ 2020-02-13  0:01               ` Kamil Rytarowski
  2020-02-28 13:18                 ` [PATCH v2] Fix arm-netbsd build error: convert from FPA to VFP Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Kamil Rytarowski @ 2020-02-13  0:01 UTC (permalink / raw)
  To: Christian Biesinger; +Cc: Alan Hayward, gdb-patches, nd


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

On 13.02.2020 00:42, Christian Biesinger via gdb-patches wrote:
> And one more question -- since VFP 1 is deprecated (not even mentioned
> on http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0472j/chr1359124231926.html)
> and your example codes don't include VFP 2, should I assume that
> fpu_present implies VFP 3 and that VFPv3-D16 doesn't happen?
> 

In theory we support what was mentioned in
http://netbsd.org/~kamil/arm-fpu.c

The kernel VFP attach function is located here:
https://nxr.netbsd.org/xref/src/sys/arch/arm/vfp/vfp_init.c#250

Today it is safe to assume VFPv3. As far as I am aware, VFPv3-D16 is
unsupported.

> Thanks again,
> Christian
> 
> On Wed, Feb 12, 2020 at 5:28 PM Christian Biesinger
> <cbiesinger@google.com> wrote:
>>
>> Thanks, Kamil!
>>
>> Please forgive the stupid question, but is SIMD in this context the
>> same as IWMMXT?
>>

iwmmxt is unsupported.

>> Christian
>>
>> On Wed, Feb 12, 2020 at 11:16 AM Kamil Rytarowski <n54@gmx.com> wrote:
>>>
>>> On 12.02.2020 18:09, Kamil Rytarowski wrote:
>>>> On 12.02.2020 17:28, Christian Biesinger wrote:
>>>>> On Wed, Feb 12, 2020 at 7:29 AM Alan Hayward <Alan.Hayward@arm.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On 11 Feb 2020, at 23:34, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
>>>>>>>
>>>>>>> On Tue, Feb 11, 2020 at 4:55 PM Christian Biesinger
>>>>>>> <cbiesinger@google.com> wrote:
>>>>>>>>
>>>>>>>> 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.
>>>>>>>>
>>>>>>>> Tested by compiling on arm-netbsd on qemu. For actually testing, there
>>>>>>>> seems to be something missing as "info registers" only shows FPA
>>>>>>>> registers and no VFP ones. I am still investigating why this is;
>>>>>>>> please let me know if you know. However, I think this is still good
>>>>>>>> to check in as-is.
>>>>>>>
>>>>>>> Hm... this is perhaps because arm_netbsd_nat_target does not implement
>>>>>>> read_description; if it returned arm_read_description
>>>>>>> (ARM_FP_TYPE_VFPV2) this may work?
>>>>>>>
>>>>>>
>>>>>> Yes, looks like netbsd isn’t using any target description functionality.
>>>>>>
>>>>>> I suspect the code is getting into arm_gdbarch_init() with a null tdesc,
>>>>>> and then using the AUTO setting. But that’s just a guess.
>>>>>>
>>>>>> Implementing read_description as you suggest should help. However,
>>>>>> read_description should probably do HWCAP checking similar to the
>>>>>> arm_linux_nat_target and arm_fbsd_nat_target versions.
>>>>>>
>>>>>> Without that, I’d worry that your patch below might start writing off the
>>>>>> end of the regcache that had been allocated for a fewer number of registers.
>>>>>
>>>>> Hm... well, I've probably spent entirely too much time on this
>>>>> already, but just in case there's an easy fix -- Kamil, does NetBSD
>>>>> provide an API similar to Linux's HWCAP API that would let me check
>>>>> which VFP version the current CPU is using? It seems AUXV does not
>>>>> contain HWCAP data on NetBSD, unlike Linux/FreeBSD, though I may be
>>>>> missing something.
>>>>>
>>>>> (Compare arm_fbsd_read_description_auxv /
>>>>> arm_linux_nat_target::read_description)
>>>>>
>>>>> Christian
>>>>>
>>>>
>>>> Thank you for your work.
>>>>
>>>> HWCAP in auxv is not supported (although there is a stub for it).
>>>>
>>>> Here is a complete algorithm to detect FPU on ARM 32-bit.
>>>>
>>>> http://netbsd.org/~kamil/arm-fpu.c
>>>>
>>>
>>> Here is another example with SIMD:
>>>
>>> http://cvsweb.netbsd.org/bsdweb.cgi/xsrc/external/mit/pixman/dist/pixman/pixman-arm.c.diff?r1=1.1&r2=1.2&f=h
>>>
>>>>>>
>>>>>>>>
>>>>>>>> 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.
>>>>>>>>        (fetch_elfcore_registers): Update.
>>>>>>>> ---
>>>>>>>> gdb/arm-nbsd-nat.c | 80 ++++++++++++++++++++--------------------------
>>>>>>>> 1 file changed, 34 insertions(+), 46 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
>>>>>>>> index 11afc289c3..8027f54dfe 100644
>>>>>>>> --- a/gdb/arm-nbsd-nat.c
>>>>>>>> +++ b/gdb/arm-nbsd-nat.c
>>>>>>>> @@ -65,15 +65,13 @@ 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;
>>>>>>>> -
>>>>>>>> -  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>>>>>>>> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>>>>>
>>>>>>>> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
>>>>>>>> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>>>>>> }
>>>>>>>>
>>>>>>>> static void
>>>>>>>> @@ -147,10 +145,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 +156,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
>>>>>>>>       return;
>>>>>>>>     }
>>>>>>>>
>>>>>>>> -  switch (regno)
>>>>>>>> +  if (regno == ARM_FPSCR_REGNUM)
>>>>>>>> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>>>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>>>>>>>     {
>>>>>>>> -    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 +183,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 +322,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 +332,15 @@ store_fp_register (const struct regcache *regcache, int regno)
>>>>>>>>       return;
>>>>>>>>     }
>>>>>>>>
>>>>>>>> -  switch (regno)
>>>>>>>> +  if (regno == ARM_FPSCR_REGNUM)
>>>>>>>> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
>>>>>>>> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>>>>>>>>     {
>>>>>>>> -    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 +352,17 @@ 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 fpreg fpregs;
>>>>>>>>
>>>>>>>> -
>>>>>>>> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
>>>>>>>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>>>>>>>>     regcache->raw_collect
>>>>>>>> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
>>>>>>>> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
>>>>>>>>
>>>>>>>> -  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"));
>>>>>>>> @@ -427,7 +415,7 @@ fetch_elfcore_registers (struct regcache *regcache,
>>>>>>>>          /* 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);
>>>>>>>> +         arm_supply_vfpregset (regcache, &fparegset);
>>>>>>>>        }
>>>>>>>>       break;
>>>>>>>>
>>>>>>>> --
>>>>>>>> 2.25.0.225.g125e21ebc7-goog
>>>>>>>>
>>>>>>
>>>>
>>>>
>>>
>>>



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

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

* [PATCH v2] Fix arm-netbsd build error: convert from FPA to VFP
  2020-02-13  0:01               ` Kamil Rytarowski
@ 2020-02-28 13:18                 ` Christian Biesinger via gdb-patches
  2020-02-28 14:35                   ` Alan Hayward
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-02-28 13:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger

[Fixes the issue pointed out by Alan based on code by Kamil.
This took way longer than it should have due to an unrelated regression,
https://sourceware.org/ml/gdb-patches/2020-02/msg01038.html]

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 | 112 +++++++++++++++++++++++----------------------
 1 file changed, 58 insertions(+), 54 deletions(-)

diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
index 11afc289c3..2e7d840adf 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,13 @@ 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;
-
-  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
+    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
 
-  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
+  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
 }
 
 static void
@@ -147,10 +148,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 +159,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
       return;
     }
 
-  switch (regno)
+  if (regno == ARM_FPSCR_REGNUM)
+    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
+  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
     {
-    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 +186,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 +325,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 +335,15 @@ store_fp_register (const struct regcache *regcache, int regno)
       return;
     }
 
-  switch (regno)
+  if (regno == ARM_FPSCR_REGNUM)
+    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
+  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
     {
-    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 +355,17 @@ 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 fpreg fpregs;
 
-
-  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
+  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
     regcache->raw_collect
-      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
+      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
 
-  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 +388,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 +428,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.1.481.gfbce0eb801-goog

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

* Re: [PATCH v2] Fix arm-netbsd build error: convert from FPA to VFP
  2020-02-28 13:18                 ` [PATCH v2] Fix arm-netbsd build error: convert from FPA to VFP Christian Biesinger via gdb-patches
@ 2020-02-28 14:35                   ` Alan Hayward
  2020-02-28 19:19                     ` [PATCH v3] " Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Hayward @ 2020-02-28 14:35 UTC (permalink / raw)
  To: Christian Biesinger; +Cc: gdb-patches, nd



> On 28 Feb 2020, at 13:18, Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
> 
> [Fixes the issue pointed out by Alan based on code by Kamil.
> This took way longer than it should have due to an unrelated regression,
> https://sourceware.org/ml/gdb-patches/2020-02/msg01038.html]
> 
> 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 | 112 +++++++++++++++++++++++----------------------
> 1 file changed, 58 insertions(+), 54 deletions(-)
> 
> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> index 11afc289c3..2e7d840adf 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,13 @@ 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;
> -
> -  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 = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
> +    regcache->raw_supply (regno, (char *) &vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> 
> -  regcache->raw_supply (ARM_FPS_REGNUM, (char *) &fparegset->fpr_fpsr);
> +  regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> }
> 
> static void
> @@ -147,10 +148,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 +159,15 @@ fetch_fp_register (struct regcache *regcache, int regno)
>       return;
>     }
> 
> -  switch (regno)
> +  if (regno == ARM_FPSCR_REGNUM)
> +    regcache->raw_supply (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>     {
> -    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 +186,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 +325,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 +335,15 @@ store_fp_register (const struct regcache *regcache, int regno)
>       return;
>     }
> 
> -  switch (regno)
> +  if (regno == ARM_FPSCR_REGNUM)
> +    regcache->raw_collect (ARM_FPSCR_REGNUM, (char *) &vfp.vfp_fpscr);
> +  else if (regno >= ARM_D0_REGNUM && regno <= ARM_D31_REGNUM)
>     {
> -    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 +355,17 @@ 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 fpreg fpregs;
> 
> -
> -  for (regno = ARM_F0_REGNUM; regno <= ARM_F7_REGNUM; regno++)
> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>     regcache->raw_collect
> -      (regno, (char *) &inferior_fp_registers.fpr[regno - ARM_F0_REGNUM]);
> +      (regno, (char *) &fpregs.fpr_vfp.vfp_regs[regno - ARM_D0_REGNUM]);
> 
> -  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);
> 

This looks fine if you have a tdesc of type ARM_FP_TYPE_VFPV3.
What happens when you create a tdesc of type ARM_FP_TYPE_NONE or AArch32?

The fetch/supply functions need to check what type the description is,
otherwise you’ll overflow the regcache.

Check for AArch32 with:
if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word == 32)

And you can probably count the number or size of registers to confirm if
it’s ARM_FP_TYPE_VFPV3 or ARM_FP_TYPE_NONE.

Alternatively, I think you could stash the arm_fp_type in the tdep structure
and then grab hold of it.

(Everything else looks ok)


Alan.

>   if (ret < 0)
>     warning (_("unable to store floating-point registers"));
> @@ -397,6 +388,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 +428,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.1.481.gfbce0eb801-goog
> 


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

* [PATCH v3] Fix arm-netbsd build error: convert from FPA to VFP
  2020-02-28 14:35                   ` Alan Hayward
@ 2020-02-28 19:19                     ` Christian Biesinger via gdb-patches
  2020-03-02 11:42                       ` Alan Hayward
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-02-28 19:19 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger

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(-)

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

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

* Re: [PATCH v3] Fix arm-netbsd build error: convert from FPA to VFP
  2020-02-28 19:19                     ` [PATCH v3] " Christian Biesinger via gdb-patches
@ 2020-03-02 11:42                       ` Alan Hayward
  2020-03-02 17:29                         ` Christian Biesinger via gdb-patches
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Hayward @ 2020-03-02 11:42 UTC (permalink / raw)
  To: Christian Biesinger; +Cc: gdb-patches, nd



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


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

* Re: [PATCH v3] Fix arm-netbsd build error: convert from FPA to VFP
  2020-03-02 11:42                       ` Alan Hayward
@ 2020-03-02 17:29                         ` Christian Biesinger via gdb-patches
  2020-03-02 17:32                           ` Kamil Rytarowski
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-03-02 17:29 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

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

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

* Re: [PATCH v3] Fix arm-netbsd build error: convert from FPA to VFP
  2020-03-02 17:29                         ` Christian Biesinger via gdb-patches
@ 2020-03-02 17:32                           ` Kamil Rytarowski
  0 siblings, 0 replies; 15+ messages in thread
From: Kamil Rytarowski @ 2020-03-02 17:32 UTC (permalink / raw)
  To: Christian Biesinger, Alan Hayward; +Cc: gdb-patches, nd


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

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



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

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

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

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11 22:55 [PATCH] Fix arm-netbsd build error Christian Biesinger via gdb-patches
2020-02-11 23:35 ` Christian Biesinger via gdb-patches
2020-02-12 13:30   ` Alan Hayward
2020-02-12 16:29     ` Christian Biesinger via gdb-patches
2020-02-12 17:10       ` Kamil Rytarowski
2020-02-12 17:17         ` Kamil Rytarowski
2020-02-12 23:28           ` Christian Biesinger via gdb-patches
2020-02-12 23:43             ` Christian Biesinger via gdb-patches
2020-02-13  0:01               ` Kamil Rytarowski
2020-02-28 13:18                 ` [PATCH v2] Fix arm-netbsd build error: convert from FPA to VFP Christian Biesinger via gdb-patches
2020-02-28 14:35                   ` Alan Hayward
2020-02-28 19:19                     ` [PATCH v3] " Christian Biesinger via gdb-patches
2020-03-02 11:42                       ` Alan Hayward
2020-03-02 17:29                         ` Christian Biesinger via gdb-patches
2020-03-02 17:32                           ` Kamil Rytarowski

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