[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

4684

drop local_regnum and use llvm::enumerate

4699

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

4663

auto :)

4670–4673

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

 //===-- ProcessGDBRemote.cpp ----------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/Config.h"
 
 #include <cerrno>
 #include <cstdlib>
 #if LLDB_ENABLE_POSIX
 #include <netinet/in.h>
 #include <sys/mman.h>
 #include <sys/socket.h>
 #include <unistd.h>
if (xml_document.ParseMemory(raw.c_str(), raw.size(), xml_filename.c_str())) {
     }
   } else {
     return false;
   }
   return true;
 }
 
 // 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())
     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)) {
     // 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);
 
     uint32_t local_regnum = 0;
     uint32_t remote_regnum = 0;
+    std::map<uint32_t, uint32_t> remote_to_local_map;
     for (RemoteRegisterInfo& remote_reg_info : registers) {
-      // 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;
+      // 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] = local_regnum++;
+
+      remote_regnum = remote_reg_info.regnum_remote + 1;
+    }
+
+    local_regnum = 0;
+    for (RemoteRegisterInfo& remote_reg_info : registers) {
+      // Translate value_regs and invalidate_regs to local regnums
+      for (uint32_t &x : remote_reg_info.value_regs) {
+        if (x != LLDB_INVALID_REGNUM)
+          x = remote_to_local_map[x];
+      }
+      for (uint32_t &x : remote_reg_info.invalidate_regs) {
+        if (x != LLDB_INVALID_REGNUM)
+          x = remote_to_local_map[x];
+      }
 
       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++,
+            remote_reg_info.regnum_remote,
             local_regnum++ },
           remote_reg_info.value_regs.data(),
           remote_reg_info.invalidate_regs.data(),
           remote_reg_info.dwarf_opcode_bytes.data(),
           remote_reg_info.dwarf_opcode_bytes.size(),
       };
 
       if (abi_sp)