[Lldb-commits] [lldb] 2af0a47 - [lldb] Consider all breakpoints in breakpoint detection
Pavel Kosov via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 25 00:06:23 PST 2023
Author: Pavel Kosov
Date: 2023-01-25T11:06:07+03:00
New Revision: 2af0a478eaee5e6236e7e9fd9b1e3207228ce2ff
URL: https://github.com/llvm/llvm-project/commit/2af0a478eaee5e6236e7e9fd9b1e3207228ce2ff
DIFF: https://github.com/llvm/llvm-project/commit/2af0a478eaee5e6236e7e9fd9b1e3207228ce2ff.diff
LOG: [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 https://github.com/llvm/llvm-project/commit/f08f5c99262ff9eaa08956334accbb2614b0f7a2 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
Added:
lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/Makefile
lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
Modified:
lldb/source/Target/StopInfo.cpp
Removed:
################################################################################
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 225234c0ffbad..2c61216ee53d5 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -256,7 +256,7 @@ class StopInfoBreakpoint : public StopInfo {
if (!m_should_perform_action)
return;
m_should_perform_action = false;
- bool internal_breakpoint = true;
+ bool all_stopping_locs_internal = true;
ThreadSP thread_sp(m_thread_wp.lock());
@@ -421,8 +421,6 @@ class StopInfoBreakpoint : public StopInfo {
continue;
}
- internal_breakpoint = bp_loc_sp->GetBreakpoint().IsInternal();
-
// First run the precondition, but since the precondition is per
// breakpoint, only run it once per breakpoint.
std::pair<std::unordered_set<break_id_t>::iterator, bool> result =
@@ -509,7 +507,7 @@ class StopInfoBreakpoint : public StopInfo {
loc_desc.GetData());
// We want this stop reported, so you will know we auto-continued
// but only for external breakpoints:
- if (!internal_breakpoint)
+ if (!bp_loc_sp->GetBreakpoint().IsInternal())
thread_sp->SetShouldReportStop(eVoteYes);
auto_continue_says_stop = false;
}
@@ -539,6 +537,9 @@ class StopInfoBreakpoint : public StopInfo {
actually_said_continue = true;
}
+ if (m_should_stop && !bp_loc_sp->GetBreakpoint().IsInternal())
+ all_stopping_locs_internal = false;
+
// If we are going to stop for this breakpoint, then remove the
// breakpoint.
if (callback_says_stop && bp_loc_sp &&
@@ -576,7 +577,7 @@ class StopInfoBreakpoint : public StopInfo {
__FUNCTION__, m_value);
}
- if ((!m_should_stop || internal_breakpoint) &&
+ if ((!m_should_stop || all_stopping_locs_internal) &&
thread_sp->CompletedPlanOverridesBreakpoint()) {
// Override should_stop decision when we have completed step plan
diff --git a/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/Makefile b/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/Makefile
new file mode 100644
index 0000000000000..2c00681fa2280
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/Makefile
@@ -0,0 +1,8 @@
+CXX_SOURCES := main.cpp
+
+ifneq (,$(findstring icc,$(CC)))
+ CXXFLAGS_EXTRAS := -debug inline-debug-info
+endif
+
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py b/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
new file mode 100644
index 0000000000000..aaecdff0069f6
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
@@ -0,0 +1,121 @@
+"""
+Test that breakpoints (reason = breakpoint) have more priority than
+plan completion (reason = step in/out/over) when reporting stop reason after step,
+in particular 'step out' and 'step over', and in addition 'step in'.
+Check for correct StopReason when stepping to the line with breakpoint,
+which should be eStopReasonBreakpoint in general,
+and eStopReasonPlanComplete when breakpoint's condition fails or it is disabled.
+"""
+
+
+import unittest2
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ThreadPlanUserBreakpointsTestCase(TestBase):
+
+ def setUp(self):
+ TestBase.setUp(self)
+
+ # Build and run to starting breakpoint
+ self.build()
+ src = lldb.SBFileSpec('main.cpp')
+ (self.target, self.process, self.thread, _) = \
+ lldbutil.run_to_source_breakpoint(self, '// Start from here', src)
+
+ # Setup two more breakpoints
+ self.breakpoints = [self.target.BreakpointCreateBySourceRegex('breakpoint_%i' % i, src)
+ for i in range(2)]
+ self.assertTrue(
+ all(bp and bp.GetNumLocations() == 1 for bp in self.breakpoints),
+ VALID_BREAKPOINT)
+
+ def check_correct_stop_reason(self, breakpoint_idx, condition):
+ self.assertState(self.process.GetState(), lldb.eStateStopped)
+ if condition:
+ # All breakpoints active, stop reason is breakpoint
+ thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[breakpoint_idx])
+ self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint %i." % breakpoint_idx)
+ else:
+ # Breakpoints are inactive, stop reason is plan complete
+ self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete,
+ 'Expected stop reason to be step into/over/out for inactive breakpoint %i line.' % breakpoint_idx)
+
+ def change_breakpoints(self, action):
+ for bp in self.breakpoints:
+ action(bp)
+
+ def check_thread_plan_user_breakpoint(self, condition, set_up_breakpoint_func):
+ # Make breakpoints active/inactive in
diff erent ways
+ self.change_breakpoints(lambda bp: set_up_breakpoint_func(condition, bp))
+
+ self.thread.StepInto()
+ # We should be stopped at the breakpoint_0 line with the correct stop reason
+ self.check_correct_stop_reason(0, condition)
+
+ # This step-over creates a step-out from `func_1` plan
+ self.thread.StepOver()
+ # We should be stopped at the breakpoint_1 line with the correct stop reason
+ self.check_correct_stop_reason(1, condition)
+
+ # Check explicit step-out
+ # Make sure we install the breakpoint at the right address:
+ # step-out might stop on
diff erent lines, if the compiler
+ # did or did not emit more instructions after the return
+ return_addr = self.thread.GetFrameAtIndex(1).GetPC()
+ step_out_breakpoint = self.target.BreakpointCreateByAddress(return_addr)
+ self.assertTrue(step_out_breakpoint, VALID_BREAKPOINT)
+ set_up_breakpoint_func(condition, step_out_breakpoint)
+ self.breakpoints.append(step_out_breakpoint)
+ self.thread.StepOut()
+ # We should be stopped somewhere in the main frame with the correct stop reason
+ self.check_correct_stop_reason(2, condition)
+
+ # Run the process until termination
+ self.process.Continue()
+ self.assertState(self.process.GetState(), lldb.eStateExited)
+
+ def set_up_breakpoints_condition(self, condition, bp):
+ # Set breakpoint condition to true/false
+ conditionStr = 'true' if condition else 'false'
+ bp.SetCondition(conditionStr)
+
+ def set_up_breakpoints_enable(self, condition, bp):
+ # Enable/disable breakpoint
+ bp.SetEnabled(condition)
+
+ def set_up_breakpoints_callback(self, condition, bp):
+ # Set breakpoint callback to return True/False
+ bp.SetScriptCallbackBody('return %s' % condition)
+
+ def test_thread_plan_user_breakpoint_conditional_active(self):
+ # Test with breakpoints having true condition
+ self.check_thread_plan_user_breakpoint(condition=True,
+ set_up_breakpoint_func=self.set_up_breakpoints_condition)
+
+ def test_thread_plan_user_breakpoint_conditional_inactive(self):
+ # Test with breakpoints having false condition
+ self.check_thread_plan_user_breakpoint(condition=False,
+ set_up_breakpoint_func=self.set_up_breakpoints_condition)
+
+ def test_thread_plan_user_breakpoint_unconditional_active(self):
+ # Test with breakpoints enabled unconditionally
+ self.check_thread_plan_user_breakpoint(condition=True,
+ set_up_breakpoint_func=self.set_up_breakpoints_enable)
+
+ def test_thread_plan_user_breakpoint_unconditional_inactive(self):
+ # Test with breakpoints disabled unconditionally
+ self.check_thread_plan_user_breakpoint(condition=False,
+ set_up_breakpoint_func=self.set_up_breakpoints_enable)
+
+ def test_thread_plan_user_breakpoint_callback_active(self):
+ # Test with breakpoints with callback that returns 'True'
+ self.check_thread_plan_user_breakpoint(condition=True,
+ set_up_breakpoint_func=self.set_up_breakpoints_callback)
+
+ def test_thread_plan_user_breakpoint_callback_inactive(self):
+ # Test with breakpoints with callback that returns 'False'
+ self.check_thread_plan_user_breakpoint(condition=False,
+ set_up_breakpoint_func=self.set_up_breakpoints_callback)
diff --git a/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp b/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
new file mode 100644
index 0000000000000..2d7e06eb6da49
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
@@ -0,0 +1,11 @@
+int func_1() { return 1; }
+
+int func_2() {
+ func_1(); // breakpoint_0
+ return 1 + func_1(); // breakpoint_1
+}
+
+int main(int argc, char const *argv[]) {
+ func_2(); // Start from here
+ return 0;
+}
More information about the lldb-commits
mailing list