Add the ability to get a C++ vtable ValueObject from another ValueObj… #67599
Conversation
clayborg
requested review from
JDevlieghere,
adrian-prantl,
labath and
jimingham
|
@llvm/pr-subscribers-lldb ChangesAdd the ability to get a C++ vtable ValueObject from another ValueObject. This patch adds the ability to ask a ValueObject for a ValueObject that represents the virtual function table for a C++ class. If the ValueObject is not a C++ class with a vtable, a valid ValueObject value will be returned that contains an appropriate error. If it is successful a valid ValueObject that represents vtable will be returned. The ValueObject that is returned will have a name that matches the demangled value for a C++ vtable mangled name like "vtable for <class-name>". It will have N children, one for each virtual function pointer. Each child's value is the function pointer itself, the summary is the symbolication of this function pointer, and the type will be a valid function pointer from the debug info if there is debug information corresponding to the virtual function pointer. The vtable SBValue will have the following:
The child SBValue objects that represent a virtual function pointer has the following values:
Examples of using this API via python: Patch is 33.01 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/67599.diff 18 Files Affected:
diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h
index b66c2d5642b6f95..333bdf1738eecaf 100644
--- a/lldb/include/lldb/API/SBValue.h
+++ b/lldb/include/lldb/API/SBValue.h
@@ -374,6 +374,34 @@ class LLDB_API SBValue {
lldb::SBWatchpoint WatchPointee(bool resolve_location, bool read, bool write,
SBError &error);
+ /// If this value represents a C++ class that has a vtable, return an value
+ /// that represents the virtual function table.
+ ///
+ /// SBValue::GetError() will be in the success state if this value represents
+ /// a C++ class with a vtable, or an appropriate error describing that the
+ /// object isn't a C++ class with a vtable or not a C++ class.
+ ///
+ /// SBValue::GetName() will be the demangled symbol name for the virtual
+ /// function table like "vtable for Baseclass".
+ ///
+ /// SBValue::GetValue() will be the address of the first vtable entry if the
+ /// current SBValue is a class with a vtable, or nothing the current SBValue
+ /// is not a C++ class or not a C++ class that has a vtable.
+ ///
+ /// SBValue::GetSummary() will contain the number of virtual function pointers
+ /// in the vtable like is done for arrays.
+ ///
+ /// SBValue::GetNumChildren() will return the number of virtual function
+ /// pointers in the vtable, or zero on error.
+ ///
+ /// SBValue::GetChildAtIndex(...) will return each virtual function pointer
+ /// as a SBValue object. The child SBValue objects name will be the array
+ /// index, value will be the virtual function pointer, summary will be the
+ /// symbolicated address description, and if the the adress resolves to a
+ /// function in debug info, the child type will be the function prototype as
+ /// a SBType object.
+ lldb::SBValue GetVTable();
+
protected:
friend class SBBlock;
friend class SBFrame;
diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
index 3af94f0a86e2fcc..20b3086138457f7 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -620,6 +620,10 @@ class ValueObject {
virtual lldb::ValueObjectSP CastPointerType(const char *name,
lldb::TypeSP &type_sp);
+ /// If this object represents a C++ class with a vtable, return an object
+ /// that represents the virtual function table. If the object isn't a class
+ /// with a vtable, return a valid ValueObject with the error set correctly.
+ lldb::ValueObjectSP GetVTable();
// The backing bits of this value object were updated, clear any descriptive
// string, so we know we have to refetch them.
void ValueUpdated() {
diff --git a/lldb/include/lldb/Core/ValueObjectChild.h b/lldb/include/lldb/Core/ValueObjectChild.h
index 07b37aa8a405f7e..46b14e6840f0dc3 100644
--- a/lldb/include/lldb/Core/ValueObjectChild.h
+++ b/lldb/include/lldb/Core/ValueObjectChild.h
@@ -73,6 +73,7 @@ class ValueObjectChild : public ValueObject {
friend class ValueObject;
friend class ValueObjectConstResult;
friend class ValueObjectConstResultImpl;
+ friend class ValueObjectVTable;
ValueObjectChild(ValueObject &parent, const CompilerType &compiler_type,
ConstString name, uint64_t byte_size,
diff --git a/lldb/include/lldb/Core/ValueObjectVTable.h b/lldb/include/lldb/Core/ValueObjectVTable.h
new file mode 100644
index 000000000000000..faa0af238e764f4
--- /dev/null
+++ b/lldb/include/lldb/Core/ValueObjectVTable.h
@@ -0,0 +1,65 @@
+//===-- ValueObjectVTable.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_CORE_VALUEOBJECTVTABLE_H
+#define LLDB_CORE_VALUEOBJECTVTABLE_H
+
+#include "lldb/Core/ValueObject.h"
+
+namespace lldb_private {
+
+/// A class that represents a virtual function table for a C++ class.
+///
+///
+class ValueObjectVTable : public ValueObject {
+public:
+ ~ValueObjectVTable() override;
+
+ static lldb::ValueObjectSP Create(ValueObject &parent);
+
+ std::optional<uint64_t> GetByteSize() override;
+
+ size_t CalculateNumChildren(uint32_t max) override;
+
+ ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
+ int32_t synthetic_index) override;
+
+ lldb::ValueType GetValueType() const override;
+
+ ConstString GetTypeName() override;
+
+ ConstString GetQualifiedTypeName() override;
+
+ ConstString GetDisplayTypeName() override;
+
+ bool IsInScope() override;
+
+protected:
+ bool UpdateValue() override;
+
+ CompilerType GetCompilerTypeImpl() override;
+
+ /// The symbol for the C++ virtual function table.
+ Symbol *m_vtable_symbol = nullptr;
+ /// Cache the number of vtable children when we update the value.
+ uint32_t m_num_vtable_entries = 0;
+ /// Cache the address size in bytes to avoid checking with the process to
+ /// many times.
+ uint32_t m_addr_size = 0;
+
+private:
+ ValueObjectVTable(ValueObject &parent);
+
+ // For ValueObject only
+ ValueObjectVTable(const ValueObjectVTable &) = delete;
+ const ValueObjectVTable &operator=(const ValueObjectVTable &) = delete;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_CORE_VALUEOBJECTVTABLE_H
diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h
index eb6e453e1aec0d0..16b912c8113f969 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -444,6 +444,10 @@ class TypeSystem : public PluginInterface,
virtual CompilerType GetBasicTypeFromAST(lldb::BasicType basic_type) = 0;
+ virtual CompilerType CreateGenericFunctionPrototype() {
+ return CompilerType();
+ }
+
virtual CompilerType
GetBuiltinTypeForEncodingAndBitSize(lldb::Encoding encoding,
size_t bit_size) = 0;
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 21e098481ce8022..b9dae9ca5573d5b 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -322,7 +322,9 @@ enum ValueType {
eValueTypeRegister = 5, ///< stack frame register value
eValueTypeRegisterSet = 6, ///< A collection of stack frame register values
eValueTypeConstResult = 7, ///< constant result variables
- eValueTypeVariableThreadLocal = 8 ///< thread local storage variable
+ eValueTypeVariableThreadLocal = 8, ///< thread local storage variable
+ eValueTypeVTable = 9, ///< virtual function table
+ eValueTypeVTableEntry = 10, ///< function pointer in virtual function table
};
/// Token size/granularities for Input Readers.
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 738773c93c49b03..cf2d862fa504844 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -114,7 +114,7 @@ class ValueImpl {
Target *target = value_sp->GetTargetSP().get();
// If this ValueObject holds an error, then it is valuable for that.
- if (value_sp->GetError().Fail())
+ if (value_sp->GetError().Fail())
return value_sp;
if (!target)
@@ -1038,8 +1038,8 @@ lldb::ValueObjectSP SBValue::GetSP(ValueLocker &locker) const {
// IsValid means that the SBValue has a value in it. But that's not the
// only time that ValueObjects are useful. We also want to return the value
// if there's an error state in it.
- if (!m_opaque_sp || (!m_opaque_sp->IsValid()
- && (m_opaque_sp->GetRootSP()
+ if (!m_opaque_sp || (!m_opaque_sp->IsValid()
+ && (m_opaque_sp->GetRootSP()
&& !m_opaque_sp->GetRootSP()->GetError().Fail()))) {
locker.GetError().SetErrorString("No value");
return ValueObjectSP();
@@ -1498,3 +1498,14 @@ lldb::SBValue SBValue::Persist() {
}
return persisted_sb;
}
+
+lldb::SBValue SBValue::GetVTable() {
+ SBValue vtable_sb;
+ ValueLocker locker;
+ lldb::ValueObjectSP value_sp(GetSP(locker));
+ if (!value_sp)
+ return vtable_sb;
+
+ vtable_sb.SetSP(value_sp->GetVTable());
+ return vtable_sb;
+}
diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp
index 1390fd8748dfaf6..2149d0f2f98da3d 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -495,6 +495,8 @@ may even involve JITing and running code in the target program.)");
case eValueTypeRegisterSet:
case eValueTypeConstResult:
case eValueTypeVariableThreadLocal:
+ case eValueTypeVTable:
+ case eValueTypeVTableEntry:
return false;
}
}
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index d7b4f2587a98bf9..b23cc5d2eaf3156 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -71,6 +71,7 @@ add_lldb_library(lldbCore
ValueObjectSyntheticFilter.cpp
ValueObjectUpdater.cpp
ValueObjectVariable.cpp
+ ValueObjectVTable.cpp
DEPENDS
clang-tablegen-targets
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 3e9116f2d922933..aa2a35bf2b2dc5f 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -17,6 +17,7 @@
#include "lldb/Core/ValueObjectDynamicValue.h"
#include "lldb/Core/ValueObjectMemory.h"
#include "lldb/Core/ValueObjectSyntheticFilter.h"
+#include "lldb/Core/ValueObjectVTable.h"
#include "lldb/DataFormatters/DataVisualization.h"
#include "lldb/DataFormatters/DumpValueObjectOptions.h"
#include "lldb/DataFormatters/FormatManager.h"
@@ -3155,3 +3156,7 @@ ValueObjectSP ValueObject::Persist() {
return persistent_var_sp->GetValueObject();
}
+
+lldb::ValueObjectSP ValueObject::GetVTable() {
+ return ValueObjectVTable::Create(*this);
+}
diff --git a/lldb/source/Core/ValueObjectVTable.cpp b/lldb/source/Core/ValueObjectVTable.cpp
new file mode 100644
index 000000000000000..31c619986b825f1
--- /dev/null
+++ b/lldb/source/Core/ValueObjectVTable.cpp
@@ -0,0 +1,325 @@
+//===-- ValueObjectVTable.cpp ---------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Core/ValueObjectVTable.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ValueObjectChild.h"
+#include "lldb/Symbol/Function.h"
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-private-enumerations.h"
+#include "clang/Tooling/Transformer/RangeSelector.h"
+#include "llvm/Support/MathExtras.h"
+#include <cstdint>
+
+using namespace lldb;
+using namespace lldb_private;
+
+class ValueObjectVTableChild : public ValueObject {
+public:
+ ValueObjectVTableChild(ValueObject &parent, uint32_t func_idx,
+ uint64_t addr_size)
+ : ValueObject(parent), m_func_idx(func_idx), m_addr_size(addr_size) {
+ SetFormat(eFormatPointer);
+ SetName(ConstString(llvm::formatv("[{0}]", func_idx).str()));
+ }
+
+ ~ValueObjectVTableChild() override = default;
+
+ std::optional<uint64_t> GetByteSize() override { return m_addr_size; };
+
+ size_t CalculateNumChildren(uint32_t max) override { return 0; };
+
+ ValueType GetValueType() const override { return eValueTypeVTableEntry; };
+
+ bool IsInScope() override {
+ ValueObject *parent = GetParent();
+ if (parent)
+ return parent->IsInScope();
+ return false;
+ };
+
+protected:
+ bool UpdateValue() override {
+ SetValueIsValid(false);
+ m_value.Clear();
+ ValueObject *parent = GetParent();
+ if (!parent) {
+ m_error.SetErrorString("no parent object");
+ return false;
+ }
+
+ addr_t parent_addr = parent->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
+ if (parent_addr == LLDB_INVALID_ADDRESS) {
+ m_error.SetErrorString("parent has invalid address");
+ return false;
+ }
+
+ ProcessSP process_sp = GetProcessSP();
+ if (!process_sp) {
+ m_error.SetErrorString("no process");
+ return false;
+ }
+
+ TargetSP target_sp = GetTargetSP();
+ if (!target_sp) {
+ m_error.SetErrorString("no target");
+ return false;
+ }
+
+ // Each `vtable_entry_addr` points to the function pointer.
+ addr_t vtable_entry_addr = parent_addr + m_func_idx * m_addr_size;
+ addr_t vfunc_ptr =
+ process_sp->ReadPointerFromMemory(vtable_entry_addr, m_error);
+ if (m_error.Fail()) {
+ m_error.SetErrorStringWithFormat(
+ "failed to read virtual function entry 0x%16.16" PRIx64,
+ vtable_entry_addr);
+ return false;
+ }
+
+ Address resolved_vfunc_ptr_address;
+ target_sp->ResolveLoadAddress(vfunc_ptr, resolved_vfunc_ptr_address);
+ if (!resolved_vfunc_ptr_address.IsValid()) {
+ m_error.SetErrorStringWithFormat(
+ "unable to resolve func ptr address: 0x%16.16" PRIx64, vfunc_ptr);
+ return false;
+ }
+
+ // Set our value to be the load address of the function pointer in memory
+ // and our type to be the function pointer type.
+ m_value.SetValueType(Value::ValueType::LoadAddress);
+ m_value.GetScalar() = vtable_entry_addr;
+
+ // See if our resolved address points to a function in the debug info. If
+ // it does, then we can report the type as a function prototype for this
+ // function.
+ Function *function =
+ resolved_vfunc_ptr_address.CalculateSymbolContextFunction();
+ if (function) {
+ m_value.SetCompilerType(function->GetCompilerType());
+ } else {
+ // Set our value's compiler type to a generic function protoype so that
+ // it displays as a hex function pointer for the value and the summary
+ // will display the address description.
+ auto type_system_or_err =
+ target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC_plus_plus);
+ if (type_system_or_err) {
+ CompilerType proto = (*type_system_or_err)->CreateGenericFunctionPrototype();
+ if (proto.IsFunctionType())
+ m_value.SetCompilerType(proto);
+ } else {
+ consumeError(type_system_or_err.takeError());
+ }
+ }
+
+ // Now read our value into m_data so that our we can use the default
+ // summary provider for C++ for function pointers which will get the
+ // address description for our function pointer.
+ if (m_error.Success()) {
+ const bool thread_and_frame_only_if_stopped = true;
+ ExecutionContext exe_ctx(
+ GetExecutionContextRef().Lock(thread_and_frame_only_if_stopped));
+ m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
+ }
+ SetValueDidChange(true);
+ SetValueIsValid(true);
+ return true;
+ };
+
+ CompilerType GetCompilerTypeImpl() override {
+ return m_value.GetCompilerType();
+ };
+
+ const uint32_t m_func_idx;
+ const uint64_t m_addr_size;
+
+private:
+ // For ValueObject only
+ ValueObjectVTableChild(const ValueObjectVTableChild &) = delete;
+ const ValueObjectVTableChild &
+ operator=(const ValueObjectVTableChild &) = delete;
+};
+
+ValueObjectSP ValueObjectVTable::Create(ValueObject &parent) {
+ return (new ValueObjectVTable(parent))->GetSP();
+}
+
+ValueObjectVTable::ValueObjectVTable(ValueObject &parent)
+ : ValueObject(parent) {
+ SetFormat(eFormatPointer);
+}
+
+std::optional<uint64_t> ValueObjectVTable::GetByteSize() {
+ if (m_vtable_symbol)
+ return m_vtable_symbol->GetByteSize();
+ else
+ return std::nullopt;
+}
+
+size_t ValueObjectVTable::CalculateNumChildren(uint32_t max) {
+ if (UpdateValueIfNeeded(false))
+ return m_num_vtable_entries <= max ? m_num_vtable_entries : max;
+ return 0;
+}
+
+ValueType ValueObjectVTable::GetValueType() const { return eValueTypeVTable; }
+
+ConstString ValueObjectVTable::GetTypeName() {
+ if (m_vtable_symbol)
+ return m_vtable_symbol->GetName();
+ return ConstString();
+}
+
+ConstString ValueObjectVTable::GetQualifiedTypeName() { return GetTypeName(); }
+
+ConstString ValueObjectVTable::GetDisplayTypeName() {
+ if (m_vtable_symbol)
+ return m_vtable_symbol->GetDisplayName();
+ return ConstString();
+}
+
+bool ValueObjectVTable::IsInScope() { return GetParent()->IsInScope(); }
+
+ValueObject *ValueObjectVTable::CreateChildAtIndex(size_t idx,
+ bool synthetic_array_member,
+ int32_t synthetic_index) {
+ if (synthetic_array_member)
+ return nullptr;
+ return new ValueObjectVTableChild(*this, idx, m_addr_size);
+}
+
+bool ValueObjectVTable::UpdateValue() {
+ m_error.Clear();
+ m_flags.m_children_count_valid = false;
+ SetValueIsValid(false);
+ m_num_vtable_entries = 0;
+ ValueObject *parent = GetParent();
+ if (!parent) {
+ m_error.SetErrorString("no parent object");
+ return false;
+ }
+
+ // Check to make sure the class has a vtable.
+
+ // If we have a pointer or reference type, get the pointee type from this
+ // so we can ask if it has virtual functions below.
+ CompilerType value_type = parent->GetCompilerType();
+ if (value_type.IsPointerOrReferenceType()) {
+ CompilerType pointee_type = value_type.GetPointeeType();
+ if (pointee_type)
+ value_type = pointee_type;
+ }
+
+ // Make sure this is a class or a struct first by checking the type class
+ // bitfield that gets returned.
+ if ((value_type.GetTypeClass() & (eTypeClassStruct | eTypeClassClass)) == 0) {
+ m_error.SetErrorStringWithFormat("type \"%s\" is not a class or struct",
+ value_type.GetTypeName().AsCString("<invalid>"));
+ return false;
+ }
+
+ // Check if the type has virtual functions by asking it if it is polymorphic.
+ if (!value_type.IsPolymorphicClass()) {
+ m_error.SetErrorStringWithFormat("type \"%s\" doesn't have a vtable",
+ value_type.GetTypeName().AsCString("<invalid>"));
+ return false;
+ }
+
+ TargetSP target_sp = GetTargetSP();
+ if (!target_sp) {
+ m_error.SetErrorString("no target");
+ return false;
+ }
+
+ // Get the address of our parent. This will update the parent value object
+ // if needed and fetch the address of the parent.
+ AddressType addr_type;
+ addr_t parent_load_addr =
+ parent->GetAddressOf(/*scalar_is_load_address=*/true, &addr_type);
+ if (addr_type == eAddressTypeFile) {
+ ModuleSP module_sp(parent->GetModule());
+ if (!module_sp) {
+ parent_load_addr = LLDB_INVALID_ADDRESS;
+ } else {
+ Address addr;
+ module_sp->ResolveFileAddress(parent_load_addr, addr);
+ parent_load_addr = addr.GetLoadAddress(target_sp.get());
+ }
+ } else if (addr_type == eAddressTypeHost ||
+ addr_type == eAddressTypeInvalid) {
+ parent_load_addr = LLDB_INVALID_ADDRESS;
+ }
+
+ if (parent_load_addr == LLDB_INVALID_ADDRESS) {
+ m_error.SetErrorString("parent is not in memory");
+ return false;
+ }
+
+ m_value.Clear();
+
+ ProcessSP process_sp = GetProcessSP();
+ if (!process_sp) {
+ m_error.SetErrorString("no process");
+ return false;
+ }
+
+ // We expect to find the vtable at the first block of memory.
+ addr_t possible_vtable_ptr =
+ process_sp->ReadPointerFromMemory(parent_load_addr, m_error);
+ if (m_error.Fail())
+ return false;
+
+ Address resolved_possible_vtable_address;
+ target_sp->ResolveLoadAddress(possible_vtable_ptr,
+ resolved_possible_vtable_address);
+ if (!resolved_possible_vtable_address.IsValid()) {
+ m_error.SetErrorStringWithFormat(
+ "unable to resolve 0x%" PRIx64 " to a section for vtable "
+ "symbol search", possible_vtable_ptr);
+ return false;
+ }
+
+ m_vtable_symbol =
+ resolved_possible_vtable_address.CalculateSymbolContextSymbol();
+ if (!(m_vtable_symbol &&
+ m_vtable_symbol->GetName().GetStringRef().startswith("vtable for "))) {
+ m_error.SetErrorStringWithFormat(
+ "no vtable symbol found containing 0x%" PRIx64, possible_vtable_ptr);
+ return false;
+ }
+
+ // Now that we know it's a vta...
[truncated]
|
lldb/include/lldb/API/SBValue.h
Outdated
| /// object isn't a C++ class with a vtable or not a C++ class. | ||
| /// | ||
| /// SBValue::GetName() will be the demangled symbol name for the virtual | ||
| /// function table like "vtable for Baseclass". |
There was a problem hiding this comment.
Shouldn't this be vtable for ChildClass in RTTI domain?
There was a problem hiding this comment.
I think "BaseClass" was just an example here, but maybe a more abstract name would be better.
For an actual C++ object, the object's vtable pointer always points to the most specific class of the object, a fact that lldb relies on to do C++ dynamic type detection.
There was a problem hiding this comment.
I can change it to "vtable for "?
| m_error.SetErrorStringWithFormat( | ||
| "unable to resolve func ptr address: 0x%16.16" PRIx64, vfunc_ptr); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
One of the reasons that people have requested the ability to see vtables in the past has been to detect corruption in the vtable. If the table is corrupt, it might very well point to bogus memory. So it might be better to have this still be valid, but have the summary value report that this looks like a bogus entry? Then when I dump the value object I'll be able to see some valid entries and some that say "Danger Will Robinson" which might be easier to work with than errors?
There was a problem hiding this comment.
Yes, good idea here. We are reading individual function pointers here, so we should still show it. This could also be JIT'ed code where we might not have a section for the address. This would be hard to test since these function pointers are in read only memory. Can you think of a way to test this? We won't get a child like this unless the parent's vtable pointer resolved to a "vtable for " symbol, so I am not sure how to test. I guess we can use our debugger superpowers to ruin the vtable pointer and test that way.
There was a problem hiding this comment.
Yes, I think that's probably the best way to test it. You could in fact just call SetValueFromCString on one of your ValueObjectVTableChild objects, that should do it.
| // Set our value's compiler type to a generic function protoype so that | ||
| // it displays as a hex function pointer for the value and the summary | ||
| // will display the address description. | ||
| auto type_system_or_err = |
There was a problem hiding this comment.
It's okay for now because we don't have any others, but there could be other languages that have the concept of a VTable, so at some point the eLanguageTypeC_plus_plus should get genericized...
There was a problem hiding this comment.
I can rename these classes to be ValueObjectCPlusPlusVTable just in case. Or the constructor can take a language enumeration and then we use that?
There was a problem hiding this comment.
Actually I can use "ValueObject::GetObjectRuntimeLanguage()"
There was a problem hiding this comment.
One interesting use case this could work for is swift's value witness tables. Although not the exact same as a vtable, it is a similar concept that I think is worth making this more generic for.
|
Looks good in general to me. Several scenarios to test:
|
| return false; | ||
| } | ||
|
|
||
| // We expect to find the vtable at the first block of memory. |
There was a problem hiding this comment.
The C++ Language Runtime has to find the VTable pointer so it can determine the dynamic type of the ValueObject, can we reuse that code here?
There was a problem hiding this comment.
We might have dynamic type detection on or off, and we will show what ever we find as the first pointer in the object if a class is polymorphic. I am happy to look at the dynamic type code, but I think that code is always going to try and adjust the pointer and we don't want to do that here, we want to accept what ever the current ValueObject is showing right?
There was a problem hiding this comment.
That's not quite what I was saying. The Dynamic Type Detection code in the ItaniumLangaugeRuntime has two phases. First, given a ValueObject, it has to find its vtable pointer. Then given the vtable pointer, it gets the dynamic class, figures out the offset to the most specific type and produces the dynamic value. That's all in one function right now. I was suggesting pulling out the part of the runtime code that gets the vtable address into a new function in the LanguageRuntime:
lldb::addr_t LanguageRuntime::GetVTablePointer(ValueObject &vo);
implementing that in the ItaniumLanguageRuntime, and then rejiggering both ItaniumLanguageRuntime::GetDynamicValueAndType and your VTable Update to use this new function.
|
This is an oft-requested feature so thanks for doing this! My only real complaint is that the code that determines the C++ "dynamic type" does much of the work that the ValueObjectVTable has to do to find the vtable pointer, and it's enough code that it seems a shame to have two copies of it. Maybe there should be a LanguageRuntime "GetVTablePointer" that factors out the part of GetDynamicTypeAndAddress that finds the VTable. Then you can get the LanguageRuntime from the Object and call this generic API. That would also make it clear that this support is for the ItaniamuABI C++ runtime, so that if another runtime comes along that does some other clever thing, it will be straightforward to support it. |
|
It might also be good to write a test that scribbles on the VTable and make sure that we present that usefully. That is a significant use case for this feature. |
I mention this in my inline comments, but we don't want to detect the dynamic type here, we just want to use what ever vtable pointer we find in the current ValueObject that we use to ask for the vtable. We might have dynamic typing on or off, and we don't want to adjust the pointer at all. If we have dynamic typing off, then we will just be in the middle of someone else's vtable, which we will locate by finding the symbol that contains it, so that is ok. If we have dynamic typing on, then the value we ask for the vtable for will point to the first vtable entry for the modified type. So there is no complexity here like there is in the GetDynamicTypeAndAddress(). We just read a pointer and look for the vtable symbol. |
Good idea. I tried to test by stripping, but then I couldn't set a breakpoint where I needed it. So if we do this in another binary and just strip the shared library, then I can test this.
Yes, if the value is polymorphic. We make sure the type has virtual functions first before doing any of this.
|
I don't think this is how it works. If you have your hands on a C++ object, even when viewed as a subclass or even one of the more complex multi-inheritance base classes that don't line up with the start of the dynamic object, and you find its vtable pointer, it will point to the vtable of the most specific class. That's actually how we figure out what the dynamic type is, we just grab the vtable pointer of the object we're handed, demangle its name and pull off the "vtable of " part, and that's always the dynamic class... So in both cases, the first step is to get the vtable from the extant object. The additional complexity in GetDynamicValueAndType is all AFTER we've found the dynamic class, where we peer further into the runtime to determine the "offset to top". But you don't need to do any of that to get the dynamic class, you just get the vtable & demangle its name... So I am pretty sure the "get the vtable" part is going to be identical between your code and the GetDynamicValueAndType, and you can factor that bit out and share it between GetDynamicValueAndType and your Update. |
| /// | ||
| /// |
| ValueObject *parent = GetParent(); | ||
| if (parent) |
There was a problem hiding this comment.
if (ValueObject *parent = GetParent()) ?
| // Set our value's compiler type to a generic function protoype so that | ||
| // it displays as a hex function pointer for the value and the summary | ||
| // will display the address description. | ||
| auto type_system_or_err = |
There was a problem hiding this comment.
One interesting use case this could work for is swift's value witness tables. Although not the exact same as a vtable, it is a similar concept that I think is worth making this more generic for.
| }; | ||
|
|
||
| ValueObjectSP ValueObjectVTable::Create(ValueObject &parent) { | ||
| return (new ValueObjectVTable(parent))->GetSP(); |
There was a problem hiding this comment.
std::make_shared<ValueObjectVTable>(parent)?
There was a problem hiding this comment.
This is some magic to have the lifetime managed by the root object.
There was a problem hiding this comment.
We have to be careful about lifetimes, somebody could do:
def FindVTableByPath(frame, path):
var = frame.GetValueForExpressionPath(path)
return var.GetVTable()
So the only thing that is held onto anymore is the VTable child. But the VTable child needs it's whole parent tree to stay alive to be able to update itself, etc.
| else | ||
| return std::nullopt; |
|
I am implementing the fixes in the LanguageRuntime stuff. I have it working. Stay tuned to updates to this patch. |
| }; | ||
|
|
||
| ValueObjectSP ValueObjectVTable::Create(ValueObject &parent) { | ||
| return (new ValueObjectVTable(parent))->GetSP(); |
There was a problem hiding this comment.
This is some magic to have the lifetime managed by the root object.
There was a problem hiding this comment.
This is much nicer looking!
I had a couple small comments about apportioning duties between the ValueObjectVTable::Update and the GetVTableInfo in the comments.
It doesn't look like you handled corrupted vtables yet? Are you still intending to do that?
Another thing it might be good to test - though from what you've done it should indeed work is if I have:
BaseClass Foo my_foo = FirstChildOfFoo();
// Stop after this line and do vtbl = frame.FindVariable("my_foo").GetVTable() and check the vtable has the right functions for FirstChildOfFoo()
my_foo = SecondChildOfFoo();
// Stop after this line and check the vtbl variable again, it should now have the functions for SecondChildOfFoo...
That will make sure both that we notice the change in the parent & update the vtable appropriately, and that we don't mess up the memory management, such that holding onto just the vtable child doesn't keep the whole tree valid.
| }; | ||
|
|
||
| ValueObjectSP ValueObjectVTable::Create(ValueObject &parent) { | ||
| return (new ValueObjectVTable(parent))->GetSP(); |
There was a problem hiding this comment.
We have to be careful about lifetimes, somebody could do:
def FindVTableByPath(frame, path):
var = frame.GetValueForExpressionPath(path)
return var.GetVTable()
So the only thing that is held onto anymore is the VTable child. But the VTable child needs it's whole parent tree to stay alive to be able to update itself, etc.
|
|
||
| // Make sure this is a class or a struct first by checking the type class | ||
| // bitfield that gets returned. | ||
| if ((value_type.GetTypeClass() & (eTypeClassStruct | eTypeClassClass)) == 0) { |
There was a problem hiding this comment.
Do these checks need to be here, or could this be folded into GetVTableInfo? Seems like it is more appropriate that the LanguageRuntime know what kinds of ValueObjects could have tables?
There was a problem hiding this comment.
Yeah, I can move this there.
| LanguageRuntime *language_runtime = process_sp->GetLanguageRuntime(language); | ||
|
|
||
| if (language_runtime == nullptr) { | ||
| m_error.SetErrorString("value doesn't have a vtable"); |
There was a problem hiding this comment.
Shouldn't this be:
m_error.SetErrorString("can't determine the Language for value, needed to find the vtable");
When this error comes back in a log we'll be happy it was different than the ones below...
| } | ||
| } | ||
| return TypeAndOrName(); | ||
| } | ||
|
|
||
| // This function can accept both pointers or references to classes as well | ||
| // as instances of classes. For dynamic type detection, only valid ValueObjects | ||
| // that return true to CouldHaveDynamicValue(...) should call this function. |
There was a problem hiding this comment.
Is there any reason to make people go through two steps here? If you pass me a ValueObject with CouldHaveDynamicInfo == false, just return no vtable? I don't have a strong feeling one way or the other about this.
There was a problem hiding this comment.
CouldHaveDynamicValue() only works on pointers. Since those are the only things we will dynamically adjust for dynamic typing. So CouldHaveDynamicValue() doesn't work for instances of a classes or structs that have virtual function pointers.
There was a problem hiding this comment.
Ah, I see. That makes sense. It might be clearer if instead of "For dynamic type detection" you said:
"If you are using this function to do dynamic type detection..."
|
|
||
| addr_t parent_addr = parent->GetValueAsUnsigned(LLDB_INVALID_ADDRESS); | ||
| if (parent_addr == LLDB_INVALID_ADDRESS) { | ||
| m_error.SetErrorString("parent has invalid address"); |
There was a problem hiding this comment.
This is public facing error right? Maybe be clear what is "parent" in this context. Like can't get parent vtable address.
| if (function) { | ||
| m_value.SetCompilerType(function->GetCompilerType()); | ||
| } else { | ||
| // Set our value's compiler type to a generic function protoype so that |
| CompilerType pointee_type = value_type.GetPointeeType(); | ||
| if (pointee_type) | ||
| value_type = pointee_type; | ||
| } |
There was a problem hiding this comment.
We can still go on if this is not a pointer/reference?
There was a problem hiding this comment.
Yes, we might have variable that is a class/struct instance that has a vtable. Unlike dynamic typing, we can always get a vtable from an instance.
I remember some compiler's multi-inheritance implementation is putting one vtable_ptr in object for each parent class. Maybe not in clang?
Right, I am not asking for a testcase, but manual testing. |
Each class has a single vtable and this table will contain a copy of all vtables that are needed for each class. |
I was talking about https://shaharmike.com/cpp/vtable-part2/ which there can be multiple vtable_ptr(s) in object and multiple vtables. But I think you are right that we only care showing the final merged vtable from most derived child class not other vtables containing non-virtual thunk methods. |
|
Maybe this was only reverted in his branch? |
|
I hope that's not the case... It would be really noisy to see what goes on on people's private forks in a PR on the main repo.
Jim
… On Oct 31, 2023, at 10:42 AM, Greg Clayton ***@***.***> wrote:
Maybe this was only reverted in his branch?
—
Reply to this email directly, view it on GitHub <#67599 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW5KUT2EHO56SZYBNLDYCEZ7NAVCNFSM6AAAAAA5J4U42CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBXGY4DOMJXGM>.
You are receiving this because your review was requested.
|
|
The change was not reverted upstream. It was reverted in that @DanielCChen 's llvm-project fork. Unfortunately that's the nature of GitHub. |
|
Phew! Still getting. used to this new workflow. Thanks for the confirmation |
|
Sorry about the noise. All the reverts are in a private branch on my fork repo. I didn't know it would broadcast to all the people who worked on the reverted commits. My apologies. |
|
That seems more like a GitHub problem than a you problem? Seems weird to have to not touch a PR commit on a private branch lest you spam everyone with the notification...
Jim
… On Nov 1, 2023, at 8:49 AM, Daniel Chen ***@***.***> wrote:
Sorry about the noise. All the reverts are in a private branch on my fork repo. I didn't know it would broadcast to all the people who worked on the reverted commits. My apologies.
—
Reply to this email directly, view it on GitHub <#67599 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW5H6FEZ7V4OTZKO4RDYCJVQRAVCNFSM6AAAAAA5J4U42CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBZGE4TQNJVGI>.
You are receiving this because your review was requested.
|
|
I think it is still my fault because it is indeed a PR branch that I did revert even though it is on my forked repo. I guess GitHub is doing the right thing to notify everyone affected as the PR could be potentially merged onto upstream branch by accident if it is approved. Again, my apologies to everyone who got distracted by this event. |
No worries! I am getting used to the new github workflow. I will watch the revert messages more closely to make sure they aren't on upstream. Sorry for the noise from me! |
The current Darwin arm64e ABI on AArch64 systems using ARMv8.3 & newer cores, adds authentication bits to the vtable pointer address. The vtable address must be in addressable memory, so running it through Process::FixDataAddress will be a no-op on other targets. This was originally a downstream change that I hadn't upstreamed yet, and it was surfaced by Greg's changes in llvm#67599 so I needed to update the local patch, and was reminded that I should upstream this.
|
FYI this did fail on the 32 bit Arm bot (https://lab.llvm.org/buildbot/#/builders/17/builds/44589), but it had been red up until then so no notification went out. I've fixed the test in 4be8a7b. |
The current Darwin arm64e ABI on AArch64 systems using ARMv8.3 & newer cores, adds authentication bits to the vtable pointer address. The vtable address must be in addressable memory, so running it through Process::FixDataAddress will be a no-op on other targets. This was originally a downstream change that I hadn't upstreamed yet, and it was surfaced by Greg's changes in #67599 so I needed to update the local patch, and was reminded that I should upstream this.
llvm#67599) Add the ability to get a C++ vtable ValueObject from another ValueObject. This patch adds the ability to ask a ValueObject for a ValueObject that represents the virtual function table for a C++ class. If the ValueObject is not a C++ class with a vtable, a valid ValueObject value will be returned that contains an appropriate error. If it is successful a valid ValueObject that represents vtable will be returned. The ValueObject that is returned will have a name that matches the demangled value for a C++ vtable mangled name like "vtable for <class-name>". It will have N children, one for each virtual function pointer. Each child's value is the function pointer itself, the summary is the symbolication of this function pointer, and the type will be a valid function pointer from the debug info if there is debug information corresponding to the virtual function pointer. The vtable SBValue will have the following: - SBValue::GetName() returns "vtable for <class>" - SBValue::GetValue() returns a string representation of the vtable address - SBValue::GetSummary() returns NULL - SBValue::GetType() returns a type appropriate for a uintptr_t type for the current process - SBValue::GetLoadAddress() returns the address of the vtable adderess - SBValue::GetValueAsUnsigned(...) returns the vtable address - SBValue::GetNumChildren() returns the number of virtual function pointers in the vtable - SBValue::GetChildAtIndex(...) returns a SBValue that represents a virtual function pointer The child SBValue objects that represent a virtual function pointer has the following values: - SBValue::GetName() returns "[%u]" where %u is the vtable function pointer index - SBValue::GetValue() returns a string representation of the virtual function pointer - SBValue::GetSummary() returns a symbolicated respresentation of the virtual function pointer - SBValue::GetType() returns the function prototype type if there is debug info, or a generic funtion prototype if there is no debug info - SBValue::GetLoadAddress() returns the address of the virtual function pointer - SBValue::GetValueAsUnsigned(...) returns the virtual function pointer - SBValue::GetNumChildren() returns 0 - SBValue::GetChildAtIndex(...) returns invalid SBValue for any index Examples of using this API via python: ``` (lldb) script vtable = lldb.frame.FindVariable("shape_ptr").GetVTable() (lldb) script vtable vtable for Shape = 0x0000000100004088 { [0] = 0x0000000100003d20 a.out`Shape::~Shape() at main.cpp:3 [1] = 0x0000000100003e4c a.out`Shape::~Shape() at main.cpp:3 [2] = 0x0000000100003e7c a.out`Shape::area() at main.cpp:4 [3] = 0x0000000100003e3c a.out`Shape::optional() at main.cpp:7 } (lldb) script c = vtable.GetChildAtIndex(0) (lldb) script c (void ()) [0] = 0x0000000100003d20 a.out`Shape::~Shape() at main.cpp:3 ``` (cherry picked from commit 7fbd427)
The current Darwin arm64e ABI on AArch64 systems using ARMv8.3 & newer cores, adds authentication bits to the vtable pointer address. The vtable address must be in addressable memory, so running it through Process::FixDataAddress will be a no-op on other targets. This was originally a downstream change that I hadn't upstreamed yet, and it was surfaced by Greg's changes in llvm#67599 so I needed to update the local patch, and was reminded that I should upstream this. (cherry picked from commit de24b0e)
llvm#67599) Add the ability to get a C++ vtable ValueObject from another ValueObject. This patch adds the ability to ask a ValueObject for a ValueObject that represents the virtual function table for a C++ class. If the ValueObject is not a C++ class with a vtable, a valid ValueObject value will be returned that contains an appropriate error. If it is successful a valid ValueObject that represents vtable will be returned. The ValueObject that is returned will have a name that matches the demangled value for a C++ vtable mangled name like "vtable for <class-name>". It will have N children, one for each virtual function pointer. Each child's value is the function pointer itself, the summary is the symbolication of this function pointer, and the type will be a valid function pointer from the debug info if there is debug information corresponding to the virtual function pointer. The vtable SBValue will have the following: - SBValue::GetName() returns "vtable for <class>" - SBValue::GetValue() returns a string representation of the vtable address - SBValue::GetSummary() returns NULL - SBValue::GetType() returns a type appropriate for a uintptr_t type for the current process - SBValue::GetLoadAddress() returns the address of the vtable adderess - SBValue::GetValueAsUnsigned(...) returns the vtable address - SBValue::GetNumChildren() returns the number of virtual function pointers in the vtable - SBValue::GetChildAtIndex(...) returns a SBValue that represents a virtual function pointer The child SBValue objects that represent a virtual function pointer has the following values: - SBValue::GetName() returns "[%u]" where %u is the vtable function pointer index - SBValue::GetValue() returns a string representation of the virtual function pointer - SBValue::GetSummary() returns a symbolicated respresentation of the virtual function pointer - SBValue::GetType() returns the function prototype type if there is debug info, or a generic funtion prototype if there is no debug info - SBValue::GetLoadAddress() returns the address of the virtual function pointer - SBValue::GetValueAsUnsigned(...) returns the virtual function pointer - SBValue::GetNumChildren() returns 0 - SBValue::GetChildAtIndex(...) returns invalid SBValue for any index Examples of using this API via python: ``` (lldb) script vtable = lldb.frame.FindVariable("shape_ptr").GetVTable() (lldb) script vtable vtable for Shape = 0x0000000100004088 { [0] = 0x0000000100003d20 a.out`Shape::~Shape() at main.cpp:3 [1] = 0x0000000100003e4c a.out`Shape::~Shape() at main.cpp:3 [2] = 0x0000000100003e7c a.out`Shape::area() at main.cpp:4 [3] = 0x0000000100003e3c a.out`Shape::optional() at main.cpp:7 } (lldb) script c = vtable.GetChildAtIndex(0) (lldb) script c (void ()) [0] = 0x0000000100003d20 a.out`Shape::~Shape() at main.cpp:3 ``` (cherry picked from commit 7fbd427)
The current Darwin arm64e ABI on AArch64 systems using ARMv8.3 & newer cores, adds authentication bits to the vtable pointer address. The vtable address must be in addressable memory, so running it through Process::FixDataAddress will be a no-op on other targets. This was originally a downstream change that I hadn't upstreamed yet, and it was surfaced by Greg's changes in llvm#67599 so I needed to update the local patch, and was reminded that I should upstream this. (cherry picked from commit de24b0e)
The current Darwin arm64e ABI on AArch64 systems using ARMv8.3 & newer cores, adds authentication bits to the vtable pointer address. The vtable address must be in addressable memory, so running it through Process::FixDataAddress will be a no-op on other targets. This was originally a downstream change that I hadn't upstreamed yet, and it was surfaced by Greg's changes in llvm/llvm-project#67599 so I needed to update the local patch, and was reminded that I should upstream this.
Add the ability to get a C++ vtable ValueObject from another ValueObject.
This patch adds the ability to ask a ValueObject for a ValueObject that represents the virtual function table for a C++ class. If the ValueObject is not a C++ class with a vtable, a valid ValueObject value will be returned that contains an appropriate error. If it is successful a valid ValueObject that represents vtable will be returned. The ValueObject that is returned will have a name that matches the demangled value for a C++ vtable mangled name like "vtable for ". It will have N children, one for each virtual function pointer. Each child's value is the function pointer itself, the summary is the symbolication of this function pointer, and the type will be a valid function pointer from the debug info if there is debug information corresponding to the virtual function pointer.
The vtable SBValue will have the following:
The child SBValue objects that represent a virtual function pointer has the following values:
Examples of using this API via python: