Skip to content

Commit 9eb7b28

Browse files
committed
Fix a bug in the breakpoint ID verifier in CommandObjectBreakpoint.
It was assuming that for any location M.N, N was always less than the number of breakpoint locations. But if you rebuild the target and rerun multiple times, when the section backing one of the locations is no longer valid, we remove the location, but we don't reuse the ID. So you can have a breakpoint that only has location 1.3. The num_locations check would say that was an invalid location.
1 parent edf0d0d commit 9eb7b28

File tree

6 files changed

+72
-2
lines changed

6 files changed

+72
-2
lines changed

lldb/source/Commands/CommandObjectBreakpoint.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2485,8 +2485,9 @@ void CommandObjectMultiwordBreakpoint::VerifyIDs(
24852485
Breakpoint *breakpoint =
24862486
target.GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
24872487
if (breakpoint != nullptr) {
2488-
const size_t num_locations = breakpoint->GetNumLocations();
2489-
if (static_cast<size_t>(cur_bp_id.GetLocationID()) > num_locations) {
2488+
lldb::break_id_t cur_loc_id = cur_bp_id.GetLocationID();
2489+
// GetLocationID returns 0 when the location isn't specified.
2490+
if (cur_loc_id != 0 && !breakpoint->FindLocationByID(cur_loc_id)) {
24902491
StreamString id_str;
24912492
BreakpointID::GetCanonicalReference(
24922493
&id_str, cur_bp_id.GetBreakpointID(), cur_bp_id.GetLocationID());
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
include Makefile.rules
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
"""
2+
When a rebuild causes a location to be removed, make sure
3+
we still handle the remaining locations correctly.
4+
"""
5+
6+
7+
import lldb
8+
import lldbsuite.test.lldbutil as lldbutil
9+
from lldbsuite.test.lldbtest import *
10+
import os
11+
12+
class TestLocationsAfterRebuild(TestBase):
13+
# If your test case doesn't stress debug info, then
14+
# set this to true. That way it won't be run once for
15+
# each debug info format.
16+
NO_DEBUG_INFO_TESTCASE = True
17+
18+
def test_remaining_location_spec(self):
19+
"""If we rebuild a couple of times some of the old locations
20+
get removed. Make sure the command-line breakpoint id
21+
validator still works correctly."""
22+
self.build(dictionary={"C_SOURCES" : "main.c", "EXE" : "a.out"})
23+
24+
path_to_exe = self.getBuildArtifact()
25+
26+
(target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint(
27+
self, "main"
28+
)
29+
30+
# Let the process continue to exit:
31+
process.Continue()
32+
self.assertEqual(process.GetState(), lldb.eStateExited, "Ran to completion")
33+
os.remove(path_to_exe)
34+
35+
# We have to rebuild twice with changed sources to get
36+
# us to remove the first set of locations:
37+
self.build(dictionary={"C_SOURCES" : "second_main.c", "EXE" : "a.out"})
38+
39+
(target, process, thread, bkpt) = lldbutil.run_to_breakpoint_do_run(self, target, bkpt)
40+
41+
# Let the process continue to exit:
42+
process.Continue()
43+
self.assertEqual(process.GetState(), lldb.eStateExited, "Ran to completion")
44+
45+
os.remove(path_to_exe)
46+
47+
self.build(dictionary={"C_SOURCES" : "third_main.c", "EXE" : "a.out"})
48+
49+
(target, process, thread, bkpt) = lldbutil.run_to_breakpoint_do_run(self, target, bkpt)
50+
51+
bkpt_id = bkpt.GetID()
52+
loc_string = f"{bkpt_id}.3"
53+
self.runCmd(f"break disable {loc_string}")
54+
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
int
2+
main() {
3+
return 1111;
4+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
int
2+
main()
3+
{
4+
return 22222;
5+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
int
2+
main()
3+
{
4+
return 33333;
5+
}

0 commit comments

Comments
 (0)