[lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

[mgorny]

Summary

Switch the gdb-remote client logic to use local (LLDB) register numbers
in value_regs/invalidate_regs rather than remote regnos. This involves
translating regnos received from lldb-server.

[labath]

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

4667–4668

drop local_regnum and use llvm::enumerate

4681

Why should we have LLDB_INVALID_REGNUM in this list?

I think that a more interesting question is what to do if the value is not located in the remote_to_local_map. This will map it to zero, which isn't very useful. I suppose we could just drop this value -- this is something that was probably happening already (if we weren't just crashing).

[mgorny]

Rebase. Apply suggested changes.

[labath]

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

4614

auto :)

4621–4624

Make this a lambda and then do llvm::transform(remote_reg_info.invalidate_regs, remote_reg_info.invalidate_regs.begin(), lambda)

[mgorny]

Both requests implemented.

[labath]

cool

lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp

void DynamicRegisterInfo::ConfigureOffsets() {
   for (auto &reg : m_regs) {
     if (reg.value_regs != nullptr) {
       // Assign a valid offset to all pseudo registers if not assigned by stub.
       // Pseudo registers with value_regs list populated will share same offset
       // as that of their corresponding primary register in value_regs list.
       if (reg.byte_offset == LLDB_INVALID_INDEX32) {
         uint32_t value_regnum = reg.value_regs[0];
         if (value_regnum != LLDB_INVALID_INDEX32)
-          reg.byte_offset =
-              GetRegisterInfoAtIndex(remote_to_local_regnum_map[value_regnum])
-                  ->byte_offset;
+          reg.byte_offset = GetRegisterInfoAtIndex(value_regnum)->byte_offset;
       }
     }
 
     reg_offset = reg.byte_offset + reg.byte_size;
     if (m_reg_data_byte_size < reg_offset)
       m_reg_data_byte_size = reg_offset;
   }
 }

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

