[Lldb-commits] [lldb] [lldb] Read the bool that a scripted thread plan's should_step returns (PR #215521)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 11 04:19:05 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: alexey-gusarov
<details>
<summary>Changes</summary>
Fixes part (1) of #<!-- -->215189.
`ScriptedThreadPlanPythonInterface::GetRunState()` reads the script's
`should_step` answer with `GetUnsignedIntegerValue()`. A Python `bool` arrives as
a `StructuredData::Boolean` -- `PythonObject::GetObjectType()` tests
`PythonBoolean::Check` before `PythonInteger::Check` -- and that accessor returns
its fail value for anything that is not an `Integer`. The answer was discarded on
every call and every scripted plan reported `eStateStepping`, so a plan returning
`False` single-stepped exactly like one returning `True`.
@<!-- -->jimingham on the issue: *"The ScriptedInterface changed the return type of
should_step from a boolean to an lldb::StateType. `should_anythings` should
return bools. That was probably just a little thinko when doing the
conversion."* That is exactly it, and it is what the replaced code did: 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`. This keeps `GetRunState()`'s
signature -- its callers want a `StateType` -- and reads the answer as the bool
it is.
The contract is documented in
`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 `lldb/examples/python/templates/scripted_thread_plan.py`
says the same in its own words, down to the `-> bool` annotation.
### Every return type, measured
| `should_step` returns | before | after |
|---|---|---|
| `True` | step | step |
| `False` | step | **run** |
| `0` or `lldb.eStateRunning` | **run** | step |
| `None`, or no such method | step | step |
**Row 3 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 -- i.e. the
natural workaround for this very bug. The documented contract is a bool, so it
goes, but it now leaves a log line instead of being dropped silently. If
honouring integers matters, read `GetAsBoolean()` first and fall back to
`GetUnsignedIntegerValue()`; say the word and I will.
One adjacent difference deliberately left alone: 9a9ec228cdcf also inverted the
*missing* `should_step` case -- the old bridge returned `false` (run) when the
method was absent (`if (!pfunc.IsAllocated()) return false;`, without setting
`got_error`), and today the early return gives `eStateStepping`. This PR keeps
stepping there; it is a separate behaviour question from the one the issue
reports.
### Testing
`lldb/test/API/functionalities/step_scripted/` gains three methods: a pair of
top-of-stack plans differing only in whether `should_step` returns `True` or
`False`, and one pinning the deliberate change in row 3 — a plan returning an
int now steps. Before the fix the `False` one fails with
`plancomplete (8) != breakpoint (3)` and the int one with `1 != 0`, on Linux and
on Windows alike.
Whole `check-lldb` on Linux x86_64 with this commit alone, compared per test id
against `main`: 34465 ids, **no id changed verdict**, 0 failed, 0 unresolved.
(An earlier round of this arm hit one unrelated flake once --
`python_api/sbplatform/TestSBPlatform.py`, a `platform connect` socket race; 10
re-runs on that build, a second full suite run, and the final arm are all
green.)
### Note on landing order
This commit wakes the `SetAutoContinue(true)` branch in
`Thread::SetupToStepOverBreakpointIfNeeded()` for scripted plans -- it is reached
only when the plan below wants to run, which a scripted plan could not say
before. That is the path #<!-- -->215522 (part (2) of #<!-- -->215189) fixes. Nothing in
the in-tree suite moves either way (measured, above), but the two are best landed
together.
---
Full diff: https://github.com/llvm/llvm-project/pull/215521.diff
4 Files Affected:
- (modified) lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp (+26-2)
- (modified) lldb/test/API/functionalities/step_scripted/Steps.py (+32)
- (modified) lldb/test/API/functionalities/step_scripted/TestStepScripted.py (+72)
- (modified) lldb/test/API/functionalities/step_scripted/main.c (+1-1)
``````````diff
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() {
``````````
</details>
https://github.com/llvm/llvm-project/pull/215521
More information about the lldb-commits
mailing list