[lldb] Move GetVTableInfo to C++ language runtime

[Nerixyz]

The original PR was reverted in #206816 due to a test failure on lldb-aarch64-ubuntu.
Since I couldn't reproduce the failure, I decided to split the PR into smaller chunks.

This is part 1/4 (the final state is on https://github.com/Nerixyz/llvm-project/tree/refactor/common-abi-runtime-take2-4-of-4).

It moves GetVTableInfo and TypeHasVTable from the Itanium ABI runtime to the C++ language runtime. Eventually, this will be used to select the ABI runtime that's able to handle a vtable. For now, we always ask and use Itanium.

[llvmorg-github-actions]

@llvm/pr-subscribers-lldb

Author: Nerixyz (Nerixyz)

Changes

The original PR was reverted in #206816 due to a test failure on lldb-aarch64-ubuntu.
Since I couldn't reproduce the failure, I decided to split the PR into smaller chunks.

This is part 1/4 (the final state is on https://github.com/Nerixyz/llvm-project/tree/refactor/common-abi-runtime-take2-4-of-4).

It moves GetVTableInfo and TypeHasVTable from the Itanium ABI runtime to the C++ language runtime. Eventually, this will be used to select the ABI runtime that's able to handle a vtable. For now, we always ask and use Itanium.

---

Full diff: https://github.com/llvm/llvm-project/pull/207010.diff

4 Files Affected:

• (modified) lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp (+119-2)
• (modified) lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h (+10)
• (modified) lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.cpp (+6-112)
• (modified) lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.h (+2-4)

diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index c517ec8611932..95830675a2b06 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -524,8 +524,16 @@ bool CPPLanguageRuntime::GetDynamicTypeAndAddress(
   if (!CouldHaveDynamicValue(in_value))
     return false;
 
+  llvm::Expected<VTableInfoEntry> entry =
+      GetVTableInfoEntry(in_value, /*check_type=*/false);
+  if (!entry) {
+    llvm::consumeError(entry.takeError());
+    return false;
+  }
+
   return m_itanium_runtime.GetDynamicTypeAndAddress(
-      in_value, use_dynamic, class_type_or_name, dynamic_address, value_type);
+      in_value, use_dynamic, entry->info, class_type_or_name, dynamic_address,
+      value_type);
 }
 
 TypeAndOrName
@@ -588,7 +596,11 @@ void CPPLanguageRuntime::Terminate() {
 
 llvm::Expected<LanguageRuntime::VTableInfo>
 CPPLanguageRuntime::GetVTableInfo(ValueObject &in_value, bool check_type) {
-  return m_itanium_runtime.GetVTableInfo(in_value, check_type);
+  llvm::Expected<VTableInfoEntry> entry =
+      GetVTableInfoEntry(in_value, check_type);
+  if (!entry)
+    return entry.takeError();
+  return entry->info;
 }
 
 BreakpointResolverSP
@@ -686,3 +698,108 @@ lldb::ValueObjectSP
 CPPLanguageRuntime::GetExceptionObjectForThread(lldb::ThreadSP thread_sp) {
   return m_itanium_runtime.GetExceptionObjectForThread(std::move(thread_sp));
 }
+
+static llvm::Error TypeHasVTable(CompilerType type) {
+  // Check to make sure the class has a vtable.
+  CompilerType original_type = type;
+  if (type.IsPointerOrReferenceType()) {
+    CompilerType pointee_type = type.GetPointeeType();
+    if (pointee_type)
+      type = pointee_type;
+  }
+
+  // Make sure this is a class or a struct first by checking the type class
+  // bitfield that gets returned.
+  if ((type.GetTypeClass() & (eTypeClassStruct | eTypeClassClass)) == 0) {
+    return llvm::createStringError(
+        std::errc::invalid_argument,
+        "type \"%s\" is not a class or struct or a pointer to one",
+        original_type.GetTypeName().AsCString("<invalid>"));
+  }
+
+  // Check if the type has virtual functions by asking it if it is polymorphic.
+  if (!type.IsPolymorphicClass()) {
+    return llvm::createStringError(std::errc::invalid_argument,
+                                   "type \"%s\" doesn't have a vtable",
+                                   type.GetTypeName().AsCString("<invalid>"));
+  }
+  return llvm::Error::success();
+}
+
+// This function can accept both pointers or references to classes as well as
+// instances of classes. If you are using this function during dynamic type
+// detection, only valid ValueObjects that return true to
+// CouldHaveDynamicValue(...) should call this function and \a check_type
+// should be set to false. This function is also used by ValueObjectVTable
+// and is can pass in instances of classes which is not suitable for dynamic
+// type detection, these cases should pass true for \a check_type.
+llvm::Expected<CPPLanguageRuntime::VTableInfoEntry>
+CPPLanguageRuntime::GetVTableInfoEntry(ValueObject &in_value, bool check_type) {
+
+  CompilerType type = in_value.GetCompilerType();
+  if (check_type) {
+    if (llvm::Error err = TypeHasVTable(type))
+      return std::move(err);
+  }
+  ExecutionContext exe_ctx(in_value.GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+  if (process == nullptr)
+    return llvm::createStringError(std::errc::invalid_argument,
+                                   "invalid process");
+
+  auto [original_ptr, address_type] =
+      type.IsPointerOrReferenceType()
+          ? in_value.GetPointerValue()
+          : in_value.GetAddressOf(/*scalar_is_load_address=*/true);
+  if (original_ptr == LLDB_INVALID_ADDRESS || address_type != eAddressTypeLoad)
+    return llvm::createStringError(std::errc::invalid_argument,
+                                   "failed to get the address of the value");
+
+  Status error;
+  lldb::addr_t vtable_load_addr =
+      process->ReadPointerFromMemory(original_ptr, error);
+
+  if (!error.Success() || vtable_load_addr == LLDB_INVALID_ADDRESS)
+    return llvm::createStringError(
+        std::errc::invalid_argument,
+        "failed to read vtable pointer from memory at 0x%" PRIx64,
+        original_ptr);
+
+  // The vtable load address can have authentication bits with
+  // AArch64 targets on Darwin.
+  vtable_load_addr = process->FixDataAddress(vtable_load_addr);
+
+  // Find the symbol that contains the "vtable_load_addr" address
+  Address vtable_addr;
+  if (!process->GetTarget().ResolveLoadAddress(vtable_load_addr, vtable_addr))
+    return llvm::createStringError(std::errc::invalid_argument,
+                                   "failed to resolve vtable pointer 0x%" PRIx64
+                                   "to a section",
+                                   vtable_load_addr);
+
+  // Check our cache first to see if we already have this info
+  {
+    std::lock_guard<std::mutex> locker(m_vtable_mutex);
+    auto pos = m_vtable_info_map.find(vtable_addr);
+    if (pos != m_vtable_info_map.end())
+      return pos->second;
+  }
+
+  Symbol *symbol = vtable_addr.CalculateSymbolContextSymbol();
+  if (symbol == nullptr)
+    return llvm::createStringError(std::errc::invalid_argument,
+                                   "no symbol found for 0x%" PRIx64,
+                                   vtable_load_addr);
+  if (m_itanium_runtime.IsVTableSymbol(symbol->GetMangled())) {
+    VTableInfoEntry entry{
+        /*info=*/VTableInfo{vtable_addr, symbol},
+    };
+    std::lock_guard<std::mutex> locker(m_vtable_mutex);
+    m_vtable_info_map[vtable_addr] = entry;
+    return entry;
+  }
+  return llvm::createStringError(std::errc::invalid_argument,
+                                 "symbol found that contains 0x%" PRIx64
+                                 " is not a vtable symbol",
+                                 vtable_load_addr);
+}
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
index 7c3dade76d703..0ed1a71b976be 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
@@ -144,6 +144,16 @@ class CPPLanguageRuntime : public LanguageRuntime {
 
   lldb::BreakpointSP m_cxx_exception_bp_sp;
   ItaniumABIRuntime m_itanium_runtime;
+
+  struct VTableInfoEntry {
+    VTableInfo info;
+  };
+
+  llvm::Expected<VTableInfoEntry> GetVTableInfoEntry(ValueObject &in_value,
+                                                     bool check_type);
+
+  std::map<Address, VTableInfoEntry> m_vtable_info_map;
+  std::mutex m_vtable_mutex;
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.cpp
index 4d9cf31c8904d..f62b4b3b4e695 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.cpp
@@ -21,6 +21,11 @@ static const char *vtable_demangled_prefix = "vtable for ";
 
 ItaniumABIRuntime::ItaniumABIRuntime(Process *process) : m_process(process) {}
 
+bool ItaniumABIRuntime::IsVTableSymbol(Mangled &mangled) const {
+  return mangled.GetDemangledName().GetStringRef().starts_with(
+      vtable_demangled_prefix);
+}
+
 TypeAndOrName
 ItaniumABIRuntime::GetTypeInfo(ValueObject &in_value,
                                const LanguageRuntime::VTableInfo &vtable_info) {
@@ -145,112 +150,9 @@ ItaniumABIRuntime::GetTypeInfo(ValueObject &in_value,
   return TypeAndOrName();
 }
 
-llvm::Error ItaniumABIRuntime::TypeHasVTable(CompilerType type) {
-  // Check to make sure the class has a vtable.
-  CompilerType original_type = type;
-  if (type.IsPointerOrReferenceType()) {
-    CompilerType pointee_type = type.GetPointeeType();
-    if (pointee_type)
-      type = pointee_type;
-  }
-
-  // Make sure this is a class or a struct first by checking the type class
-  // bitfield that gets returned.
-  if ((type.GetTypeClass() & (eTypeClassStruct | eTypeClassClass)) == 0) {
-    return llvm::createStringError(
-        std::errc::invalid_argument,
-        "type \"%s\" is not a class or struct or a pointer to one",
-        original_type.GetTypeName().AsCString("<invalid>"));
-  }
-
-  // Check if the type has virtual functions by asking it if it is polymorphic.
-  if (!type.IsPolymorphicClass()) {
-    return llvm::createStringError(std::errc::invalid_argument,
-                                   "type \"%s\" doesn't have a vtable",
-                                   type.GetTypeName().AsCString("<invalid>"));
-  }
-  return llvm::Error::success();
-}
-
-// This function can accept both pointers or references to classes as well as
-// instances of classes. If you are using this function during dynamic type
-// detection, only valid ValueObjects that return true to
-// CouldHaveDynamicValue(...) should call this function and \a check_type
-// should be set to false. This function is also used by ValueObjectVTable
-// and is can pass in instances of classes which is not suitable for dynamic
-// type detection, these cases should pass true for \a check_type.
-llvm::Expected<LanguageRuntime::VTableInfo>
-ItaniumABIRuntime::GetVTableInfo(ValueObject &in_value, bool check_type) {
-
-  CompilerType type = in_value.GetCompilerType();
-  if (check_type) {
-    if (llvm::Error err = TypeHasVTable(type))
-      return std::move(err);
-  }
-  ExecutionContext exe_ctx(in_value.GetExecutionContextRef());
-  Process *process = exe_ctx.GetProcessPtr();
-  if (process == nullptr)
-    return llvm::createStringError(std::errc::invalid_argument,
-                                   "invalid process");
-
-  auto [original_ptr, address_type] =
-      type.IsPointerOrReferenceType()
-          ? in_value.GetPointerValue()
-          : in_value.GetAddressOf(/*scalar_is_load_address=*/true);
-  if (original_ptr == LLDB_INVALID_ADDRESS || address_type != eAddressTypeLoad)
-    return llvm::createStringError(std::errc::invalid_argument,
-                                   "failed to get the address of the value");
-
-  Status error;
-  lldb::addr_t vtable_load_addr =
-      process->ReadPointerFromMemory(original_ptr, error);
-
-  if (!error.Success() || vtable_load_addr == LLDB_INVALID_ADDRESS)
-    return llvm::createStringError(
-        std::errc::invalid_argument,
-        "failed to read vtable pointer from memory at 0x%" PRIx64,
-        original_ptr);
-
-  // The vtable load address can have authentication bits with
-  // AArch64 targets on Darwin.
-  vtable_load_addr = process->FixDataAddress(vtable_load_addr);
-
-  // Find the symbol that contains the "vtable_load_addr" address
-  Address vtable_addr;
-  if (!process->GetTarget().ResolveLoadAddress(vtable_load_addr, vtable_addr))
-    return llvm::createStringError(std::errc::invalid_argument,
-                                   "failed to resolve vtable pointer 0x%" PRIx64
-                                   "to a section",
-                                   vtable_load_addr);
-
-  // Check our cache first to see if we already have this info
-  {
-    std::lock_guard<std::mutex> locker(m_mutex);
-    auto pos = m_vtable_info_map.find(vtable_addr);
-    if (pos != m_vtable_info_map.end())
-      return pos->second;
-  }
-
-  Symbol *symbol = vtable_addr.CalculateSymbolContextSymbol();
-  if (symbol == nullptr)
-    return llvm::createStringError(std::errc::invalid_argument,
-                                   "no symbol found for 0x%" PRIx64,
-                                   vtable_load_addr);
-  llvm::StringRef name = symbol->GetMangled().GetDemangledName().GetStringRef();
-  if (name.starts_with(vtable_demangled_prefix)) {
-    LanguageRuntime::VTableInfo info = {vtable_addr, symbol};
-    std::lock_guard<std::mutex> locker(m_mutex);
-    auto pos = m_vtable_info_map[vtable_addr] = info;
-    return info;
-  }
-  return llvm::createStringError(std::errc::invalid_argument,
-                                 "symbol found that contains 0x%" PRIx64
-                                 " is not a vtable symbol",
-                                 vtable_load_addr);
-}
-
 bool ItaniumABIRuntime::GetDynamicTypeAndAddress(
     ValueObject &in_value, lldb::DynamicValueType use_dynamic,
+    const LanguageRuntime::VTableInfo &vtable_info,
     TypeAndOrName &class_type_or_name, Address &dynamic_address,
     Value::ValueType &value_type) {
   // For Itanium, if the type has a vtable pointer in the object, it will be at
@@ -266,14 +168,6 @@ bool ItaniumABIRuntime::GetDynamicTypeAndAddress(
   // want GetVTableInfo to check the type since we accept void * as a possible
   // dynamic type and that won't pass the type check. We already checked the
   // type above in CouldHaveDynamicValue(...).
-  llvm::Expected<LanguageRuntime::VTableInfo> vtable_info_or_err =
-      GetVTableInfo(in_value, /*check_type=*/false);
-  if (!vtable_info_or_err) {
-    llvm::consumeError(vtable_info_or_err.takeError());
-    return false;
-  }
-
-  const LanguageRuntime::VTableInfo &vtable_info = vtable_info_or_err.get();
   class_type_or_name = GetTypeInfo(in_value, vtable_info);
 
   if (!class_type_or_name)
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.h b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.h
index 75a8142b26d93..b276e7df60951 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABIRuntime.h
@@ -20,11 +20,11 @@ class ItaniumABIRuntime {
 public:
   ItaniumABIRuntime(Process *process);
 
-  llvm::Expected<LanguageRuntime::VTableInfo>
-  GetVTableInfo(ValueObject &in_value, bool check_type);
+  bool IsVTableSymbol(Mangled &manged) const;
 
   bool GetDynamicTypeAndAddress(ValueObject &in_value,
                                 lldb::DynamicValueType use_dynamic,
+                                const LanguageRuntime::VTableInfo &vtable_info,
                                 TypeAndOrName &class_type_or_name,
                                 Address &dynamic_address,
                                 Value::ValueType &value_type);
@@ -42,8 +42,6 @@ class ItaniumABIRuntime {
   TypeAndOrName GetTypeInfo(ValueObject &in_value,
                             const LanguageRuntime::VTableInfo &vtable_info);
 
-  llvm::Error TypeHasVTable(CompilerType type);
-
   TypeAndOrName GetDynamicTypeInfo(const lldb_private::Address &vtable_addr);
 
   void SetDynamicTypeInfo(const lldb_private::Address &vtable_addr,

[Nerixyz]

Ping

[Michael137]

Dropping the error here seems suspicious. But that' not relevant to this PR

[Michael137]

Can the corresponding member on the ItaniumABIRuntime be removed?

[Nerixyz]

Yes!