[lldb] Fix dynamic type resolutions for core files #138698
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We're reading from the object's vtable to determine the pointer to the full object. The vtable is normally in the "rodata" section of the executable, which is often not included in the core file because it's not supposed to change and the debugger can extrapolate its contents from the executable file. We weren't doing that. This patch changes the read operation to use the target class (which falls back onto the executable module as expected) and adds the missing ReadSignedIntegerFromMemory API. The fix is tested by creating a core (minidump) file which deliberately omits the vtable pointer.
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesWe're reading from the object's vtable to determine the pointer to the full object. The vtable is normally in the "rodata" section of the executable, which is often not included in the core file because it's not supposed to change and the debugger can extrapolate its contents from the executable file. We weren't doing that. This patch changes the read operation to use the target class (which falls back onto the executable module as expected) and adds the missing ReadSignedIntegerFromMemory API. The fix is tested by creating a core (minidump) file which deliberately omits the vtable pointer. Full diff: https://github.com/llvm/llvm-project/pull/138698.diff 4 Files Affected:
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 73f27dc934b46..0d4e11b65339e 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1158,6 +1158,11 @@ class Target : public std::enable_shared_from_this<Target>,
Status &error,
bool force_live_memory = false);
+ int64_t ReadSignedIntegerFromMemory(const Address &addr,
+ size_t integer_byte_size,
+ int64_t fail_value, Status &error,
+ bool force_live_memory = false);
+
uint64_t ReadUnsignedIntegerFromMemory(const Address &addr,
size_t integer_byte_size,
uint64_t fail_value, Status &error,
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
index 8faf7135217ac..0d068ed5950d5 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
@@ -350,7 +350,7 @@ bool ItaniumABILanguageRuntime::GetDynamicTypeAndAddress(
if (offset_to_top_location >= vtable_load_addr)
return false;
Status error;
- const int64_t offset_to_top = m_process->ReadSignedIntegerFromMemory(
+ const int64_t offset_to_top = target.ReadSignedIntegerFromMemory(
offset_to_top_location, addr_byte_size, INT64_MIN, error);
if (offset_to_top == INT64_MIN)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e90e748191a7e..7f61f8689fb95 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2270,6 +2270,17 @@ size_t Target::ReadScalarIntegerFromMemory(const Address &addr, uint32_t byte_si
return 0;
}
+int64_t Target::ReadSignedIntegerFromMemory(const Address &addr,
+ size_t integer_byte_size,
+ int64_t fail_value, Status &error,
+ bool force_live_memory) {
+ Scalar scalar;
+ if (ReadScalarIntegerFromMemory(addr, integer_byte_size, false, scalar, error,
+ force_live_memory))
+ return scalar.SLongLong(fail_value);
+ return fail_value;
+}
+
uint64_t Target::ReadUnsignedIntegerFromMemory(const Address &addr,
size_t integer_byte_size,
uint64_t fail_value, Status &error,
diff --git a/lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py b/lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py
index 634bd13d7c71a..b15b5f1dde377 100644
--- a/lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py
+++ b/lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py
@@ -279,3 +279,53 @@ def test_from_forward_decl(self):
"frame var -d run-target --ptr-depth=1 --show-types a",
substrs=["(B *) a", "m_b_value = 10"],
)
+
+ @no_debug_info_test
+ @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24663")
+ def test_from_core_file(self):
+ """Test fetching C++ dynamic values from core files. Specifically, test
+ that we can determine the dynamic type of the value if the core file
+ does not contain the type vtable."""
+ self.build()
+ lldbutil.run_to_name_breakpoint(self, "take_A")
+
+ # Get the address of our object and its vtable
+ a = self.frame().FindVariable("a")
+ self.assertSuccess(a.GetError())
+ vtable = a.GetVTable()
+ self.assertSuccess(vtable.GetError())
+ a = a.GetValueAsAddress()
+ vtable = vtable.GetValueAsAddress()
+
+ # Create a core file which will only contain the memory region
+ # containing `a`. The object is on the stack, so this will automatically
+ # include the stack of the main thread.
+ core = self.getBuildArtifact("a.dmp")
+ options = lldb.SBSaveCoreOptions()
+ options.SetPluginName("minidump")
+ options.SetStyle(lldb.eSaveCoreCustomOnly)
+ options.SetOutputFile(lldb.SBFileSpec(core))
+ region = lldb.SBMemoryRegionInfo()
+ self.assertSuccess(self.process().GetMemoryRegionInfo(a, region))
+ self.assertSuccess(options.AddMemoryRegionToSave(region))
+
+ # Save the core file and load it.
+ self.assertSuccess(self.process().SaveCore(options))
+ self.process().Kill()
+ error = lldb.SBError()
+ self.target().LoadCore(core, error)
+ self.assertSuccess(error)
+
+ # Sanity check -- the process should be able to read the object but not
+ # its vtable..
+ self.process().ReadPointerFromMemory(a, error)
+ self.assertSuccess(error)
+ self.process().ReadPointerFromMemory(vtable, error)
+ self.assertTrue(error.Fail())
+
+ # .. but we should still be able to see the dynamic type by reading the
+ # vtable from the executable file.
+ self.expect(
+ "frame var -d run-target --ptr-depth=1 --show-types a",
+ substrs=["(B *) a", "m_b_value = 10"],
+ )
|
Michael137
approved these changes
May 6, 2025
|
✅ With the latest revision this PR passed the Python code formatter. |
|
@DavidSpickett, I am confused by the error on this arm builder: https://lab.llvm.org/buildbot/#/builders/18/builds/15619 It says arm is not supported, but I think that error message is coming from here, and that switch has a |
|
There's two places that could make that error, I will check both locally. |
|
There are? Where's the other one? |
|
Oh, it's this one. That makes more sense now. I'll skip the test on arm. Thanks for catching that. |
|
Yep, called from AddThreadList: |
|
Btw I think this is causing an asan failure in https://green.lab.llvm.org/job/lldb-cmake-sanitized/1671/ |
|
Lol, I think its this: llvm-project/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp Lines 718 to 721 in 6babd63 I'm done for this week, but could someone try if changing that to std::min fixes this? |
felipepiovezan
added a commit
that referenced
this pull request
May 8, 2025
As per comment in #138698 (comment)
|
Pushed your suggestion
|
|
Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We're reading from the object's vtable to determine the pointer to the full object. The vtable is normally in the "rodata" section of the executable, which is often not included in the core file because it's not supposed to change and the debugger can extrapolate its contents from the executable file. We weren't doing that.
This patch changes the read operation to use the target class (which falls back onto the executable module as expected) and adds the missing ReadSignedIntegerFromMemory API. The fix is tested by creating a core (minidump) file which deliberately omits the vtable pointer.