[Lldb-commits] [lldb] 58bdef2 - [lldb][Symbol] Make sure we decrement PC before checking location list (#74772)

[via lldb-commits]

Author: Michael Buch
Date: 2023-12-08T07:32:23Z
New Revision: 58bdef2be75263a9b6bf93faf3baccc76e31e082

URL: https://github.com/llvm/llvm-project/commit/58bdef2be75263a9b6bf93faf3baccc76e31e082
DIFF: https://github.com/llvm/llvm-project/commit/58bdef2be75263a9b6bf93faf3baccc76e31e082.diff

LOG: [lldb][Symbol] Make sure we decrement PC before checking location list (#74772)

Added: 
    lldb/test/API/functionalities/location-list-lookup/main.cpp

Modified: 
    lldb/source/Symbol/Variable.cpp
    lldb/test/API/functionalities/location-list-lookup/Makefile
    lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py

Removed: 
    lldb/test/API/functionalities/location-list-lookup/main.c


################################################################################
diff  --git a/lldb/source/Symbol/Variable.cpp b/lldb/source/Symbol/Variable.cpp
index 85ceadd20c611..db740cb7cb6e4 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/Makefile b/lldb/test/API/functionalities/location-list-lookup/Makefile
index 78b0b11cb7484..8e453681d7b39 100644
--- a/lldb/test/API/functionalities/location-list-lookup/Makefile
+++ b/lldb/test/API/functionalities/location-list-lookup/Makefile
@@ -1,3 +1,3 @@
-C_SOURCES := main.c
+CXX_SOURCES := main.cpp
 CFLAGS_EXTRAS := -O1
 include Makefile.rules

diff  --git a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 4793447c59413..07f306a6ed78b 100644
--- a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++ b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -7,16 +7,11 @@
 
 
 class LocationListLookupTestCase(TestBase):
-    def setUp(self):
-        # Call super's setUp().
-        TestBase.setUp(self)
-
     @skipIf(oslist=["linux"], archs=["arm"])
     def test_loclist(self):
         self.build()
-        exe = self.getBuildArtifact("a.out")
 
-        # Create a target by the debugger.
+        exe = self.getBuildArtifact("a.out")
         target = self.dbg.CreateTarget(exe)
         self.assertTrue(target, VALID_TARGET)
         self.dbg.SetAsync(False)
@@ -27,12 +22,15 @@ def test_loclist(self):
         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]
+        # Find `bar` on the stack, then
+        # make sure we can read out the local
+        # variables (with both `frame var` and `expr`)
         for f in process.GetSelectedThread().frames:
-            if f.GetDisplayFunctionName() == "main":
+            if f.GetDisplayFunctionName().startswith("Foo::bar"):
                 argv = f.GetValueForVariablePath("argv").GetChildAtIndex(0)
                 strm = lldb.SBStream()
                 argv.GetDescription(strm)
                 self.assertNotEqual(strm.GetData().find("a.out"), -1)
+
+                process.GetSelectedThread().SetSelectedFrame(f.idx)
+                self.expect_expr("this", result_type="Foo *")

diff  --git a/lldb/test/API/functionalities/location-list-lookup/main.c b/lldb/test/API/functionalities/location-list-lookup/main.c
deleted file mode 100644
index 852772ee52ca2..0000000000000
--- a/lldb/test/API/functionalities/location-list-lookup/main.c
+++ /dev/null
@@ -1,23 +0,0 @@
-#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 -O1 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); }

diff  --git a/lldb/test/API/functionalities/location-list-lookup/main.cpp b/lldb/test/API/functionalities/location-list-lookup/main.cpp
new file mode 100644
index 0000000000000..4ccdadbddbb55
--- /dev/null
+++ b/lldb/test/API/functionalities/location-list-lookup/main.cpp
@@ -0,0 +1,23 @@
+#include <cstdio>
+#include <cstdlib>
+
+void func(int in);
+
+struct Foo {
+  int x;
+  [[clang::noinline]] void bar(char **argv);
+};
+
+int main(int argc, char **argv) {
+  Foo f{.x = 5};
+  std::printf("%p\n", &f.x);
+  f.bar(argv);
+  return f.x;
+}
+
+void Foo::bar(char **argv) {
+  std::printf("%p %p\n", argv, this);
+  std::abort(); /// 'this' should be still accessible
+}
+
+void func(int in) { printf("%d\n", in); }