[LLDB] Add per-thread register infos shared pointer in gdb-remote

In gdb-remote process we have register infos defind as a refernce object of
GDBRemoteDynamicRegisterInfo class. In past register infos have remained
constant througout the life time of a process.

This has changed after AArch64 SVE support where register infos will have
per-thread configuration. SVE registers will have per-thread size and can
be updated while running. This patch aims to build up for that support by
changing GDBRemoteDynamicRegisterInfo reference to a shared pointer deinfed
per-thread.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D82857

Author: omjavaid

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

@@ -650,6 +650,8 @@ void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
   }
 }
 
+bool DynamicRegisterInfo::IsReconfigurable() { return m_is_reconfigurable; }
+
 size_t DynamicRegisterInfo::GetNumRegisters() const { return m_regs.size(); }
 
 size_t DynamicRegisterInfo::GetNumRegisterSets() const { return m_sets.size(); }

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

@@ -17,6 +17,10 @@
 #include "lldb/lldb-private.h"
 
 class DynamicRegisterInfo {
+protected:
+  DynamicRegisterInfo(DynamicRegisterInfo &) = default;
+  DynamicRegisterInfo &operator=(DynamicRegisterInfo &) = default;
+
 public:
   DynamicRegisterInfo() = default;
 
@@ -25,9 +29,6 @@ class DynamicRegisterInfo {
 
   virtual ~DynamicRegisterInfo() = default;
 
-  DynamicRegisterInfo(DynamicRegisterInfo &) = delete;
-  void operator=(DynamicRegisterInfo &) = delete;
-
   DynamicRegisterInfo(DynamicRegisterInfo &&info);
   DynamicRegisterInfo &operator=(DynamicRegisterInfo &&info);
 
@@ -63,6 +64,8 @@ class DynamicRegisterInfo {
 
   void Clear();
 
+  bool IsReconfigurable();
+
 protected:
   // Classes that inherit from DynamicRegisterInfo can see and modify these
   typedef std::vector<lldb_private::RegisterInfo> reg_collection;
@@ -89,5 +92,6 @@ class DynamicRegisterInfo {
   size_t m_reg_data_byte_size = 0u; // The number of bytes required to store
                                     // all registers
   bool m_finalized = false;
+  bool m_is_reconfigurable = false;
 };
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_DYNAMICREGISTERINFO_H

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

@@ -31,19 +31,20 @@ using namespace lldb_private::process_gdb_remote;
 // GDBRemoteRegisterContext constructor
 GDBRemoteRegisterContext::GDBRemoteRegisterContext(
     ThreadGDBRemote &thread, uint32_t concrete_frame_idx,
-    GDBRemoteDynamicRegisterInfo &reg_info, bool read_all_at_once,
+    GDBRemoteDynamicRegisterInfoSP reg_info_sp, bool read_all_at_once,
     bool write_all_at_once)
-    : RegisterContext(thread, concrete_frame_idx), m_reg_info(reg_info),
-      m_reg_valid(), m_reg_data(), m_read_all_at_once(read_all_at_once),
+    : RegisterContext(thread, concrete_frame_idx),
+      m_reg_info_sp(std::move(reg_info_sp)), m_reg_valid(), m_reg_data(),
+      m_read_all_at_once(read_all_at_once),
       m_write_all_at_once(write_all_at_once) {
   // Resize our vector of bools to contain one bool for every register. We will
   // use these boolean values to know when a register value is valid in
   // m_reg_data.
-  m_reg_valid.resize(reg_info.GetNumRegisters());
+  m_reg_valid.resize(m_reg_info_sp->GetNumRegisters());
 
   // Make a heap based buffer that is big enough to store all registers
   DataBufferSP reg_data_sp(
-      new DataBufferHeap(reg_info.GetRegisterDataByteSize(), 0));
+      new DataBufferHeap(m_reg_info_sp->GetRegisterDataByteSize(), 0));
   m_reg_data.SetData(reg_data_sp);
   m_reg_data.SetByteOrder(thread.GetProcess()->GetByteOrder());
 }
@@ -62,12 +63,12 @@ void GDBRemoteRegisterContext::SetAllRegisterValid(bool b) {
 }
 
 size_t GDBRemoteRegisterContext::GetRegisterCount() {
-  return m_reg_info.GetNumRegisters();
+  return m_reg_info_sp->GetNumRegisters();
 }
 
 const RegisterInfo *
 GDBRemoteRegisterContext::GetRegisterInfoAtIndex(size_t reg) {
-  RegisterInfo *reg_info = m_reg_info.GetRegisterInfoAtIndex(reg);
+  RegisterInfo *reg_info = m_reg_info_sp->GetRegisterInfoAtIndex(reg);
 
   if (reg_info && reg_info->dynamic_size_dwarf_expr_bytes) {
     const ArchSpec &arch = m_thread.GetProcess()->GetTarget().GetArchitecture();
@@ -78,11 +79,11 @@ GDBRemoteRegisterContext::GetRegisterInfoAtIndex(size_t reg) {
 }
 
 size_t GDBRemoteRegisterContext::GetRegisterSetCount() {
-  return m_reg_info.GetNumRegisterSets();
+  return m_reg_info_sp->GetNumRegisterSets();
 }
 
 const RegisterSet *GDBRemoteRegisterContext::GetRegisterSet(size_t reg_set) {
-  return m_reg_info.GetRegisterSet(reg_set);
+  return m_reg_info_sp->GetRegisterSet(reg_set);
 }
 
 bool GDBRemoteRegisterContext::ReadRegister(const RegisterInfo *reg_info,
@@ -209,9 +210,10 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info,
           SetAllRegisterValid(true);
           return true;
         } else if (buffer_sp->GetByteSize() > 0) {
-          const int regcount = m_reg_info.GetNumRegisters();
+          const int regcount = m_reg_info_sp->GetNumRegisters();
           for (int i = 0; i < regcount; i++) {
-            struct RegisterInfo *reginfo = m_reg_info.GetRegisterInfoAtIndex(i);
+            struct RegisterInfo *reginfo =
+                m_reg_info_sp->GetRegisterInfoAtIndex(i);
             if (reginfo->byte_offset + reginfo->byte_size 
                    <= buffer_sp->GetByteSize()) {
               m_reg_valid[i] = true;
@@ -506,7 +508,7 @@ bool GDBRemoteRegisterContext::ReadAllRegisterValues(
       // m_reg_data buffer
     }
     data_sp = std::make_shared<DataBufferHeap>(
-        m_reg_data.GetDataStart(), m_reg_info.GetRegisterDataByteSize());
+        m_reg_data.GetDataStart(), m_reg_info_sp->GetRegisterDataByteSize());
     return true;
   } else {
 
@@ -708,7 +710,7 @@ bool GDBRemoteRegisterContext::WriteAllRegisterValues(
 
 uint32_t GDBRemoteRegisterContext::ConvertRegisterKindToRegisterNumber(
     lldb::RegisterKind kind, uint32_t num) {
-  return m_reg_info.ConvertRegisterKindToRegisterNumber(kind, num);
+  return m_reg_info_sp->ConvertRegisterKindToRegisterNumber(kind, num);
 }
 
 void GDBRemoteDynamicRegisterInfo::HardcodeARMRegisters(bool from_scratch) {

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

@@ -27,8 +27,12 @@ namespace process_gdb_remote {
 
 class ThreadGDBRemote;
 class ProcessGDBRemote;
+class GDBRemoteDynamicRegisterInfo;
 
-class GDBRemoteDynamicRegisterInfo : public DynamicRegisterInfo {
+typedef std::shared_ptr<GDBRemoteDynamicRegisterInfo>
+    GDBRemoteDynamicRegisterInfoSP;
+
+class GDBRemoteDynamicRegisterInfo final : public DynamicRegisterInfo {
 public:
   GDBRemoteDynamicRegisterInfo() : DynamicRegisterInfo() {}
 
@@ -40,7 +44,7 @@ class GDBRemoteDynamicRegisterInfo : public DynamicRegisterInfo {
 class GDBRemoteRegisterContext : public RegisterContext {
 public:
   GDBRemoteRegisterContext(ThreadGDBRemote &thread, uint32_t concrete_frame_idx,
-                           GDBRemoteDynamicRegisterInfo &reg_info,
+                           GDBRemoteDynamicRegisterInfoSP reg_info_sp,
                            bool read_all_at_once, bool write_all_at_once);
 
   ~GDBRemoteRegisterContext() override;
@@ -106,7 +110,7 @@ class GDBRemoteRegisterContext : public RegisterContext {
       m_reg_valid[reg] = valid;
   }
 
-  GDBRemoteDynamicRegisterInfo &m_reg_info;
+  GDBRemoteDynamicRegisterInfoSP m_reg_info_sp;
   std::vector<bool> m_reg_valid;
   DataExtractor m_reg_data;
   bool m_read_all_at_once;

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

@@ -249,7 +249,7 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP target_sp,
                                    ListenerSP listener_sp)
     : Process(target_sp, listener_sp),
       m_debugserver_pid(LLDB_INVALID_PROCESS_ID), m_last_stop_packet_mutex(),
-      m_register_info(),
+      m_register_info_sp(nullptr),
       m_async_broadcaster(nullptr, "lldb.process.gdb-remote.async-broadcaster"),
       m_async_listener_sp(
           Listener::MakeListener("lldb.process.gdb-remote.async-listener")),
@@ -368,8 +368,8 @@ bool ProcessGDBRemote::ParsePythonTargetDefinition(
           m_breakpoint_pc_offset = breakpoint_pc_int_value->GetValue();
       }
 
-      if (m_register_info.SetRegisterInfo(*target_definition_sp,
-                                          GetTarget().GetArchitecture()) > 0) {
+      if (m_register_info_sp->SetRegisterInfo(
+              *target_definition_sp, GetTarget().GetArchitecture()) > 0) {
         return true;
       }
     }
@@ -396,10 +396,10 @@ static size_t SplitCommaSeparatedRegisterNumberString(
 }
 
 void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) {
-  if (!force && m_register_info.GetNumRegisters() > 0)
+  if (!force && m_register_info_sp)
     return;
 
-  m_register_info.Clear();
+  m_register_info_sp = std::make_shared<GDBRemoteDynamicRegisterInfo>();
 
   // Check if qHostInfo specified a specific packet timeout for this
   // connection. If so then lets update our setting so the user knows what the
@@ -581,7 +581,7 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) {
         if (ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use))
           abi_sp->AugmentRegisterInfo(reg_info);
 
-        m_register_info.AddRegister(reg_info, reg_name, alt_name, set_name);
+        m_register_info_sp->AddRegister(reg_info, reg_name, alt_name, set_name);
       } else {
         break; // ensure exit before reg_num is incremented
       }
@@ -590,8 +590,8 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) {
     }
   }
 
-  if (m_register_info.GetNumRegisters() > 0) {
-    m_register_info.Finalize(GetTarget().GetArchitecture());
+  if (m_register_info_sp->GetNumRegisters() > 0) {
+    m_register_info_sp->Finalize(GetTarget().GetArchitecture());
     return;
   }
 
@@ -600,21 +600,21 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) {
   // 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.
-  bool from_scratch = (m_register_info.GetNumRegisters() == 0);
+  bool from_scratch = (m_register_info_sp->GetNumRegisters() == 0);
 
   if (!target_arch.IsValid()) {
     if (arch_to_use.IsValid() &&
         (arch_to_use.GetMachine() == llvm::Triple::arm ||
          arch_to_use.GetMachine() == llvm::Triple::thumb) &&
         arch_to_use.GetTriple().getVendor() == llvm::Triple::Apple)
-      m_register_info.HardcodeARMRegisters(from_scratch);
+      m_register_info_sp->HardcodeARMRegisters(from_scratch);
   } else if (target_arch.GetMachine() == llvm::Triple::arm ||
              target_arch.GetMachine() == llvm::Triple::thumb) {
-    m_register_info.HardcodeARMRegisters(from_scratch);
+    m_register_info_sp->HardcodeARMRegisters(from_scratch);
   }
 
   // At this point, we can finalize our register info.
-  m_register_info.Finalize(GetTarget().GetArchitecture());
+  m_register_info_sp->Finalize(GetTarget().GetArchitecture());
 }
 
 Status ProcessGDBRemote::WillLaunch(lldb_private::Module *module) {
@@ -4567,7 +4567,7 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess(
       // 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,
+        ParseRegisters(feature_node, target_info, *this->m_register_info_sp,
                        abi_to_use_sp, reg_num_remote, reg_num_local);
       }
 
@@ -4597,9 +4597,9 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) {
   uint32_t reg_num_local = 0;
   if (GetGDBServerRegisterInfoXMLAndProcess(arch_to_use, "target.xml",
                                             reg_num_remote, reg_num_local))
-    this->m_register_info.Finalize(arch_to_use);
+    this->m_register_info_sp->Finalize(arch_to_use);
 
-  return m_register_info.GetNumRegisters() > 0;
+  return m_register_info_sp->GetNumRegisters() > 0;
 }
 
 llvm::Expected<LoadedModuleInfoList> ProcessGDBRemote::GetLoadedModuleList() {

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

@@ -254,7 +254,7 @@ class ProcessGDBRemote : public Process,
                                                              // the last stop
                                                              // packet variable
   std::recursive_mutex m_last_stop_packet_mutex;
-  GDBRemoteDynamicRegisterInfo m_register_info;
+  GDBRemoteDynamicRegisterInfoSP m_register_info_sp;
   Broadcaster m_async_broadcaster;
   lldb::ListenerSP m_async_listener_sp;
   HostThread m_async_thread;

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

@@ -42,6 +42,14 @@ ThreadGDBRemote::ThreadGDBRemote(Process &process, lldb::tid_t tid)
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD));
   LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this, process.GetID(),
            GetID());
+  // At this point we can clone reg_info for architectures supporting
+  // run-time update to register sizes and offsets..
+  auto &gdb_process = static_cast<ProcessGDBRemote &>(process);
+  if (!gdb_process.m_register_info_sp->IsReconfigurable())
+    m_reg_info_sp = gdb_process.m_register_info_sp;
+  else
+    m_reg_info_sp = std::make_shared<GDBRemoteDynamicRegisterInfo>(
+        *gdb_process.m_register_info_sp);
 }
 
 ThreadGDBRemote::~ThreadGDBRemote() {
@@ -307,8 +315,8 @@ ThreadGDBRemote::CreateRegisterContextForFrame(StackFrame *frame) {
           !pSupported || gdb_process->m_use_g_packet_for_reading;
       bool write_all_registers_at_once = !pSupported;
       reg_ctx_sp = std::make_shared<GDBRemoteRegisterContext>(
-          *this, concrete_frame_idx, gdb_process->m_register_info,
-          read_all_registers_at_once, write_all_registers_at_once);
+          *this, concrete_frame_idx, m_reg_info_sp, read_all_registers_at_once,
+          write_all_registers_at_once);
     }
   } else {
     reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);

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

@@ -14,6 +14,8 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/StructuredData.h"
 
+#include "GDBRemoteRegisterContext.h"
+
 class StringExtractor;
 
 namespace lldb_private {
@@ -101,6 +103,8 @@ class ThreadGDBRemote : public Thread {
       m_queue_serial_number; // Queue info from stop reply/stop info for thread
   lldb_private::LazyBool m_associated_with_libdispatch_queue;
 
+  GDBRemoteDynamicRegisterInfoSP m_reg_info_sp;
+
   bool PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef<uint8_t> data);
 
   bool PrivateSetRegisterValue(uint32_t reg, uint64_t regval);