[PING] [PATCH v2] gdb: specify sh pointer register types

["Sébastien Michelland" <sebastien.michelland@lcis.grenoble-inp.fr>]

Hi everyone, kindly pinging for process for this patch.

Simon approved the approach on a previous version but this one still 
needs validation, and ultimately a push if accepted.

I understand there is little maintenance going on for the sh target. Is 
there additional testing I should do to help with this patch?

On 2024-04-01 11:55, Sébastien Michelland wrote:
> This patch fixes a pretty funny issue on sh targets that occurred
> because $pc (and similar registers) were typed as int. When $pc is in
> the upper half of the address space (i.e. kernel code on sh), `x/i $pc'
> would resolve to a negative value. At least in the case of a remote
> target with an Xfer memory map, this leads to a spurious "cannot access
> memory" error as negative addresses are out of bounds.
> 
> (gdb) x/i $pc
>      0x8c202c04:    Cannot access memory at address 0x8c202c04
> (gdb) x/i 0x8c202c04
> => 0x8c202c04 <gintctl_gint_gdb+304>:    mov.l    @r1,r10
> 
> The issue is fixed by specifying pointer types for pc and other pointer
> registers. Code pointer registers on sh include pc, pr (return address
> of a call), vbr (interrupt handler) and spc (return address after
> interrupt). Data pointers include r15 (stack pointer) and gbr (base
> register for a few specific addressing modes).
> ---
> 
> Compared to v1, this patch applies to all sh architectures. It also adds
> r15 (stack pointer) as a data pointer and sets gbr to a data rather than
> code pointer (which was a mistake).
> 
>   gdb/sh-tdep.c | 23 ++++++++++++++++++++++-
>   1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/sh-tdep.c b/gdb/sh-tdep.c
> index 1c67ea42..b76efa3a 100644
> --- a/gdb/sh-tdep.c
> +++ b/gdb/sh-tdep.c
> @@ -1400,6 +1400,11 @@ sh_sh2a_register_type (struct gdbarch *gdbarch, int reg_nr)
>       return builtin_type (gdbarch)->builtin_float;
>     else if (reg_nr >= DR0_REGNUM && reg_nr <= DR_LAST_REGNUM)
>       return builtin_type (gdbarch)->builtin_double;
> +  else if (reg_nr == PC_REGNUM || reg_nr == PR_REGNUM || reg_nr == VBR_REGNUM
> +      || reg_nr == SPC_REGNUM)
> +    return builtin_type (gdbarch)->builtin_func_ptr;
> +  else if (reg_nr == R0_REGNUM+15 || reg_nr == GBR_REGNUM)
> +    return builtin_type (gdbarch)->builtin_data_ptr;
>     else
>       return builtin_type (gdbarch)->builtin_int;
>   }
> @@ -1412,6 +1417,11 @@ sh_sh3e_register_type (struct gdbarch *gdbarch, int reg_nr)
>     if ((reg_nr >= gdbarch_fp0_regnum (gdbarch)
>          && (reg_nr <= FP_LAST_REGNUM)) || (reg_nr == FPUL_REGNUM))
>       return builtin_type (gdbarch)->builtin_float;
> +  else if (reg_nr == PC_REGNUM || reg_nr == PR_REGNUM || reg_nr == VBR_REGNUM
> +      || reg_nr == SPC_REGNUM)
> +    return builtin_type (gdbarch)->builtin_func_ptr;
> +  else if (reg_nr == R0_REGNUM+15 || reg_nr == GBR_REGNUM)
> +    return builtin_type (gdbarch)->builtin_data_ptr;
>     else
>       return builtin_type (gdbarch)->builtin_int;
>   }
> @@ -1433,6 +1443,11 @@ sh_sh4_register_type (struct gdbarch *gdbarch, int reg_nr)
>       return builtin_type (gdbarch)->builtin_double;
>     else if (reg_nr >= FV0_REGNUM && reg_nr <= FV_LAST_REGNUM)
>       return sh_sh4_build_float_register_type (gdbarch, 3);
> +  else if (reg_nr == PC_REGNUM || reg_nr == PR_REGNUM || reg_nr == VBR_REGNUM
> +      || reg_nr == SPC_REGNUM)
> +    return builtin_type (gdbarch)->builtin_func_ptr;
> +  else if (reg_nr == R0_REGNUM+15 || reg_nr == GBR_REGNUM)
> +    return builtin_type (gdbarch)->builtin_data_ptr;
>     else
>       return builtin_type (gdbarch)->builtin_int;
>   }
> @@ -1440,7 +1455,13 @@ sh_sh4_register_type (struct gdbarch *gdbarch, int reg_nr)
>   static struct type *
>   sh_default_register_type (struct gdbarch *gdbarch, int reg_nr)
>   {
> -  return builtin_type (gdbarch)->builtin_int;
> +  if (reg_nr == PC_REGNUM || reg_nr == PR_REGNUM || reg_nr == VBR_REGNUM
> +      || reg_nr == SPC_REGNUM)
> +    return builtin_type (gdbarch)->builtin_func_ptr;
> +  else if (reg_nr == R0_REGNUM+15 || reg_nr == GBR_REGNUM)
> +    return builtin_type (gdbarch)->builtin_data_ptr;
> +  else
> +    return builtin_type (gdbarch)->builtin_int;
>   }
>   
>   /* Is a register in a reggroup?