jasonmolenda created this revision.
jasonmolenda added reviewers: jingham, JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added subscribers: pengfei, kristof.beyls.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
When lldb is looking for a location list entry for a variable, it is using the
pc value -- or return-pc value when up the stack -- to pick the correct DWARF
location list entry. Because it is using return-pc, the entry selected may be
an entirely different basic block, or function. We should be using the "pc for
symbolication" -- return-pc decremented in most cases -- to select the correct
block we should be looking up. I added RegisterContext::GetPCForSymbolication
and StackFrame::GetFrameCodeAddressForSymbolication last year in
https://reviews.llvm.org/D97644 and this is a simple patch with those in place.
The bug that drove this is in the middle of a complex project, but I can elicit
incorrect behavior with a small C file that calls abort() as its last
instruction, with a different function immediately after that. It must be
built at -O1 with clang (on x86_64, AArch64) so that the debug info tracks
variables in different registers during the function lifetime, using a DWARF
location list.
I constructed a test case that does this, but it depends on clang's current
codegen at -O1 on these two architectures. If the codegen were to shift
around, the test could pass without correctly exercising this codepath. One
possibility would be to include a test binary, DWARF for it, and corefile; that
would eliminate the possibility of codegen changing. But it's a lot of data,
and I'm not sure how necessary it is. Given that I get the same codegen on two
architectures with clang at -O1, maybe this is a small enough exposure to the
optimizer that we can use it in the testsuite. Open to suggestions.
Without this patch, this test case will show argv as unavailable when you try
to print it. In the actual bug that I was tracking down, the return-pc pointed
to a different location list entry for the variable so lldb showed the wrong
value for that variable. A much more subtle and confusing bug for the user.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124597
Files:
lldb/source/Expression/DWARFExpression.cpp
lldb/test/API/functionalities/location-list-lookup/Makefile
lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
lldb/test/API/functionalities/location-list-lookup/main.c
Index: lldb/test/API/functionalities/location-list-lookup/main.c
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/location-list-lookup/main.c
@@ -0,0 +1,26 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+// The goal with this test is:
+// 1. Have main() followed by foo()
+// 2. Have the no-return call to abort() in main be the last instruction
+// 3. Have the next instruction be the start of foo()
+// 4. The debug info for argv uses a location list.
+// clang at -O0 on x86_64 or arm64 has debuginfo like
+// DW_AT_location (0x00000049:
+// [0x0000000100003f15, 0x0000000100003f25): DW_OP_reg4 RSI
+// [0x0000000100003f25, 0x0000000100003f5b): DW_OP_reg15 R15)
+
+void foo(int);
+int main (int argc, char **argv)
+{
+ char *file = argv[0];
+ char f0 = file[0];
+ printf ("%c\n", f0);
+ foo(f0);
+ printf ("%s %d\n", argv[0], argc);
+ abort(); /// argv is still be accessible here
+}
+void foo(int in) {
+ printf ("%d\n", in);
+}
Index: lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -0,0 +1,40 @@
+"""Test that lldb picks the correct DWARF location list entry with a return-pc out of bounds."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class LocationListLookupTestCase(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ def setUp(self):
+ # Call super's setUp().
+ TestBase.setUp(self)
+
+ def test_loclist(self):
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+
+ # Create a target by the debugger.
+ target = self.dbg.CreateTarget(exe)
+ self.assertTrue(target, VALID_TARGET)
+ self.dbg.SetAsync(False)
+
+ li = lldb.SBLaunchInfo(["a.out"])
+ error = lldb.SBError()
+ process = target.Launch(li, error)
+ self.assertTrue(process.IsValid())
+ self.assertTrue(process.is_stopped)
+
+ # Find `main` on the stack, then
+ # find `argv` local variable, then
+ # check that we can read the c-string in argv[0]
+ for f in process.GetSelectedThread().frames:
+ if f.GetDisplayFunctionName() == "main":
+ argv = f.GetValueForVariablePath("argv").GetChildAtIndex(0)
+ strm = lldb.SBStream()
+ argv.GetDescription(strm)
+ self.assertNotEqual(strm.GetData().find('a.out'), -1)
Index: lldb/test/API/functionalities/location-list-lookup/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/location-list-lookup/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -O1
+include Makefile.rules
Index: lldb/source/Expression/DWARFExpression.cpp
===================================================================
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -858,29 +858,28 @@
ModuleSP module_sp = m_module_wp.lock();
if (IsLocationList()) {
- addr_t pc;
+ Address pc;
StackFrame *frame = nullptr;
- if (reg_ctx)
- pc = reg_ctx->GetPC();
- else {
+ if (!reg_ctx || !reg_ctx->GetPCForSymbolication(pc)) {
frame = exe_ctx->GetFramePtr();
if (!frame)
return false;
RegisterContextSP reg_ctx_sp = frame->GetRegisterContext();
if (!reg_ctx_sp)
return false;
- pc = reg_ctx_sp->GetPC();
+ reg_ctx_sp->GetPCForSymbolication(pc);
}
if (func_load_addr != LLDB_INVALID_ADDRESS) {
- if (pc == LLDB_INVALID_ADDRESS) {
+ if (!pc.IsValid()) {
if (error_ptr)
error_ptr->SetErrorString("Invalid PC in frame.");
return false;
}
- if (llvm::Optional<DataExtractor> expr =
- GetLocationExpression(func_load_addr, pc)) {
+ Target *target = exe_ctx->GetTargetPtr();
+ if (llvm::Optional<DataExtractor> expr = GetLocationExpression(
+ func_load_addr, pc.GetLoadAddress(target))) {
return DWARFExpression::Evaluate(
exe_ctx, reg_ctx, module_sp, *expr, m_dwarf_cu, m_reg_kind,
initial_value_ptr, object_address_ptr, result, error_ptr);
@@ -2862,7 +2861,7 @@
if (load_function_start == LLDB_INVALID_ADDRESS)
return false;
- addr_t pc = frame.GetFrameCodeAddress().GetLoadAddress(
+ addr_t pc = frame.GetFrameCodeAddressForSymbolication().GetLoadAddress(
frame.CalculateTarget().get());
if (llvm::Optional<DataExtractor> expr =
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
