Author: Jim Ingham Date: 2021-06-01T18:22:27-07:00 New Revision: 658f6ed1523b0e61ddee494ce1691f29a701c317
URL: https://github.com/llvm/llvm-project/commit/658f6ed1523b0e61ddee494ce1691f29a701c317 DIFF: https://github.com/llvm/llvm-project/commit/658f6ed1523b0e61ddee494ce1691f29a701c317.diff LOG: Make ignore counts work as "after stop" modifiers so they play nicely with conditions Previously ignore counts were checked when we stopped to do the sync callback in Breakpoint::ShouldStop. That meant we would do all the ignore count work even when there is also a condition says the breakpoint should not stop. That's wrong, lldb treats breakpoint hits that fail the thread or condition checks as "not having hit the breakpoint". So the ignore count check should happen after the condition and thread checks in StopInfoBreakpoint::PerformAction. The one side-effect of doing this is that if you have a breakpoint with a synchronous callback, it will run the synchronous callback before checking the ignore count. That is probably a good thing, since this was already true of the condition and thread checks, so this removes an odd asymmetry. And breakpoints with sync callbacks are all internal lldb breakpoints and there's not a really good reason why you would want one of these to use an ignore count (but not a condition or thread check...) Differential Revision https://reviews.llvm.org/D103217 Added: Modified: lldb/include/lldb/Breakpoint/Breakpoint.h lldb/include/lldb/Breakpoint/BreakpointLocation.h lldb/source/Breakpoint/Breakpoint.cpp lldb/source/Breakpoint/BreakpointLocation.cpp lldb/source/Target/StopInfo.cpp lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/TestBreakpointIgnoreCount.py lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/main.c Removed: ################################################################################ diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h index ddb70b719d6ed..37710bb33118f 100644 --- a/lldb/include/lldb/Breakpoint/Breakpoint.h +++ b/lldb/include/lldb/Breakpoint/Breakpoint.h @@ -617,14 +617,6 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, void DecrementIgnoreCount(); - // BreakpointLocation::IgnoreCountShouldStop & - // Breakpoint::IgnoreCountShouldStop can only be called once per stop, and - // BreakpointLocation::IgnoreCountShouldStop should be tested first, and if - // it returns false we should continue, otherwise we should test - // Breakpoint::IgnoreCountShouldStop. - - bool IgnoreCountShouldStop(); - private: // To call from CopyFromBreakpoint. Breakpoint(Target &new_target, const Breakpoint &bp_to_copy_from); diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h index a12696be91e38..ca1739161c0cc 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h @@ -297,6 +297,10 @@ class BreakpointLocation void DecrementIgnoreCount(); + /// BreakpointLocation::IgnoreCountShouldStop can only be called once + /// per stop. This method checks first against the loc and then the owner. + /// It also takes care of decrementing the ignore counters. + /// If it returns false we should continue, otherwise stop. bool IgnoreCountShouldStop(); private: diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp index a09efe79c4781..45e23f65aea62 100644 --- a/lldb/source/Breakpoint/Breakpoint.cpp +++ b/lldb/source/Breakpoint/Breakpoint.cpp @@ -326,18 +326,6 @@ uint32_t Breakpoint::GetIgnoreCount() const { return m_options_up->GetIgnoreCount(); } -bool Breakpoint::IgnoreCountShouldStop() { - uint32_t ignore = GetIgnoreCount(); - if (ignore != 0) { - // When we get here we know the location that caused the stop doesn't have - // an ignore count, since by contract we call it first... So we don't have - // to find & decrement it, we only have to decrement our own ignore count. - DecrementIgnoreCount(); - return false; - } else - return true; -} - uint32_t Breakpoint::GetHitCount() const { return m_hit_counter.GetValue(); } bool Breakpoint::IsOneShot() const { return m_options_up->IsOneShot(); } diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp index 617b89bf19643..acdffa12369cf 100644 --- a/lldb/source/Breakpoint/BreakpointLocation.cpp +++ b/lldb/source/Breakpoint/BreakpointLocation.cpp @@ -357,15 +357,16 @@ void BreakpointLocation::DecrementIgnoreCount() { } bool BreakpointLocation::IgnoreCountShouldStop() { - if (m_options_up != nullptr) { - uint32_t loc_ignore = m_options_up->GetIgnoreCount(); - if (loc_ignore != 0) { - m_owner.DecrementIgnoreCount(); - DecrementIgnoreCount(); // Have to decrement our owners' ignore count, - // since it won't get a - // chance to. - return false; - } + uint32_t owner_ignore = GetBreakpoint().GetIgnoreCount(); + uint32_t loc_ignore = 0; + if (m_options_up != nullptr) + loc_ignore = m_options_up->GetIgnoreCount(); + + if (loc_ignore != 0 || owner_ignore != 0) { + m_owner.DecrementIgnoreCount(); + DecrementIgnoreCount(); // Have to decrement our owners' ignore count, + // since it won't get a chance to. + return false; } return true; } @@ -400,12 +401,6 @@ bool BreakpointLocation::ShouldStop(StoppointCallbackContext *context) { if (!IsEnabled()) return false; - if (!IgnoreCountShouldStop()) - return false; - - if (!m_owner.IgnoreCountShouldStop()) - return false; - // We only run synchronous callbacks in ShouldStop: context->is_synchronous = true; should_stop = InvokeCallback(context); diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index ae29b42e0e50c..466a0fa9cc727 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -483,6 +483,15 @@ class StopInfoBreakpoint : public StopInfo { } } + // We've done all the checks whose failure means "we consider lldb + // not to have hit the breakpoint". Now we're going to check for + // conditions that might continue after hitting. Start with the + // ignore count: + if (!bp_loc_sp->IgnoreCountShouldStop()) { + actually_said_continue = true; + continue; + } + // Check the auto-continue bit on the location, do this before the // callback since it may change this, but that would be for the // NEXT hit. Note, you might think you could check auto-continue diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/TestBreakpointIgnoreCount.py b/lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/TestBreakpointIgnoreCount.py index c54584abab687..ba5df2b7a18c3 100644 --- a/lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/TestBreakpointIgnoreCount.py +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/TestBreakpointIgnoreCount.py @@ -27,10 +27,21 @@ def test_with_python_api(self): self.build() self.breakpoint_ignore_count_python() + @skipIfWindows # This test will hang on windows llvm.org/pr21753 + def test_ignore_vrs_condition_bkpt(self): + self.build() + self.ignore_vrs_condition(False) + + @skipIfWindows # This test will hang on windows llvm.org/pr21753 + def test_ignore_vrs_condition_loc(self): + self.build() + self.ignore_vrs_condition(True) + def setUp(self): # Call super's setUp(). TestBase.setUp(self) # Find the line number to of function 'c'. + self.stop_in_main = "Stop here at start of main" self.line1 = line_number( 'main.c', '// Find the line number of function "c" here.') self.line2 = line_number( @@ -99,8 +110,9 @@ def breakpoint_ignore_count(self): def breakpoint_ignore_count_python(self): """Use Python APIs to set breakpoint ignore count.""" - target = self.createTestTarget() - + target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(self, + self.stop_in_main, + lldb.SBFileSpec("main.c")) # Now create a breakpoint on main.c by name 'c'. breakpoint = target.BreakpointCreateByName('c', 'a.out') self.assertTrue(breakpoint and @@ -119,10 +131,8 @@ def breakpoint_ignore_count_python(self): self.assertEqual(location.GetIgnoreCount(), 2, "SetIgnoreCount() works correctly") - # Now launch the process, and do not stop at entry point. - process = target.LaunchSimple( - None, None, self.get_process_working_directory()) - self.assertTrue(process, PROCESS_IS_VALID) + # Now continue and hit our breakpoint on c: + process.Continue() # Frame#0 should be on main.c:37, frame#1 should be on main.c:25, and # frame#2 should be on main.c:48. @@ -143,4 +153,31 @@ def breakpoint_ignore_count_python(self): # The hit count for the breakpoint should be 3. self.assertEqual(breakpoint.GetHitCount(), 3) - process.Continue() + def ignore_vrs_condition(self, use_location): + main_spec = lldb.SBFileSpec("main.c") + target, process, _ , _ = lldbutil.run_to_source_breakpoint(self, + self.stop_in_main, + main_spec) + + # Now make a breakpoint on the loop, and set a condition and ignore count. + # Make sure that the condition fails don't count against the ignore count. + bkpt = target.BreakpointCreateBySourceRegex("Set a breakpoint here, with i", main_spec) + self.assertEqual(bkpt.GetNumLocations(), 1, "Wrong number of locations") + + if use_location: + loc = bkpt.location[0] + self.assertTrue(loc.IsValid(), "Got a valid location") + loc.SetIgnoreCount(2) + loc.SetCondition("i >= 3") + else: + bkpt.SetIgnoreCount(2) + bkpt.SetCondition("i >= 3") + + threads = lldbutil.continue_to_breakpoint(process, bkpt) + self.assertEqual(len(threads), 1, "Hit the breakpoint") + var = threads[0].frame[0].FindVariable("i") + self.assertTrue(var.IsValid(), "Didn't find the i variable") + val = var.GetValueAsUnsigned(10000) + self.assertNotEqual(val, 10000, "Got the fail value for i") + self.assertEqual(val, 5, "We didn't stop the right number of times") + self.assertEqual(bkpt.GetHitCount(), 3, "Hit count is not right") diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/main.c b/lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/main.c index 3f80f22fb2bba..b39b8d0442e44 100644 --- a/lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/main.c +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/main.c @@ -29,9 +29,15 @@ int c(int val) return val + 3; // Find the line number of function "c" here. } +void spin_a_bit () { + for (unsigned int i = 0; i < 10; i++) { + printf("Set a breakpoint here, with i = %d.\n", i); + } +} + int main (int argc, char const *argv[]) { - int A1 = a(1); // a(1) -> b(1) -> c(1) + int A1 = a(1); // a(1) -> b(1) -> c(1) // Stop here at start of main printf("a(1) returns %d\n", A1); int B2 = b(2); // b(2) -> c(2) Find the call site of b(2). @@ -42,5 +48,7 @@ int main (int argc, char const *argv[]) int C1 = c(5); // Find the call site of c in main. printf ("c(5) returns %d\n", C1); + + spin_a_bit(); return 0; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
