[Lldb-commits] [lldb] 803f957 - Fix a thinko in the CallSite handling code: (#114896)

[via lldb-commits]

Author: jimingham
Date: 2024-11-05T11:23:23-08:00
New Revision: 803f957e87e4083f6d61c8991171eeeaf0e6bd61

URL: https://github.com/llvm/llvm-project/commit/803f957e87e4083f6d61c8991171eeeaf0e6bd61
DIFF: https://github.com/llvm/llvm-project/commit/803f957e87e4083f6d61c8991171eeeaf0e6bd61.diff

LOG: Fix a thinko in the CallSite handling code: (#114896)

I have to check for the sc list size being changed by the call-site
search, not just that it had more than one element.

Added a test for multiple CU's with the same name in a given module,
which would have caught this mistake.

We were also doing all the work to find call sites when the found decl
and specified decl's only difference was a column, but the incoming
specification hadn't specified a column (column number == 0).

Added: 
    lldb/test/API/functionalities/breakpoint/same_cu_name/Makefile
    lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py
    lldb/test/API/functionalities/breakpoint/same_cu_name/common.cpp
    lldb/test/API/functionalities/breakpoint/same_cu_name/main.cpp

Modified: 
    lldb/source/Symbol/CompileUnit.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index 73389b2e8479b3..166f111ef62207 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -328,14 +328,17 @@ void CompileUnit::ResolveSymbolContext(
   // We will miss functions that ONLY exist as a call site entry.
 
   if (line_entry.IsValid() &&
-      (line_entry.line != line || line_entry.column != column_num) &&
-      resolve_scope & eSymbolContextLineEntry && check_inlines) {
+      (line_entry.line != line ||
+       (column_num != 0 && line_entry.column != column_num)) &&
+      (resolve_scope & eSymbolContextLineEntry) && check_inlines) {
     // We don't move lines over function boundaries, so the address in the
     // line entry will be the in function that contained the line that might
     // be a CallSite, and we can just iterate over that function to find any
     // inline records, and dig up their call sites.
     Address start_addr = line_entry.range.GetBaseAddress();
     Function *function = start_addr.CalculateSymbolContextFunction();
+    // Record the size of the list to see if we added to it:
+    size_t old_sc_list_size = sc_list.GetSize();
 
     Declaration sought_decl(file_spec, line, column_num);
     // We use this recursive function to descend the block structure looking
@@ -417,7 +420,7 @@ void CompileUnit::ResolveSymbolContext(
     // FIXME: Should I also do this for "call site line exists between the
     // given line number and the later line we found in the line table"?  That's
     // a closer approximation to our general sliding algorithm.
-    if (sc_list.GetSize())
+    if (sc_list.GetSize() > old_sc_list_size)
       return;
   }
 

diff  --git a/lldb/test/API/functionalities/breakpoint/same_cu_name/Makefile b/lldb/test/API/functionalities/breakpoint/same_cu_name/Makefile
new file mode 100644
index 00000000000000..4bfdb15e777d99
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/Makefile
@@ -0,0 +1,19 @@
+CXX_SOURCES := main.cpp
+LD_EXTRAS := ns1.o ns2.o ns3.o ns4.o
+
+a.out: main.o ns1.o ns2.o ns3.o ns4.o
+
+ns1.o: common.cpp
+	$(CC) -g -c -DNAMESPACE=ns1 -o $@ $<
+
+ns2.o: common.cpp
+	$(CC) -g -c -DNAMESPACE=ns2 -o $@ $<
+
+ns3.o: common.cpp
+	$(CC) -g -c -DNAMESPACE=ns3 -o $@ $<
+
+ns4.o: common.cpp
+	$(CC) -g -c -DNAMESPACE=ns4 -o $@ $<
+
+
+include Makefile.rules

diff  --git a/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py b/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py
new file mode 100644
index 00000000000000..dc10d407d72302
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py
@@ -0,0 +1,32 @@
+"""
+Test setting a breakpoint by file and line when many instances of the
+same file name exist in the CU list.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestBreakpointSameCU(TestBase):
+    def test_breakpoint_same_cu(self):
+        self.build()
+        target = self.createTestTarget()
+
+        # Break both on the line before the code:
+        comment_line = line_number("common.cpp", "// A comment here")
+        self.assertNotEqual(comment_line, 0, "line_number worked")
+        bkpt = target.BreakpointCreateByLocation("common.cpp", comment_line)
+        self.assertEqual(
+            bkpt.GetNumLocations(), 4, "Got the right number of breakpoints"
+        )
+
+        # And break on the code, both should work:
+        code_line = line_number("common.cpp", "// The line with code")
+        self.assertNotEqual(comment_line, 0, "line_number worked again")
+        bkpt = target.BreakpointCreateByLocation("common.cpp", code_line)
+        self.assertEqual(
+            bkpt.GetNumLocations(), 4, "Got the right number of breakpoints"
+        )

diff  --git a/lldb/test/API/functionalities/breakpoint/same_cu_name/common.cpp b/lldb/test/API/functionalities/breakpoint/same_cu_name/common.cpp
new file mode 100644
index 00000000000000..ed9a43f27b173a
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/common.cpp
@@ -0,0 +1,8 @@
+namespace NAMESPACE {
+static int g_value = 0;
+void DoSomeStuff() {
+  // A comment here
+  g_value++; // The line with code
+}
+
+} // namespace NAMESPACE

diff  --git a/lldb/test/API/functionalities/breakpoint/same_cu_name/main.cpp b/lldb/test/API/functionalities/breakpoint/same_cu_name/main.cpp
new file mode 100644
index 00000000000000..43d9e3271ece2a
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/main.cpp
@@ -0,0 +1,24 @@
+namespace ns1 {
+extern void DoSomeStuff();
+}
+
+namespace ns2 {
+extern void DoSomeStuff();
+}
+
+namespace ns3 {
+extern void DoSomeStuff();
+}
+
+namespace ns4 {
+extern void DoSomeStuff();
+}
+
+int main(int argc, char *argv[]) {
+  ns1::DoSomeStuff();
+  ns2::DoSomeStuff();
+  ns3::DoSomeStuff();
+  ns4::DoSomeStuff();
+
+  return 0;
+}