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 <jing...@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 <jing...@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(); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits