Re: [PATCH] [gdb/tdep, aarch64] Remove fp and sp reg aliases, add x31 reg alias

[Luis Machado <luis.machado@arm.com>]

Hi Tom,

Thanks for addressing this.

On 1/19/23 12:37, Tom de Vries wrote:
> From: Tom de Vries <tdevries@space.suse.cz>
> 
> In aarch64-tdep.c we find these register aliases:
> ...
> {
>    /* 64-bit register names.  */
>    {"fp", AARCH64_FP_REGNUM},
>    {"lr", AARCH64_LR_REGNUM},
>    {"sp", AARCH64_SP_REGNUM},
> ...
> 
> The sp alias is superfluous, because the canonical name of x31 is already sp.
> 
> The fp alias is superfluous, because it's already taken by the default meaning
> of fp, assigned here in _initialize_frame_reg:
> ...
>    user_reg_add_builtin ("fp", value_of_builtin_frame_fp_reg, NULL);
> ...
> 
> Fix this by removing the fp and sp aliases.
> 
> While we're at it, add an x31 alias for sp.
> 
> Tested on aarch64-linux.
> 
> PR tdep/30012
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30012
> ---
>   gdb/aarch64-tdep.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index b576d3b9d99..1dcedac31f2 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -61,17 +61,20 @@
>   /* All possible aarch64 target descriptors.  */
>   static std::unordered_map <aarch64_features, target_desc *> tdesc_aarch64_map;
>   
> -/* The standard register names, and all the valid aliases for them.  */
> +/* The standard register names, and all the valid aliases for them.
> +   We're not adding fp here, that name is already taken, see
> +   _initialize_frame_reg.  */
>   static const struct
>   {
>     const char *const name;
>     int regnum;
>   } aarch64_register_aliases[] =
>   {
> -  /* 64-bit register names.  */
> -  {"fp", AARCH64_FP_REGNUM},
> +  /* Link register alias for x30.  */
>     {"lr", AARCH64_LR_REGNUM},
> -  {"sp", AARCH64_SP_REGNUM},
> +  /* Sp is the canonical name for x31 according to aarch64_r_register_names,

Sp -> SP, in the comment.

Otherwise this looks good to me. Thanks!

Approved-by: Luis Machado <luis.machado@arm.com>

> +     so we're adding an x31 alias for sp.  */
> +  {"x31", AARCH64_SP_REGNUM},
>     /*  specials */
>     {"ip0", AARCH64_X0_REGNUM + 16},
>     {"ip1", AARCH64_X0_REGNUM + 17}
> 
> base-commit: 75c2efc6f19fe454413aa70d2c37e543f04a67e1