[Lldb-commits] [PATCH] D111142: [lldb] [ABI] Apply AugmentRegisterInfo() to RemoteRegisterInfos

Wed, 06 Oct 2021 07:23:29 -0700 

mgorny updated this revision to Diff 377538.
mgorny retitled this revision from "[lldb] [ABI] Make AugmentRegisterInfo() 
take whole DynamicRegisterInfo" to "[lldb] [ABI] Apply AugmentRegisterInfo() to 
RemoteRegisterInfos".
mgorny edited the summary of this revision.
mgorny added a comment.

Pass a vector of `RemoteRegisterInfo`s instead of `DynamicRegisterInfo` class.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111142/new/

https://reviews.llvm.org/D111142

Files:
  lldb/include/lldb/Target/ABI.h
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/include/lldb/Target/RemoteRegisterInfo.h
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/ABI.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===================================================================
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -808,3 +808,11 @@
       return &reg_info;
   return nullptr;
 }
+
+lldb_private::RegisterInfo *
+DynamicRegisterInfo::GetRegisterInfo(llvm::StringRef reg_name) {
+  for (auto &reg_info : m_regs)
+    if (reg_info.name == reg_name)
+      return &reg_info;
+  return nullptr;
+}
Index: lldb/source/Target/ABI.cpp
===================================================================
--- lldb/source/Target/ABI.cpp
+++ lldb/source/Target/ABI.cpp
@@ -214,33 +214,38 @@
   return info_up;
 }
 
-void RegInfoBasedABI::AugmentRegisterInfo(RegisterInfo &info) {
-  if (info.kinds[eRegisterKindEHFrame] != LLDB_INVALID_REGNUM &&
-      info.kinds[eRegisterKindDWARF] != LLDB_INVALID_REGNUM)
-    return;
-
-  RegisterInfo abi_info;
-  if (!GetRegisterInfoByName(info.name, abi_info))
-    return;
-
-  if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
-    info.kinds[eRegisterKindEHFrame] = abi_info.kinds[eRegisterKindEHFrame];
-  if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
-    info.kinds[eRegisterKindDWARF] = abi_info.kinds[eRegisterKindDWARF];
-  if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
-    info.kinds[eRegisterKindGeneric] = abi_info.kinds[eRegisterKindGeneric];
+void RegInfoBasedABI::AugmentRegisterInfo(
+    std::vector<RemoteRegisterInfo> &regs) {
+  for (RemoteRegisterInfo &info : regs) {
+    if (info.regnum_ehframe != LLDB_INVALID_REGNUM &&
+        info.regnum_dwarf != LLDB_INVALID_REGNUM)
+      continue;
+
+    RegisterInfo abi_info;
+    if (!GetRegisterInfoByName(info.name.GetStringRef(), abi_info))
+      continue;
+
+    if (info.regnum_ehframe == LLDB_INVALID_REGNUM)
+      info.regnum_ehframe = abi_info.kinds[eRegisterKindEHFrame];
+    if (info.regnum_dwarf == LLDB_INVALID_REGNUM)
+      info.regnum_dwarf = abi_info.kinds[eRegisterKindDWARF];
+    if (info.regnum_generic == LLDB_INVALID_REGNUM)
+      info.regnum_generic = abi_info.kinds[eRegisterKindGeneric];
+  }
 }
 
-void MCBasedABI::AugmentRegisterInfo(RegisterInfo &info) {
-  uint32_t eh, dwarf;
-  std::tie(eh, dwarf) = GetEHAndDWARFNums(info.name);
-
-  if (info.kinds[eRegisterKindEHFrame] == LLDB_INVALID_REGNUM)
-    info.kinds[eRegisterKindEHFrame] = eh;
-  if (info.kinds[eRegisterKindDWARF] == LLDB_INVALID_REGNUM)
-    info.kinds[eRegisterKindDWARF] = dwarf;
-  if (info.kinds[eRegisterKindGeneric] == LLDB_INVALID_REGNUM)
-    info.kinds[eRegisterKindGeneric] = GetGenericNum(info.name);
+void MCBasedABI::AugmentRegisterInfo(std::vector<RemoteRegisterInfo> &regs) {
+  for (RemoteRegisterInfo &info : regs) {
+    uint32_t eh, dwarf;
+    std::tie(eh, dwarf) = GetEHAndDWARFNums(info.name.GetStringRef());
+
+    if (info.regnum_ehframe == LLDB_INVALID_REGNUM)
+      info.regnum_ehframe = eh;
+    if (info.regnum_dwarf == LLDB_INVALID_REGNUM)
+      info.regnum_dwarf = dwarf;
+    if (info.regnum_generic == LLDB_INVALID_REGNUM)
+      info.regnum_generic = GetGenericNum(info.name.GetStringRef());
+  }
 }
 
 std::pair<uint32_t, uint32_t>
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -20,6 +20,7 @@
 #include "lldb/Core/ThreadSafeValue.h"
 #include "lldb/Host/HostThread.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Target/RemoteRegisterInfo.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
@@ -44,22 +45,6 @@
 }
 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;
-};
-
 class ThreadGDBRemote;
 
 class ProcessGDBRemote : public Process,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -545,7 +545,6 @@
     m_register_info_sp->HardcodeARMRegisters(from_scratch);
   }
 
-  // At this point, we can finalize our register info.
   m_register_info_sp->Finalize(GetTarget().GetArchitecture());
 }
 
