Skip to content

Commit 803f957

Browse files
authored
Fix a thinko in the CallSite handling code: (llvm#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).
1 parent c695a32 commit 803f957

File tree

5 files changed

+89
-3
lines changed

5 files changed

+89
-3
lines changed

lldb/source/Symbol/CompileUnit.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,14 +328,17 @@ void CompileUnit::ResolveSymbolContext(
328328
// We will miss functions that ONLY exist as a call site entry.
329329

330330
if (line_entry.IsValid() &&
331-
(line_entry.line != line || line_entry.column != column_num) &&
332-
resolve_scope & eSymbolContextLineEntry && check_inlines) {
331+
(line_entry.line != line ||
332+
(column_num != 0 && line_entry.column != column_num)) &&
333+
(resolve_scope & eSymbolContextLineEntry) && check_inlines) {
333334
// We don't move lines over function boundaries, so the address in the
334335
// line entry will be the in function that contained the line that might
335336
// be a CallSite, and we can just iterate over that function to find any
336337
// inline records, and dig up their call sites.
337338
Address start_addr = line_entry.range.GetBaseAddress();
338339
Function *function = start_addr.CalculateSymbolContextFunction();
340+
// Record the size of the list to see if we added to it:
341+
size_t old_sc_list_size = sc_list.GetSize();
339342

340343
Declaration sought_decl(file_spec, line, column_num);
341344
// We use this recursive function to descend the block structure looking
@@ -417,7 +420,7 @@ void CompileUnit::ResolveSymbolContext(
417420
// FIXME: Should I also do this for "call site line exists between the
418421
// given line number and the later line we found in the line table"? That's
419422
// a closer approximation to our general sliding algorithm.
420-
if (sc_list.GetSize())
423+
if (sc_list.GetSize() > old_sc_list_size)
421424
return;
422425
}
423426

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
CXX_SOURCES := main.cpp
2+
LD_EXTRAS := ns1.o ns2.o ns3.o ns4.o
3+
4+
a.out: main.o ns1.o ns2.o ns3.o ns4.o
5+
6+
ns1.o: common.cpp
7+
$(CC) -g -c -DNAMESPACE=ns1 -o $@ $<
8+
9+
ns2.o: common.cpp
10+
$(CC) -g -c -DNAMESPACE=ns2 -o $@ $<
11+
12+
ns3.o: common.cpp
13+
$(CC) -g -c -DNAMESPACE=ns3 -o $@ $<
14+
15+
ns4.o: common.cpp
16+
$(CC) -g -c -DNAMESPACE=ns4 -o $@ $<
17+
18+
19+
include Makefile.rules
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
"""
2+
Test setting a breakpoint by file and line when many instances of the
3+
same file name exist in the CU list.
4+
"""
5+
6+
7+
import lldb
8+
from lldbsuite.test.decorators import *
9+
from lldbsuite.test.lldbtest import *
10+
from lldbsuite.test import lldbutil
11+
12+
13+
class TestBreakpointSameCU(TestBase):
14+
def test_breakpoint_same_cu(self):
15+
self.build()
16+
target = self.createTestTarget()
17+
18+
# Break both on the line before the code:
19+
comment_line = line_number("common.cpp", "// A comment here")
20+
self.assertNotEqual(comment_line, 0, "line_number worked")
21+
bkpt = target.BreakpointCreateByLocation("common.cpp", comment_line)
22+
self.assertEqual(
23+
bkpt.GetNumLocations(), 4, "Got the right number of breakpoints"
24+
)
25+
26+
# And break on the code, both should work:
27+
code_line = line_number("common.cpp", "// The line with code")
28+
self.assertNotEqual(comment_line, 0, "line_number worked again")
29+
bkpt = target.BreakpointCreateByLocation("common.cpp", code_line)
30+
self.assertEqual(
31+
bkpt.GetNumLocations(), 4, "Got the right number of breakpoints"
32+
)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace NAMESPACE {
2+
static int g_value = 0;
3+
void DoSomeStuff() {
4+
// A comment here
5+
g_value++; // The line with code
6+
}
7+
8+
} // namespace NAMESPACE
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
namespace ns1 {
2+
extern void DoSomeStuff();
3+
}
4+
5+
namespace ns2 {
6+
extern void DoSomeStuff();
7+
}
8+
9+
namespace ns3 {
10+
extern void DoSomeStuff();
11+
}
12+
13+
namespace ns4 {
14+
extern void DoSomeStuff();
15+
}
16+
17+
int main(int argc, char *argv[]) {
18+
ns1::DoSomeStuff();
19+
ns2::DoSomeStuff();
20+
ns3::DoSomeStuff();
21+
ns4::DoSomeStuff();
22+
23+
return 0;
24+
}

0 commit comments

Comments
 (0)