[PATCH] Offsets of FPR should be multiplied by 4 on 32-bit ppc

[PATCH] Offsets of FPR should be multiplied by 4 on 32-bit ppc

[Huangsikai]

From: huangsikai <huangsikai@huawei.com>

GDB:ppc32: calculate FPR address correctly on PPC32 with hard float

GDB has trouble getting values from FPRs on PPC32 systems with hard float enabled.

GDB first calculates the address of the register through the following code:
if (tdep->ppc_fp0_regnum >= 0
      && regno >= tdep->ppc_fp0_regnum
      && regno < tdep->ppc_fp0_regnum + ppc_num_fprs)
    u_addr = (PT_FPR0 * wordsize) + ((regno - tdep->ppc_fp0_regnum) * 8);

Then the address is passed into ptrace to fetch the value.

And in kernel, ptrace gets the register index by the following code:

#ifdef CONFIG_PPC32
                index = addr >> 2;
                if ((addr & 3) || (index > PT_FPSCR)
                    || (child->thread.regs == NULL))
#else
                index = addr >> 3;
                if ((addr & 7) || (index > PT_FPSCR))
#endif
                        break;
...
unsigned int fpidx = index - PT_FPR0;

It's easy to see the difference. GDB multiplies the register offset by 8,
adds the starting address of FPR0 to get the address. But the kernel divides
the address by 4, and then subtract the starting address of FPR0 to get the index.
Consequently, suppose GDB tries to get the value from FPR1,  the address passed
into ptrace will mean FPR2.

Therefore this constant should vary accordingly. As it's 4 in ppc32, 8 in ppc64,
wordsize(which equals to sizeof(long)) will do the trick.

Thanks,
Allan

Signed-off-by: huangsikai <huangsikai@huawei.com>
---
 gdb/ppc-linux-nat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 6be8f022a7..c78e554416 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -659,7 +659,7 @@ ppc_register_u_addr (struct gdbarch *gdbarch, int regno)
   if (tdep->ppc_fp0_regnum >= 0
       && regno >= tdep->ppc_fp0_regnum
       && regno < tdep->ppc_fp0_regnum + ppc_num_fprs)
-    u_addr = (PT_FPR0 * wordsize) + ((regno - tdep->ppc_fp0_regnum) * 8);
+    u_addr = ((regno - tdep->ppc_fp0_regnum + PT_FPR0) * wordsize);
 
   /* UISA special purpose registers: 1 slot each.  */
   if (regno == gdbarch_pc_regnum (gdbarch))
-- 
2.12.3

Re: [PATCH] Offsets of FPR should be multiplied by 4 on 32-bit ppc

[Simon Marchi]

On 2020-06-05 5:46 a.m., Huangsikai wrote:
> From: huangsikai <huangsikai@huawei.com>
> 
> GDB:ppc32: calculate FPR address correctly on PPC32 with hard float
> 
> GDB has trouble getting values from FPRs on PPC32 systems with hard float enabled.
> 
> GDB first calculates the address of the register through the following code:
> if (tdep->ppc_fp0_regnum >= 0
>       && regno >= tdep->ppc_fp0_regnum
>       && regno < tdep->ppc_fp0_regnum + ppc_num_fprs)
>     u_addr = (PT_FPR0 * wordsize) + ((regno - tdep->ppc_fp0_regnum) * 8);
> 
> Then the address is passed into ptrace to fetch the value.
> 
> And in kernel, ptrace gets the register index by the following code:
> 
> #ifdef CONFIG_PPC32
>                 index = addr >> 2;
>                 if ((addr & 3) || (index > PT_FPSCR)
>                     || (child->thread.regs == NULL))
> #else
>                 index = addr >> 3;
>                 if ((addr & 7) || (index > PT_FPSCR))
> #endif
>                         break;
> ...
> unsigned int fpidx = index - PT_FPR0;
> 
> It's easy to see the difference. GDB multiplies the register offset by 8,
> adds the starting address of FPR0 to get the address. But the kernel divides
> the address by 4, and then subtract the starting address of FPR0 to get the index.
> Consequently, suppose GDB tries to get the value from FPR1,  the address passed
> into ptrace will mean FPR2.
> 
> Therefore this constant should vary accordingly. As it's 4 in ppc32, 8 in ppc64,
> wordsize(which equals to sizeof(long)) will do the trick.
> 
> Thanks,
> Allan
> 
> Signed-off-by: huangsikai <huangsikai@huawei.com>
> ---
>  gdb/ppc-linux-nat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
> index 6be8f022a7..c78e554416 100644
> --- a/gdb/ppc-linux-nat.c
> +++ b/gdb/ppc-linux-nat.c
> @@ -659,7 +659,7 @@ ppc_register_u_addr (struct gdbarch *gdbarch, int regno)
>    if (tdep->ppc_fp0_regnum >= 0
>        && regno >= tdep->ppc_fp0_regnum
>        && regno < tdep->ppc_fp0_regnum + ppc_num_fprs)
> -    u_addr = (PT_FPR0 * wordsize) + ((regno - tdep->ppc_fp0_regnum) * 8);
> +    u_addr = ((regno - tdep->ppc_fp0_regnum + PT_FPR0) * wordsize);
>  
>    /* UISA special purpose registers: 1 slot each.  */
>    if (regno == gdbarch_pc_regnum (gdbarch))
> -- 
> 2.12.3
> 

By inspection, that makes sense to me.  We would need to test for this.  Is there a
test in the GDB testsuite that is currently failing that this patch fixes?  If not,
please write or modify one to exercise that code.

You can check the testsuite/gdb.arch/ppc-* files, which are the ppc-specific tests.
There are already some floating point tests, so see if you can modify an existing
one, or add a new one.

Simon