[Lldb-commits] [lldb] Fix a thinko in the CallSite handling code: (PR #114896)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Nov 4 17:01:21 PST 2024
https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/114896
>From 38c7625fc7899f91190711818c144f27a39423c0 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Mon, 4 Nov 2024 15:56:26 -0800
Subject: [PATCH 1/2] Fix a thinko in the CallSite handling code:
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.
---
lldb/source/Symbol/CompileUnit.cpp | 10 ++++---
.../breakpoint/same_cu_name/Makefile | 19 ++++++++++++
.../TestFileBreakpoinsSameCUName.py | 29 +++++++++++++++++++
.../breakpoint/same_cu_name/common.cpp | 12 ++++++++
.../breakpoint/same_cu_name/main.cpp | 25 ++++++++++++++++
5 files changed, 91 insertions(+), 4 deletions(-)
create mode 100644 lldb/test/API/functionalities/breakpoint/same_cu_name/Makefile
create mode 100644 lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py
create mode 100644 lldb/test/API/functionalities/breakpoint/same_cu_name/common.cpp
create mode 100644 lldb/test/API/functionalities/breakpoint/same_cu_name/main.cpp
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index 73389b2e8479b3..d7df6ee1f221b3 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -326,16 +326,18 @@ void CompileUnit::ResolveSymbolContext(
// the function containing the PC of the line table match. That way we can
// limit the call site search to that function.
// 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 +419,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..74524685b55771
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py
@@ -0,0 +1,29 @@
+"""
+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..a33a7f1d5b5c47
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/common.cpp
@@ -0,0 +1,12 @@
+#define XSTR(x) STR(x)
+#define STR(x) #x
+
+
+namespace NAMESPACE {
+ static int g_value = 0;
+void DoSomeStuff() {
+ // A comment here
+ g_value++; // The line with code
+}
+
+} // end 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..e4313db70a4935
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/main.cpp
@@ -0,0 +1,25 @@
+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;
+}
>From 721d79d7ef2ad5ce547359b3cdc09f23de3e57f9 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Mon, 4 Nov 2024 17:00:50 -0800
Subject: [PATCH 2/2] clang-format, remove unneeded defines
---
lldb/source/Symbol/CompileUnit.cpp | 7 ++++---
.../same_cu_name/TestFileBreakpoinsSameCUName.py | 9 ++++++---
.../breakpoint/same_cu_name/common.cpp | 8 ++------
.../functionalities/breakpoint/same_cu_name/main.cpp | 11 +++++------
4 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index d7df6ee1f221b3..166f111ef62207 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -326,10 +326,11 @@ void CompileUnit::ResolveSymbolContext(
// the function containing the PC of the line table match. That way we can
// limit the call site search to that function.
// We will miss functions that ONLY exist as a call site entry.
-
+
if (line_entry.IsValid() &&
- (line_entry.line != line || (column_num != 0 && 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
diff --git a/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py b/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py
index 74524685b55771..dc10d407d72302 100644
--- a/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/TestFileBreakpoinsSameCUName.py
@@ -19,11 +19,14 @@ def test_breakpoint_same_cu(self):
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")
+ 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")
-
+ 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
index a33a7f1d5b5c47..ed9a43f27b173a 100644
--- a/lldb/test/API/functionalities/breakpoint/same_cu_name/common.cpp
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/common.cpp
@@ -1,12 +1,8 @@
-#define XSTR(x) STR(x)
-#define STR(x) #x
-
-
namespace NAMESPACE {
- static int g_value = 0;
+static int g_value = 0;
void DoSomeStuff() {
// A comment here
g_value++; // The line with code
}
-} // end NAMESPACE
+} // 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
index e4313db70a4935..43d9e3271ece2a 100644
--- a/lldb/test/API/functionalities/breakpoint/same_cu_name/main.cpp
+++ b/lldb/test/API/functionalities/breakpoint/same_cu_name/main.cpp
@@ -1,21 +1,20 @@
namespace ns1 {
- extern void DoSomeStuff();
+extern void DoSomeStuff();
}
namespace ns2 {
- extern void DoSomeStuff();
+extern void DoSomeStuff();
}
namespace ns3 {
- extern void DoSomeStuff();
+extern void DoSomeStuff();
}
namespace ns4 {
- extern void DoSomeStuff();
+extern void DoSomeStuff();
}
-
-int main(int argc, char* argv[]) {
+int main(int argc, char *argv[]) {
ns1::DoSomeStuff();
ns2::DoSomeStuff();
ns3::DoSomeStuff();
More information about the lldb-commits
mailing list