[lldb] [ABI/AArch64] Add pseudo-regs if missing

[mgorny]

Summary

Create pseudo-registers on the AArch64 target if they are not provided
by the remote server. This is the case for gdbserver. The created
registers are:

• 32-bit wN partials for 64-bit xN registers
• double precision floating-point dN registers (overlapping with vN)
• single precision floating-point sN registers (overlapping with vN)

[mgorny]

(so far with ugly hacks, to get some initial ideas on how to proceed with this)

[mgorny]

@labath, I think the biggest problem here is that DynamicRegisterInfo is defined only inside source tree. I'm not sure if I should continue with a forward declaration like this or aim to move DynamicRegisterInfo into public headers. if the former, should I move it into lldb_private namespace?

[mgorny]

Use public API to determine next regnums.

[labath]

I think we could move the DynamicRegisterInfo class into non-plugin code, but I need to give it more thought.

In the mean time (and maybe before that), I'd like to understand why do you need to mess with the process plugin register numbers in this class. It doesn't seem like a good idea for that class to be assuming anything about how those numbers are allocated and used..

[mgorny]

In D109876#3003630, @labath wrote:

In the mean time (and maybe before that), I'd like to understand why do you need to mess with the process plugin register numbers in this class. It doesn't seem like a good idea for that class to be assuming anything about how those numbers are allocated and used..

For some reason, I have assumed that I need to allocate some unique numbers for the new registers. Thinking about it, I'm going to try if things work if I just put LLDB_INVALID_REGNUM there.

[mgorny]

Ok, I definitely need some 'process plugin' IDs to handle invalidate_regs between pseudo-registers.

[mgorny]

Cover FPU registers.

[mgorny]

Rebase. Clang-format. Eliminate remote regnums entirely.

[mgorny]

Update for DynamicRegisterInfo move.

[labath]

lldb/include/lldb/Target/ABI.h

131–134 ↗

(On Diff #376549)

Ideally, I would like to subsume the AugmentRegisterInfo functionality into this function, so that one would just call abi->Augment(Dynamic)RegisterInfo(dyn_reg_info) and it would automatically fill it in with all the necessary information. The old AugmentRegisterInfo function could become a private/protected implementation detail.

Would such a thing be possible? Perhaps with some preparatory refactoring patch?

[mgorny]

Update to use the new AugmentRegisterInfo() API.

[mgorny]

Rewrite for std::vector<RemoteRegisterInfo>-based augmentation.

[mgorny]

Rebase.

[mgorny]

Arrays need explicit initialization.

[mgorny]

Remove unused next_regnum_lldb arg.

[mgorny]

Rename the set to 'supplementary registers'.

[labath]

lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp

112–118

This might be reason enough to make me replace array<uint32_t> with array<Optional<uint32_t> and array<bool> with std::bitset

149–153

It might be worth making a utility function for this as well (or maybe moving the for loop into the existing utility function).

lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

383

what's up with the [:]? Can we just drop that?

[mgorny]

lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp

112–118

Makes sense.

149–153

I inlined the for loop and renamed the function to addPartialRegisters().

lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

383

It was necessary to modify the reg_data variable from higher function instead of declaring a new local variable but I think we can move it to class variable now to make it cleaner.

lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp

return llvm::StringSwitch<uint32_t>(name)
       .Case("x3", LLDB_REGNUM_GENERIC_ARG4)
       .Case("x4", LLDB_REGNUM_GENERIC_ARG5)
       .Case("x5", LLDB_REGNUM_GENERIC_ARG6)
       .Case("x6", LLDB_REGNUM_GENERIC_ARG7)
       .Case("x7", LLDB_REGNUM_GENERIC_ARG8)
       .Default(LLDB_INVALID_REGNUM);
 }
 
