Re: [PATCH] Fix arm-netbsd build error

[Alan Hayward <Alan.Hayward@arm.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 7983 bytes --]



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

\x16º&Öéj×!zÊÞ¶êç×®5ãÉb²Ö«r\x18\x1dn–­r\x17¬