llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) <details> <summary>Changes</summary> In optimized code we can end up with return address being the next instruction in a different block. If we are dealing with location lists, we want to decrement the PC value so it's within the calling block range that we're checking against the loclists. This was fixed in https://reviews.llvm.org/D124597 (`6e56c4961a106b8fde69ffcf9f803fe0890722fa`), by introducing a `GetFrameCodeAddressForSymbolication`. This fixed `frame variable`, but `expr` calls into `Variable::LocationIsValidForFrame`, where this new API wasn't used yet. So in the associated test-case, running `frame var this` works, but `expr this` doesn't. With `dwim-print` this makes the situation more surprising for the user because `p this`, `v this` and `v *this` work, but `p *this` doesn't because it falls back onto `expr` (due to the dereference). This patch makes sure we lookup loclists using the correct PC by using this new `GetFrameCodeAddressForSymbolication` API. --- Full diff: https://github.com/llvm/llvm-project/pull/74772.diff 4 Files Affected: - (modified) lldb/source/Symbol/Variable.cpp (+2-1) - (added) lldb/test/API/functionalities/location-list-lookup-cpp-member/Makefile (+3) - (added) lldb/test/API/functionalities/location-list-lookup-cpp-member/TestCppMemberLocationListLookup.py (+29) - (added) lldb/test/API/functionalities/location-list-lookup-cpp-member/main.cpp (+23) ``````````diff diff --git a/lldb/source/Symbol/Variable.cpp b/lldb/source/Symbol/Variable.cpp index 85ceadd20c611e..db740cb7cb6e41 100644 --- a/lldb/source/Symbol/Variable.cpp +++ b/lldb/source/Symbol/Variable.cpp @@ -227,7 +227,8 @@ bool Variable::LocationIsValidForFrame(StackFrame *frame) { // contains the current address when converted to a load address return m_location_list.ContainsAddress( loclist_base_load_addr, - frame->GetFrameCodeAddress().GetLoadAddress(target_sp.get())); + frame->GetFrameCodeAddressForSymbolication().GetLoadAddress( + target_sp.get())); } } return false; diff --git a/lldb/test/API/functionalities/location-list-lookup-cpp-member/Makefile b/lldb/test/API/functionalities/location-list-lookup-cpp-member/Makefile new file mode 100644 index 00000000000000..8e453681d7b390 --- /dev/null +++ b/lldb/test/API/functionalities/location-list-lookup-cpp-member/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp +CFLAGS_EXTRAS := -O1 +include Makefile.rules diff --git a/lldb/test/API/functionalities/location-list-lookup-cpp-member/TestCppMemberLocationListLookup.py b/lldb/test/API/functionalities/location-list-lookup-cpp-member/TestCppMemberLocationListLookup.py new file mode 100644 index 00000000000000..846386ceffc6ac --- /dev/null +++ b/lldb/test/API/functionalities/location-list-lookup-cpp-member/TestCppMemberLocationListLookup.py @@ -0,0 +1,29 @@ +"""Test that lldb picks the correct DWARF location list entry with a return-pc out of bounds.""" + +import lldb +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class CppMemberLocationListLookupTestCase(TestBase): + def test(self): + self.build() + + exe = self.getBuildArtifact("a.out") + 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 `bar` on the stack, then + # find `this` local variable, then + # check that we can read out the pointer value + for f in process.GetSelectedThread().frames: + if f.GetDisplayFunctionName().startswith("Foo::bar"): + process.GetSelectedThread().SetSelectedFrame(f.idx) + self.expect_expr("this", result_type="Foo *") diff --git a/lldb/test/API/functionalities/location-list-lookup-cpp-member/main.cpp b/lldb/test/API/functionalities/location-list-lookup-cpp-member/main.cpp new file mode 100644 index 00000000000000..96817f141b82cb --- /dev/null +++ b/lldb/test/API/functionalities/location-list-lookup-cpp-member/main.cpp @@ -0,0 +1,23 @@ +#include <cstdio> +#include <cstdlib> + +void func(int in); + +struct Foo { + int x; + [[clang::noinline]] void bar(Foo *f); +}; + +int main(int argc, char **argv) { + Foo f{.x = 5}; + std::printf("%p\n", &f.x); + f.bar(&f); + return f.x; +} + +void Foo::bar(Foo *f) { + std::printf("%p %p\n", f, this); + std::abort(); /// 'this' should be still accessible +} + +void func(int in) { printf("%d\n", in); } `````````` </details> https://github.com/llvm/llvm-project/pull/74772 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
