[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