[lldb][LoongArch] Complete register alias name in AugmentRegisterInfo

[wangleiat]

Fixes: #123903

[llvmbot]

@llvm/pr-subscribers-testing-tools

@llvm/pr-subscribers-lldb

Author: wanglei (wangleiat)

Changes

Fixes: #123903

---

Full diff: https://github.com/llvm/llvm-project/pull/124059.diff

1 Files Affected:

• (modified) lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp (+14-24)

diff --git a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
index dc7e9bba000676..272c6a6be529ff 100644
--- a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
+++ b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
@@ -644,32 +644,22 @@ void ABISysV_loongarch::AugmentRegisterInfo(
     std::vector<lldb_private::DynamicRegisterInfo::Register> &regs) {
   lldb_private::RegInfoBasedABI::AugmentRegisterInfo(regs);
 
+  static const std::unordered_map<std::string, std::string> reg_aliases = {
+      {"r0", "zero"}, {"r1", "ra"},  {"r2", "tp"},  {"r3", "sp"},
+      {"r4", "a0"},   {"r5", "a1"},  {"r6", "a2"},  {"r7", "a3"},
+      {"r8", "a4"},   {"r9", "a5"},  {"r10", "a6"}, {"r11", "a7"},
+      {"r12", "t0"},  {"r13", "t1"}, {"r14", "t2"}, {"r15", "t3"},
+      {"r16", "t4"},  {"r17", "t5"}, {"r18", "t6"}, {"r19", "t7"},
+      {"r20", "t8"},  {"r22", "fp"}, {"r23", "s0"}, {"r24", "s1"},
+      {"r25", "s2"},  {"r26", "s3"}, {"r27", "s4"}, {"r28", "s5"},
+      {"r29", "s6"},  {"r30", "s7"}, {"r31", "s8"}};
+
   for (auto it : llvm::enumerate(regs)) {
     // Set alt name for certain registers for convenience
-    if (it.value().name == "r0")
-      it.value().alt_name.SetCString("zero");
-    else if (it.value().name == "r1")
-      it.value().alt_name.SetCString("ra");
-    else if (it.value().name == "r3")
-      it.value().alt_name.SetCString("sp");
-    else if (it.value().name == "r22")
-      it.value().alt_name.SetCString("fp");
-    else if (it.value().name == "r4")
-      it.value().alt_name.SetCString("a0");
-    else if (it.value().name == "r5")
-      it.value().alt_name.SetCString("a1");
-    else if (it.value().name == "r6")
-      it.value().alt_name.SetCString("a2");
-    else if (it.value().name == "r7")
-      it.value().alt_name.SetCString("a3");
-    else if (it.value().name == "r8")
-      it.value().alt_name.SetCString("a4");
-    else if (it.value().name == "r9")
-      it.value().alt_name.SetCString("a5");
-    else if (it.value().name == "r10")
-      it.value().alt_name.SetCString("a6");
-    else if (it.value().name == "r11")
-      it.value().alt_name.SetCString("a7");
+    std::string reg_name = it.value().name.GetStringRef().str();
+    if (auto alias = reg_aliases.find(reg_name); alias != reg_aliases.end()) {
+      it.value().alt_name.SetCString(alias->second.c_str());
+    }
 
     // Set generic regnum so lldb knows what the PC, etc is
     it.value().regnum_generic = GetGenericNum(it.value().name.GetStringRef());

[DavidSpickett]

ABI details are someone else's job but on generic things this is the right idea.

It needs tests, I suggest you find the tests that read general registers and add aliased name reads to those. Though it's unlikely to break for just one use case, I would add this to core file and live process tests, since it's simple and cheap to do.

[wangleiat]

ABI details are someone else's job but on generic things this is the right idea.

It needs tests, I suggest you find the tests that read general registers and add aliased name reads to those. Though it's unlikely to break for just one use case, I would add this to core file and live process tests, since it's simple and cheap to do.

Sorry, I couldn't find any reference test cases. Since the Chinese Spring Festival holiday started today, the follow-up changes for this patch will have to wait until after the holiday(two weeks later). Or interested individuals can continue to improve it.

[DavidSpickett]

lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py - test_loongarch64_regs for example. You should be able to read all those registers using the aliases as well as the normal names. If you look through the LoongArch lldb changes you'll probably find the live process equivalent quite quickly.

[wangleiat]

gentle ping

[SixWeining]

LGTM

[DavidSpickett]

Please add a test like lldb/test/Shell/Register/aarch64-gp-read.test which tests the normal and alias names using a live process.

Note that the main "trick" of that test is to manually insert a breakpoint instruction:

  asm volatile(
    "ldp      x0,  x1,  [%0]\n\t"
<...>
    "brk      #0\n\t"
<...>
  );

Then lldb will stop there, before the compiler restores anything.

The usual way to do these tests is set each register to its number plus some offset. x0 = 1, x1 = 2, etc.

One is being added for RISC-V in #124475, so you can use that as an example.

[DavidSpickett]

Everything in the PR at the moment is fine, just need the extra test case.

[wangleiat]

Please add a test like lldb/test/Shell/Register/aarch64-gp-read.test which tests the normal and alias names using a live process.

Note that the main "trick" of that test is to manually insert a breakpoint instruction:

  asm volatile(
    "ldp      x0,  x1,  [%0]\n\t"
<...>
    "brk      #0\n\t"
<...>
  );

Then lldb will stop there, before the compiler restores anything.

The usual way to do these tests is set each register to its number plus some offset. x0 = 1, x1 = 2, etc.

One is being added for RISC-V in #124475, so you can use that as an example.

Thank you, the test case has been added.

[DavidSpickett]

LGTM with my one new comment addressed.