[Lldb-commits] [lldb] [lldb] Consult Policy for expression evaluation capabilities (PR #225312)
Med Ismail Bennani via lldb-commits
lldb-commits at lists.llvm.org
Tue Sep 22 00:01:39 PDT 2026
https://github.com/medismailben created https://github.com/llvm/llvm-project/pull/225312
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
>From 0c8f2af927c17514dc7fbae38d0da3ebab1c915f Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani <ismail at bennani.ma>
Date: Mon, 21 Sep 2026 23:57:09 -0700
Subject: [PATCH] [lldb] Consult Policy for expression evaluation capabilities
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
Signed-off-by: Med Ismail Bennani <ismail at bennani.ma>
---
lldb/include/lldb/Target/Process.h | 2 +-
lldb/source/Expression/UserExpression.cpp | 9 +++++++++
lldb/source/Target/Process.cpp | 14 +++++++++++++-
lldb/source/Target/Target.cpp | 12 +-----------
lldb/source/Utility/Policy.cpp | 6 ++++++
lldb/unittests/Utility/PolicyTest.cpp | 13 +++++++++++++
6 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index c530204e770441..1f156bd28ab2be 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1292,7 +1292,7 @@ class Process : public std::enable_shared_from_this<Process>,
lldb::ExpressionResults
RunThreadPlan(ExecutionContext &exe_ctx, lldb::ThreadPlanSP &thread_plan_sp,
- const EvaluateExpressionOptions &options,
+ const EvaluateExpressionOptions &requested_options,
DiagnosticManager &diagnostic_manager);
void GetStatus(Stream &ostrm, bool is_verbose = false);
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index 537a9cc4c4e355..8b4cab94887ed7 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -38,6 +38,7 @@
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Policy.h"
#include "lldb/Utility/State.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/ValueObject/ValueObjectConstResult.h"
@@ -152,6 +153,14 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
exe_ctx.GetBestExecutionContextScope(), std::move(error));
};
+ if (!PolicyStack::Get().Current().capabilities.can_evaluate_expressions) {
+ LLDB_LOG(log, "== [UserExpression::Evaluate] The current policy doesn't "
+ "allow evaluating expressions ==");
+ set_error(Status::FromErrorString(
+ "expression evaluation is not allowed in this context"));
+ return lldb::eExpressionSetupError;
+ }
+
if (ctx_obj) {
static unsigned const ctx_type_mask = lldb::TypeFlags::eTypeIsClass |
lldb::TypeFlags::eTypeIsStructUnion |
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 1b3715917e9478..d8c05b72bcb53d 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -5184,7 +5184,7 @@ HandleStoppedEvent(lldb::tid_t thread_id, const ThreadPlanSP &thread_plan_sp,
ExpressionResults
Process::RunThreadPlan(ExecutionContext &exe_ctx,
lldb::ThreadPlanSP &thread_plan_sp,
- const EvaluateExpressionOptions &options,
+ const EvaluateExpressionOptions &requested_options,
DiagnosticManager &diagnostic_manager) {
ExpressionResults return_value = eExpressionSetupError;
@@ -5220,6 +5220,18 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx,
// to run the expression exits during the expression evaluation.
lldb::tid_t expr_thread_id = thread->GetID();
+ // The all-threads retry exists only to resume the other threads, so it is
+ // coupled to can_run_all_threads rather than checked independently.
+ EvaluateExpressionOptions options = requested_options;
+ const Policy policy = PolicyStack::Get().Current();
+ if (!policy.capabilities.can_run_all_threads) {
+ options.SetStopOthers(true);
+ options.SetTryAllThreads(false);
+ thread_plan_sp->SetStopOthers(true);
+ } else if (!policy.capabilities.can_try_all_threads) {
+ options.SetTryAllThreads(false);
+ }
+
// We need to change some of the thread plan attributes for the thread plan
// runner. This will restore them when we are done:
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 20c94d98eca567..fde9e1d056117d 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3003,19 +3003,9 @@ ExpressionResults Target::EvaluateExpression(
result_valobj_sp = persistent_var_sp->GetValueObject();
execution_results = eExpressionCompleted;
} else {
- // If this expression is being evaluated from inside a frame provider,
- // force single-thread execution. Resuming all threads while a provider
- // is mid-construction could cause unwanted process state changes.
- EvaluateExpressionOptions effective_options = options;
- if (ThreadSP thread_sp = exe_ctx.GetThreadSP()) {
- if (thread_sp->IsAnyProviderActive()) {
- effective_options.SetStopOthers(true);
- effective_options.SetTryAllThreads(false);
- }
- }
llvm::StringRef prefix = GetExpressionPrefixContents();
execution_results =
- UserExpression::Evaluate(exe_ctx, effective_options, expr, prefix,
+ UserExpression::Evaluate(exe_ctx, options, expr, prefix,
result_valobj_sp, fixed_expression, ctx_obj);
}
diff --git a/lldb/source/Utility/Policy.cpp b/lldb/source/Utility/Policy.cpp
index 04293d7a03f85c..bd17bf90bd59e1 100644
--- a/lldb/source/Utility/Policy.cpp
+++ b/lldb/source/Utility/Policy.cpp
@@ -64,9 +64,15 @@ Policy Policy::CreatePublicStateRunningExpression() {
return p;
}
+// A scripted extension invoked by the debugger must not perturb the state it
+// was asked to describe, so an expression started from one stays on its own
+// thread. This scope is not pushed for scripted commands, which the user
+// invokes directly.
Policy Policy::CreateScriptedExtensionCall() {
Policy p = PolicyStack::Get().Current();
p.capabilities.can_bypass_target_api_mutex = true;
+ p.capabilities.can_run_all_threads = false;
+ p.capabilities.can_try_all_threads = false;
return p;
}
diff --git a/lldb/unittests/Utility/PolicyTest.cpp b/lldb/unittests/Utility/PolicyTest.cpp
index 66c08f24eb5a5c..cc514b879f5fd9 100644
--- a/lldb/unittests/Utility/PolicyTest.cpp
+++ b/lldb/unittests/Utility/PolicyTest.cpp
@@ -74,11 +74,24 @@ TEST(PolicyTest, PublicStateRunningExpression) {
TEST(PolicyTest, ScriptedExtensionCall) {
Policy p = Policy::CreateScriptedExtensionCall();
EXPECT_TRUE(p.capabilities.can_bypass_target_api_mutex);
+ EXPECT_FALSE(p.capabilities.can_run_all_threads);
+ EXPECT_FALSE(p.capabilities.can_try_all_threads);
+ // An extension may still evaluate expressions and run commands; it just
+ // can't let the inferior's other threads run while doing so.
+ EXPECT_TRUE(p.capabilities.can_evaluate_expressions);
PolicyStack::Guard guard = PolicyStack::Get().PushPrivateState();
Policy nested = Policy::CreateScriptedExtensionCall();
EXPECT_EQ(nested.view, Policy::View::Private);
EXPECT_TRUE(nested.capabilities.can_bypass_target_api_mutex);
+ EXPECT_FALSE(nested.capabilities.can_run_all_threads);
+}
+
+TEST(PolicyTest, ScriptedExtensionCallWithdrawalIsInherited) {
+ PolicyStack::Guard guard = PolicyStack::Get().PushScriptedExtensionCall();
+ Policy nested = Policy::CreatePrivateState();
+ EXPECT_FALSE(nested.capabilities.can_run_all_threads);
+ EXPECT_FALSE(nested.capabilities.can_try_all_threads);
}
TEST(PolicyTest, StackDefaultIsPublicState) {
More information about the lldb-commits
mailing list