kpdev42 created this revision.
kpdev42 added reviewers: clayborg, davide, k8stone, DavidSpickett.
kpdev42 added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.
Currently in some cases lldb reports stop reason as "step out" or "step over"
(from thread plan completion) over "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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D140368
Files:
lldb/source/Target/StopInfo.cpp
lldb/test/API/functionalities/breakpoint/step_out_breakpoint/Makefile
lldb/test/API/functionalities/breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py
lldb/test/API/functionalities/breakpoint/step_out_breakpoint/main.cpp
Index: lldb/test/API/functionalities/breakpoint/step_out_breakpoint/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/step_out_breakpoint/main.cpp
@@ -0,0 +1,11 @@
+int func_1() { return 1; }
+
+int func_2() {
+ func_1(); // breakpoint_1
+ return 1 + func_1(); // breakpoint_2
+}
+
+int main(int argc, char const *argv[]) {
+ func_2(); // breakpoint_0
+ return 0; // breakpoint_3
+}
Index: lldb/test/API/functionalities/breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py
@@ -0,0 +1,85 @@
+"""
+Test that breakpoints do not affect stepping.
+Check for correct StopReason when stepping to the line with breakpoint
+which should be eStopReasonBreakpoint in general,
+and eStopReasonPlanComplete when breakpoint's condition fails.
+"""
+
+
+import unittest2
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class StepOverBreakpointsTestCase(TestBase):
+
+ def setUp(self):
+ TestBase.setUp(self)
+
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ src = lldb.SBFileSpec("main.cpp")
+
+ # Create a target by the debugger.
+ self.target = self.dbg.CreateTarget(exe)
+ self.assertTrue(self.target, VALID_TARGET)
+
+ # Setup four breakpoints
+ self.lines = [line_number('main.cpp', "breakpoint_%i" % i) for i in range(4)]
+
+ self.breakpoints = [self.target.BreakpointCreateByLocation(src, line) for line in self.lines]
+ self.assertTrue(
+ self.breakpoints[0] and self.breakpoints[0].GetNumLocations() == 1,
+ VALID_BREAKPOINT)
+
+ # Start debugging
+ self.process = self.target.LaunchSimple(
+ None, None, self.get_process_working_directory())
+ self.assertIsNotNone(self.process, PROCESS_IS_VALID)
+ self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[0])
+ self.assertIsNotNone(self.thread, "Didn't stop at breakpoint 0.")
+
+ 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 check_step_out_conditional(self, condition):
+ conditionStr = 'true' if condition else 'false'
+
+ self.breakpoints[1].GetLocationAtIndex(0).SetCondition(conditionStr)
+ self.breakpoints[2].GetLocationAtIndex(0).SetCondition(conditionStr)
+ self.breakpoints[3].GetLocationAtIndex(0).SetCondition(conditionStr)
+
+ self.thread.StepInto()
+ # We should be stopped at the breakpoint_1 line with the correct stop reason
+ self.check_correct_stop_reason(1, condition)
+
+ # This step-over creates a step-out from `func_1` plan
+ self.thread.StepOver()
+ # We should be stopped at the breakpoint_2 line with the correct stop reason
+ self.check_correct_stop_reason(2, condition)
+
+ # Check explicit step-out
+ self.thread.StepOut()
+ # We should be stopped at the breakpoint_3 line with the correct stop reason
+ self.check_correct_stop_reason(3, condition)
+
+ # Run the process until termination
+ self.process.Continue()
+ self.assertState(self.process.GetState(), lldb.eStateExited)
+
+ def test_step_out_active(self):
+ # Test with breakpoints 1, 2, 3 enabled
+ self.check_step_out_conditional(True)
+
+ def test_step_out_inactive(self):
+ # Test with breakpoints 1, 2, 3 having false condition
+ self.check_step_out_conditional(False)
Index: lldb/test/API/functionalities/breakpoint/step_out_breakpoint/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/step_out_breakpoint/Makefile
@@ -0,0 +1,8 @@
+CXX_SOURCES := main.cpp
+
+ifneq (,$(findstring icc,$(CC)))
+ CXXFLAGS_EXTRAS := -debug inline-debug-info
+endif
+
+
+include Makefile.rules
Index: lldb/source/Target/StopInfo.cpp
===================================================================
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -421,8 +421,6 @@
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 @@
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 @@
actually_said_continue = true;
}
+ if (m_should_stop && !bp_loc_sp->GetBreakpoint().IsInternal())
+ internal_breakpoint = false;
+
// If we are going to stop for this breakpoint, then remove the
// breakpoint.
if (callback_says_stop && bp_loc_sp &&
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