@@ -4472,11 +4471,6 @@
 
 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)) {
@@ -4492,10 +4486,7 @@
     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();
-
+  for (RemoteRegisterInfo &remote_reg_info : registers) {
     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
@@ -4506,6 +4497,17 @@
                     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);
+  }
+
+  // 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.
+  if (ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use))
+    abi_sp->AugmentRegisterInfo(registers);
+
+  for (auto it : llvm::enumerate(registers)) {
+    uint32_t local_regnum = it.index();
+    RemoteRegisterInfo &remote_reg_info = it.value();
 
     auto regs_with_sentinel = [](std::vector<uint32_t> &vec) -> uint32_t * {
       if (!vec.empty()) {
@@ -4525,9 +4527,6 @@
           regs_with_sentinel(remote_reg_info.value_regs),
           regs_with_sentinel(remote_reg_info.invalidate_regs),
     };
-
-    if (abi_sp)
-      abi_sp->AugmentRegisterInfo(reg_info);
     m_register_info_sp->AddRegister(reg_info, remote_reg_info.set_name);
   };
 
Index: lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
===================================================================
--- lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
+++ lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
@@ -11,7 +11,7 @@
 
 #include "lldb/Target/ABI.h"
 
-class ABIAArch64: public lldb_private::MCBasedABI {
+class ABIAArch64 : public lldb_private::MCBasedABI {
 public:
   static void Initialize();
   static void Terminate();
@@ -31,7 +31,8 @@
 
   uint32_t GetGenericNum(llvm::StringRef name) override;
 
-  void AugmentRegisterInfo(lldb_private::RegisterInfo &info) override;
+  void AugmentRegisterInfo(
+      std::vector<lldb_private::RemoteRegisterInfo> &regs) override;
 
   using lldb_private::MCBasedABI::MCBasedABI;
 };
Index: lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
===================================================================
--- lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
@@ -71,10 +71,14 @@
       .Default(LLDB_INVALID_REGNUM);
 }
 
-void ABIAArch64::AugmentRegisterInfo(lldb_private::RegisterInfo &info) {
-  lldb_private::MCBasedABI::AugmentRegisterInfo(info);
+void ABIAArch64::AugmentRegisterInfo(
+    std::vector<lldb_private::RemoteRegisterInfo> &regs) {
+  lldb_private::MCBasedABI::AugmentRegisterInfo(regs);
 
-  // GDB sends x31 as "sp".  Add the "x31" alt_name for convenience.
-  if (!strcmp(info.name, "sp") && !info.alt_name)
-    info.alt_name = "x31";
+  lldb_private::ConstString sp_string{"sp"};
+  for (lldb_private::RemoteRegisterInfo &info : regs) {
+    // GDB sends x31 as "sp".  Add the "x31" alt_name for convenience.
+    if (info.name == sp_string && !info.alt_name)
+      info.alt_name.SetCString("x31");
+  }
 }
Index: lldb/include/lldb/Target/RemoteRegisterInfo.h
===================================================================
--- /dev/null
+++ lldb/include/lldb/Target/RemoteRegisterInfo.h
@@ -0,0 +1,37 @@
+//===-- RemoteRegisterInfo.h ------------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TARGET_REMOTEREGISTERINFO_H
+#define LLDB_TARGET_REMOTEREGISTERINFO_H
+
+#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-private.h"
+
+#include <vector>
+
+namespace lldb_private {
+
+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;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_REMOTEREGISTERINFO_H
Index: lldb/include/lldb/Target/DynamicRegisterInfo.h
===================================================================
--- lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -77,6 +77,9 @@
   const lldb_private::RegisterInfo *
   GetRegisterInfo(llvm::StringRef reg_name) const;
 
+  lldb_private::RegisterInfo *
+  GetRegisterInfo(llvm::StringRef reg_name);
+
   typedef std::vector<lldb_private::RegisterInfo> reg_collection;
   llvm::iterator_range<reg_collection::const_iterator> registers() const {
     return llvm::iterator_range<reg_collection::const_iterator>(m_regs);
Index: lldb/include/lldb/Target/ABI.h
===================================================================
--- lldb/include/lldb/Target/ABI.h
+++ lldb/include/lldb/Target/ABI.h
@@ -11,6 +11,7 @@
 
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Target/RemoteRegisterInfo.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private.h"
 
@@ -127,7 +128,7 @@
 
   llvm::MCRegisterInfo &GetMCRegisterInfo() { return *m_mc_register_info_up; }
 
-  virtual void AugmentRegisterInfo(RegisterInfo &info) = 0;
+  virtual void AugmentRegisterInfo(std::vector<RemoteRegisterInfo> &regs) = 0;
 
   virtual bool GetPointerReturnRegister(const char *&name) { return false; }
 
@@ -159,7 +160,7 @@
 
 class RegInfoBasedABI : public ABI {
 public:
-  void AugmentRegisterInfo(RegisterInfo &info) override;
+  void AugmentRegisterInfo(std::vector<RemoteRegisterInfo> &regs) override;
 
 protected:
   using ABI::ABI;
@@ -171,7 +172,7 @@
 
 class MCBasedABI : public ABI {
 public:
-  void AugmentRegisterInfo(RegisterInfo &info) override;
+  void AugmentRegisterInfo(std::vector<RemoteRegisterInfo> &regs) override;
 
   /// If the register name is of the form "<from_prefix>[<number>]" then change
   /// the name to "<to_prefix>[<number>]". Otherwise, leave the name unchanged.

_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to