https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/208816
>From 97a612ea8e3eae00b91e38b5cda91961c7aba6a6 Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Fri, 10 Jul 2026 16:45:04 -0700 Subject: [PATCH 1/2] [lldb] Make PolicyStack::Get() out-of-line LLDB builds with hidden visibility by default, so a function-local static in an inline function is not shared across shared library boundaries: each dylib that includes this header and calls an inline Get() gets its own private copy of the thread_local stack, silently splitting one logical per-thread stack into several. Declaring it out-of-line ensures every dylib resolves to the single instance defined in Policy.cpp. Nothing currently pushes a policy from outside liblldb, so this has no observable effect yet, but any future capability that needs to be set in one shared library and read in another depends on this. Signed-off-by: Med Ismail Bennani <[email protected]> --- lldb/include/lldb/Utility/Policy.h | 5 +---- lldb/source/Utility/Policy.cpp | 8 ++++++++ lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Utility/Policy.h b/lldb/include/lldb/Utility/Policy.h index c841daf47a97d..afeeab19c2ed0 100644 --- a/lldb/include/lldb/Utility/Policy.h +++ b/lldb/include/lldb/Utility/Policy.h @@ -95,10 +95,7 @@ struct Policy { /// thread's stack when the task starts. class PolicyStack { public: - static PolicyStack &Get() { - static thread_local PolicyStack s_stack; - return s_stack; - } + static PolicyStack &Get(); Policy Current() const; diff --git a/lldb/source/Utility/Policy.cpp b/lldb/source/Utility/Policy.cpp index 3c2fa8f7fc867..301f7ee36e897 100644 --- a/lldb/source/Utility/Policy.cpp +++ b/lldb/source/Utility/Policy.cpp @@ -15,6 +15,14 @@ using namespace lldb_private; +// Declared out-of-line so every shared library resolves to the same +// instance: a function-local static in an inline function is not +// shared across shared library boundaries under hidden visibility. +PolicyStack &PolicyStack::Get() { + static thread_local PolicyStack s_stack; + return s_stack; +} + Policy PolicyStack::Current() const { Policy p = m_stack.back(); if (Log *log = GetLog(LLDBLog::Process)) { diff --git a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp index 7dd0a4a0f1871..bb1e0d8cd1fea 100644 --- a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp +++ b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp @@ -94,7 +94,7 @@ class SymbolFilePDBTests : public testing::Test { if (!left.FileEquals(right)) return false; // If BOTH have a directory, also compare the directories. - if (left.GetDirectory() && right.GetDirectory()) + if (!left.GetDirectory().empty() && !right.GetDirectory().empty()) return left.DirectoryEquals(right); // If one has a directory but not the other, they match. >From 9f87b3e6a683bd75432647332d908dfa8233078e Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Fri, 10 Jul 2026 15:32:20 -0700 Subject: [PATCH 2/2] [lldb] Fix scripted frame provider cross-thread re-entrant deadlock GetStoppedExecutionContext unconditionally blocked acquiring the target's API mutex. A thread already holding that mutex (e.g. a `bt` command thread, via CommandObjectParsed's eCommandTryTargetAPILock) can end up waiting on a StackFrameList lock held by another thread (e.g. the debugger's event-handler thread) that is itself blocked re-acquiring the API mutex from inside a scripted frame provider's Python code that touches SB API -- a classic AB-BA deadlock. Introduce Policy::Capabilities::can_reenter_target_api_mutex, pushed around every scripted-extension callback in ScriptedPythonInterface::Dispatch and CallStaticMethod. The point isn't "avoid this deadlock" as a goal in itself: a thread running one of these callbacks isn't servicing a client-facing SB API entry point, it's doing internal work on the callback's behalf, so it doesn't need the same locking guarantees a top-level SB API call does. Avoiding the deadlock is a beneficial side effect of giving the thread the permissions that match what it's actually doing. Encapsulate the policy check in a new Target::GetAPIMutexLock(), next to GetAPIMutex() (which already picks between the public/private mutex based on the policy's view), instead of leaving a free-standing `PolicyStack::Get()...` check at the call site. It returns a real lock normally, or a no-op unlocked one when the current thread's policy says it doesn't need one. Adds a regression test for the deadlock. It's a genuine cross-thread race (the command thread vs. the debugger's event-handler thread), so like the sibling runlock_reentrant_deadlock/was_hit_deadlock tests, it raises the odds of hitting it within a single invocation but can't guarantee it. Also updates PolicyTest's DumpPublicState/DumpPrivateState expectations for the new reenter_api_mutex field in Policy::Dump(). Signed-off-by: Med Ismail Bennani <[email protected]> --- lldb/include/lldb/Target/ExecutionContext.h | 11 ++- lldb/include/lldb/Target/Target.h | 4 + lldb/include/lldb/Utility/Policy.h | 11 +++ .../Interfaces/ScriptedPythonInterface.h | 7 ++ lldb/source/Target/ExecutionContext.cpp | 4 +- lldb/source/Target/Target.cpp | 8 ++ lldb/source/Utility/Policy.cpp | 7 ++ .../Makefile | 2 + ...ProviderRegisterCommandAPIMutexDeadlock.py | 89 +++++++++++++++++++ .../frame_provider.py | 28 ++++++ .../main.c | 7 ++ lldb/unittests/Utility/PolicyTest.cpp | 6 +- 12 files changed, 176 insertions(+), 8 deletions(-) create mode 100644 lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/Makefile create mode 100644 lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/TestFrameProviderRegisterCommandAPIMutexDeadlock.py create mode 100644 lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/frame_provider.py create mode 100644 lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/main.c diff --git a/lldb/include/lldb/Target/ExecutionContext.h b/lldb/include/lldb/Target/ExecutionContext.h index bf976f4db8c87..b155b2ce436f3 100644 --- a/lldb/include/lldb/Target/ExecutionContext.h +++ b/lldb/include/lldb/Target/ExecutionContext.h @@ -14,6 +14,7 @@ #include "lldb/Host/ProcessRunLock.h" #include "lldb/Target/StackID.h" #include "lldb/Target/SyntheticFrameProvider.h" +#include "lldb/Utility/Policy.h" #include "lldb/lldb-private.h" namespace lldb_private { @@ -561,9 +562,9 @@ class ExecutionContext { }; /// A wrapper class representing an execution context with non-null Target -/// and Process pointers, a locked API mutex and a locked ProcessRunLock. -/// The locks are private by design: to unlock them, destroy the -/// StoppedExecutionContext. +/// and Process pointers, a locked ProcessRunLock, and (unless the current +/// thread's policy allows skipping it) a locked API mutex. The locks are +/// private by design; to unlock them, destroy the StoppedExecutionContext. struct StoppedExecutionContext : ExecutionContext { StoppedExecutionContext(lldb::TargetSP &target_sp, lldb::ProcessSP &process_sp, @@ -574,7 +575,9 @@ struct StoppedExecutionContext : ExecutionContext { : m_api_lock(std::move(api_lock)), m_stop_locker(std::move(stop_locker)) { assert(target_sp); assert(process_sp); - assert(m_api_lock.owns_lock()); + assert( + m_api_lock.owns_lock() || + PolicyStack::Get().Current().capabilities.can_reenter_target_api_mutex); assert(m_stop_locker.IsLocked()); SetTargetSP(target_sp); SetProcessSP(process_sp); diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index c8b7c477505ee..270c956fb118c 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -763,6 +763,10 @@ class Target : public std::enable_shared_from_this<Target>, std::recursive_mutex &GetAPIMutex(); + /// Returns a lock on the API mutex, or an already-unlocked lock if the + /// current thread's policy says it doesn't need one. + std::unique_lock<std::recursive_mutex> GetAPIMutexLock(); + void DeleteCurrentProcess(); void CleanupProcess(); diff --git a/lldb/include/lldb/Utility/Policy.h b/lldb/include/lldb/Utility/Policy.h index afeeab19c2ed0..63a17523c8b36 100644 --- a/lldb/include/lldb/Utility/Policy.h +++ b/lldb/include/lldb/Utility/Policy.h @@ -50,6 +50,11 @@ struct Policy { bool can_run_breakpoint_actions = true; bool can_load_frame_providers = true; bool can_run_frame_recognizers = true; + /// Whether SB API calls made on this thread may skip re-acquiring the + /// target's API mutex, because the thread is already running under + /// whatever protections its caller set up rather than servicing a + /// top-level SB API entry point itself. + bool can_reenter_target_api_mutex = false; }; /// Why a private-state policy is being pushed. Distinguishes a PST's @@ -75,6 +80,7 @@ struct Policy { static Policy CreatePrivateState( PrivateStatePurpose purpose = PrivateStatePurpose::Default); static Policy CreatePublicStateRunningExpression(); + static Policy CreateScriptedExtensionCall(); /// @} void Dump(Stream &s) const; @@ -140,6 +146,11 @@ class PolicyStack { return Guard(); } + [[nodiscard]] Guard PushScriptedExtensionCall() { + Push(Policy::CreateScriptedExtensionCall()); + return Guard(); + } + private: void Push(Policy policy) { m_stack.push_back(std::move(policy)); } diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h index 7d0d4cdd3c6d1..79ca76577ffa9 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h @@ -17,6 +17,7 @@ #include "lldb/Interpreter/Interfaces/ScriptedInterface.h" #include "lldb/Utility/DataBufferHeap.h" +#include "lldb/Utility/Policy.h" #include "../PythonDataObjects.h" #include "../SWIGPythonBridge.h" @@ -415,6 +416,9 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { return ErrorWithMessage<T>(caller_signature, "missing script class name", error); + PolicyStack::Guard policy_guard = + PolicyStack::Get().PushScriptedExtensionCall(); + Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); @@ -510,6 +514,9 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { return ErrorWithMessage<T>(caller_signature, "python object ill-formed", error); + PolicyStack::Guard policy_guard = + PolicyStack::Get().PushScriptedExtensionCall(); + Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); diff --git a/lldb/source/Target/ExecutionContext.cpp b/lldb/source/Target/ExecutionContext.cpp index e4b2f07d8d8d1..2818d7ff23c01 100644 --- a/lldb/source/Target/ExecutionContext.cpp +++ b/lldb/source/Target/ExecutionContext.cpp @@ -145,8 +145,8 @@ lldb_private::GetStoppedExecutionContext( return llvm::createStringError( "StoppedExecutionContext created with a null target"); - auto api_lock = - std::unique_lock<std::recursive_mutex>(target_sp->GetAPIMutex()); + std::unique_lock<std::recursive_mutex> api_lock = + target_sp->GetAPIMutexLock(); auto process_sp = exe_ctx_ref_ptr->GetProcessSP(); if (!process_sp) diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index daa1b1c43efa8..f47d31b32412e 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -6000,6 +6000,14 @@ std::recursive_mutex &Target::GetAPIMutex() { return m_mutex; } +std::unique_lock<std::recursive_mutex> Target::GetAPIMutexLock() { + Policy policy = PolicyStack::Get().Current(); + if (policy.capabilities.can_reenter_target_api_mutex) + return {}; + + return std::unique_lock<std::recursive_mutex>(GetAPIMutex()); +} + /// Get metrics associated with this target in JSON format. llvm::json::Value Target::ReportStatistics(const lldb_private::StatisticsOptions &options) { diff --git a/lldb/source/Utility/Policy.cpp b/lldb/source/Utility/Policy.cpp index 301f7ee36e897..a375cf03181b3 100644 --- a/lldb/source/Utility/Policy.cpp +++ b/lldb/source/Utility/Policy.cpp @@ -59,6 +59,12 @@ Policy Policy::CreatePublicStateRunningExpression() { return p; } +Policy Policy::CreateScriptedExtensionCall() { + Policy p = PolicyStack::Get().Current(); + p.capabilities.can_reenter_target_api_mutex = true; + return p; +} + PolicyStack::Guard::~Guard() { if (!m_active) return; @@ -103,6 +109,7 @@ void Policy::Dump(Stream &s) const { s << " bp_actions=" << capabilities.can_run_breakpoint_actions; s << " frame_providers=" << capabilities.can_load_frame_providers; s << " frame_recognizers=" << capabilities.can_run_frame_recognizers; + s << " reenter_api_mutex=" << capabilities.can_reenter_target_api_mutex; s << '}'; } diff --git a/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/Makefile b/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/Makefile new file mode 100644 index 0000000000000..c9319d6e6888a --- /dev/null +++ b/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/Makefile @@ -0,0 +1,2 @@ +C_SOURCES := main.c +include Makefile.rules diff --git a/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/TestFrameProviderRegisterCommandAPIMutexDeadlock.py b/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/TestFrameProviderRegisterCommandAPIMutexDeadlock.py new file mode 100644 index 0000000000000..09345b11cd84c --- /dev/null +++ b/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/TestFrameProviderRegisterCommandAPIMutexDeadlock.py @@ -0,0 +1,89 @@ +""" +Test that a scripted frame provider whose get_frame_at_index touches SB +API (self.input_frames) does not deadlock when running `bt` from the +command interpreter. + +GetStoppedExecutionContext (used by SBFrame::IsValid, among others) +unconditionally blocked acquiring the target's API mutex. The command +thread running `bt` already holds that mutex (CommandObjectParsed's +eCommandTryTargetAPILock) and can end up waiting on a StackFrameList +lock held by the debugger's event-handler thread, which is itself +blocked re-acquiring the API mutex from inside this provider's Python +code -- an AB-BA deadlock between the command thread and the +event-handler thread. + +The event-handler thread only runs when commands are driven through +SBDebugger.RunCommandInterpreter (what the lldb driver itself uses), +not through plain HandleCommand, so this test drives commands that way. + +Note: this is a genuine cross-thread race (the command thread vs. the +debugger's event-handler thread), not a deterministic sequential +deadlock, so this test is best-effort -- like the sibling +runlock_reentrant_deadlock/was_hit_deadlock tests, it raises the odds of +hitting the race within a single invocation but cannot guarantee it. +""" + +import os +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + + +class TestFrameProviderRegisterCommandAPIMutexDeadlock(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def test_register_command_then_bt_no_deadlock(self): + """ + Register a scripted frame provider whose get_frame_at_index + touches SB API, then repeatedly run `bt` through + RunCommandInterpreter. Should complete without deadlocking. + """ + self.build() + + lldbutil.run_to_name_breakpoint(self, "frame3") + + provider_path = os.path.join(self.getSourceDir(), "frame_provider.py") + + commands = ["command script import " + provider_path] + commands.append( + "target frame-provider register -C frame_provider.DictFrameProvider" + ) + # Run `bt` several times to raise the odds of hitting the race + # between the command thread and the debugger's event-handler + # thread within a single test invocation. + commands.extend(["bt"] * 20) + commands.append("quit") + + stdin_path = self.getBuildArtifact("stdin.txt") + stdout_path = self.getBuildArtifact("stdout.txt") + with open(stdin_path, "w") as f: + f.write("\n".join(commands) + "\n") + + with open(stdin_path, "r") as in_fileH, open(stdout_path, "w") as out_fileH: + in_sbf = lldb.SBFile(in_fileH.fileno(), "r", False) + out_sbf = lldb.SBFile(out_fileH.fileno(), "w", False) + self.assertSuccess(self.dbg.SetInputFile(in_sbf)) + self.assertSuccess(self.dbg.SetOutputFile(out_sbf)) + self.assertSuccess(self.dbg.SetErrorFile(out_sbf)) + + options = lldb.SBCommandInterpreterRunOptions() + options.SetEchoCommands(False) + options.SetPrintResults(True) + options.SetStopOnError(False) + options.SetStopOnCrash(False) + + # If the API-mutex deadlock regresses, this call hangs forever + # (timing out the test run). + n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter( + True, False, options, 0, False, False + ) + + with open(stdout_path, "r") as out_fileH: + output = out_fileH.read() + + self.assertFalse(has_crashed, "lldb should not have crashed") + self.assertTrue(quit_requested, "quit command should have been processed") + self.assertEqual(n_errors, 0, f"unexpected errors in output:\n{output}") + + self.assertIn("successfully registered scripted frame provider", output) diff --git a/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/frame_provider.py b/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/frame_provider.py new file mode 100644 index 0000000000000..2bfab64b1c4d8 --- /dev/null +++ b/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/frame_provider.py @@ -0,0 +1,28 @@ +""" +Frame provider that returns dict-based synthetic frames (never identity +forwarding), while touching self.input_frames from get_frame_at_index. + +Returning a dict keeps this test isolated from the frame-aliasing bug: +dict-based frames always go through ScriptedFrameProvider's +create_frame_from_dict helper, which builds a brand new StackFrame and +never reuses (or wraps via BorrowedStackFrame) the parent list's frame +object. Only the API-mutex deadlock is reachable through this path. +""" + +from lldb.plugins.scripted_frame_provider import ScriptedFrameProvider + + +class DictFrameProvider(ScriptedFrameProvider): + @staticmethod + def get_description(): + return "Provider that returns dict-based synthetic frames" + + def get_frame_at_index(self, index): + if index >= len(self.input_frames): + return None + # __getitem__ calls SBFrame.IsValid() internally, which is what + # exercises GetStoppedExecutionContext. + frame = self.input_frames[index] + if frame is None: + return None + return {"idx": index, "pc": frame.GetPC()} diff --git a/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/main.c b/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/main.c new file mode 100644 index 0000000000000..1aa56e3eddf7a --- /dev/null +++ b/lldb/test/API/functionalities/scripted_frame_provider/register_command_api_mutex_deadlock/main.c @@ -0,0 +1,7 @@ +int frame3() { return 3; } + +int frame2() { return frame3(); } + +int frame1() { return frame2(); } + +int main() { return frame1(); } diff --git a/lldb/unittests/Utility/PolicyTest.cpp b/lldb/unittests/Utility/PolicyTest.cpp index 5ad045a03d30b..f98989ab4e3eb 100644 --- a/lldb/unittests/Utility/PolicyTest.cpp +++ b/lldb/unittests/Utility/PolicyTest.cpp @@ -145,7 +145,8 @@ TEST(PolicyTest, DumpPublicState) { EXPECT_EQ(s.GetString(), "policy: view=public, capabilities={" "eval_expr=true run_all=true try_all=true " - "bp_actions=true frame_providers=true frame_recognizers=true}"); + "bp_actions=true frame_providers=true frame_recognizers=true " + "reenter_api_mutex=false}"); } TEST(PolicyTest, DumpPrivateState) { @@ -154,7 +155,8 @@ 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=true frame_recognizers=true}"); + "bp_actions=true frame_providers=true frame_recognizers=true " + "reenter_api_mutex=false}"); } TEST(PolicyTest, DumpStack) { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
