[Lldb-commits] [lldb] [lldb] Adopt Policy for private/public view and capability decisions (PR #195771)
Med Ismail Bennani via lldb-commits
lldb-commits at lists.llvm.org
Wed May 6 16:51:05 PDT 2026
https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/195771
>From 83e26275ba34ab4f3dcf034faa3b693746b12950 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani <ismail at bennani.ma>
Date: Wed, 6 May 2026 16:50:36 -0700
Subject: [PATCH] [lldb] Adopt Policy for private/public view and capability
decisions
Push Policy::PrivateState() at all sites where the private state
thread needs to see the private reality instead of the public illusion:
- StopInfoBreakpoint::ShouldStopSynchronous (sync callbacks, WasHit)
- StopInfoBreakpoint::PerformAction (async callbacks, conditions)
- StopInfoWatchpoint::ShouldStopSynchronous
- StopInfoWatchpoint::PerformAction
- RunThreadPlan (original PST during expression evaluation)
- RunPrivateStateThread (override PSTs)
Consult the policy at all view decision points:
- Process::GetState() -- private vs public state
- Target::GetAPIMutex() -- private vs public mutex
- PrivateStateThread::GetRunLock() -- private vs public run lock
- Thread::GetStackFrameList() -- parent vs provider-augmented frames
- StackFrameList::SelectMostRelevantFrame() -- skip frame recognizers
- StopInfoBreakpoint::PerformAction() -- skip breakpoint actions
The existing host thread identity checks remain as fallbacks until
Policy is pushed for all PSTs unconditionally.
rdar://176223894
Signed-off-by: Med Ismail Bennani <ismail at bennani.ma>
---
lldb/include/lldb/Target/Policy.h | 10 ++--
lldb/include/lldb/Target/Process.h | 29 ++---------
lldb/source/Target/Policy.cpp | 21 ++++++--
lldb/source/Target/Process.cpp | 22 +++++----
lldb/source/Target/StackFrameList.cpp | 5 ++
lldb/source/Target/StopInfo.cpp | 24 +++++++++-
lldb/source/Target/Target.cpp | 9 +++-
lldb/source/Target/Thread.cpp | 15 +++---
lldb/unittests/Target/PolicyTest.cpp | 69 +++++++++++++--------------
9 files changed, 115 insertions(+), 89 deletions(-)
diff --git a/lldb/include/lldb/Target/Policy.h b/lldb/include/lldb/Target/Policy.h
index 4f3d6f4bee7ff..b398b6bfd650a 100644
--- a/lldb/include/lldb/Target/Policy.h
+++ b/lldb/include/lldb/Target/Policy.h
@@ -84,17 +84,17 @@ struct Policy {
/// thread's stack when the task starts.
class PolicyStack {
public:
- static PolicyStack &GetForCurrentThread() {
+ static PolicyStack &Get() {
static thread_local PolicyStack s_stack;
return s_stack;
}
- Policy Current() const { return m_stack.back(); }
+ Policy Current() const;
void Push(Policy policy) { m_stack.push_back(std::move(policy)); }
void Pop() {
- assert(!m_stack.empty() && "can't pop the last policy");
+ assert(!m_stack.empty() && "can't pop the base policy");
m_stack.pop_back();
}
@@ -103,8 +103,8 @@ class PolicyStack {
/// RAII guard that pushes a policy on construction and pops on destruction.
class Guard {
public:
- explicit Guard(Policy policy) { GetForCurrentThread().Push(policy); }
- ~Guard() { GetForCurrentThread().Pop(); }
+ explicit Guard(Policy policy) { Get().Push(std::move(policy)); }
+ ~Guard() { Get().Pop(); }
Guard(const Guard &) = delete;
Guard &operator=(const Guard &) = delete;
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 67a9a883cd36c..27d374ee5dc63 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -41,6 +41,7 @@
#include "lldb/Target/InstrumentationRuntime.h"
#include "lldb/Target/Memory.h"
#include "lldb/Target/MemoryTagManager.h"
+#include "lldb/Target/Policy.h"
#include "lldb/Target/QueueList.h"
#include "lldb/Target/ThreadList.h"
#include "lldb/Target/ThreadPlanStack.h"
@@ -3334,10 +3335,12 @@ void PruneThreadPlans();
}
ProcessRunLock &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;
- else
- return m_public_run_lock;
+ return m_public_run_lock;
}
Process &m_process;
@@ -3719,28 +3722,6 @@ class UtilityFunctionScope {
}
};
-/// RAII guard that marks the current thread as a private state thread.
-///
-/// When RunThreadPlan detects it is running on the private state thread, it
-/// spins up an override thread and reassigns m_current_private_state_thread_sp
-/// to it. The original PST continues processing events via DoOnRemoval
-/// callbacks, but CurrentThreadIsPrivateStateThread() no longer recognizes it.
-/// This guard sets a thread_local flag so that GetStackFrameList can identify
-/// the original PST and return parent frames instead of provider-augmented
-/// frames.
-struct PrivateStateThreadGuard {
- PrivateStateThreadGuard() { g_is_private_state_thread = true; }
- ~PrivateStateThreadGuard() { g_is_private_state_thread = false; }
- static bool IsPrivateStateThread() { return g_is_private_state_thread; }
-
- // Non-copyable, non-movable.
- PrivateStateThreadGuard(const PrivateStateThreadGuard &) = delete;
- PrivateStateThreadGuard &operator=(const PrivateStateThreadGuard &) = delete;
-
-private:
- static thread_local bool g_is_private_state_thread;
-};
-
} // namespace lldb_private
#endif // LLDB_TARGET_PROCESS_H
diff --git a/lldb/source/Target/Policy.cpp b/lldb/source/Target/Policy.cpp
index b21ac3e074ca2..0757bcc1bf397 100644
--- a/lldb/source/Target/Policy.cpp
+++ b/lldb/source/Target/Policy.cpp
@@ -7,20 +7,33 @@
//===----------------------------------------------------------------------===//
#include "lldb/Target/Policy.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
#include "lldb/Utility/Stream.h"
+#include "lldb/Utility/StreamString.h"
using namespace lldb_private;
+Policy PolicyStack::Current() const {
+ Policy p = m_stack.back();
+ if (Log *log = GetLog(LLDBLog::Process)) {
+ StreamString s;
+ p.Dump(s);
+ LLDB_LOG(log, "{0}", s.GetData());
+ }
+ return p;
+}
+
void Policy::Dump(Stream &s) const {
- s.Printf("view=%s", view == View::Public ? "public" : "private");
- s.PutCString(", capabilities={");
+ s << "policy: view=" << (view == View::Public ? "public" : "private")
+ << ", capabilities={";
s.Printf("eval_expr=%d", capabilities.can_evaluate_expressions);
s.Printf(" run_all=%d", capabilities.can_run_all_threads);
s.Printf(" try_all=%d", capabilities.can_try_all_threads);
s.Printf(" bp_actions=%d", capabilities.can_run_breakpoint_actions);
s.Printf(" frame_providers=%d", capabilities.can_load_frame_providers);
s.Printf(" frame_recognizers=%d", capabilities.can_run_frame_recognizers);
- s.PutChar('}');
+ s << '}';
}
void PolicyStack::Dump(Stream &s) const {
@@ -28,6 +41,6 @@ void PolicyStack::Dump(Stream &s) const {
for (size_t i = 0; i < m_stack.size(); i++) {
s.Printf(" [%zu] ", i);
m_stack[i].Dump(s);
- s.PutChar('\n');
+ s << '\n';
}
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 9fa6d5b26e9de..b846acf2f4482 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -52,6 +52,7 @@
#include "lldb/Target/MemoryRegionInfo.h"
#include "lldb/Target/OperatingSystem.h"
#include "lldb/Target/Platform.h"
+#include "lldb/Target/Policy.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StopInfo.h"
@@ -73,6 +74,7 @@
#include "lldb/Utility/ProcessInfo.h"
#include "lldb/Utility/SelectHelper.h"
#include "lldb/Utility/State.h"
+#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/Timer.h"
using namespace lldb;
@@ -1277,10 +1279,14 @@ StateType Process::GetState() {
if (!m_current_private_state_thread_sp)
return eStateUnloaded;
+ Policy policy = PolicyStack::Get().Current();
+ if (policy.view == Policy::View::Private)
+ return GetPrivateState();
+
if (CurrentThreadPosesAsPrivateStateThread())
return GetPrivateState();
- else
- return GetPublicState();
+
+ return GetPublicState();
}
void Process::SetPublicState(StateType new_state, bool restarted) {
@@ -4328,14 +4334,12 @@ Status Process::HaltPrivate() {
return error;
}
-thread_local bool PrivateStateThreadGuard::g_is_private_state_thread = false;
-
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<PrivateStateThreadGuard> pst_guard;
+ std::optional<PolicyStack::Guard> policy_guard;
if (is_override)
- pst_guard.emplace();
+ policy_guard.emplace(Policy::PrivateState());
bool control_only = true;
@@ -5533,9 +5537,9 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx,
// If we spawned an override PST, mark the current (original) PST so
// GetStackFrameList returns parent frames during event processing.
- std::optional<PrivateStateThreadGuard> private_state_thread_guard;
+ std::optional<PolicyStack::Guard> policy_guard;
if (backup_private_state_thread)
- private_state_thread_guard.emplace();
+ policy_guard.emplace(Policy::PrivateState());
while (true) {
// We usually want to resume the process if we get to the top of the
@@ -5937,7 +5941,7 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx,
}
} // END WAIT LOOP
- private_state_thread_guard.reset();
+ policy_guard.reset();
// If we had to start up a temporary private state thread to run this
// thread plan, shut it down now.
diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index 5423785c2fd51..97b54403bec8b 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -15,6 +15,7 @@
#include "lldb/Symbol/Block.h"
#include "lldb/Symbol/Function.h"
#include "lldb/Symbol/Symbol.h"
+#include "lldb/Target/Policy.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StackFrame.h"
@@ -806,6 +807,10 @@ void StackFrameList::SelectMostRelevantFrame() {
// Don't call into the frame recognizers on the private state thread as
// 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())
return;
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 2a7ea2808d87f..768d2e8fc0dae 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -16,6 +16,7 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Expression/UserExpression.h"
#include "lldb/Symbol/Block.h"
+#include "lldb/Target/Policy.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/Target.h"
@@ -153,6 +154,10 @@ class StopInfoBreakpoint : public StopInfo {
StopReason GetStopReason() const override { return eStopReasonBreakpoint; }
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());
+
ThreadSP thread_sp(m_thread_wp.lock());
if (thread_sp) {
if (!m_should_stop_is_valid) {
@@ -393,7 +398,9 @@ class StopInfoBreakpoint : public StopInfo {
ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
Process *process = exe_ctx.GetProcessPtr();
- if (process->GetModIDRef().IsRunningExpression()) {
+ Policy policy = PolicyStack::Get().Current();
+ if (!policy.capabilities.can_run_breakpoint_actions ||
+ process->GetModIDRef().IsRunningExpression()) {
// 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
@@ -857,6 +864,10 @@ 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());
+
// If we are running our step-over the watchpoint plan, stop if it's done
// and continue if it's not:
if (m_should_stop_is_valid)
@@ -965,6 +976,17 @@ class StopInfoWatchpoint : public StopInfo {
void PerformAction(Event *event_ptr) override {
Log *log = GetLog(LLDBLog::Watchpoints);
+
+ Policy policy = PolicyStack::Get().Current();
+ if (!policy.capabilities.can_run_breakpoint_actions) {
+ m_should_stop = false;
+ m_should_stop_is_valid = true;
+ LLDB_LOGF(log, "StopInfoWatchpoint::PerformAction - Hit a "
+ "watchpoint while running an expression,"
+ " not running commands to avoid recursion.");
+ return;
+ }
+
// We're going to calculate if we should stop or not in some way during the
// course of this code. Also by default we're going to stop, so set that
// here.
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index d7611f470b9b6..1c2e96ab59517 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -51,6 +51,7 @@
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Language.h"
#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Target/Policy.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/RegisterTypeBuilder.h"
#include "lldb/Target/SectionLoadList.h"
@@ -5889,10 +5890,14 @@ Target::TargetEventData::GetModuleListFromEvent(const Event *event_ptr) {
}
std::recursive_mutex &Target::GetAPIMutex() {
+ Policy policy = PolicyStack::Get().Current();
+ if (policy.view == Policy::View::Private)
+ return m_private_mutex;
+
if (GetProcessSP() && GetProcessSP()->CurrentThreadIsPrivateStateThread())
return m_private_mutex;
- else
- return m_mutex;
+
+ return m_mutex;
}
/// Get metrics associated with this target in JSON format.
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 70053b8822bdc..89e8bb468de7c 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -24,6 +24,7 @@
#include "lldb/Target/DynamicLoader.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Target/Policy.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/ScriptedThreadPlan.h"
@@ -1533,7 +1534,7 @@ StackFrameListSP Thread::GetStackFrameList() {
// thread that is already mid-construction.
//
// 2. Current thread is a private state thread that should see the
- // private reality (PrivateStateThreadGuard::IsPrivateStateThread).
+ // private reality (Policy::View::Private).
//
// For case 1, if a provider is active we return its input (parent)
// frames. For case (2), we return/create the unwinder frame list
@@ -1558,13 +1559,11 @@ StackFrameListSP Thread::GetStackFrameList() {
if (m_curr_frames_sp)
return m_curr_frames_sp;
- // For case 2, PST must see the private reality, not the public illusion.
- // PrivateStateThreadGuard is a thread_local flag set by RunThreadPlan
- // (for the original PST) and RunPrivateStateThread (for override PSTs).
- // We cannot use CurrentThreadIsPrivateStateThread() here because
- // RunThreadPlan reassigns m_current_private_state_thread_sp to the
- // override, so the original PST is no longer recognized.
- if (PrivateStateThreadGuard::IsPrivateStateThread()) {
+ // The private state thread must see the raw unwinder frames, not the
+ // provider-augmented public view. Policy::PrivateState is pushed by
+ // RunThreadPlan and RunPrivateStateThread.
+ Policy policy = PolicyStack::Get().Current();
+ if (policy.view == Policy::View::Private) {
if (!m_unwinder_frames_sp)
m_unwinder_frames_sp = std::make_shared<StackFrameList>(
*this, m_prev_frames_sp, true, /*provider_id=*/0);
diff --git a/lldb/unittests/Target/PolicyTest.cpp b/lldb/unittests/Target/PolicyTest.cpp
index 691ca52b34c31..0b6a34df4f1fe 100644
--- a/lldb/unittests/Target/PolicyTest.cpp
+++ b/lldb/unittests/Target/PolicyTest.cpp
@@ -59,74 +59,72 @@ TEST(PolicyTest, PublicStateRunningExpression) {
}
TEST(PolicyTest, StackDefaultIsPublicState) {
- PolicyStack &stack = PolicyStack::GetForCurrentThread();
- Policy current = stack.Current();
+ Policy current = PolicyStack::Get().Current();
EXPECT_EQ(current.view, Policy::View::Public);
EXPECT_TRUE(current.capabilities.can_evaluate_expressions);
EXPECT_TRUE(current.capabilities.can_load_frame_providers);
}
TEST(PolicyTest, StackPushPop) {
- PolicyStack &stack = PolicyStack::GetForCurrentThread();
+ PolicyStack::Get().Push(Policy::PrivateState());
+ EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private);
+ EXPECT_FALSE(
+ PolicyStack::Get().Current().capabilities.can_load_frame_providers);
- stack.Push(Policy::PrivateState());
- EXPECT_EQ(stack.Current().view, Policy::View::Private);
- EXPECT_FALSE(stack.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);
- stack.Push(Policy::PublicStateRunningExpression());
- EXPECT_EQ(stack.Current().view, Policy::View::Public);
- EXPECT_FALSE(stack.Current().capabilities.can_run_breakpoint_actions);
+ PolicyStack::Get().Pop();
+ EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private);
- stack.Pop();
- EXPECT_EQ(stack.Current().view, Policy::View::Private);
-
- stack.Pop();
- EXPECT_EQ(stack.Current().view, Policy::View::Public);
+ PolicyStack::Get().Pop();
+ EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public);
}
TEST(PolicyTest, GuardRAII) {
- PolicyStack &stack = PolicyStack::GetForCurrentThread();
- EXPECT_EQ(stack.Current().view, Policy::View::Public);
+ EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public);
{
PolicyStack::Guard guard(Policy::PrivateState());
- EXPECT_EQ(stack.Current().view, Policy::View::Private);
- EXPECT_FALSE(stack.Current().capabilities.can_load_frame_providers);
+ 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(stack.Current().view, Policy::View::Public);
- EXPECT_FALSE(stack.Current().capabilities.can_run_breakpoint_actions);
+ EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public);
+ EXPECT_FALSE(
+ PolicyStack::Get().Current().capabilities.can_run_breakpoint_actions);
}
- EXPECT_EQ(stack.Current().view, Policy::View::Private);
+ EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private);
}
- EXPECT_EQ(stack.Current().view, Policy::View::Public);
+ EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Public);
}
TEST(PolicyTest, StackIsPerThread) {
- PolicyStack &main_stack = PolicyStack::GetForCurrentThread();
- main_stack.Push(Policy::PrivateState());
+ PolicyStack::Get().Push(Policy::PrivateState());
Policy::View other_thread_view;
std::thread t([&other_thread_view]() {
- PolicyStack &stack = PolicyStack::GetForCurrentThread();
- other_thread_view = stack.Current().view;
+ other_thread_view = PolicyStack::Get().Current().view;
});
t.join();
- EXPECT_EQ(main_stack.Current().view, Policy::View::Private);
+ EXPECT_EQ(PolicyStack::Get().Current().view, Policy::View::Private);
EXPECT_EQ(other_thread_view, Policy::View::Public);
- main_stack.Pop();
+ PolicyStack::Get().Pop();
}
TEST(PolicyTest, DumpPublicState) {
StreamString s;
Policy::PublicState().Dump(s);
EXPECT_EQ(s.GetString(),
- "view=public, capabilities={"
+ "policy: view=public, capabilities={"
"eval_expr=1 run_all=1 try_all=1 "
"bp_actions=1 frame_providers=1 frame_recognizers=1}");
}
@@ -135,20 +133,19 @@ TEST(PolicyTest, DumpPrivateState) {
StreamString s;
Policy::PrivateState().Dump(s);
EXPECT_EQ(s.GetString(),
- "view=private, capabilities={"
+ "policy: view=private, capabilities={"
"eval_expr=1 run_all=1 try_all=1 "
"bp_actions=1 frame_providers=0 frame_recognizers=0}");
}
TEST(PolicyTest, DumpStack) {
- PolicyStack &stack = PolicyStack::GetForCurrentThread();
- stack.Push(Policy::PrivateState());
+ PolicyStack::Get().Push(Policy::PrivateState());
StreamString s;
- stack.Dump(s);
+ PolicyStack::Get().Dump(s);
EXPECT_NE(s.GetString().find("depth=2"), std::string::npos);
- EXPECT_NE(s.GetString().find("[0] view=public"), std::string::npos);
- EXPECT_NE(s.GetString().find("[1] view=private"), 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);
- stack.Pop();
+ PolicyStack::Get().Pop();
}
More information about the lldb-commits
mailing list