[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

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

79–85

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

98–102

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

79–85

Makes sense.

98–102

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/include/lldb/Target/ABI.h

 //===-- ABI.h ---------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_TARGET_ABI_H
 #define LLDB_TARGET_ABI_H
 
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/MC/MCRegisterInfo.h"
 
 namespace llvm {
 class Type;
public:
   /// plug-ins would strip those bits.
   /// @{
   virtual lldb::addr_t FixCodeAddress(lldb::addr_t pc) { return pc; }
   virtual lldb::addr_t FixDataAddress(lldb::addr_t pc) { return pc; }
   /// @}
 
   llvm::MCRegisterInfo &GetMCRegisterInfo() { return *m_mc_register_info_up; }
 
   virtual void AugmentRegisterInfo(RegisterInfo &info) = 0;
+  // Routine called before DynamicRegisterInfo::Finalize().
+  virtual void
+  PreFinalizeDynamicRegisterInfo(lldb_private::DynamicRegisterInfo &info) {}
 
   virtual bool GetPointerReturnRegister(const char *&name) { return false; }
 
   static lldb::ABISP FindPlugin(lldb::ProcessSP process_sp, const ArchSpec &arch);
 
 protected:
   ABI(lldb::ProcessSP process_sp, std::unique_ptr<llvm::MCRegisterInfo> info_up)
       : m_process_wp(process_sp), m_mc_register_info_up(std::move(info_up)) {

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

protected:
   std::pair<uint32_t, uint32_t>
   GetEHAndDWARFNums(llvm::StringRef name) override;
 
   std::string GetMCName(std::string reg) override;
 
   uint32_t GetGenericNum(llvm::StringRef name) override;
 
   void AugmentRegisterInfo(lldb_private::RegisterInfo &info) override;
+  void PreFinalizeDynamicRegisterInfo(
+      lldb_private::DynamicRegisterInfo &info) override;
 
   using lldb_private::MCBasedABI::MCBasedABI;
 };
 #endif

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

return llvm::StringSwitch<uint32_t>(name)
       .Default(LLDB_INVALID_REGNUM);
 }
 
 void ABIAArch64::AugmentRegisterInfo(lldb_private::RegisterInfo &info) {
   lldb_private::MCBasedABI::AugmentRegisterInfo(info);
 
   // GDB sends x31 as "sp".  Add the "x31" alt_name for convenience.
   if (!strcmp(info.name, "sp") && !info.alt_name)
     info.alt_name = "x31";
 }
+
+static void AddPartialRegister(lldb_private::DynamicRegisterInfo &info,
+                               const lldb_private::RegisterInfo *full_reg,
+                               uint32_t full_reg_size,
+                               const std::string &partial_reg_name,
+                               uint32_t partial_reg_size,
+                               lldb::Encoding encoding, lldb::Format format,
+                               uint32_t &next_regnum_lldb) {
+  if (!full_reg || full_reg->byte_size != full_reg_size)
+    return;
+  if (info.GetRegisterInfo(partial_reg_name))
+    return;
+
+  lldb_private::ConstString group{"partial registers"};
+
+  lldb_private::ConstString partial_reg_name_const{partial_reg_name};
+  uint32_t value_regs[] = {
+      full_reg->kinds[lldb::eRegisterKindProcessPlugin],
+      LLDB_INVALID_REGNUM,
+  };
+
+  struct lldb_private::RegisterInfo partial_reg {
+    partial_reg_name_const.AsCString(),
+        nullptr, partial_reg_size, LLDB_INVALID_INDEX32, encoding, format,
+        {
+            LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
+            LLDB_INVALID_REGNUM, next_regnum_lldb++,
+        },
+        value_regs, nullptr, nullptr, 0
+  };
+
+  info.AddSupplementaryRegister(partial_reg, group);
+}
+
+void ABIAArch64::PreFinalizeDynamicRegisterInfo(
+    lldb_private::DynamicRegisterInfo &info) {
+  uint32_t next_regnum_lldb = info.GetNumRegisters();
+
+  // Create aliases for partial registers: wN for xN, and sN/dN for vN.
+  for (int i = 0; i < 32; i++)
+    AddPartialRegister(info,
+                       info.GetRegisterInfo(llvm::formatv("x{0}", i).str()), 8,
+                       llvm::formatv("w{0}", i), 4, lldb::eEncodingUint,
+                       lldb::eFormatHex, next_regnum_lldb);
+
+  for (int i = 0; i < 32; i++)
+    AddPartialRegister(info,
+                       info.GetRegisterInfo(llvm::formatv("v{0}", i).str()), 16,
+                       llvm::formatv("s{0}", i), 4, lldb::eEncodingIEEE754,
+                       lldb::eFormatFloat, next_regnum_lldb);
+
+  for (int i = 0; i < 32; i++)
+    AddPartialRegister(info,
+                       info.GetRegisterInfo(llvm::formatv("v{0}", i).str()), 16,
+                       llvm::formatv("d{0}", i), 8, lldb::eEncodingIEEE754,
+                       lldb::eFormatFloat, next_regnum_lldb);
+}

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

else
     arch_to_use = remote_host_arch;
 
   if (!arch_to_use.IsValid())
     arch_to_use = target_arch;
 
   if (GetGDBServerRegisterInfo(arch_to_use))
     return;
 
+  // We have to make a temporary ABI here, and not use the GetABI because
+  // this code gets called in DidAttach, when the target architecture
+  // (and consequently the ABI we'll get from the process) may be wrong.
+  ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
   char packet[128];
   std::vector<RemoteRegisterInfo> registers;
   uint32_t reg_num = 0;
   for (StringExtractorGDBRemote::ResponseType response_type =
            StringExtractorGDBRemote::eResponse;
        response_type == StringExtractorGDBRemote::eResponse; ++reg_num) {
     const int packet_len =
         ::snprintf(packet, sizeof(packet), "qRegisterInfo%x", reg_num);
if (arch_to_use.IsValid() &&
         arch_to_use.GetTriple().getVendor() == llvm::Triple::Apple)
       m_register_info_sp->HardcodeARMRegisters(from_scratch);
   } else if (target_arch.GetMachine() == llvm::Triple::arm ||
              target_arch.GetMachine() == llvm::Triple::thumb) {
     m_register_info_sp->HardcodeARMRegisters(from_scratch);
   }
 
   // At this point, we can finalize our register info.
+  if (abi_sp)
+    abi_sp->PreFinalizeDynamicRegisterInfo(*m_register_info_sp);
   m_register_info_sp->Finalize(GetTarget().GetArchitecture());
 }
 
 Status ProcessGDBRemote::WillLaunch(lldb_private::Module *module) {
   return WillLaunchOrAttach();
 }
 
 Status ProcessGDBRemote::WillAttachToProcessWithID(lldb::pid_t pid) {
struct RegisterInfo reg_info {
           remote_reg_info.dwarf_opcode_bytes.size(),
     };
 
     if (abi_sp)
       abi_sp->AugmentRegisterInfo(reg_info);
     m_register_info_sp->AddRegister(reg_info, remote_reg_info.set_name);
   };
 
+  if (abi_sp)
+    abi_sp->PreFinalizeDynamicRegisterInfo(*m_register_info_sp);
   m_register_info_sp->Finalize(arch_to_use);
 }
 
 // query the target of gdb-remote for extended target information returns
 // true on success (got register definitions), false on failure (did not).
 bool ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) {
   // Make sure LLDB has an XML parser it can use first
   if (!XMLDocument::XMLEnabled())

lldb/source/Target/DynamicRegisterInfo.cpp

for (size_t i = 0; i < num_sets; ++i) {
     }
     s.Printf("]\n");
   }
 }
 
 const lldb_private::RegisterInfo *
 DynamicRegisterInfo::GetRegisterInfo(llvm::StringRef reg_name) const {
   for (auto &reg_info : m_regs)
-    if (reg_info.name == reg_name)
+    if (reg_info.name == reg_name ||
+        (reg_info.alt_name && reg_info.alt_name == reg_name))
       return &reg_info;
   return nullptr;
 }

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"])