Skip to content

Commit 2af0a47

Browse files
committed
[lldb] Consider all breakpoints in breakpoint detection
Currently in some cases lldb reports stop reason as "step out" or "step over" (from thread plan completion) instead of "breakpoint", if the user breakpoint happens to be set on the same address. The part of llvm@f08f5c9 seems to overwrite internal breakpoint detection logic, so that only the last breakpoint for the current stop address is considered. Together with step-out plans not clearing its breakpoint until they are destrouyed, this creates a situation when there is a user breakpoint set for address, but internal breakpoint makes lldb report a plan completion stop reason instead of breakpoint. This patch reverts that internal breakpoint detection logic to consider all breakpoints Reviewed By: jingham Differential Revision: https://reviews.llvm.org/D140368
1 parent 22cdc6a commit 2af0a47

File tree

4 files changed

+146
-5
lines changed

4 files changed

+146
-5
lines changed

lldb/source/Target/StopInfo.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ class StopInfoBreakpoint : public StopInfo {
256256
if (!m_should_perform_action)
257257
return;
258258
m_should_perform_action = false;
259-
bool internal_breakpoint = true;
259+
bool all_stopping_locs_internal = true;
260260

261261
ThreadSP thread_sp(m_thread_wp.lock());
262262

@@ -421,8 +421,6 @@ class StopInfoBreakpoint : public StopInfo {
421421
continue;
422422
}
423423

424-
internal_breakpoint = bp_loc_sp->GetBreakpoint().IsInternal();
425-
426424
// First run the precondition, but since the precondition is per
427425
// breakpoint, only run it once per breakpoint.
428426
std::pair<std::unordered_set<break_id_t>::iterator, bool> result =
@@ -509,7 +507,7 @@ class StopInfoBreakpoint : public StopInfo {
509507
loc_desc.GetData());
510508
// We want this stop reported, so you will know we auto-continued
511509
// but only for external breakpoints:
512-
if (!internal_breakpoint)
510+
if (!bp_loc_sp->GetBreakpoint().IsInternal())
513511
thread_sp->SetShouldReportStop(eVoteYes);
514512
auto_continue_says_stop = false;
515513
}
@@ -539,6 +537,9 @@ class StopInfoBreakpoint : public StopInfo {
539537
actually_said_continue = true;
540538
}
541539

540+
if (m_should_stop && !bp_loc_sp->GetBreakpoint().IsInternal())
541+
all_stopping_locs_internal = false;
542+
542543
// If we are going to stop for this breakpoint, then remove the
543544
// breakpoint.
544545
if (callback_says_stop && bp_loc_sp &&
@@ -576,7 +577,7 @@ class StopInfoBreakpoint : public StopInfo {
576577
__FUNCTION__, m_value);
577578
}
578579

