[lldb][AArch64] Move register info reconfigure into architecture plugin (#70950)

This removes AArch64 specific code from the GDB* classes.

To do this I've added 2 new methods to Architecture:
* RegisterWriteCausesReconfigure to check if what you are about to do
  will trash the register info.
* ReconfigureRegisterInfo to do the reconfiguring. This tells you if
  anything changed so that we only invalidate registers when needed.

So that ProcessGDBRemote can call ReconfigureRegisterInfo in
SetThreadStopInfo,
I've added forwarding calls to GDBRemoteRegisterContext and the base
class
RegisterContext.

(which removes a slightly sketchy static cast as well)

RegisterContext defaults to doing nothing for both the methods
so anything other than GDBRemoteRegisterContext will do nothing.

Author: DavidSpickett

lldb/include/lldb/Core/Architecture.h

@@ -10,6 +10,7 @@
 #define LLDB_CORE_ARCHITECTURE_H
 
 #include "lldb/Core/PluginInterface.h"
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Target/MemoryTagManager.h"
 
 namespace lldb_private {
@@ -109,6 +110,25 @@ class Architecture : public PluginInterface {
   virtual const MemoryTagManager *GetMemoryTagManager() const {
     return nullptr;
   }
+
+  // This returns true if a write to the named register should cause lldb to
+  // reconfigure its register information. For example on AArch64 writing to vg
+  // to change the vector length means lldb has to change the size of registers.
+  virtual bool
+  RegisterWriteCausesReconfigure(const llvm::StringRef name) const {
+    return false;
+  }
+
+  // Call this after writing a register for which RegisterWriteCausesReconfigure
+  // returns true. This method will update the layout of registers according to
+  // the new state e.g. the new length of scalable vector registers.
+  // Returns true if anything changed, which means existing register values must
+  // be invalidated.
+  virtual bool ReconfigureRegisterInfo(DynamicRegisterInfo &reg_info,
+                                       DataExtractor &reg_data,
+                                       RegisterContext &reg_context) const {
+    return false;
+  }
 };
 
 } // namespace lldb_private

lldb/include/lldb/Target/DynamicRegisterInfo.h

@@ -93,6 +93,10 @@ class DynamicRegisterInfo {
     return llvm::iterator_range<reg_collection::const_iterator>(m_regs);
   }
 
+  llvm::iterator_range<reg_collection::iterator> registers_mutable() {
+    return llvm::iterator_range<reg_collection::iterator>(m_regs);
+  }
+
   void ConfigureOffsets();
 
 protected:

lldb/include/lldb/Target/RegisterContext.h

@@ -51,6 +51,12 @@ class RegisterContext : public std::enable_shared_from_this<RegisterContext>,
     return false;
   }
 
+  virtual bool RegisterWriteCausesReconfigure(const llvm::StringRef name) {
+    return false;
+  }
+
+  virtual bool ReconfigureRegisterInfo() { return false; }
+
   // These two functions are used to implement "push" and "pop" of register
   // states.  They are used primarily for expression evaluation, where we need
   // to push a new state (storing the old one in data_sp) and then restoring

lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp

@@ -8,7 +8,10 @@
 
 #include "Plugins/Architecture/AArch64/ArchitectureAArch64.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/DataExtractor.h"
 
 using namespace lldb_private;
 using namespace lldb;
@@ -34,3 +37,91 @@ ArchitectureAArch64::Create(const ArchSpec &arch) {
   }
   return std::unique_ptr<Architecture>(new ArchitectureAArch64());
 }