if (reg_info->value_regs) {
       bool success = true;
       for (uint32_t idx = 0; success; ++idx) {
         const uint32_t prim_reg = reg_info->value_regs[idx];
         if (prim_reg == LLDB_INVALID_REGNUM)
           break;
         // We have a valid primordial register as our constituent. Grab the
         // corresponding register info.
         const RegisterInfo *prim_reg_info =
-            GetRegisterInfo(eRegisterKindProcessPlugin, prim_reg);
+            GetRegisterInfo(eRegisterKindLLDB, prim_reg);
         if (prim_reg_info == nullptr)
           success = false;
         else {
           // Read the containing register if it hasn't already been read
           if (!GetRegisterIsValid(prim_reg))
             success = GetPrimordialRegister(prim_reg_info, gdb_comm);
         }
       }
if (lock) {
 
           for (uint32_t idx = 0; success; ++idx) {
             const uint32_t reg = reg_info->value_regs[idx];
             if (reg == LLDB_INVALID_REGNUM)
               break;
             // We have a valid primordial register as our constituent. Grab the
             // corresponding register info.
             const RegisterInfo *value_reg_info =
-                GetRegisterInfo(eRegisterKindProcessPlugin, reg);
+                GetRegisterInfo(eRegisterKindLLDB, reg);
             if (value_reg_info == nullptr)
               success = false;
             else
               success = SetPrimordialRegister(value_reg_info, gdb_comm);
           }
         } else {
           // This is an actual register, write it
           success = SetPrimordialRegister(reg_info, gdb_comm);
 
           if (success && do_reconfigure_arm64_sve)
             AArch64SVEReconfigure();
         }
 
         // Check if writing this register will invalidate any other register
         // values? If so, invalidate them
         if (reg_info->invalidate_regs) {
           for (uint32_t idx = 0, reg = reg_info->invalidate_regs[0];
                reg != LLDB_INVALID_REGNUM;
                reg = reg_info->invalidate_regs[++idx])
             SetRegisterIsValid(ConvertRegisterKindToRegisterNumber(
-                                   eRegisterKindProcessPlugin, reg),
+                                   eRegisterKindLLDB, reg),
                                false);
         }
 
         return success;
       }
     } else {
       Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_THREAD |
                                                              GDBR_LOG_PACKETS));

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

 
 void ProcessGDBRemote::AddRemoteRegisters(
     std::vector<RemoteRegisterInfo> &registers, const ArchSpec &arch_to_use) {
   // Don't use Process::GetABI, this code gets called from DidAttach, and
   // in that context we haven't set the Target's architecture yet, so the
   // ABI is also potentially incorrect.
   ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
 
+  std::map<uint32_t, uint32_t> remote_to_local_map;
   uint32_t remote_regnum = 0;
   for (auto it : llvm::enumerate(registers)) {
+    RemoteRegisterInfo &remote_reg_info = it.value();
+
+    // Assign successive remote regnums if missing.
+    if (remote_reg_info.regnum_remote == LLDB_INVALID_REGNUM)
+      remote_reg_info.regnum_remote = remote_regnum;
+
+    // Create a mapping from remote to local regnos.
+    remote_to_local_map[remote_reg_info.regnum_remote] = it.index();
+
+    remote_regnum = remote_reg_info.regnum_remote + 1;
+  }
+
+  for (auto it : llvm::enumerate(registers)) {
     uint32_t local_regnum = it.index();
     RemoteRegisterInfo &remote_reg_info = it.value();
-    // Use remote regnum if available, previous remote regnum + 1 when not.
-    if (remote_reg_info.regnum_remote != LLDB_INVALID_REGNUM)
-      remote_regnum = remote_reg_info.regnum_remote;
+
+    auto proc_to_lldb = [&remote_to_local_map](uint32_t process_regnum) {
+      auto lldb_regit = remote_to_local_map.find(process_regnum);
+      return lldb_regit != remote_to_local_map.end() ? lldb_regit->second
+                                                     : LLDB_INVALID_REGNUM;
+    };
+
+    llvm::transform(remote_reg_info.value_regs,
+                    remote_reg_info.value_regs.begin(), proc_to_lldb);
+    llvm::transform(remote_reg_info.invalidate_regs,
+                    remote_reg_info.invalidate_regs.begin(), proc_to_lldb);
 
     auto regs_with_sentinel = [](std::vector<uint32_t> &vec) -> uint32_t * {
       if (!vec.empty()) {
         vec.push_back(LLDB_INVALID_REGNUM);
         return vec.data();
       }
       return nullptr;
     };
 
     struct RegisterInfo reg_info {
       remote_reg_info.name.AsCString(), remote_reg_info.alt_name.AsCString(),
           remote_reg_info.byte_size, remote_reg_info.byte_offset,
           remote_reg_info.encoding, remote_reg_info.format,
           {remote_reg_info.regnum_ehframe, remote_reg_info.regnum_dwarf,
-           remote_reg_info.regnum_generic, remote_regnum++, local_regnum},
+           remote_reg_info.regnum_generic, remote_reg_info.regnum_remote,
+           local_regnum},
           regs_with_sentinel(remote_reg_info.value_regs),
           regs_with_sentinel(remote_reg_info.invalidate_regs),
           !remote_reg_info.dwarf_opcode_bytes.empty()
               ? remote_reg_info.dwarf_opcode_bytes.data()
               : nullptr,
           remote_reg_info.dwarf_opcode_bytes.size(),
     };
 
bool ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) {
   if (!XMLDocument::XMLEnabled())
     return false;
 
   // check that we have extended feature read support
   if (!m_gdb_comm.GetQXferFeaturesReadSupported())
     return false;
 
   std::vector<RemoteRegisterInfo> registers;
   if (GetGDBServerRegisterInfoXMLAndProcess(arch_to_use, "target.xml",
                                             registers))
     AddRemoteRegisters(registers, arch_to_use);
 
   return m_register_info_sp->GetNumRegisters() > 0;
 }
 
 llvm::Expected<LoadedModuleInfoList> ProcessGDBRemote::GetLoadedModuleList() {
   // Make sure LLDB has an XML parser it can use first
   if (!XMLDocument::XMLEnabled())
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "XML parsing not available");
 
   Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS);
   LLDB_LOGF(log, "ProcessGDBRemote::%s", __FUNCTION__);
 
   LoadedModuleInfoList list;
   GDBRemoteCommunicationClient &comm = m_gdb_comm;
   bool can_use_svr4 = GetGlobalPluginProperties()->GetUseSVR4();
 
   // check that we have extended feature read support
   if (can_use_svr4 && comm.GetQXferLibrariesSVR4ReadSupported()) {
     // request the loaded library list