Re: [PATCH] Fix arm-netbsd build error

[Kamil Rytarowski <n54@gmx.com>]

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