From: Jim Ingham Date: Tue, 1 Jun 2021 23:14:08 +0000 (-0700) Subject: Make ignore counts work as "after stop" modifiers so they play nicely with conditions X-Git-Tag: llvmorg-14-init~5110 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=658f6ed1523b0e61ddee494ce1691f29a701c317;p=platform%2Fupstream%2Fllvm.git 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 --- diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h index ddb70b7..37710bb 100644 --- a/lldb/include/lldb/Breakpoint/Breakpoint.h +++ b/lldb/include/lldb/Breakpoint/Breakpoint.h @@ -617,14 +617,6 @@ protected: 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 a12696b..ca17391 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h @@ -297,6 +297,10 @@ protected: 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 a09efe7..45e23f6 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 617b89b..acdffa1 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 ae29b42..466a0fa 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -483,6 +483,15 @@ protected: } } + // 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 c54584a..ba5df2b 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 @@ class BreakpointIgnoreCountTestCase(TestBase): 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 @@ class BreakpointIgnoreCountTestCase(TestBase): 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 @@ class BreakpointIgnoreCountTestCase(TestBase): 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 @@ class BreakpointIgnoreCountTestCase(TestBase): # 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 3f80f22..b39b8d0 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; }