[lldb] [gdb-remote] Refactor getting remote regs to use local vector

[mgorny]

Summary

Refactor remote register getters to collect them into a local
std::vector rather than adding them straight into DynamicRegisterInfo.
The purpose of this change is to lay groundwork for switching value_regs
and invalidate_regs to use local LLDB register numbers rather than
remote numbers.

[mgorny]

@labath, please tell me if you like this design. If it's ok, I'm going to convert the other routine to it, then move the conversion to DynamicRegisterInfo into a common function.

(also I haven't reformatted the code yet)

[labath]

I like it.

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

4318–4321

struct process_gdb_remote::RemoteRegisterInfo { should work too. Putting the declaration in the header would also be ok.

[mgorny]

Put the struct into the header. Put the common conversion and finalization code into a helper method. Use llvm::enumerate for local regnums. Convert qRegisterInfo getters.

[labath]

Looks good, just be careful about sentinels.

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

571–578

It looks like you're not doing it here.

4460–4461

Can we avoid pushing the sentinel here? I'd hope this can be done during the conversion to the "RegisterInfo" format...

4597

i.e., add a push(sentinel) here.

[mgorny]

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

4460–4461

Yes, that makes sense and reduces code duplication even more. Thanks!

4597

Hmm, actually you made me realize I'm pushing empty arrays instead of nullptr sometimes. Let's fix that as well.

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

 #include "llvm/ADT/DenseMap.h"
 
 namespace lldb_private {
 namespace repro {
 class Loader;
 }
 namespace process_gdb_remote {
 
+struct RemoteRegisterInfo {
+  ConstString name;
+  ConstString alt_name;
+  ConstString set_name;
+  uint32_t byte_size = LLDB_INVALID_INDEX32;
+  uint32_t byte_offset = LLDB_INVALID_INDEX32;
+  lldb::Encoding encoding = lldb::eEncodingUint;
+  lldb::Format format = lldb::eFormatHex;
+  uint32_t regnum_dwarf = LLDB_INVALID_REGNUM;
+  uint32_t regnum_ehframe = LLDB_INVALID_REGNUM;
+  uint32_t regnum_generic = LLDB_INVALID_REGNUM;
+  uint32_t regnum_remote = LLDB_INVALID_REGNUM;
+  std::vector<uint32_t> value_regs;
+  std::vector<uint32_t> invalidate_regs;
+  std::vector<uint8_t> dwarf_opcode_bytes;
+};
+
 class ThreadGDBRemote;
 
 class ProcessGDBRemote : public Process,
                          private GDBRemoteClientBase::ContinueDelegate {
 public:
   ProcessGDBRemote(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
 
   ~ProcessGDBRemote() override;
protected:
 
   Status ConnectToDebugserver(llvm::StringRef host_port);
 
   const char *GetDispatchQueueNameForThread(lldb::addr_t thread_dispatch_qaddr,
                                             std::string &dispatch_queue_name);
 
   DynamicLoader *GetDynamicLoader() override;
 
-  bool GetGDBServerRegisterInfoXMLAndProcess(ArchSpec &arch_to_use,
-                                             std::string xml_filename,
-                                             uint32_t &cur_reg_remote,
-                                             uint32_t &cur_reg_local);
-
+  bool GetGDBServerRegisterInfoXMLAndProcess(
+    ArchSpec &arch_to_use, std::string xml_filename,
+    std::vector<RemoteRegisterInfo> &registers);
+
+  // Convert RemoteRegisterInfos into RegisterInfos and add to the dynamic
+  // register list.
+  void AddRemoteRegisters(std::vector<RemoteRegisterInfo> &registers,
+                          const ArchSpec &arch_to_use);
   // Query remote GDBServer for register information
   bool GetGDBServerRegisterInfo(ArchSpec &arch);
 
   lldb::ModuleSP LoadModuleAtAddress(const FileSpec &file,
                                      lldb::addr_t link_map,
                                      lldb::addr_t base_addr,
                                      bool value_is_offset);
 

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

void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) {
 
   if (!arch_to_use.IsValid())
     arch_to_use = target_arch;
 
   if (GetGDBServerRegisterInfo(arch_to_use))
     return;
 
   char packet[128];
-  uint32_t reg_offset = LLDB_INVALID_INDEX32;
+  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);
     assert(packet_len < (int)sizeof(packet));
     UNUSED_IF_ASSERT_DISABLED(packet_len);
     StringExtractorGDBRemote response;
     if (m_gdb_comm.SendPacketAndWaitForResponse(packet, response) ==
         GDBRemoteCommunication::PacketResult::Success) {
       response_type = response.GetResponseType();
       if (response_type == StringExtractorGDBRemote::eResponse) {
         llvm::StringRef name;
         llvm::StringRef value;
-        ConstString reg_name;
-        ConstString alt_name;
-        ConstString set_name;
-        std::vector<uint32_t> value_regs;
-        std::vector<uint32_t> invalidate_regs;
-        std::vector<uint8_t> dwarf_opcode_bytes;
-        RegisterInfo reg_info = {
-            nullptr,       // Name
-            nullptr,       // Alt name
-            0,             // byte size
-            reg_offset,    // offset
-            eEncodingUint, // encoding
-            eFormatHex,    // format
-            {
-                LLDB_INVALID_REGNUM, // eh_frame reg num
-                LLDB_INVALID_REGNUM, // DWARF reg num
-                LLDB_INVALID_REGNUM, // generic reg num
-                reg_num,             // process plugin reg num
-                reg_num              // native register number
-            },
-            nullptr,
-            nullptr,
-            nullptr, // Dwarf expression opcode bytes pointer
-            0        // Dwarf expression opcode bytes length
-        };
+        RemoteRegisterInfo reg_info;
 
         while (response.GetNameColonValue(name, value)) {
           if (name.equals("name")) {
-            reg_name.SetString(value);
+            reg_info.name.SetString(value);
           } else if (name.equals("alt-name")) {
-            alt_name.SetString(value);
+            reg_info.alt_name.SetString(value);
           } else if (name.equals("bitsize")) {
-            value.getAsInteger(0, reg_info.byte_size);
-            reg_info.byte_size /= CHAR_BIT;
+            reg_info.byte_size =
+                StringConvert::ToUInt32(value.data(), 0, 0) / CHAR_BIT;
           } else if (name.equals("offset")) {
-            if (value.getAsInteger(0, reg_offset))
-              reg_offset = UINT32_MAX;
+            reg_info.byte_offset =
+                StringConvert::ToUInt32(value.data(), LLDB_INVALID_INDEX32, 0);
           } else if (name.equals("encoding")) {
             const Encoding encoding = Args::StringToEncoding(value);
             if (encoding != eEncodingInvalid)
               reg_info.encoding = encoding;
           } else if (name.equals("format")) {
-            Format format = eFormatInvalid;
-            if (OptionArgParser::ToFormat(value.str().c_str(), format, nullptr)
+            if (!OptionArgParser::ToFormat(value.str().c_str(), reg_info.format, nullptr)
                     .Success())
-              reg_info.format = format;
-            else {
               reg_info.format =
                   llvm::StringSwitch<Format>(value)
                       .Case("binary", eFormatBinary)
                       .Case("decimal", eFormatDecimal)
                       .Case("hex", eFormatHex)
                       .Case("float", eFormatFloat)
                       .Case("vector-sint8", eFormatVectorOfSInt8)
                       .Case("vector-uint8", eFormatVectorOfUInt8)
                       .Case("vector-sint16", eFormatVectorOfSInt16)
                       .Case("vector-uint16", eFormatVectorOfUInt16)
                       .Case("vector-sint32", eFormatVectorOfSInt32)
                       .Case("vector-uint32", eFormatVectorOfUInt32)
                       .Case("vector-float32", eFormatVectorOfFloat32)
                       .Case("vector-uint64", eFormatVectorOfUInt64)
                       .Case("vector-uint128", eFormatVectorOfUInt128)
                       .Default(eFormatInvalid);
-            }
           } else if (name.equals("set")) {
-            set_name.SetString(value);
+            reg_info.set_name.SetString(value);
           } else if (name.equals("gcc") || name.equals("ehframe")) {
-            if (value.getAsInteger(0, reg_info.kinds[eRegisterKindEHFrame]))
-              reg_info.kinds[eRegisterKindEHFrame] = LLDB_INVALID_REGNUM;
+            reg_info.regnum_ehframe =
+                StringConvert::ToUInt32(value.data(), LLDB_INVALID_REGNUM, 0);
           } else if (name.equals("dwarf")) {
-            if (value.getAsInteger(0, reg_info.kinds[eRegisterKindDWARF]))
-              reg_info.kinds[eRegisterKindDWARF] = LLDB_INVALID_REGNUM;
+            reg_info.regnum_dwarf =
+                StringConvert::ToUInt32(value.data(), LLDB_INVALID_REGNUM, 0);
           } else if (name.equals("generic")) {
-            reg_info.kinds[eRegisterKindGeneric] =
-                Args::StringToGenericRegister(value);
+            reg_info.regnum_generic =
+                StringConvert::ToUInt32(value.data(), LLDB_INVALID_REGNUM, 0);
           } else if (name.equals("container-regs")) {
-            SplitCommaSeparatedRegisterNumberString(value, value_regs, 16);
+            SplitCommaSeparatedRegisterNumberString(value, reg_info.value_regs, 16);
           } else if (name.equals("invalidate-regs")) {
-            SplitCommaSeparatedRegisterNumberString(value, invalidate_regs, 16);
+            SplitCommaSeparatedRegisterNumberString(value, reg_info.invalidate_regs, 16);
           } else if (name.equals("dynamic_size_dwarf_expr_bytes")) {
             size_t dwarf_opcode_len = value.size() / 2;
             assert(dwarf_opcode_len > 0);
 
-            dwarf_opcode_bytes.resize(dwarf_opcode_len);
-            reg_info.dynamic_size_dwarf_len = dwarf_opcode_len;
+            reg_info.dwarf_opcode_bytes.resize(dwarf_opcode_len);
 
             StringExtractor opcode_extractor(value);
             uint32_t ret_val =
-                opcode_extractor.GetHexBytesAvail(dwarf_opcode_bytes);
+                opcode_extractor.GetHexBytesAvail(reg_info.dwarf_opcode_bytes);
             assert(dwarf_opcode_len == ret_val);
             UNUSED_IF_ASSERT_DISABLED(ret_val);
-            reg_info.dynamic_size_dwarf_expr_bytes = dwarf_opcode_bytes.data();
           }
         }
 
-        reg_info.byte_offset = reg_offset;
         assert(reg_info.byte_size != 0);
-        reg_offset = LLDB_INVALID_INDEX32;
-        if (!value_regs.empty()) {
-          value_regs.push_back(LLDB_INVALID_REGNUM);
-          reg_info.value_regs = value_regs.data();
-        }
-        if (!invalidate_regs.empty()) {
-          invalidate_regs.push_back(LLDB_INVALID_REGNUM);
-          reg_info.invalidate_regs = invalidate_regs.data();
-        }
-
-        reg_info.name = reg_name.AsCString();
-        reg_info.alt_name = alt_name.AsCString();
-        // 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.
-        if (ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use))
-          abi_sp->AugmentRegisterInfo(reg_info);
-
-        m_register_info_sp->AddRegister(reg_info, set_name);
       } else {
         break; // ensure exit before reg_num is incremented
       }
     } else {
       break;
     }
   }
 
-  if (m_register_info_sp->GetNumRegisters() > 0) {
-    m_register_info_sp->Finalize(GetTarget().GetArchitecture());
+  if (!registers.empty()) {
+    AddRemoteRegisters(registers, GetTarget().GetArchitecture());
     return;
   }
 
   // We didn't get anything if the accumulated reg_num is zero.  See if we are
   // debugging ARM and fill with a hard coded register set until we can get an
   // updated debugserver down on the devices. On the other hand, if the
   // accumulated reg_num is positive, see if we can add composite registers to
   // the existing primordial ones.

 llvm::VersionTuple ProcessGDBRemote::GetHostOSVersion() {
   return m_gdb_comm.GetOSVersion();
 }
 
 llvm::VersionTuple ProcessGDBRemote::GetHostMacCatalystVersion() {
   return m_gdb_comm.GetMacCatalystVersion();
 }
 
 namespace {
 
 typedef std::vector<std::string> stringVec;
 
 typedef std::vector<struct GdbServerRegisterInfo> GDBServerRegisterVec;
 struct RegisterSetInfo {
   ConstString name;
 };
 
 typedef std::map<uint32_t, RegisterSetInfo> RegisterSetMap;
 
 struct GdbServerTargetInfo {
   std::string arch;
   std::string osabi;
   stringVec includes;
   RegisterSetMap reg_set_map;
 };
 
 bool ParseRegisters(XMLNode feature_node, GdbServerTargetInfo &target_info,
-                    GDBRemoteDynamicRegisterInfo &dyn_reg_info, ABISP abi_sp,
-                    uint32_t &reg_num_remote, uint32_t &reg_num_local) {
+                    std::vector<RemoteRegisterInfo> &registers) {
   if (!feature_node)
     return false;
 
-  uint32_t reg_offset = LLDB_INVALID_INDEX32;
   feature_node.ForEachChildElementWithName(
-      "reg", [&target_info, &dyn_reg_info, &reg_num_remote, &reg_num_local,
-              &reg_offset, &abi_sp](const XMLNode &reg_node) -> bool {
+      "reg", [&target_info, &registers](const XMLNode &reg_node) -> bool {
         std::string gdb_group;
         std::string gdb_type;
-        ConstString reg_name;
-        ConstString alt_name;
-        ConstString set_name;
-        std::vector<uint32_t> value_regs;
-        std::vector<uint32_t> invalidate_regs;
-        std::vector<uint8_t> dwarf_opcode_bytes;
+        RemoteRegisterInfo reg_info;
         bool encoding_set = false;
         bool format_set = false;
-        RegisterInfo reg_info = {
-            nullptr,       // Name
-            nullptr,       // Alt name
-            0,             // byte size
-            reg_offset,    // offset
-            eEncodingUint, // encoding
-            eFormatHex,    // format
-            {
-                LLDB_INVALID_REGNUM, // eh_frame reg num
-                LLDB_INVALID_REGNUM, // DWARF reg num
-                LLDB_INVALID_REGNUM, // generic reg num
-                reg_num_remote,      // process plugin reg num
-                reg_num_local        // native register number
-            },
-            nullptr,
-            nullptr,
-            nullptr, // Dwarf Expression opcode bytes pointer
-            0        // Dwarf Expression opcode bytes length
-        };
 
         reg_node.ForEachAttribute([&target_info, &gdb_group, &gdb_type,
-                                   &reg_name, &alt_name, &set_name, &value_regs,
-                                   &invalidate_regs, &encoding_set, &format_set,
-                                   &reg_info, &reg_offset, &dwarf_opcode_bytes](
+                                   &encoding_set, &format_set, &reg_info](
                                       const llvm::StringRef &name,
                                       const llvm::StringRef &value) -> bool {
           if (name == "name") {
-            reg_name.SetString(value);
+            reg_info.name.SetString(value);
           } else if (name == "bitsize") {
             reg_info.byte_size =
                 StringConvert::ToUInt32(value.data(), 0, 0) / CHAR_BIT;
           } else if (name == "type") {
             gdb_type = value.str();
           } else if (name == "group") {
             gdb_group = value.str();
           } else if (name == "regnum") {
             const uint32_t regnum =
                 StringConvert::ToUInt32(value.data(), LLDB_INVALID_REGNUM, 0);
             if (regnum != LLDB_INVALID_REGNUM) {
-              reg_info.kinds[eRegisterKindProcessPlugin] = regnum;
+              reg_info.regnum_remote = regnum;
             }
           } else if (name == "offset") {
-            reg_offset = StringConvert::ToUInt32(value.data(), UINT32_MAX, 0);
+            reg_info.byte_offset =
+                StringConvert::ToUInt32(value.data(), LLDB_INVALID_INDEX32, 0);
           } else if (name == "altname") {
-            alt_name.SetString(value);
+            reg_info.alt_name.SetString(value);
           } else if (name == "encoding") {
             encoding_set = true;
             reg_info.encoding = Args::StringToEncoding(value, eEncodingUint);
           } else if (name == "format") {
             format_set = true;
-            Format format = eFormatInvalid;
-            if (OptionArgParser::ToFormat(value.data(), format, nullptr)
+            if (!OptionArgParser::ToFormat(value.data(), reg_info.format,
+                                           nullptr)
                      .Success())
-              reg_info.format = format;
-            else if (value == "vector-sint8")
-              reg_info.format = eFormatVectorOfSInt8;
-            else if (value == "vector-uint8")
-              reg_info.format = eFormatVectorOfUInt8;
-            else if (value == "vector-sint16")
-              reg_info.format = eFormatVectorOfSInt16;
-            else if (value == "vector-uint16")
-              reg_info.format = eFormatVectorOfUInt16;
-            else if (value == "vector-sint32")
-              reg_info.format = eFormatVectorOfSInt32;
-            else if (value == "vector-uint32")
-              reg_info.format = eFormatVectorOfUInt32;
-            else if (value == "vector-float32")
-              reg_info.format = eFormatVectorOfFloat32;
-            else if (value == "vector-uint64")
-              reg_info.format = eFormatVectorOfUInt64;
-            else if (value == "vector-uint128")
-              reg_info.format = eFormatVectorOfUInt128;
+              reg_info.format =
+                  llvm::StringSwitch<lldb::Format>(value)
+                      .Case("vector-sint8", eFormatVectorOfSInt8)
+                      .Case("vector-uint8", eFormatVectorOfUInt8)
+                      .Case("vector-sint16", eFormatVectorOfSInt16)
+                      .Case("vector-uint16", eFormatVectorOfUInt16)
+                      .Case("vector-sint32", eFormatVectorOfSInt32)
+                      .Case("vector-uint32", eFormatVectorOfUInt32)
+                      .Case("vector-float32", eFormatVectorOfFloat32)
+                      .Case("vector-uint64", eFormatVectorOfUInt64)
+                      .Case("vector-uint128", eFormatVectorOfUInt128)
+                      .Default(eFormatInvalid);
           } else if (name == "group_id") {
             const uint32_t set_id =
                 StringConvert::ToUInt32(value.data(), UINT32_MAX, 0);
             RegisterSetMap::const_iterator pos =
                 target_info.reg_set_map.find(set_id);
             if (pos != target_info.reg_set_map.end())
-              set_name = pos->second.name;
+              reg_info.set_name = pos->second.name;
           } else if (name == "gcc_regnum" || name == "ehframe_regnum") {
-            reg_info.kinds[eRegisterKindEHFrame] =
+            reg_info.regnum_ehframe =
                 StringConvert::ToUInt32(value.data(), LLDB_INVALID_REGNUM, 0);
           } else if (name == "dwarf_regnum") {
-            reg_info.kinds[eRegisterKindDWARF] =
+            reg_info.regnum_dwarf =
                 StringConvert::ToUInt32(value.data(), LLDB_INVALID_REGNUM, 0);
           } else if (name == "generic") {
-            reg_info.kinds[eRegisterKindGeneric] =
-                Args::StringToGenericRegister(value);
+            reg_info.regnum_generic = Args::StringToGenericRegister(value);
           } else if (name == "value_regnums") {
-            SplitCommaSeparatedRegisterNumberString(value, value_regs, 0);
+            SplitCommaSeparatedRegisterNumberString(value, reg_info.value_regs,
+                                                    0);
           } else if (name == "invalidate_regnums") {
-            SplitCommaSeparatedRegisterNumberString(value, invalidate_regs, 0);
+            SplitCommaSeparatedRegisterNumberString(
+                value, reg_info.invalidate_regs, 0);
           } else if (name == "dynamic_size_dwarf_expr_bytes") {
             std::string opcode_string = value.str();
             size_t dwarf_opcode_len = opcode_string.length() / 2;
             assert(dwarf_opcode_len > 0);
 
-            dwarf_opcode_bytes.resize(dwarf_opcode_len);
-            reg_info.dynamic_size_dwarf_len = dwarf_opcode_len;
+            reg_info.dwarf_opcode_bytes.resize(dwarf_opcode_len);
             StringExtractor opcode_extractor(opcode_string);
             uint32_t ret_val =
-                opcode_extractor.GetHexBytesAvail(dwarf_opcode_bytes);
+                opcode_extractor.GetHexBytesAvail(reg_info.dwarf_opcode_bytes);
             assert(dwarf_opcode_len == ret_val);
             UNUSED_IF_ASSERT_DISABLED(ret_val);
-            reg_info.dynamic_size_dwarf_expr_bytes = dwarf_opcode_bytes.data();
           } else {
             printf("unhandled attribute %s = %s\n", name.data(), value.data());
           }
           return true; // Keep iterating through all attributes
         });
 
         if (!gdb_type.empty() && !(encoding_set || format_set)) {
           if (llvm::StringRef(gdb_type).startswith("int")) {
feature_node.ForEachChildElementWithName(
             reg_info.format = eFormatVectorOfUInt8;
             reg_info.encoding = eEncodingVector;
           }
         }
 
         // Only update the register set name if we didn't get a "reg_set"
         // attribute. "set_name" will be empty if we didn't have a "reg_set"
         // attribute.
-        if (!set_name) {
+        if (!reg_info.set_name) {
           if (!gdb_group.empty()) {
-            set_name.SetCString(gdb_group.c_str());
+            reg_info.set_name.SetCString(gdb_group.c_str());
           } else {
             // If no register group name provided anywhere,
             // we'll create a 'general' register set
-            set_name.SetCString("general");
+            reg_info.set_name.SetCString("general");
           }
         }
 
-        reg_info.byte_offset = reg_offset;
         assert(reg_info.byte_size != 0);
-        reg_offset = LLDB_INVALID_INDEX32;
-        if (!value_regs.empty()) {
-          value_regs.push_back(LLDB_INVALID_REGNUM);
-          reg_info.value_regs = value_regs.data();
-        }
-        if (!invalidate_regs.empty()) {
-          invalidate_regs.push_back(LLDB_INVALID_REGNUM);
-          reg_info.invalidate_regs = invalidate_regs.data();
-        }
-
-        reg_num_remote = reg_info.kinds[eRegisterKindProcessPlugin] + 1;
-        ++reg_num_local;
-        reg_info.name = reg_name.AsCString();
-        reg_info.alt_name = alt_name.AsCString();
-        if (abi_sp)
-          abi_sp->AugmentRegisterInfo(reg_info);
-        dyn_reg_info.AddRegister(reg_info, set_name);
-
+        registers.push_back(reg_info);
         return true; // Keep iterating through all "reg" elements
       });
   return true;
 }
 
 } // namespace
 
 // This method fetches a register description feature xml file from
 // the remote stub and adds registers/register groupsets/architecture
 // information to the current process.  It will call itself recursively
 // for nested register definition files.  It returns true if it was able
 // to fetch and parse an xml file.
 bool ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess(
-    ArchSpec &arch_to_use, std::string xml_filename, uint32_t &reg_num_remote,
-    uint32_t &reg_num_local) {
+    ArchSpec &arch_to_use, std::string xml_filename, std::vector<RemoteRegisterInfo> &registers) {
   // request the target xml file
   std::string raw;
   lldb_private::Status lldberr;
   if (!m_gdb_comm.ReadExtFeature(ConstString("features"),
                                  ConstString(xml_filename.c_str()), raw,
                                  lldberr)) {
     return false;
   }
if (!arch_to_use.IsValid() && !target_info.arch.empty()) {
                                 .Default(target_info.arch) +
                             "--");
 
       if (arch_to_use.IsValid())
         GetTarget().MergeArchitecture(arch_to_use);
     }
 
     if (arch_to_use.IsValid()) {
-      // 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_to_use_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
       for (auto &feature_node : feature_nodes) {
-        ParseRegisters(feature_node, target_info, *this->m_register_info_sp,
-                       abi_to_use_sp, reg_num_remote, reg_num_local);
+        ParseRegisters(feature_node, target_info,
+                       registers);
       }
 
       for (const auto &include : target_info.includes) {
         GetGDBServerRegisterInfoXMLAndProcess(arch_to_use, include,
-                                              reg_num_remote, reg_num_local);
+                                              registers);
       }
     }
   } else {
     return false;
   }
   return true;
 }
 
+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);
+
+  uint32_t remote_regnum = 0;
+  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 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},
+          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(),
+    };
+
+    if (abi_sp)
+      abi_sp->AugmentRegisterInfo(reg_info);
+    m_register_info_sp->AddRegister(reg_info, remote_reg_info.set_name);
+  };
+
+  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())
     return false;
 
   // check that we have extended feature read support
   if (!m_gdb_comm.GetQXferFeaturesReadSupported())
     return false;
 
-  uint32_t reg_num_remote = 0;
-  uint32_t reg_num_local = 0;
+  std::vector<RemoteRegisterInfo> registers;
   if (GetGDBServerRegisterInfoXMLAndProcess(arch_to_use, "target.xml",
-                                            reg_num_remote, reg_num_local))
-    this->m_register_info_sp->Finalize(arch_to_use);
+                                            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(),