Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

[tatsuo]

Summary

RegInfoBasedABI::GetRegisterInfoByName was failing because mips/mips64 ABIs don't use ConstString in their register info array.

[teemperor]

To clarify the description a bit: RegInfoBasedABI::GetRegisterInfoByName is comparing the name/alt_name by doing a const char * pointer value comparison. That only works if both strings that are compared are coming from a ConstString. Since b0060c3a7868ef026d95d0cf8a076791ef74f474 GetRegisterInfoByName is checking with an assert that both C strings came from a ConstString (which is failing as these ABI implementation changed here are lacking the ConstString'ification code).

Obviously copying this code around isn't ideal, so this patch is just to get the backend at least running again until I get around to refactor this code.

Anyway, this LGTM.

[tatsuo]

@teemperor Thanks for the quick review and context!
I don't have commit access, that'd be great if you could merge it in.

[teemperor]

Done. Thanks for the patch!

[JDevlieghere]

Instead of copy-pasting this code can we extract a helper and call it with g_register_infos_mips64 and g_register_infos respectively?

[teemperor]

Yeah, this is just to prevent the MIPS backend from hitting the assert (well, and get the related code to actually do what it's supposed to do). This whole code on my "to refactor" list.

[labath]

Not long ago. I added a MCBasedABI class which makes this goo mostly disappear as it gets its information from llvm MC layer. I haven't converted the mips abi due to a combination of being preempted by higher priority task, not having a mips machine to test on, and this code not being particularly egregious.

But if anyone wants to try this (and better yet, has ability to test it), I would strongly encourage converting these classes to an MCBasedABI. The only gotcha there is that the names of registers used by llvm mc and lldb/other tools don't always match, so it's sometimes necessary to write a small translation function. But that's still better than hundreds of lines of goo.

lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp

enum dwarf_regnums {
   dwarf_sr,
   dwarf_lo,
   dwarf_hi,
   dwarf_bad,
   dwarf_cause,
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos[] = {
+static RegisterInfo g_register_infos[] = {
     //  NAME      ALT    SZ OFF ENCODING        FORMAT         EH_FRAME
     //  DWARF                   GENERIC                     PROCESS PLUGINS
     //  LLDB NATIVE            VALUE REGS  INVALIDATE REGS
     //  ========  ======  == === =============  ===========    ============
     //  ==============          ============                =================
     //  ===================     ========== =================
     {"r0",
      "zero",
static RegisterInfo g_register_infos[] = {
      nullptr,
      nullptr,
      nullptr,
      0},
 };
 
 static const uint32_t k_num_register_infos =
     llvm::array_lengthof(g_register_infos);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+    g_register_info_names_constified = true;
+    for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+      if (g_register_infos[i].name)
+        g_register_infos[i].name =
+            ConstString(g_register_infos[i].name).GetCString();
+      if (g_register_infos[i].alt_name)
+        g_register_infos[i].alt_name =
+            ConstString(g_register_infos[i].alt_name).GetCString();
+    }
+  }
   count = k_num_register_infos;
   return g_register_infos;
 }
 
 size_t ABISysV_mips::GetRedZoneSize() const { return 0; }
 
 // Static Functions
 

lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp

enum dwarf_regnums {
   dwarf_sr,
   dwarf_lo,
   dwarf_hi,
   dwarf_bad,
   dwarf_cause,
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos_mips64[] = {
+static RegisterInfo g_register_infos_mips64[] = {
     //  NAME      ALT    SZ OFF ENCODING        FORMAT         EH_FRAME
     //  DWARF                   GENERIC                     PROCESS PLUGIN
     //  LLDB NATIVE
     //  ========  ======  == === =============  ==========     =============
     //  =================       ====================        =================
     //  ====================
     {"r0",
      "zero",
static RegisterInfo g_register_infos_mips64[] = {
      nullptr,
      nullptr,
      nullptr,
      0},
 };
 
 static const uint32_t k_num_register_infos =
     llvm::array_lengthof(g_register_infos_mips64);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips64::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+    g_register_info_names_constified = true;
+    for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+      if (g_register_infos_mips64[i].name)
+        g_register_infos_mips64[i].name =
+            ConstString(g_register_infos_mips64[i].name).GetCString();
+      if (g_register_infos_mips64[i].alt_name)
+        g_register_infos_mips64[i].alt_name =
+            ConstString(g_register_infos_mips64[i].alt_name).GetCString();
+    }
+  }
   count = k_num_register_infos;
   return g_register_infos_mips64;
 }
 
 size_t ABISysV_mips64::GetRedZoneSize() const { return 0; }
 
 // Static Functions