https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/195775
>From 53beffd2becacd46b5fb31ebfa6eefe03f3c4187 Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Wed, 1 Jul 2026 00:18:51 -0700 Subject: [PATCH 1/2] [lldb] Harden PolicyStack against cross-thread Guard misuse Two related correctness/clarity improvements to the Policy infrastructure introduced by 504a11278112: PolicyStack::Guard now stores the std::thread::id of the thread that created it. Destruction and move operations call llvm::report_fatal_error when they happen on a different thread, since the PolicyStack is thread_local: popping from the wrong thread would silently corrupt that thread's stack. Push/Pop on PolicyStack are now private. Callers go through named factories (PushPrivateState, PushPublicStateRunningExpression) that return RAII Guards. The transition factories on Policy (CreatePrivateState, CreatePublicStateRunningExpression) inherit from PolicyStack::Get().Current() and apply their named change on top, so pushed policies preserve existing stack state rather than resetting unrelated fields. CreatePublicState remains the baseline reference value (returns a default Policy{}); the stack returns to public state by popping the private guards, not by pushing a "public" policy on top. rdar://176223894 Signed-off-by: Med Ismail Bennani <[email protected]> --- lldb/include/lldb/Utility/Policy.h | 79 +++++++++++++------- lldb/source/Target/Process.cpp | 12 ++-- lldb/source/Target/StopInfo.cpp | 4 +- lldb/source/Target/Thread.cpp | 2 +- lldb/source/Utility/Policy.cpp | 56 +++++++++++++++ lldb/unittests/Utility/PolicyTest.cpp | 100 +++++++++++++++++++------- 6 files changed, 194 insertions(+), 59 deletions(-) diff --git a/lldb/include/lldb/Utility/Policy.h b/lldb/include/lldb/Utility/Policy.h index e785f13ab9287..0435bdd4a0e15 100644 --- a/lldb/include/lldb/Utility/Policy.h +++ b/lldb/include/lldb/Utility/Policy.h @@ -12,6 +12,7 @@ #include "llvm/ADT/SmallVector.h" #include <cassert> +#include <thread> namespace lldb_private { @@ -28,7 +29,7 @@ class Stream; /// top of the private unwinder stack. The private state thread must see the /// raw unwinder frames, while public clients see the augmented view. Rather /// than checking thread identity at every callsite, the private state thread -/// pushes Policy::PrivateState() and the rest follows from the policy. +/// pushes Policy::CreatePrivateState() and the rest follows from the policy. struct Policy { /// What view of the process this thread sees. enum class View { @@ -54,21 +55,16 @@ struct Policy { View view = View::Public; Capabilities capabilities; - static Policy PublicState() { return {}; } - - static Policy PrivateState() { - Policy p; - p.view = View::Private; - p.capabilities.can_load_frame_providers = false; - p.capabilities.can_run_frame_recognizers = false; - return p; - } - - static Policy PublicStateRunningExpression() { - Policy p; - p.capabilities.can_run_breakpoint_actions = false; - return p; - } + /// @name Factories + /// + /// CreatePublicState is the baseline (returns default Policy{}). The + /// transition factories below start from PolicyStack::Get().Current() and + /// apply their named change on top. + /// @{ + static Policy CreatePublicState(); + static Policy CreatePrivateState(); + static Policy CreatePublicStateRunningExpression(); + /// @} void Dump(Stream &s) const; }; @@ -79,6 +75,10 @@ struct Policy { /// initialized with a default-constructed base entry that is never popped. /// RAII guards (Guard) push and pop policies. /// +/// Policies are pushed via named factory methods (PushPrivateState, etc.) +/// that return an RAII Guard. Direct Push is private to prevent callers +/// from assembling arbitrary capability combinations. +/// /// For thread pool workers that don't inherit thread_local storage, the /// policy must be passed into the lambda and pushed onto the worker /// thread's stack when the task starts. @@ -91,26 +91,53 @@ class PolicyStack { Policy Current() const; - void Push(Policy policy) { m_stack.push_back(std::move(policy)); } - - void Pop() { - assert(!m_stack.empty() && "can't pop the base policy"); - m_stack.pop_back(); - } - void Dump(Stream &s) const; - /// RAII guard that pushes a policy on construction and pops on destruction. + /// RAII guard that pops a policy on destruction. + /// + /// A Guard is bound to the thread that created it: the policy stack lives + /// in thread_local storage, so popping from a different thread would + /// corrupt that thread's stack. Guards may be moved, but only on the + /// owning thread; a cross-thread move or destruction is a fatal error. class Guard { + friend class PolicyStack; + public: - explicit Guard(Policy policy) { Get().Push(std::move(policy)); } - ~Guard() { Get().Pop(); } + ~Guard(); + Guard(Guard &&other); + Guard &operator=(Guard &&other); Guard(const Guard &) = delete; Guard &operator=(const Guard &) = delete; + + private: + Guard() : m_thread_id(std::this_thread::get_id()), m_active(true) {} + std::thread::id m_thread_id; + bool m_active = false; }; + /// All Push* methods delegate to the named static factories on Policy, + /// which already inherit from Current(). So the pushed policy preserves + /// existing stack state instead of resetting unrelated fields. + + [[nodiscard]] Guard PushPrivateState() { + Push(Policy::CreatePrivateState()); + return Guard(); + } + + [[nodiscard]] Guard PushPublicStateRunningExpression() { + Push(Policy::CreatePublicStateRunningExpression()); + return Guard(); + } + private: + void Push(Policy policy) { m_stack.push_back(std::move(policy)); } + + void Pop() { + assert(!m_stack.empty() && "can't pop the base policy"); + m_stack.pop_back(); + } + llvm::SmallVector<Policy> m_stack = {Policy{}}; }; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 1661ae5734b0a..4360250b21475 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -4372,7 +4372,7 @@ thread_result_t Process::RunPrivateStateThread(bool is_override) { // They must see parent frames, not provider-augmented frames. std::optional<PolicyStack::Guard> policy_guard; if (is_override) - policy_guard.emplace(Policy::PrivateState()); + policy_guard = PolicyStack::Get().PushPrivateState(); bool control_only = true; @@ -5427,7 +5427,7 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, // GetStackFrameList returns parent frames during event processing. std::optional<PolicyStack::Guard> policy_guard; if (backup_private_state_thread) - policy_guard.emplace(Policy::PrivateState()); + policy_guard = PolicyStack::Get().PushPrivateState(); while (true) { // We usually want to resume the process if we get to the top of the @@ -5499,10 +5499,10 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, Halt(clear_thread_plans, use_run_lock); } - diagnostic_manager.Printf( - lldb::eSeverityError, - "didn't get running event after initial resume, got %s instead.", - StateAsCString(stop_state)); + diagnostic_manager.Printf(lldb::eSeverityError, + "didn't get running event after initial " + "resume, got %s instead.", + StateAsCString(stop_state)); return_value = eExpressionSetupError; break; } diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index f438d368c9ba8..c20b0ed07ee3c 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -193,7 +193,7 @@ class StopInfoBreakpoint : public StopInfo { bool ShouldStopSynchronous(Event *event_ptr) override { // Breakpoint callbacks run on the PST during stop processing. Push // private state context so callback code sees the private reality. - PolicyStack::Guard policy_guard(Policy::PrivateState()); + PolicyStack::Guard policy_guard = PolicyStack::Get().PushPrivateState(); ThreadSP thread_sp(m_thread_wp.lock()); if (thread_sp) { @@ -903,7 +903,7 @@ class StopInfoWatchpoint : public StopInfo { bool ShouldStopSynchronous(Event *event_ptr) override { // Watchpoint callbacks run on the PST during stop processing. Push // private state context so callback code sees the private reality. - PolicyStack::Guard policy_guard(Policy::PrivateState()); + PolicyStack::Guard policy_guard = PolicyStack::Get().PushPrivateState(); // If we are running our step-over the watchpoint plan, stop if it's done // and continue if it's not: diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index 87ec77452f2cd..f25a14fbe92e8 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -1558,7 +1558,7 @@ StackFrameListSP Thread::GetStackFrameList() { return m_curr_frames_sp; // The private state thread must see the raw unwinder frames, not the - // provider-augmented public view. Policy::PrivateState is pushed by + // provider-augmented public view. Policy::CreatePrivateState is pushed by // RunThreadPlan and RunPrivateStateThread. Policy policy = PolicyStack::Get().Current(); if (policy.view == Policy::View::Private) { diff --git a/lldb/source/Utility/Policy.cpp b/lldb/source/Utility/Policy.cpp index 2abc0d6281161..104df17df7a97 100644 --- a/lldb/source/Utility/Policy.cpp +++ b/lldb/source/Utility/Policy.cpp @@ -11,6 +11,7 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/Stream.h" #include "lldb/Utility/StreamString.h" +#include "llvm/Support/ErrorHandling.h" using namespace lldb_private; @@ -24,6 +25,61 @@ Policy PolicyStack::Current() const { return p; } +// CreatePublicState is the baseline, not a transition. The stack returns to +// public state by popping the private-state guards, not by pushing a +// "public" policy on top. This factory exists only as a reference value +// (tests, dump comparisons); it never reads the current stack. +Policy Policy::CreatePublicState() { return {}; } + +Policy Policy::CreatePrivateState() { + Policy p = PolicyStack::Get().Current(); + p.view = View::Private; + p.capabilities.can_load_frame_providers = false; + p.capabilities.can_run_frame_recognizers = false; + return p; +} + +Policy Policy::CreatePublicStateRunningExpression() { + Policy p = PolicyStack::Get().Current(); + p.capabilities.can_run_breakpoint_actions = false; + return p; +} + +PolicyStack::Guard::~Guard() { + if (!m_active) + return; + if (m_thread_id != std::this_thread::get_id()) + llvm::report_fatal_error( + "PolicyStack::Guard destroyed on a different thread than the one " + "that created it"); + Get().Pop(); +} + +PolicyStack::Guard::Guard(Guard &&other) + : m_thread_id(other.m_thread_id), m_active(other.m_active) { + if (m_active && m_thread_id != std::this_thread::get_id()) + llvm::report_fatal_error("PolicyStack::Guard moved across threads"); + other.m_active = false; +} + +PolicyStack::Guard &PolicyStack::Guard::operator=(Guard &&other) { + if (this != &other) { + if (other.m_active && other.m_thread_id != std::this_thread::get_id()) + llvm::report_fatal_error("PolicyStack::Guard moved across threads"); + if (m_active) { + if (m_thread_id != std::this_thread::get_id()) + llvm::report_fatal_error( + "PolicyStack::Guard destroyed on a different thread than the " + "one that created it"); + Get().Pop(); + } + m_thread_id = other.m_thread_id; + m_active = other.m_active; + other.m_active = false; + } + return *this; +} + void Policy::Dump(Stream &s) const { s << "policy: view=" << (view == View::Public ? "public" : "private"); s << ", capabilities={"; diff --git a/lldb/unittests/Utility/PolicyTest.cpp b/lldb/unittests/Utility/PolicyTest.cpp index b7771e52b46e4..c582f16a3d7bb 100644 --- a/lldb/unittests/Utility/PolicyTest.cpp +++ b/lldb/unittests/Utility/PolicyTest.cpp @@ -26,7 +26,7 @@ TEST(PolicyTest, DefaultIsPublicWithAllCapabilities) { } TEST(PolicyTest, PublicState) { - Policy p = Policy::PublicState(); + Policy p = Policy::CreatePublicState(); EXPECT_EQ(p.view, Policy::View::Public); EXPECT_TRUE(p.capabilities.can_evaluate_expressions); EXPECT_TRUE(p.capabilities.can_run_all_threads); @@ -37,7 +37,7 @@ TEST(PolicyTest, PublicState) { } TEST(PolicyTest, PrivateState) { - Policy p = Policy::PrivateState(); + Policy p = Policy::CreatePrivateState(); EXPECT_EQ(p.view, Policy::View::Private); EXPECT_TRUE(p.capabilities.can_evaluate_expressions); EXPECT_TRUE(p.capabilities.can_run_all_threads); @@ -48,7 +48,7 @@ TEST(PolicyTest, PrivateState) { } TEST(PolicyTest, PublicStateRunningExpression) { - Policy p = Policy::PublicStateRunningExpression(); + Policy p = Policy::CreatePublicStateRunningExpression(); EXPECT_EQ(p.view, Policy::View::Public); EXPECT_TRUE(p.capabilities.can_evaluate_expressions); EXPECT_TRUE(p.capabilities.can_run_all_threads); @@ -66,20 +66,25 @@ TEST(PolicyTest, StackDefaultIsPublicState) { } TEST(PolicyTest, StackPushPop) { - PolicyStack::Get().Push(Policy::PrivateState()); - EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); - EXPECT_FALSE( - PolicyStack::Get().Current().capabilities.can_load_frame_providers); + { + PolicyStack::Guard guard = PolicyStack::Get().PushPrivateState(); + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); + EXPECT_FALSE( + PolicyStack::Get().Current().capabilities.can_load_frame_providers); - PolicyStack::Get().Push(Policy::PublicStateRunningExpression()); - EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public); - EXPECT_FALSE( - PolicyStack::Get().Current().capabilities.can_run_breakpoint_actions); + { + PolicyStack::Guard inner = + PolicyStack::Get().PushPublicStateRunningExpression(); + // PushPublicStateRunningExpression inherits from Current() and only + // toggles bp_actions; view stays Private. + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); + EXPECT_FALSE( + PolicyStack::Get().Current().capabilities.can_run_breakpoint_actions); + } - PolicyStack::Get().Pop(); - EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); + } - PolicyStack::Get().Pop(); EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public); } @@ -87,14 +92,16 @@ TEST(PolicyTest, GuardRAII) { EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public); { - PolicyStack::Guard guard(Policy::PrivateState()); + PolicyStack::Guard guard = PolicyStack::Get().PushPrivateState(); EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); EXPECT_FALSE( PolicyStack::Get().Current().capabilities.can_load_frame_providers); { - PolicyStack::Guard inner(Policy::PublicStateRunningExpression()); - EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public); + PolicyStack::Guard inner = + PolicyStack::Get().PushPublicStateRunningExpression(); + // Inherits Private view from outer guard. + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); EXPECT_FALSE( PolicyStack::Get().Current().capabilities.can_run_breakpoint_actions); } @@ -106,7 +113,7 @@ TEST(PolicyTest, GuardRAII) { } TEST(PolicyTest, StackIsPerThread) { - PolicyStack::Get().Push(Policy::PrivateState()); + PolicyStack::Guard guard = PolicyStack::Get().PushPrivateState(); Policy::View other_thread_view; std::thread t([&other_thread_view]() { @@ -116,13 +123,11 @@ TEST(PolicyTest, StackIsPerThread) { EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); EXPECT_EQ(other_thread_view, Policy::View::Public); - - PolicyStack::Get().Pop(); } TEST(PolicyTest, DumpPublicState) { StreamString s; - Policy::PublicState().Dump(s); + Policy::CreatePublicState().Dump(s); EXPECT_EQ(s.GetString(), "policy: view=public, capabilities={" "eval_expr=true run_all=true try_all=true " @@ -131,7 +136,7 @@ TEST(PolicyTest, DumpPublicState) { TEST(PolicyTest, DumpPrivateState) { StreamString s; - Policy::PrivateState().Dump(s); + Policy::CreatePrivateState().Dump(s); EXPECT_EQ(s.GetString(), "policy: view=private, capabilities={" "eval_expr=true run_all=true try_all=true " @@ -139,13 +144,60 @@ TEST(PolicyTest, DumpPrivateState) { } TEST(PolicyTest, DumpStack) { - PolicyStack::Get().Push(Policy::PrivateState()); + PolicyStack::Guard guard = PolicyStack::Get().PushPrivateState(); StreamString s; PolicyStack::Get().Dump(s); EXPECT_NE(s.GetString().find("depth=2"), std::string::npos); EXPECT_NE(s.GetString().find("[0] policy: view=public"), std::string::npos); EXPECT_NE(s.GetString().find("[1] policy: view=private"), std::string::npos); +} - PolicyStack::Get().Pop(); +TEST(PolicyTest, GuardSameThreadMove) { + // Move on the same thread is fine; the moved-into Guard still pops on + // destruction and the moved-from Guard becomes a no-op. + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public); + { + PolicyStack::Guard outer = PolicyStack::Get().PushPrivateState(); + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); + + PolicyStack::Guard moved = std::move(outer); + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); + } + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public); +} + +#if GTEST_HAS_DEATH_TEST +TEST(PolicyStackDeathTest, GuardDestroyedOnDifferentThread) { + PolicyStack::Guard outer = PolicyStack::Get().PushPrivateState(); + // The move into the closure happens here, on the constructing thread, so + // it doesn't trip the thread-affinity check. The closure (and thus the + // Guard) is destroyed on the worker thread once it returns, which is + // where the violation is detected. + EXPECT_DEATH( + { + std::thread t([guard = std::move(outer)]() mutable { (void)guard; }); + t.join(); + }, + "PolicyStack::Guard"); +} +#endif + +TEST(PolicyTest, PushInheritsFromCurrent) { + // Push* methods inherit from Current() rather than starting from a + // default Policy: stacking PushPublicStateRunningExpression on top of + // PushPrivateState must preserve the Private view. + PolicyStack::Guard outer = PolicyStack::Get().PushPrivateState(); + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); + + PolicyStack::Guard inner = + PolicyStack::Get().PushPublicStateRunningExpression(); + // Capability from inner push. + EXPECT_FALSE( + PolicyStack::Get().Current().capabilities.can_run_breakpoint_actions); + // View inherited from outer push (would be Public if Push reset state). + EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); + // Capabilities from outer push also inherited. + EXPECT_FALSE( + PolicyStack::Get().Current().capabilities.can_load_frame_providers); } >From c49557ebdfbfaeff1d96378ca75e4aae6fce272b Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Mon, 4 May 2026 18:58:54 -0700 Subject: [PATCH 2/2] [lldb] Push ExpressionEvaluation policy and remove identity check fallbacks Push PolicyStack::Get().PushPublicStateRunningExpression() at all three expression evaluation entry points (LLVMUserExpression::DoExecute, FunctionCaller::ExecuteFunction, IRInterpreter). This policy sets can_run_breakpoint_actions=false, preventing recursive breakpoint callback execution during expression eval. Push PolicyStack::Get().PushPrivateState() unconditionally for all PSTs in RunPrivateStateThread (not just overrides), giving every PST the private view while keeping frame providers and recognizers enabled for normal stop processing. Override PSTs use PushPrivateStateRunningExpression() which additionally disables providers and recognizers. With all PSTs and expression eval sites now covered by the policy, remove all host thread identity check fallbacks: - CurrentThreadPosesAsPrivateStateThread() in Process::GetState() - CurrentThreadIsPrivateStateThread() in Target::GetAPIMutex() - IsOnThread() in PrivateStateThread::GetRunLock() - CurrentThreadPosesAsPrivateStateThread() in SelectMostRelevantFrame() - IsRunningExpression() in StopInfoBreakpoint::PerformAction() SelectMostRelevantFrame now checks !can_run_frame_recognizers instead of View::Private, so recognizers run during normal PST stop processing but are skipped during expression evaluation. rdar://176223894 Signed-off-by: Med Ismail Bennani <[email protected]> --- lldb/include/lldb/Utility/Policy.h | 6 +++++ lldb/source/Expression/FunctionCaller.cpp | 4 ++++ lldb/source/Expression/IRInterpreter.cpp | 4 ++++ lldb/source/Expression/LLVMUserExpression.cpp | 4 ++++ lldb/source/Target/Process.cpp | 18 ++++++-------- lldb/source/Target/StackFrameList.cpp | 5 +--- lldb/source/Target/StopInfo.cpp | 3 +-- lldb/source/Target/Target.cpp | 3 --- lldb/source/Utility/Policy.cpp | 5 ++++ lldb/unittests/Utility/PolicyTest.cpp | 24 +++++++++++++++---- 10 files changed, 51 insertions(+), 25 deletions(-) diff --git a/lldb/include/lldb/Utility/Policy.h b/lldb/include/lldb/Utility/Policy.h index 0435bdd4a0e15..3ff73a7e81725 100644 --- a/lldb/include/lldb/Utility/Policy.h +++ b/lldb/include/lldb/Utility/Policy.h @@ -63,6 +63,7 @@ struct Policy { /// @{ static Policy CreatePublicState(); static Policy CreatePrivateState(); + static Policy CreatePrivateStateRunningExpression(); static Policy CreatePublicStateRunningExpression(); /// @} @@ -125,6 +126,11 @@ class PolicyStack { return Guard(); } + [[nodiscard]] Guard PushPrivateStateRunningExpression() { + Push(Policy::CreatePrivateStateRunningExpression()); + return Guard(); + } + [[nodiscard]] Guard PushPublicStateRunningExpression() { Push(Policy::CreatePublicStateRunningExpression()); return Guard(); diff --git a/lldb/source/Expression/FunctionCaller.cpp b/lldb/source/Expression/FunctionCaller.cpp index 6ea3faee98688..0a7dbb5b4ec6e 100644 --- a/lldb/source/Expression/FunctionCaller.cpp +++ b/lldb/source/Expression/FunctionCaller.cpp @@ -25,6 +25,7 @@ #include "lldb/Utility/ErrorMessages.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/Policy.h" #include "lldb/Utility/State.h" #include "lldb/ValueObject/ValueObject.h" #include "lldb/ValueObject/ValueObjectList.h" @@ -384,6 +385,9 @@ lldb::ExpressionResults FunctionCaller::ExecuteFunction( if (exe_ctx.GetProcessPtr()) exe_ctx.GetProcessPtr()->SetRunningUserExpression(true); + PolicyStack::Guard expr_policy_guard = + PolicyStack::Get().PushPublicStateRunningExpression(); + return_value = exe_ctx.GetProcessRef().RunThreadPlan( exe_ctx, call_plan_sp, real_options, diagnostic_manager); diff --git a/lldb/source/Expression/IRInterpreter.cpp b/lldb/source/Expression/IRInterpreter.cpp index 69e7d0b327803..163f9dac384eb 100644 --- a/lldb/source/Expression/IRInterpreter.cpp +++ b/lldb/source/Expression/IRInterpreter.cpp @@ -18,6 +18,7 @@ #include "lldb/Utility/Endian.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/Policy.h" #include "lldb/Utility/Scalar.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/StreamString.h" @@ -1581,6 +1582,9 @@ bool IRInterpreter::Interpret(llvm::Module &module, llvm::Function &function, process->SetRunningUserExpression(true); + lldb_private::PolicyStack::Guard expr_policy_guard = + lldb_private::PolicyStack::Get().PushPublicStateRunningExpression(); + // Execute the actual function call thread plan lldb::ExpressionResults res = process->RunThreadPlan(exe_ctx, call_plan_sp, options, diagnostics); diff --git a/lldb/source/Expression/LLVMUserExpression.cpp b/lldb/source/Expression/LLVMUserExpression.cpp index 2d59194027b57..d2c06cbf3ba72 100644 --- a/lldb/source/Expression/LLVMUserExpression.cpp +++ b/lldb/source/Expression/LLVMUserExpression.cpp @@ -31,6 +31,7 @@ #include "lldb/Utility/ErrorMessages.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/Policy.h" #include "lldb/Utility/StreamString.h" #include "lldb/ValueObject/ValueObjectConstResult.h" @@ -174,6 +175,9 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager, if (exe_ctx.GetProcessPtr()) exe_ctx.GetProcessPtr()->SetRunningUserExpression(true); + PolicyStack::Guard expr_policy_guard = + PolicyStack::Get().PushPublicStateRunningExpression(); + lldb::ExpressionResults execution_result = exe_ctx.GetProcessRef().RunThreadPlan(exe_ctx, call_plan_sp, options, diagnostic_manager); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 4360250b21475..9c11813041ea4 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -1287,9 +1287,6 @@ StateType Process::GetState() { if (policy.view == Policy::View::Private) return GetPrivateState(); - if (CurrentThreadPosesAsPrivateStateThread()) - return GetPrivateState(); - return GetPublicState(); } @@ -4107,8 +4104,6 @@ ProcessRunLock &Process::PrivateStateThread::GetRunLock() { Policy policy = PolicyStack::Get().Current(); if (policy.view == Policy::View::Private) return m_private_run_lock; - if (IsOnThread(Host::GetCurrentThread())) - return m_private_run_lock; return m_public_run_lock; } @@ -4368,11 +4363,12 @@ Status Process::HaltPrivate() { } thread_result_t Process::RunPrivateStateThread(bool is_override) { - // Override PSTs exist solely to service RunThreadPlan expression evaluation. - // They must see parent frames, not provider-augmented frames. - std::optional<PolicyStack::Guard> policy_guard; - if (is_override) - policy_guard = PolicyStack::Get().PushPrivateState(); + // All PSTs see the private reality (private state, private run lock). + // Override PSTs additionally skip frame providers and recognizers since + // they exist solely to service RunThreadPlan expression evaluation. + PolicyStack::Guard policy_guard = + is_override ? PolicyStack::Get().PushPrivateStateRunningExpression() + : PolicyStack::Get().PushPrivateState(); bool control_only = true; @@ -5427,7 +5423,7 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, // GetStackFrameList returns parent frames during event processing. std::optional<PolicyStack::Guard> policy_guard; if (backup_private_state_thread) - policy_guard = PolicyStack::Get().PushPrivateState(); + policy_guard = PolicyStack::Get().PushPrivateStateRunningExpression(); while (true) { // We usually want to resume the process if we get to the top of the diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index 01ce5870a6edb..60d5585777084 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -808,10 +808,7 @@ void StackFrameList::SelectMostRelevantFrame() { // they can cause code to run in the target, and that can cause deadlocks // when fetching stop events for the expression. Policy policy = PolicyStack::Get().Current(); - if (policy.view == Policy::View::Private) - return; - - if (m_thread.GetProcess()->CurrentThreadPosesAsPrivateStateThread()) + if (!policy.capabilities.can_run_frame_recognizers) return; Log *log = GetLog(LLDBLog::Thread); diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index c20b0ed07ee3c..f2cf1a75851c6 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -436,8 +436,7 @@ class StopInfoBreakpoint : public StopInfo { ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0)); Process *process = exe_ctx.GetProcessPtr(); Policy policy = PolicyStack::Get().Current(); - if (!policy.capabilities.can_run_breakpoint_actions || - process->GetModIDRef().IsRunningExpression()) { + if (!policy.capabilities.can_run_breakpoint_actions) { // If we are in the middle of evaluating an expression, don't run // asynchronous breakpoint commands or expressions. That could // lead to infinite recursion if the command or condition re-calls diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 891732bc40dff..15b93d384a5ce 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -5963,9 +5963,6 @@ std::recursive_mutex &Target::GetAPIMutex() { if (policy.view == Policy::View::Private) return m_private_mutex; - if (GetProcessSP() && GetProcessSP()->CurrentThreadIsPrivateStateThread()) - return m_private_mutex; - return m_mutex; } diff --git a/lldb/source/Utility/Policy.cpp b/lldb/source/Utility/Policy.cpp index 104df17df7a97..2284985903e6f 100644 --- a/lldb/source/Utility/Policy.cpp +++ b/lldb/source/Utility/Policy.cpp @@ -34,6 +34,11 @@ Policy Policy::CreatePublicState() { return {}; } Policy Policy::CreatePrivateState() { Policy p = PolicyStack::Get().Current(); p.view = View::Private; + return p; +} + +Policy Policy::CreatePrivateStateRunningExpression() { + Policy p = CreatePrivateState(); p.capabilities.can_load_frame_providers = false; p.capabilities.can_run_frame_recognizers = false; return p; diff --git a/lldb/unittests/Utility/PolicyTest.cpp b/lldb/unittests/Utility/PolicyTest.cpp index c582f16a3d7bb..bef3920e03e24 100644 --- a/lldb/unittests/Utility/PolicyTest.cpp +++ b/lldb/unittests/Utility/PolicyTest.cpp @@ -43,6 +43,17 @@ TEST(PolicyTest, PrivateState) { EXPECT_TRUE(p.capabilities.can_run_all_threads); EXPECT_TRUE(p.capabilities.can_try_all_threads); EXPECT_TRUE(p.capabilities.can_run_breakpoint_actions); + EXPECT_TRUE(p.capabilities.can_load_frame_providers); + EXPECT_TRUE(p.capabilities.can_run_frame_recognizers); +} + +TEST(PolicyTest, PrivateStateRunningExpression) { + Policy p = Policy::CreatePrivateStateRunningExpression(); + EXPECT_EQ(p.view, Policy::View::Private); + EXPECT_TRUE(p.capabilities.can_evaluate_expressions); + EXPECT_TRUE(p.capabilities.can_run_all_threads); + EXPECT_TRUE(p.capabilities.can_try_all_threads); + EXPECT_TRUE(p.capabilities.can_run_breakpoint_actions); EXPECT_FALSE(p.capabilities.can_load_frame_providers); EXPECT_FALSE(p.capabilities.can_run_frame_recognizers); } @@ -67,7 +78,8 @@ TEST(PolicyTest, StackDefaultIsPublicState) { TEST(PolicyTest, StackPushPop) { { - PolicyStack::Guard guard = PolicyStack::Get().PushPrivateState(); + PolicyStack::Guard guard = + PolicyStack::Get().PushPrivateStateRunningExpression(); EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); EXPECT_FALSE( PolicyStack::Get().Current().capabilities.can_load_frame_providers); @@ -92,7 +104,8 @@ TEST(PolicyTest, GuardRAII) { EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public); { - PolicyStack::Guard guard = PolicyStack::Get().PushPrivateState(); + PolicyStack::Guard guard = + PolicyStack::Get().PushPrivateStateRunningExpression(); EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); EXPECT_FALSE( PolicyStack::Get().Current().capabilities.can_load_frame_providers); @@ -140,7 +153,7 @@ TEST(PolicyTest, DumpPrivateState) { EXPECT_EQ(s.GetString(), "policy: view=private, capabilities={" "eval_expr=true run_all=true try_all=true " - "bp_actions=true frame_providers=false frame_recognizers=false}"); + "bp_actions=true frame_providers=true frame_recognizers=true}"); } TEST(PolicyTest, DumpStack) { @@ -186,8 +199,9 @@ TEST(PolicyStackDeathTest, GuardDestroyedOnDifferentThread) { TEST(PolicyTest, PushInheritsFromCurrent) { // Push* methods inherit from Current() rather than starting from a // default Policy: stacking PushPublicStateRunningExpression on top of - // PushPrivateState must preserve the Private view. - PolicyStack::Guard outer = PolicyStack::Get().PushPrivateState(); + // PushPrivateStateRunningExpression must preserve the Private view. + PolicyStack::Guard outer = + PolicyStack::Get().PushPrivateStateRunningExpression(); EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private); PolicyStack::Guard inner = _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