579-
if ((!m_should_stop || internal_breakpoint) &&
580+
if ((!m_should_stop || all_stopping_locs_internal) &&
580581
thread_sp->CompletedPlanOverridesBreakpoint()) {
581582

582583
// Override should_stop decision when we have completed step plan
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
CXX_SOURCES := main.cpp
2+
3+
ifneq (,$(findstring icc,$(CC)))
4+
CXXFLAGS_EXTRAS := -debug inline-debug-info
5+
endif
6+
7+
8+
include Makefile.rules
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
"""
2+
Test that breakpoints (reason = breakpoint) have more priority than
3+
plan completion (reason = step in/out/over) when reporting stop reason after step,
4+
in particular 'step out' and 'step over', and in addition 'step in'.
5+
Check for correct StopReason when stepping to the line with breakpoint,
6+
which should be eStopReasonBreakpoint in general,
7+
and eStopReasonPlanComplete when breakpoint's condition fails or it is disabled.
8+
"""
9+
10+
11+
import unittest2
12+
import lldb
13+
from lldbsuite.test.decorators import *
14+
from lldbsuite.test.lldbtest import *
15+
from lldbsuite.test import lldbutil
16+
17+
class ThreadPlanUserBreakpointsTestCase(TestBase):
18+
19+
def setUp(self):
20+
TestBase.setUp(self)
21+
22+
# Build and run to starting breakpoint
23+
self.build()
24+
src = lldb.SBFileSpec('main.cpp')
25+
(self.target, self.process, self.thread, _) = \
26+
lldbutil.run_to_source_breakpoint(self, '// Start from here', src)
27+
28+
# Setup two more breakpoints
29+
self.breakpoints = [self.target.BreakpointCreateBySourceRegex('breakpoint_%i' % i, src)
30+
for i in range(2)]
31+
self.assertTrue(
32+
all(bp and bp.GetNumLocations() == 1 for bp in self.breakpoints),
33+
VALID_BREAKPOINT)
34+
35+
def check_correct_stop_reason(self, breakpoint_idx, condition):
36+
self.assertState(self.process.GetState(), lldb.eStateStopped)
37+
if condition:
38+
# All breakpoints active, stop reason is breakpoint
39+
thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[breakpoint_idx])
40+
self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint %i." % breakpoint_idx)
41+
else:
42+
# Breakpoints are inactive, stop reason is plan complete
43+
self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete,
44+
'Expected stop reason to be step into/over/out for inactive breakpoint %i line.' % breakpoint_idx)
45+
46+
def change_breakpoints(self, action):
47+
for bp in self.breakpoints:
48+
action(bp)
49+
50+
def check_thread_plan_user_breakpoint(self, condition, set_up_breakpoint_func):
51+
# Make breakpoints active/inactive in different ways
52+
self.change_breakpoints(lambda bp: set_up_breakpoint_func(condition, bp))
53+
54+
self.thread.StepInto()
55+
# We should be stopped at the breakpoint_0 line with the correct stop reason
56+
self.check_correct_stop_reason(0, condition)
57+
58+
# This step-over creates a step-out from `func_1` plan
59+
self.thread.StepOver()
60+
# We should be stopped at the breakpoint_1 line with the correct stop reason
61+
self.check_correct_stop_reason(1, condition)
62+
63+
# Check explicit step-out
64+
# Make sure we install the breakpoint at the right address:
65+
# step-out might stop on different lines, if the compiler
66+
# did or did not emit more instructions after the return
67+
return_addr = self.thread.GetFrameAtIndex(1).GetPC()
68+
step_out_breakpoint = self.target.BreakpointCreateByAddress(return_addr)
69+
self.assertTrue(step_out_breakpoint, VALID_BREAKPOINT)
70+
set_up_breakpoint_func(condition, step_out_breakpoint)
71+
self.breakpoints.append(step_out_breakpoint)
72+
self.thread.StepOut()
73+
# We should be stopped somewhere in the main frame with the correct stop reason
74+
self.check_correct_stop_reason(2, condition)
75+
76+
# Run the process until termination
77+
self.process.Continue()
78+
self.assertState(self.process.GetState(), lldb.eStateExited)
79+
80+
def set_up_breakpoints_condition(self, condition, bp):
81+
# Set breakpoint condition to true/false
82+
conditionStr = 'true' if condition else 'false'
83+
bp.SetCondition(conditionStr)
84+
85+
def set_up_breakpoints_enable(self, condition, bp):
86+
# Enable/disable breakpoint
87+
bp.SetEnabled(condition)
88+
89+
def set_up_breakpoints_callback(self, condition, bp):
90+
# Set breakpoint callback to return True/False
91+
bp.SetScriptCallbackBody('return %s' % condition)
92+
93+
def test_thread_plan_user_breakpoint_conditional_active(self):
94+
# Test with breakpoints having true condition
95+
self.check_thread_plan_user_breakpoint(condition=True,
96+
set_up_breakpoint_func=self.set_up_breakpoints_condition)
97+
98+
def test_thread_plan_user_breakpoint_conditional_inactive(self):
99+
# Test with breakpoints having false condition
100+
self.check_thread_plan_user_breakpoint(condition=False,
101+
set_up_breakpoint_func=self.set_up_breakpoints_condition)
102+
103+
def test_thread_plan_user_breakpoint_unconditional_active(self):
104+
# Test with breakpoints enabled unconditionally
105+
self.check_thread_plan_user_breakpoint(condition=True,
106+
set_up_breakpoint_func=self.set_up_breakpoints_enable)
107+
108+
def test_thread_plan_user_breakpoint_unconditional_inactive(self):
109+
# Test with breakpoints disabled unconditionally
110+
self.check_thread_plan_user_breakpoint(condition=False,
111+
set_up_breakpoint_func=self.set_up_breakpoints_enable)
112+
113+
def test_thread_plan_user_breakpoint_callback_active(self):
114+
# Test with breakpoints with callback that returns 'True'
115+
self.check_thread_plan_user_breakpoint(condition=True,
116+
set_up_breakpoint_func=self.set_up_breakpoints_callback)
117+
118+
def test_thread_plan_user_breakpoint_callback_inactive(self):
119+
# Test with breakpoints with callback that returns 'False'
120+
self.check_thread_plan_user_breakpoint(condition=False,
121+
set_up_breakpoint_func=self.set_up_breakpoints_callback)
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
int func_1() { return 1; }
2+
3+
int func_2() {
4+
func_1(); // breakpoint_0
5+
return 1 + func_1(); // breakpoint_1
6+
}
7+
8+
int main(int argc, char const *argv[]) {
9+
func_2(); // Start from here
10+
return 0;
11+
}

0 commit comments

Comments
 (0)