+static void addPartialRegister(
+    std::vector<lldb_private::DynamicRegisterInfo::Register> &regs,
+    uint32_t full_reg_index, uint32_t full_reg_size,
+    const std::string &partial_reg_name, uint32_t partial_reg_size,
+    lldb::Encoding encoding, lldb::Format format) {
+  if (full_reg_index == LLDB_INVALID_REGNUM ||
+      regs[full_reg_index].byte_size != full_reg_size)
+    return;
+
+  lldb_private::DynamicRegisterInfo::Register partial_reg{
+      lldb_private::ConstString(partial_reg_name),
+      lldb_private::ConstString(),
+      lldb_private::ConstString("supplementary registers"),
+      partial_reg_size,
+      LLDB_INVALID_INDEX32,
+      encoding,
+      format,
+      LLDB_INVALID_REGNUM,
+      LLDB_INVALID_REGNUM,
+      LLDB_INVALID_REGNUM,
+      LLDB_INVALID_REGNUM,
+      {full_reg_index},
+      {}};
+
+  addSupplementaryRegister(regs, partial_reg);
+}
+
 void ABIAArch64::AugmentRegisterInfo(
     std::vector<lldb_private::DynamicRegisterInfo::Register> &regs) {
   lldb_private::MCBasedABI::AugmentRegisterInfo(regs);
 
   lldb_private::ConstString sp_string{"sp"};
-  for (lldb_private::DynamicRegisterInfo::Register &info : regs) {
+
+  std::array<uint32_t, 32> x_regs;
+  std::array<uint32_t, 32> v_regs;
+  std::array<bool, 32> have_w_regs;
+  std::array<bool, 32> have_s_regs;
+  std::array<bool, 32> have_d_regs;
+  for (int i = 0; i < 32; i++) {
+    x_regs[i] = LLDB_INVALID_REGNUM;
+    v_regs[i] = LLDB_INVALID_REGNUM;
+    have_w_regs[i] = false;
+    have_s_regs[i] = false;
+    have_d_regs[i] = false;
+  }
+
+  for (auto it : llvm::enumerate(regs)) {
+    lldb_private::DynamicRegisterInfo::Register &info = it.value();
     // GDB sends x31 as "sp".  Add the "x31" alt_name for convenience.
     if (info.name == sp_string && !info.alt_name)
       info.alt_name.SetCString("x31");
+
+    unsigned int reg_num;
+    auto get_reg = [&info, &reg_num](const char *prefix) {
+      llvm::StringRef reg_name = info.name.GetStringRef();
+      llvm::StringRef alt_name = info.alt_name.GetStringRef();
+      return (reg_name.consume_front(prefix) &&
+              llvm::to_integer(reg_name, reg_num, 10) && reg_num < 32) ||
+             (alt_name.consume_front(prefix) &&
+              llvm::to_integer(alt_name, reg_num, 10) && reg_num < 32);
+    };
+
+    if (get_reg("x"))
+      x_regs[reg_num] = it.index();
+    if (get_reg("v"))
+      v_regs[reg_num] = it.index();
+    if (get_reg("w"))
+      have_w_regs[reg_num] = true;
+    if (get_reg("s"))
+      have_s_regs[reg_num] = true;
+    if (get_reg("d"))
+      have_d_regs[reg_num] = true;
+  }
+
+  // Create aliases for partial registers: wN for xN, and sN/dN for vN.
+  for (int i = 0; i < 32; i++) {
+    if (!have_w_regs[i])
+      addPartialRegister(regs, x_regs[i], 8, llvm::formatv("w{0}", i), 4,
+                         lldb::eEncodingUint, lldb::eFormatHex);
+  }
+
+  for (int i = 0; i < 32; i++) {
+    if (!have_s_regs[i])
+      addPartialRegister(regs, v_regs[i], 16, llvm::formatv("s{0}", i), 4,
+                         lldb::eEncodingIEEE754, lldb::eFormatFloat);
+  }
+
+  for (int i = 0; i < 32; i++) {
+    if (!have_d_regs[i])
+      addPartialRegister(regs, v_regs[i], 16, llvm::formatv("d{0}", i), 8,
+                         lldb::eEncodingIEEE754, lldb::eFormatFloat);
   }
 }

lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

def test_aarch64_regs(self):
 
             def readRegister(self, regnum):
                 return ""
 
             def readRegisters(self):
                 return "".join(reg_data)
 
             def writeRegisters(self, reg_hex):
+                reg_data[:] = [reg_hex]
                 return "OK"
 
             def haltReason(self):
                 return "T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;07:0102030405060708;10:1112131415161718;"
 
         self.server.responder = MyResponder()
 
         target = self.createTarget("basic_eh_frame-aarch64.yaml")
def test_aarch64_regs(self):
         self.match("register read flags",
                    ["cpsr = 0x74737271"])
 
         # test vector registers
         self.match("register read v0",
                    ["v0 = {0x81 0x82 0x83 0x84 0x85 0x86 0x87 0x88 0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
         self.match("register read v31",
                    ["v31 = {0xa1 0xa2 0xa3 0xa4 0xa5 0xa6 0xa7 0xa8 0xa9 0xaa 0xab 0xac 0xad 0xae 0xaf 0xb0}"])
+
+        # test partial registers
+        self.match("register read w0",
+                   ["w0 = 0x04030201"])
+        self.runCmd("register write w0 0xfffefdfc")
+        self.match("register read x0",
+                   ["x0 = 0x08070605fffefdfc"])
+
+        self.match("register read w1",
+                   ["w1 = 0x14131211"])
+        self.runCmd("register write w1 0xefeeedec")
+        self.match("register read x1",
+                   ["x1 = 0x18171615efeeedec"])
+
+        self.match("register read w30",
+                   ["w30 = 0x44434241"])
+        self.runCmd("register write w30 0xdfdedddc")
+        self.match("register read x30",
+                   ["x30 = 0x48474645dfdedddc"])
+
+        self.match("register read w31",
+                   ["w31 = 0x54535251"])
+        self.runCmd("register write w31 0xcfcecdcc")
+        self.match("register read x31",
+                   ["sp = 0x58575655cfcecdcc"])
+
+        # test FPU registers (overlapping with vector registers)
+        self.runCmd("register write d0 16")
+        self.match("register read v0",
+                   ["v0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x30 0x40 0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+        self.runCmd("register write v31 '{0x00 0x00 0x00 0x00 0x00 0x00 0x50 0x40 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff}'")
+        self.match("register read d31",
+                   ["d31 = 64"])
+
+        self.runCmd("register write s0 32")
+        self.match("register read v0",
+                   ["v0 = {0x00 0x00 0x00 0x42 0x00 0x00 0x30 0x40 0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+        self.runCmd("register write v31 '{0x00 0x00 0x00 0x43 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff}'")
+        self.match("register read s31",
+                   ["s31 = 128"])