[Lldb-commits] [lldb] [lldb] Consult Policy for expression evaluation capabilities (PR #225312)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)

<details>
<summary>Changes</summary>

3fe311f215d0 introduced can_evaluate_expressions, can_run_all_threads and can_try_all_threads, and the adoption patch that followed wired up neither, so all three have been declared but never read outside Policy.h, Policy.cpp and their unit test.

Two of them already had an implementation. Target::EvaluateExpression forces single-threaded execution while a frame provider is active, a check added in e1cd55879b5f six weeks before the capabilities that were written to describe it. Because it sits at a caller rather than a chokepoint, it only covers expressions routed through Target and misses FunctionCaller and IRInterpreter.

Consult the capabilities where the decisions are actually made: can_evaluate_expressions in UserExpression::Evaluate, and the two thread capabilities in Process::RunThreadPlan, which now derives the effective options from the policy once rather than at each of the decision points below it. Withdrawing can_run_all_threads also disables the all-threads retry, because that retry exists only to resume the other threads, so keeping it would spend the second timeout to reach the same interruption.

Withdraw both in CreateScriptedExtensionCall rather than in a provider-specific scope. The rule is a property of being a debugger-initiated callback, not of being a frame provider: the debugger is mid-operation, and letting an expression started from there resume the inferior's other threads can change the state the callback was asked to describe. This is broader than the check it replaces, reaching synthetic children, OS plugin thread lists and scripted thread plans as well. Scripted commands stay exempt, since UserCanRunDirectly() keeps the scope off the stack for them.

Nothing withdraws can_evaluate_expressions yet, so that one is a chokepoint without a withdrawal site for now.

rdar://176223894

---
Full diff: https://github.com/llvm/llvm-project/pull/225312.diff


6 Files Affected:

- (modified) lldb/include/lldb/Target/Process.h (+1-1) 
- (modified) lldb/source/Expression/UserExpression.cpp (+9) 
- (modified) lldb/source/Target/Process.cpp (+13-1) 
- (modified) lldb/source/Target/Target.cpp (+1-11) 
- (modified) lldb/source/Utility/Policy.cpp (+6) 
- (modified) lldb/unittests/Utility/PolicyTest.cpp (+13) 


``````````diff
The server is unavailable at this time. Please wait a few minutes before you try again.
``````````

</details>


https://github.com/llvm/llvm-project/pull/225312