https://github.com/alexey-gusarov updated https://github.com/llvm/llvm-project/pull/215522
>From 5b30377be2b94cf40530acf7ebc4322ba9ecdae4 Mon Sep 17 00:00:00 2001 From: alexey-gusarov <[email protected]> Date: Tue, 11 Aug 2026 09:49:17 +0200 Subject: [PATCH] [lldb] Don't consult the plan stack for a stop an auto-continue will discard Thread::ShouldStop() walks the plan stack from the top, asking each plan whether to stop and popping the ones that are done. A plan that is done and wants the process to keep going sets override_stop, and after the walk if (override_stop) should_stop = false; discards whatever the plans below it answered. Asking them anyway is not free, because the asking consumes them: a plan that answers MischiefManaged() is popped there, and it tears its state down on the way out -- ThreadPlanRunToAddress::MischiefManaged() deletes the breakpoint it was running to. With its vote discarded and its breakpoint gone, nothing is left to stop the thread and the process runs to exit. The plan that reaches this is the one nothing on the stack owns: Thread::SetupToStepOverBreakpointIfNeeded(), called from ThreadList::WillResume(), interposes a ThreadPlanStepOverBreakpoint just before a resume and gives it auto-continue exactly when it was interposed for a plan that wanted to run rather than step. The stop being processed is the end of the single instruction it stepped on that plan's behalf, not an event any plan below asked to see. So pop it and resume instead of polling the plans below: they keep their state, and a BreakpointSite the thread is sitting on but has not executed is still installed, so it is hit for real on that resume -- which is also what keeps a user breakpoint at that address reported as a hit. It reproduces with no scripted thread plan involved: // thread stopped on an enabled breakpoint site thread.RunToAddress(next_instruction_address); // process runs away The new test covers that, the same plan queued one instruction off the site as a control, the same plan queued by a scripted thread plan (the shape the issue reports), that the stepped-over breakpoint is still hit on a later pass, and a user breakpoint at the address run to, which must still report a hit -- that last one passes before this change as well, and is there because it is what a narrower fix would have lost. One consequence worth stating: at the stop where this fires the plans below are no longer asked ShouldStop()/MischiefManaged(), so a scripted thread plan under the interposed one sees one fewer callback per breakpoint it resumes off. That callback was for a manufactured internal stop the plan never asked to see, and its answer was discarded anyway, so this is the intended half of the change. ThreadPlanSingleThreadTimeout is the one in-tree plan whose deadline depends on being polled: its MischiefManaged() returns true so that the poll pops it, and the next resume re-arms it with a fresh timeout. None is armed across the interposed plan's single step -- one is only created while the current plan answers SupportsResumeOthers(), and ThreadPlanStepOverBreakpoint answers false there for exactly that reason. One armed by an earlier resume and still alive below would keep its deadline instead of being re-armed at this stop: a bounded effect on when target.process.thread.single-thread-plan-timeout fires, not on where the thread stops. This is part (2) of #215189. Assisted-by: Claude Code (Claude Opus 5) --- lldb/source/Target/Thread.cpp | 34 ++++- .../step_over_breakpoint_site/Makefile | 3 + .../TestStepOverBreakpointSite.py | 142 ++++++++++++++++++ .../step_over_breakpoint_site/main.c | 10 ++ .../run_to_address_plan.py | 21 +++ 5 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/Makefile create mode 100644 lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/TestStepOverBreakpointSite.py create mode 100644 lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/main.c create mode 100644 lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/run_to_address_plan.py diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index 6aaec4686cde9..3d77a9ae99327 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -940,12 +940,44 @@ bool Thread::ShouldStop(Event *event_ptr) { if (should_stop) current_plan->WillStop(); - if (current_plan->ShouldAutoContinue(event_ptr)) { + const bool auto_continue = + current_plan->ShouldAutoContinue(event_ptr); + if (auto_continue) { override_stop = true; LLDB_LOGF(log, "Plan %s auto-continue: true.", current_plan->GetName()); } + // A plan that auto-continues has already settled that this stop will + // not be reported: override_stop discards whatever the plans below + // answer. Asking them anyway is not free, because the asking + // consumes them -- a plan that answers MischiefManaged() is popped + // here, and it tears its state down on the way out, the way + // ThreadPlanRunToAddress deletes the breakpoint it was running to. + // With its vote discarded and its breakpoint gone, nothing is left + // to stop the thread and the process runs away. + // + // The plan that does this is the one nothing on the stack owns: + // SetupToStepOverBreakpointIfNeeded() interposes a + // ThreadPlanStepOverBreakpoint just before the resume, and gives it + // auto-continue exactly when it was interposed for a plan that + // wanted to run rather than step. This stop is the end of the one + // instruction it stepped on that plan's behalf, not an event any + // plan below asked to see. So pop it and resume instead: the plans + // below keep their state, and a BreakpointSite the thread is sitting + // on but has not executed is still installed, so it is hit for real + // on that resume -- which is also what keeps a user breakpoint there + // reported as a hit. + if (auto_continue && + current_plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { + LLDB_LOGF(log, + "Plan %s auto-continues; not asking the plans below it " + "about a stop that will not be reported.", + current_plan->GetName()); + PopPlan(); + break; + } + // If a Controlling Plan wants to stop, we let it. Otherwise, see if // the plan's parent wants to stop. diff --git a/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/Makefile b/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/TestStepOverBreakpointSite.py b/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/TestStepOverBreakpointSite.py new file mode 100644 index 0000000000000..34840c276f12c --- /dev/null +++ b/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/TestStepOverBreakpointSite.py @@ -0,0 +1,142 @@ +""" +Test that a thread plan queued while the thread sits on a breakpoint site keeps +control of the process. + +Resuming from a stop on an enabled breakpoint site makes lldb push a +ThreadPlanStepOverBreakpoint to single-step off the site first. That plan +auto-continues, and the stop it produces must not consume the plans below it: +they keep their state, and the site the single step lands on is hit for real on +the resume. +""" + +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + + +class StepOverBreakpointSiteTestCase(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def test_run_to_address_from_a_breakpoint_site(self): + """The pc is on an enabled breakpoint site when the plan is queued.""" + thread, target, process = self.setup_target() + self.expect_run_to_next_instruction(target, process, thread) + + def test_run_to_address_off_a_breakpoint_site(self): + """The control: the same plan, with the pc one instruction past the + site, which needs no step-over plan at all.""" + thread, target, process = self.setup_target() + thread.StepInstruction(False) + self.assertState(process.GetState(), lldb.eStateStopped) + self.expect_run_to_next_instruction(target, process, thread) + + def test_user_breakpoint_at_the_target_is_still_reported(self): + """A user breakpoint at the address the plan runs to must still be + reported as a hit: the thread steps off the site it was stopped on and + the site it lands on is hit on the resume, not swallowed.""" + thread, target, process = self.setup_target() + next_pc = self.next_pc(target, thread) + user_bp = target.BreakpointCreateByAddress(next_pc) + self.assertTrue(user_bp.GetNumLocations() > 0, VALID_BREAKPOINT) + + thread.RunToAddress(next_pc) + + self.assertState(process.GetState(), lldb.eStateStopped) + self.assertEqual(thread.GetFrameAtIndex(0).GetPC(), next_pc) + self.assertEqual( + user_bp.GetHitCount(), + 1, + "the user breakpoint at the address run to was not reported as hit", + ) + self.assertStopReason(thread.GetStopReason(), lldb.eStopReasonBreakpoint) + + def test_run_to_address_two_instructions_from_a_breakpoint_site(self): + """A target further than one instruction away must be reached, not + merely stepped towards: the single step off the site lands short of it, + and the thread has to carry on to the address it was given.""" + thread, target, process = self.setup_target() + pc = thread.GetFrameAtIndex(0).GetPCAddress() + instructions = target.ReadInstructions(pc, 3) + self.assertEqual(instructions.GetSize(), 3) + target_pc = ( + instructions.GetInstructionAtIndex(2).GetAddress().GetLoadAddress(target) + ) + + thread.RunToAddress(target_pc) + + self.assertState(process.GetState(), lldb.eStateStopped) + self.assertEqual( + thread.GetFrameAtIndex(0).GetPC(), + target_pc, + "the thread stopped short of the address the plan was given", + ) + + def test_the_stepped_over_breakpoint_is_hit_again(self): + """The site the thread was parked on must be re-enabled after the plan + that stepped off it is popped, so a later pass hits it again.""" + thread, target, process = self.setup_target() + breakpoint = target.GetBreakpointAtIndex(0) + self.assertEqual(breakpoint.GetHitCount(), 1) + + self.expect_run_to_next_instruction(target, process, thread) + process.Continue() + + self.assertState(process.GetState(), lldb.eStateStopped) + self.assertEqual( + breakpoint.GetHitCount(), + 2, + "the breakpoint that was stepped over was not hit on the next pass", + ) + + def test_scripted_plan_queueing_the_run_to_address(self): + """The shape the issue reports: the run-to-address plan is queued by a + scripted thread plan rather than by the API directly.""" + thread, target, process = self.setup_target() + self.runCmd("command script import run_to_address_plan.py") + next_pc = self.next_pc(target, thread) + + args = lldb.SBStructuredData() + args.SetFromJSON('{"addr":%d}' % next_pc) + err = thread.StepUsingScriptedThreadPlan( + "run_to_address_plan.RunToAddress", args, True + ) + self.assertSuccess(err) + + self.assertState( + process.GetState(), + lldb.eStateStopped, + "the process ran away instead of stopping at the queued address", + ) + self.assertEqual(thread.GetFrameAtIndex(0).GetPC(), next_pc) + + def next_pc(self, target, thread): + pc = thread.GetFrameAtIndex(0).GetPCAddress() + instructions = target.ReadInstructions(pc, 2) + self.assertEqual( + instructions.GetSize(), 2, "could not read two instructions at the pc" + ) + return instructions.GetInstructionAtIndex(1).GetAddress().GetLoadAddress(target) + + def expect_run_to_next_instruction(self, target, process, thread): + next_pc = self.next_pc(target, thread) + + thread.RunToAddress(next_pc) + + self.assertState( + process.GetState(), + lldb.eStateStopped, + "the process was not stopped at the address the plan was given", + ) + self.assertEqual( + thread.GetFrameAtIndex(0).GetPC(), + next_pc, + "the thread did not stop at the address the plan was given", + ) + + def setup_target(self): + self.build() + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "Set a breakpoint here", lldb.SBFileSpec("main.c") + ) + return thread, target, process diff --git a/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/main.c b/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/main.c new file mode 100644 index 0000000000000..0390a53881454 --- /dev/null +++ b/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/main.c @@ -0,0 +1,10 @@ +static int f(int x) { + int y = x + 1; // Set a breakpoint here. + return y; +} + +int main() { + int a = f(1); + a += f(2); + return a; +} diff --git a/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/run_to_address_plan.py b/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/run_to_address_plan.py new file mode 100644 index 0000000000000..6220dd04b513b --- /dev/null +++ b/lldb/test/API/functionalities/thread_plan/step_over_breakpoint_site/run_to_address_plan.py @@ -0,0 +1,21 @@ +import lldb + + +class RunToAddress: + """The shape the issue reports: a scripted plan whose only job is to queue a + run-to-address sub-plan for an address it is handed.""" + + def __init__(self, thread_plan, args_data): + self.thread_plan = thread_plan + target = thread_plan.GetThread().GetProcess().GetTarget() + addr = args_data.GetValueForKey("addr").GetUnsignedIntegerValue() + self.sub_plan = thread_plan.QueueThreadPlanForRunToAddress( + lldb.SBAddress(addr, target), lldb.SBError() + ) + + def explains_stop(self, event): + return False + + def should_stop(self, event): + self.thread_plan.SetPlanComplete(True) + return True _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