+
+static void UpdateARM64SVERegistersInfos(
+    llvm::iterator_range<
+        lldb_private::DynamicRegisterInfo::reg_collection::iterator>
+        regs,
+    uint64_t vg) {
+  // SVE Z register size is vg x 8 bytes.
+  uint32_t z_reg_byte_size = vg * 8;
+
+  // SVE vector length has changed, accordingly set size of Z, P and FFR
+  // registers. Also invalidate register offsets it will be recalculated
+  // after SVE register size update.
+  for (auto &reg : regs) {
+    if (reg.value_regs == nullptr) {
+      if (reg.name[0] == 'z' && isdigit(reg.name[1]))
+        reg.byte_size = z_reg_byte_size;
+      else if (reg.name[0] == 'p' && isdigit(reg.name[1]))
+        reg.byte_size = vg;
+      else if (strcmp(reg.name, "ffr") == 0)
+        reg.byte_size = vg;
+    }
+    reg.byte_offset = LLDB_INVALID_INDEX32;
+  }
+}
+
+static void UpdateARM64SMERegistersInfos(
+    llvm::iterator_range<
+        lldb_private::DynamicRegisterInfo::reg_collection::iterator>
+        regs,
+    uint64_t svg) {
+  for (auto &reg : regs) {
+    if (strcmp(reg.name, "za") == 0) {
+      // ZA is a register with size (svg*8) * (svg*8). A square essentially.
+      reg.byte_size = (svg * 8) * (svg * 8);
+    }
+    reg.byte_offset = LLDB_INVALID_INDEX32;
+  }
+}
+
+bool ArchitectureAArch64::ReconfigureRegisterInfo(DynamicRegisterInfo &reg_info,
+                                                  DataExtractor &reg_data,
+                                                  RegisterContext &reg_context
+
+) const {
+  // Once we start to reconfigure registers, we cannot read any of them.
+  // So we must read VG and SVG up front.
+
+  const uint64_t fail_value = LLDB_INVALID_ADDRESS;
+  std::optional<uint64_t> vg_reg_value;
+  const RegisterInfo *vg_reg_info = reg_info.GetRegisterInfo("vg");
+  if (vg_reg_info) {
+    uint32_t vg_reg_num = vg_reg_info->kinds[eRegisterKindLLDB];
+    uint64_t reg_value =
+        reg_context.ReadRegisterAsUnsigned(vg_reg_num, fail_value);
+    if (reg_value != fail_value && reg_value <= 32)
+      vg_reg_value = reg_value;
+  }
+
+  std::optional<uint64_t> svg_reg_value;
+  const RegisterInfo *svg_reg_info = reg_info.GetRegisterInfo("svg");
+  if (svg_reg_info) {
+    uint32_t svg_reg_num = svg_reg_info->kinds[eRegisterKindLLDB];
+    uint64_t reg_value =
+        reg_context.ReadRegisterAsUnsigned(svg_reg_num, fail_value);
+    if (reg_value != fail_value && reg_value <= 32)
+      svg_reg_value = reg_value;
+  }
+
+  if (!vg_reg_value && !svg_reg_value)
+    return false;
+
+  if (vg_reg_value)
+    UpdateARM64SVERegistersInfos(reg_info.registers_mutable(), *vg_reg_value);
+  if (svg_reg_value)
+    UpdateARM64SMERegistersInfos(reg_info.registers_mutable(), *svg_reg_value);
+
+  // At this point if we have updated any registers, their offsets will all be
+  // invalid. If we did, we need to update them all.
+  reg_info.ConfigureOffsets();
+  // From here we are able to read registers again.
+
+  // Make a heap based buffer that is big enough to store all registers
+  reg_data.SetData(
+      std::make_shared<DataBufferHeap>(reg_info.GetRegisterDataByteSize(), 0));
+  reg_data.SetByteOrder(reg_context.GetByteOrder());
+
+  return true;
+}

lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h

@@ -28,6 +28,17 @@ class ArchitectureAArch64 : public Architecture {
     return &m_memory_tag_manager;
   }
 
+  bool
+  RegisterWriteCausesReconfigure(const llvm::StringRef name) const override {
+    // lldb treats svg as read only, so only vg can be written. This results in
+    // the SVE registers changing size.
+    return name == "vg";
+  }
+
+  bool ReconfigureRegisterInfo(DynamicRegisterInfo &reg_info,
+                               DataExtractor &reg_data,
+                               RegisterContext &reg_context) const override;
+
 private:
   static std::unique_ptr<Architecture> Create(const ArchSpec &arch);
   ArchitectureAArch64() = default;

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

@@ -8,18 +8,19 @@
 
 #include "GDBRemoteRegisterContext.h"
 
+#include "ProcessGDBRemote.h"
+#include "ProcessGDBRemoteLog.h"
+#include "ThreadGDBRemote.h"
+#include "Utility/ARM_DWARF_Registers.h"
+#include "Utility/ARM_ehframe_Registers.h"
+#include "lldb/Core/Architecture.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Scalar.h"
 #include "lldb/Utility/StreamString.h"
