https://github.com/alexey-gusarov updated https://github.com/llvm/llvm-project/pull/215521
>From 9f0d6c1fa874ce179d6ed8be782d3de37670828f Mon Sep 17 00:00:00 2001 From: alexey-gusarov <[email protected]> Date: Tue, 11 Aug 2026 09:49:17 +0200 Subject: [PATCH] [lldb] Read the bool that a scripted thread plan's should_step returns ScriptedThreadPlanPythonInterface::GetRunState() asked the script for should_step and then read the answer with static_cast<lldb::StateType>(obj->GetUnsignedIntegerValue( static_cast<uint32_t>(lldb::eStateStepping))); A Python bool arrives as a StructuredData::Boolean -- PythonObject:: GetObjectType() tests PythonBoolean::Check before PythonInteger::Check -- and StructuredData::Object::GetUnsignedIntegerValue() returns its fail value for anything that is not an Integer. The script's answer was therefore discarded on every call and every scripted thread plan reported eStateStepping, so a plan whose should_step returns False single-stepped exactly like one returning True instead of running on to the next breakpoint. That contradicts the documented contract, which is a question and not a StateType: lldb/docs/use/tutorials/automating-stepping-logic.md "Return `True` if you want lldb to instruction step one instruction, or False to continue till the next breakpoint is hit." and what lldb/examples/python/templates/scripted_thread_plan.py says in its own words, down to the `-> bool` annotation on the method. and it is what this code used to do: before 9a9ec228cdcf the SWIG bridge compared the result against Py_True/Py_False and returned a bool, which ScriptedThreadPlanGetRunState() mapped with `should_step ? eStateStepping : eStateRunning`. Read the boolean, and log the answers that cannot be read instead of discarding them silently. Measured, one scripted plan per row: should_step returns before after True step step False step run 0 or lldb.eStateRunning run step None, or no such method step step The third row is a behaviour this changes rather than fixes, and it is the one thing to weigh: an integer return used to be reinterpreted as a StateType, which before this fix was the only way to make a scripted plan run at all. The documented contract is a bool, so it goes -- but it now leaves a log line instead of being silently dropped. This is part (1) of #215189. Assisted-by: Claude Code (Claude Opus 5) --- .../ScriptedThreadPlanPythonInterface.cpp | 28 +++++++- .../functionalities/step_scripted/Steps.py | 32 +++++++++ .../step_scripted/TestStepScripted.py | 72 +++++++++++++++++++ .../API/functionalities/step_scripted/main.c | 2 +- 4 files changed, 131 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp index b18823ea60960..b887ede843e5b 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp @@ -10,7 +10,9 @@ #include "lldb/Core/PluginManager.h" #include "lldb/Target/ThreadPlan.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/StreamString.h" #include "lldb/lldb-enumerations.h" #include "../SWIGPythonBridge.h" @@ -86,8 +88,30 @@ lldb::StateType ScriptedThreadPlanPythonInterface::GetRunState() { error)) return lldb::eStateStepping; - return static_cast<lldb::StateType>(obj->GetUnsignedIntegerValue( - static_cast<uint32_t>(lldb::eStateStepping))); + // `should_step` answers a question, it does not return a StateType: the + // documented contract is "Return `True` if you want lldb to instruction step + // one instruction, or False to continue till the next breakpoint is hit" + // (lldb/docs/use/tutorials/automating-stepping-logic.md). A Python bool + // arrives here as a + // StructuredData::Boolean, and GetUnsignedIntegerValue() returns its fail + // value for anything that is not an Integer -- so reading the answer as a + // StateType discarded it, and a plan returning False single-stepped exactly + // like one returning True. + if (StructuredData::Boolean *should_step = obj->GetAsBoolean()) + return should_step->GetValue() ? lldb::eStateStepping : lldb::eStateRunning; + + // Anything else is not an answer to the question that was asked. Step, as + // this function has always done when it could not read a reply -- but say so + // in the log, because an int return used to be reinterpreted as a StateType + // and is the one thing whose behaviour changes here. + if (Log *log = GetLog(LLDBLog::Script)) { + StreamString returned; + obj->Dump(returned, /*pretty_print=*/false); + LLDB_LOG(log, + "{0}: should_step returned {1}, which is not a bool; stepping.", + LLVM_PRETTY_FUNCTION, returned.GetData()); + } + return lldb::eStateStepping; } llvm::Error diff --git a/lldb/test/API/functionalities/step_scripted/Steps.py b/lldb/test/API/functionalities/step_scripted/Steps.py index b36bb5f5f9048..768733bd48865 100644 --- a/lldb/test/API/functionalities/step_scripted/Steps.py +++ b/lldb/test/API/functionalities/step_scripted/Steps.py @@ -140,3 +140,35 @@ def should_step(self): def explains_stop(self, event): return True + + +# These two are top of the plan stack -- they queue no child plan -- so the +# thread's run state comes from their should_step, and that is the only thing +# that differs between them. +class RunToNextBreakpoint: + def __init__(self, thread_plan, args_data): + self.thread_plan = thread_plan + + def explains_stop(self, event): + return False + + def should_stop(self, event): + self.thread_plan.SetPlanComplete(True) + return True + + def should_step(self): + return False + + +class StepOneInstruction(RunToNextBreakpoint): + def should_step(self): + return True + + +class ReturnsAnIntFromShouldStep(RunToNextBreakpoint): + """should_step's contract is a bool. An int is not an answer, and the plan + steps -- which it did not always do: the int used to be reinterpreted as a + StateType.""" + + def should_step(self): + return 0 diff --git a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py index a08a39a710596..126c619a4acf2 100644 --- a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py +++ b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py @@ -45,6 +45,78 @@ def test_constructor_error_preserves_traceback(self): "ValueError: scripted plan construction failed", result.GetError() ) + def test_should_step_false_runs_to_the_next_breakpoint(self): + """A scripted plan whose should_step returns False must let the process + run until the next breakpoint, not single-step one instruction.""" + self.build() + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "Set a breakpoint here", self.main_source_file + ) + second = target.BreakpointCreateBySourceRegex( + "Run to this breakpoint", self.main_source_file + ) + self.assertTrue(second.GetNumLocations() > 0, VALID_BREAKPOINT) + + err = thread.StepUsingScriptedThreadPlan("Steps.RunToNextBreakpoint") + self.assertSuccess(err) + + self.assertStopReason( + thread.GetStopReason(), + lldb.eStopReasonBreakpoint, + "should_step returned False, so the thread should have run on to " + "the next breakpoint", + ) + self.assertEqual(second.GetHitCount(), 1) + + def test_should_step_true_steps_one_instruction(self): + """The sibling of the test above: returning True from should_step still + single-steps, so the later breakpoint is not reached.""" + self.build() + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "Set a breakpoint here", self.main_source_file + ) + second = target.BreakpointCreateBySourceRegex( + "Run to this breakpoint", self.main_source_file + ) + self.assertTrue(second.GetNumLocations() > 0, VALID_BREAKPOINT) + + err = thread.StepUsingScriptedThreadPlan("Steps.StepOneInstruction") + self.assertSuccess(err) + + self.assertEqual( + second.GetHitCount(), + 0, + "should_step returned True, so the thread should not have run as " + "far as the next breakpoint", + ) + self.assertStopReason( + thread.GetStopReason(), + lldb.eStopReasonPlanComplete, + "the scripted plan should have completed after one instruction", + ) + + def test_should_step_returning_an_int_steps(self): + """should_step answers a question with a bool; anything else is not an + answer, and the plan steps rather than running free.""" + self.build() + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "Set a breakpoint here", self.main_source_file + ) + second = target.BreakpointCreateBySourceRegex( + "Run to this breakpoint", self.main_source_file + ) + self.assertTrue(second.GetNumLocations() > 0, VALID_BREAKPOINT) + + err = thread.StepUsingScriptedThreadPlan("Steps.ReturnsAnIntFromShouldStep") + self.assertSuccess(err) + + self.assertEqual(second.GetHitCount(), 0) + self.assertStopReason( + thread.GetStopReason(), + lldb.eStopReasonPlanComplete, + "the plan should have completed after one instruction", + ) + def step_out_with_scripted_plan(self, name): (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( self, "Set a breakpoint here", self.main_source_file diff --git a/lldb/test/API/functionalities/step_scripted/main.c b/lldb/test/API/functionalities/step_scripted/main.c index 9023120c44312..75497be1e377a 100644 --- a/lldb/test/API/functionalities/step_scripted/main.c +++ b/lldb/test/API/functionalities/step_scripted/main.c @@ -4,7 +4,7 @@ void foo() { int foo = 10; printf("%d\n", foo); // Set a breakpoint here. foo = 20; - printf("%d\n", foo); + printf("%d\n", foo); // Run to this breakpoint. } int main() { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
