gdb/riscv: Improved register alias name creation

This commit does two things:

 1. Makes use of the DECLARE_CSR_ALIAS definitions in riscv-opc.h to
 add additional aliases for CSRs.

 2. Only creates aliases for registers that are actually present on
 the target (as announced in the target XML description).

This means that the 'csr%d' aliases that exist will only be created
for those CSRs the target actually has, which is a nice improvement,
as accessing one of the CSRs that didn't exist would cause GDB to
crash with this error:

  valprint.c:1560: internal-error: bool maybe_negate_by_bytes(const gdb_byte*, unsigned int, bfd_endian, gdb::byte_vector*): Assertion `len > 0' failed.

When we look at the DECLARE_CSR_ALIAS lines in riscv-opc.h, these can
be split into three groups:

 DECLARE_CSR_ALIAS(misa, 0xf10, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P9P1)

The 'misa' register used to exist of offset 0xf10, but was moved to
its current offset (0x301) in with privilege spec 1.9.1.  We don't
want GDB to create an alias called 'misa' as we will already have a
'misa' register created by the DECLARE_CSR(misa ....) call earlier in
riscv-opc.h

 DECLARE_CSR_ALIAS(ubadaddr, CSR_UTVAL, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR_ALIAS(sbadaddr, CSR_STVAL, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR_ALIAS(sptbr, CSR_SATP, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR_ALIAS(mbadaddr, CSR_MTVAL, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)
 DECLARE_CSR_ALIAS(mucounteren, CSR_MCOUNTINHIBIT, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P10)

These aliases are all CSRs that were removed in privilege spec 1.10,
and whose addresses were reused by new CSRs.  The names meaning of the
old names is totally different to the new CSRs that have taken their
place.  I don't believe we should add these as aliases into GDB.  If
the new CSR exists in the target then that should be enough.

 DECLARE_CSR_ALIAS(dscratch, CSR_DSCRATCH0, CSR_CLASS_I, PRIV_SPEC_CLASS_1P9, PRIV_SPEC_CLASS_1P11)

In privilege spec 1.11 the 'dscratch' register was renamed to
'dscratch0', however the meaning of the register didn't change.
Adding the 'dscratch' alias makes sense I think.

Looking then at the final PRIV_SPEC_CLASS_* field for each alias then
we can see that currently we only want to take the alias from
PRIV_SPEC_CLASS_1P11.  For now then this is what I'm using to filter
the aliases within GDB.

In the future there's no telling how DECLARE_CSR_ALIAS will be used.
I've heard it said that future RISC-V privilege specs will not reuse
CSR offsets again.  But it could happen.  We just don't know.

If / when it does we may need to revisit how aliases are created for
GDB, but for now this seems to be OK.

gdb/ChangeLog:

	* riscv-tdep.c (riscv_create_csr_aliases): Handle csr aliases from
	riscv-opc.h.
	(class riscv_pending_register_alias): New class.
	(riscv_check_tdesc_feature): Take vector of pending aliases and
	populate it as appropriate.
	(riscv_setup_register_aliases): Delete.
	(riscv_gdbarch_init): Create vector of pending aliases and pass it
	to riscv_check_tdesc_feature in all cases.  Use the vector to
	create the register aliases.

gdb/testsuite/ChangeLog:

	* gdb.arch/riscv-tdesc-regs-32.xml: New file.
	* gdb.arch/riscv-tdesc-regs-64.xml: New file.
	* gdb.arch/riscv-tdesc-regs.c: New file.
	* gdb.arch/riscv-tdesc-regs.exp: New file.

1 files changed, 66 insertions, 29 deletions

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index fa43c8d..55929ae 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -258,6 +258,16 @@ riscv_create_csr_aliases ()
       int csr_num = reg.regnum - RISCV_FIRST_CSR_REGNUM;
       const char *alias = xstrprintf ("csr%d", csr_num);
       reg.names.push_back (alias);
+
+      /* Setup the other csr aliases.  We don't use a switch table here in
+         case there are multiple aliases with the same value.  Also filter
+         based on ABRT_VER in order to avoid a very old alias for misa that
+         duplicates the name "misa" but at a different CSR address.  */
+#define DECLARE_CSR_ALIAS(NAME,VALUE,CLASS,DEF_VER,ABRT_VER)	 \
+      if (csr_num == VALUE && ABRT_VER >= PRIV_SPEC_CLASS_1P11)  \
+        reg.names.push_back ( # NAME );
+#include "opcode/riscv-opc.h"
+#undef DECLARE_CSR_ALIAS
     }
 }
 
@@ -2945,6 +2955,37 @@ riscv_find_default_target_description (const struct gdbarch_info info)
   return riscv_lookup_target_description (features);
 }
 
+/* Information about a register alias that needs to be set up for this
+   target.  These are collected when the target's XML description is
+   analysed, and then processed later, once the gdbarch has been created.  */
+
+class riscv_pending_register_alias
+{
+public:
+  /* Constructor.  */
+
+  riscv_pending_register_alias (const char *name, const void *baton)
+    : m_name (name),
+      m_baton (baton)
+  { /* Nothing.  */ }
+
+  /* Convert this into a user register for GDBARCH.  */
+
+  void create (struct gdbarch *gdbarch) const
+  {
+    user_reg_add (gdbarch, m_name, value_of_riscv_user_reg, m_baton);
+  }
+
+private:
+  /* The name for this alias.  */
+  const char *m_name;
+
+  /* The baton value for passing to user_reg_add.  This must point to some
+     data that will live for at least as long as the gdbarch object to
+     which the user register is attached.  */
+  const void *m_baton;
+};
+
 /* All of the registers in REG_SET are checked for in FEATURE, TDESC_DATA
    is updated with the register numbers for each register as listed in
    REG_SET.  If any register marked as required in REG_SET is not found in
@@ -2953,7 +2994,8 @@ riscv_find_default_target_description (const struct gdbarch_info info)
 static bool
 riscv_check_tdesc_feature (struct tdesc_arch_data *tdesc_data,
                            const struct tdesc_feature *feature,
-                           const struct riscv_register_feature *reg_set)
+                           const struct riscv_register_feature *reg_set,
+                           std::vector<riscv_pending_register_alias> *aliases)
 {
   for (const auto &reg : reg_set->registers)
     {
@@ -2965,7 +3007,15 @@ riscv_check_tdesc_feature (struct tdesc_arch_data *tdesc_data,
 	    tdesc_numbered_register (feature, tdesc_data, reg.regnum, name);
 
 	  if (found)
-	    break;
+            {
+              /* We know that the target description mentions this
+                 register.  In RISCV_REGISTER_NAME we ensure that GDB
+                 always uses the first name for each register, so here we
+                 add aliases for all of the remaining names.  */
+              for (int i = 0; i < reg.names.size (); ++i)
+		aliases->emplace_back (reg.names[i], (void *) &reg.regnum);
+              break;
+            }
 	}
 
       if (!found && reg.required_p)
@@ -2993,24 +3043,6 @@ riscv_add_reggroups (struct gdbarch *gdbarch)
   reggroup_add (gdbarch, csr_reggroup);
 }
 
-/* Create register aliases for all the alternative names that exist for
-   registers in REG_SET.  */
-
-static void
-riscv_setup_register_aliases (struct gdbarch *gdbarch,
-                              const struct riscv_register_feature *reg_set)
-{
-  for (auto &reg : reg_set->registers)
-    {
-      /* The first item in the names list is the preferred name for the
-         register, this is what RISCV_REGISTER_NAME returns, and so we
-         don't need to create an alias with that name here.  */
-      for (int i = 1; i < reg.names.size (); ++i)
-        user_reg_add (gdbarch, reg.names[i], value_of_riscv_user_reg,
-                      &reg.regnum);
-    }
-}
-
 /* Implement the "dwarf2_reg_to_regnum" gdbarch method.  */
 
 static int
@@ -3114,10 +3146,12 @@ riscv_gdbarch_init (struct gdbarch_info info,
     return NULL;
 
   struct tdesc_arch_data *tdesc_data = tdesc_data_alloc ();
+  std::vector<riscv_pending_register_alias> pending_aliases;
 
   bool valid_p = riscv_check_tdesc_feature (tdesc_data,
                                             feature_cpu,
-                                            &riscv_xreg_feature);
+                                            &riscv_xreg_feature,
+                                            &pending_aliases);
   if (valid_p)
     {
       /* Check that all of the core cpu registers have the same bitsize.  */
@@ -3137,7 +3171,8 @@ riscv_gdbarch_init (struct gdbarch_info info,
   if (feature_fpu != NULL)
     {
       valid_p &= riscv_check_tdesc_feature (tdesc_data, feature_fpu,
-                                            &riscv_freg_feature);
+                                            &riscv_freg_feature,
+                                            &pending_aliases);
 
       /* Search for the first floating point register (by any alias), to
          determine the bitsize.  */
@@ -3173,11 +3208,13 @@ riscv_gdbarch_init (struct gdbarch_info info,
 
   if (feature_virtual)
     riscv_check_tdesc_feature (tdesc_data, feature_virtual,
-                               &riscv_virtual_feature);
+                               &riscv_virtual_feature,
+                               &pending_aliases);
 
   if (feature_csr)
     riscv_check_tdesc_feature (tdesc_data, feature_csr,
-                               &riscv_csr_feature);
+                               &riscv_csr_feature,
+                               &pending_aliases);
 
   if (!valid_p)
     {
@@ -3315,11 +3352,11 @@ riscv_gdbarch_init (struct gdbarch_info info,
      want, ignoring what the target tells us.  */
   set_gdbarch_register_reggroup_p (gdbarch, riscv_register_reggroup_p);
 
-  /* Create register aliases for alternative register names.  */
-  riscv_setup_register_aliases (gdbarch, &riscv_xreg_feature);
-  if (riscv_has_fp_regs (gdbarch))
-    riscv_setup_register_aliases (gdbarch, &riscv_freg_feature);
-  riscv_setup_register_aliases (gdbarch, &riscv_csr_feature);
+  /* Create register aliases for alternative register names.  We only
+     create aliases for registers which were mentioned in the target
+     description.  */
+  for (const auto &alias : pending_aliases)
+    alias.create (gdbarch);
 
   /* Compile command hooks.  */
   set_gdbarch_gcc_target_options (gdbarch, riscv_gcc_target_options);