-#include "ProcessGDBRemote.h"
-#include "ProcessGDBRemoteLog.h"
-#include "ThreadGDBRemote.h"
-#include "Utility/ARM_DWARF_Registers.h"
-#include "Utility/ARM_ehframe_Registers.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include <memory>
@@ -373,14 +374,8 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
   if (dst == nullptr)
     return false;
 
-  // Code below is specific to AArch64 target in SVE or SME state
-  // If vector granule (vg) register is being written then thread's
-  // register context reconfiguration is triggered on success.
-  // We do not allow writes to SVG so it is not mentioned here.
-  const ArchSpec &arch = process->GetTarget().GetArchitecture();
-  bool do_reconfigure_arm64_sve = arch.IsValid() &&
-                                  arch.GetTriple().isAArch64() &&
-                                  (strcmp(reg_info->name, "vg") == 0);
+  const bool should_reconfigure_registers =
+      RegisterWriteCausesReconfigure(reg_info->name);
 
   if (data.CopyByteOrderedData(data_offset,                // src offset
                                reg_info->byte_size,        // src length
@@ -400,8 +395,8 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
                 {m_reg_data.GetDataStart(), size_t(m_reg_data.GetByteSize())}))
 
         {
-          if (do_reconfigure_arm64_sve)
-            AArch64Reconfigure();
+          if (should_reconfigure_registers)
+            ReconfigureRegisterInfo();
 
           InvalidateAllRegisters();
 
@@ -447,10 +442,9 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info,
                                false);
         }
 
-        if (success && do_reconfigure_arm64_sve) {
-          AArch64Reconfigure();
+        if (success && should_reconfigure_registers &&
+            ReconfigureRegisterInfo())
           InvalidateAllRegisters();
-        }
 
         return success;
       }
@@ -762,75 +756,20 @@ uint32_t GDBRemoteRegisterContext::ConvertRegisterKindToRegisterNumber(
   return m_reg_info_sp->ConvertRegisterKindToRegisterNumber(kind, num);
 }
 
