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

Message ID 20230119123744.18276-1-tdevries@suse.de
State Committed
Headers
Series [gdb/tdep,aarch64] Remove fp and sp reg aliases, add x31 reg alias |

Commit Message

Tom de Vries Jan. 19, 2023, 12:37 p.m. UTC
  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(-)


base-commit: 75c2efc6f19fe454413aa70d2c37e543f04a67e1
  

Comments

Luis Machado Jan. 19, 2023, 12:47 p.m. UTC | #1
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
  

Patch

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,
+     so we're adding an x31 alias for sp.  */
+  {"x31", AARCH64_SP_REGNUM},
   /*  specials */
   {"ip0", AARCH64_X0_REGNUM + 16},
   {"ip1", AARCH64_X0_REGNUM + 17}