-void GDBRemoteRegisterContext::AArch64Reconfigure() {
-  assert(m_reg_info_sp);
-
-  // Once we start to reconfigure registers, we cannot read any of them.
-  // So we must read VG and SVG up front.
-
-  const uint64_t fail_value = LLDB_INVALID_ADDRESS;
-  std::optional<uint64_t> vg_reg_value;
-  const RegisterInfo *vg_reg_info = m_reg_info_sp->GetRegisterInfo("vg");
-  if (vg_reg_info) {
-    uint32_t vg_reg_num = vg_reg_info->kinds[eRegisterKindLLDB];
-    uint64_t reg_value = ReadRegisterAsUnsigned(vg_reg_num, fail_value);
-    if (reg_value != fail_value && reg_value <= 32)
-      vg_reg_value = reg_value;
-  }
-
-  std::optional<uint64_t> svg_reg_value;
-  const RegisterInfo *svg_reg_info = m_reg_info_sp->GetRegisterInfo("svg");
-  if (svg_reg_info) {
-    uint32_t svg_reg_num = svg_reg_info->kinds[eRegisterKindLLDB];
-    uint64_t reg_value = ReadRegisterAsUnsigned(svg_reg_num, fail_value);
-    if (reg_value != fail_value && reg_value <= 32)
-      svg_reg_value = reg_value;
-  }
-
-  if (vg_reg_value)
-    m_reg_info_sp->UpdateARM64SVERegistersInfos(*vg_reg_value);
-  if (svg_reg_value)
-    m_reg_info_sp->UpdateARM64SMERegistersInfos(*svg_reg_value);
-
-  // At this point if we have updated any registers, their offsets will all be
-  // invalid. If we did, we need to update them all.
-  if (vg_reg_value || svg_reg_value) {
-    m_reg_info_sp->ConfigureOffsets();
-    // From here we are able to read registers again.
-
-    // Make a heap based buffer that is big enough to store all registers
-    m_reg_data.SetData(std::make_shared<DataBufferHeap>(
-        m_reg_info_sp->GetRegisterDataByteSize(), 0));
-    m_reg_data.SetByteOrder(GetByteOrder());
-  }
-}
-
-void GDBRemoteDynamicRegisterInfo::UpdateARM64SVERegistersInfos(uint64_t vg) {
-  // SVE Z register size is vg x 8 bytes.
-  uint32_t z_reg_byte_size = vg * 8;
-
-  // SVE vector length has changed, accordingly set size of Z, P and FFR
-  // registers. Also invalidate register offsets it will be recalculated
-  // after SVE register size update.
-  for (auto &reg : m_regs) {
-    if (reg.value_regs == nullptr) {
-      if (reg.name[0] == 'z' && isdigit(reg.name[1]))
-        reg.byte_size = z_reg_byte_size;
-      else if (reg.name[0] == 'p' && isdigit(reg.name[1]))
-        reg.byte_size = vg;
-      else if (strcmp(reg.name, "ffr") == 0)
-        reg.byte_size = vg;
-    }
-    reg.byte_offset = LLDB_INVALID_INDEX32;
-  }
+bool GDBRemoteRegisterContext::RegisterWriteCausesReconfigure(
+    const llvm::StringRef name) {
+  ExecutionContext exe_ctx(CalculateThread());
+  const Architecture *architecture =
+      exe_ctx.GetProcessRef().GetTarget().GetArchitecturePlugin();
+  return architecture && architecture->RegisterWriteCausesReconfigure(name);
 }
 
-void GDBRemoteDynamicRegisterInfo::UpdateARM64SMERegistersInfos(uint64_t svg) {
-  for (auto &reg : m_regs) {
-    if (strcmp(reg.name, "za") == 0) {
-      // ZA is a register with size (svg*8) * (svg*8). A square essentially.
-      reg.byte_size = (svg * 8) * (svg * 8);
-    }
-    reg.byte_offset = LLDB_INVALID_INDEX32;
-  }
+bool GDBRemoteRegisterContext::ReconfigureRegisterInfo() {
+  ExecutionContext exe_ctx(CalculateThread());
+  const Architecture *architecture =
+      exe_ctx.GetProcessRef().GetTarget().GetArchitecturePlugin();
+  if (architecture)
+    return architecture->ReconfigureRegisterInfo(*(m_reg_info_sp.get()),
+                                                 m_reg_data, *this);
+  return false;
 }

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

@@ -78,8 +78,9 @@ class GDBRemoteRegisterContext : public RegisterContext {
   uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
                                                uint32_t num) override;
 
-  // Reconfigure variable sized registers for AArch64 SVE and SME.
-  void AArch64Reconfigure();
+  bool RegisterWriteCausesReconfigure(const llvm::StringRef name) override;
+
+  bool ReconfigureRegisterInfo() override;
 
 protected:
   friend class ThreadGDBRemote;

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

@@ -1667,27 +1667,14 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
 
   ParseExpeditedRegisters(expedited_register_map, thread_sp);
 
-  // AArch64 SVE/SME specific code below updates SVE and ZA register sizes and
-  // offsets if value of VG or SVG registers has changed since last stop.
-  const ArchSpec &arch = GetTarget().GetArchitecture();
-  if (arch.IsValid() && arch.GetTriple().isAArch64()) {
-    GDBRemoteRegisterContext *reg_ctx_sp =
-        static_cast<GDBRemoteRegisterContext *>(
-            gdb_thread->GetRegisterContext().get());
-
-    if (reg_ctx_sp) {
-      reg_ctx_sp->AArch64Reconfigure();
-      // Now we have changed the offsets of all the registers, so the values
-      // will be corrupted.
-      reg_ctx_sp->InvalidateAllRegisters();
-
-      // Expedited registers values will never contain registers that would be
-      // resized by AArch64Reconfigure. So we are safe to continue using these
-      // values. These values include vg, svg and useful general purpose
-      // registers so this saves a few read packets each time we make use of
-      // them.
-      ParseExpeditedRegisters(expedited_register_map, thread_sp);
-    }
+  if (reg_ctx_sp->ReconfigureRegisterInfo()) {
+    // Now we have changed the offsets of all the registers, so the values
+    // will be corrupted.
+    reg_ctx_sp->InvalidateAllRegisters();
+    // Expedited registers values will never contain registers that would be
+    // resized by a reconfigure. So we are safe to continue using these
+    // values.
+    ParseExpeditedRegisters(expedited_register_map, thread_sp);
   }
 
   thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str());