https://github.com/medismailben created https://github.com/llvm/llvm-project/pull/208992
`ScriptedFrameProvider::GetFrameAtIndex` reused the parent list's live StackFrame directly whenever a provider's `get_frame_at_index` forwarded a frame under its own index (`real_frame_index` == `idx`), instead of wrapping it in a `BorrowedStackFrame` like every other forwarding case. `SyntheticStackFrameList::FetchFramesUpTo` unconditionally re-tags the returned frame's `m_frame_list_id` to this child list before caching it. Reusing the parent list's object means this re-tag corrupts the parent list's own cached frame to claim it belongs to the child list instead. Later, resolving that frame's identity back to a frame list (via `ExecutionContextRef::GetFrameSP`, used by `SBFrame` validity checks) follows the corrupted tag to the child list. If the resolving thread is the one already fetching frames on that child list -- holding its writer lock via `GetFramesUpTo`, it self-deadlocks trying to take the (non-reentrant) reader lock on the same list. This changes the frame to be always wrapped in a `BorrowedStackFrame`, even when the index is unchanged, so the object added to the child list is never the same object cached in the parent list. This patch also adds a regression test using an identity-forwarding provider (mirroring the shape that surfaced this bug) to isolate it from the API-mutex deadlock fixed in the parent commit. Like that test, this is a best-effort race reproduction, not a guaranteed one. Signed-off-by: Med Ismail Bennani <[email protected]> >From 4acab8f6e4102fc271c3b3ef374788581b6c3c37 Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Fri, 10 Jul 2026 15:32:20 -0700 Subject: [PATCH 1/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. Encode the check inside Target::GetAPIMutex() itself: when the current thread's policy allows re-entrancy, it hands out a thread-local mutex instead of the real one, so every existing caller -- there are close to 200 across the SB* classes -- becomes deadlock-safe automatically, without having to convert each call site to a new wrapper. 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 | 4 +- lldb/include/lldb/Target/Target.h | 5 ++ lldb/include/lldb/Utility/Policy.h | 11 +++ .../Interfaces/ScriptedPythonInterface.h | 7 ++ lldb/source/Target/ExecutionContext.cpp | 3 +- lldb/source/Target/Target.cpp | 10 +++ 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, 173 insertions(+), 6 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..706fd6f64ba53 100644 --- a/lldb/include/lldb/Target/ExecutionContext.h +++ b/lldb/include/lldb/Target/ExecutionContext.h @@ -561,8 +561,8 @@ 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 +/// and Process pointers, a locked ProcessRunLock, and a locked API mutex. +/// The locks are private by design; to unlock them, destroy the /// StoppedExecutionContext. struct StoppedExecutionContext : ExecutionContext { StoppedExecutionContext(lldb::TargetSP &target_sp, diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index c8b7c477505ee..9eb17dda1755f 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -761,6 +761,11 @@ class Target : public std::enable_shared_from_this<Target>, static TargetProperties &GetGlobalProperties(); + /// Returns the mutex a caller should serialize on before touching the + /// target through the SB API. When the current thread's policy says it + /// doesn't need to serialize, this returns a mutex private to that thread + /// instead, so callers can lock it unconditionally without ever + /// contending with whichever thread holds the real one. std::recursive_mutex &GetAPIMutex(); void DeleteCurrentProcess(); 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..f2ac6a0bc8e24 100644 --- a/lldb/source/Target/ExecutionContext.cpp +++ b/lldb/source/Target/ExecutionContext.cpp @@ -145,8 +145,7 @@ 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->GetAPIMutex()); 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..2777f29a3f65c 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -5994,6 +5994,16 @@ Target::TargetEventData::GetModuleListFromEvent(const Event *event_ptr) { std::recursive_mutex &Target::GetAPIMutex() { Policy policy = PolicyStack::Get().Current(); + + // A thread whose policy says it doesn't need to serialize on the API mutex + // gets a mutex of its own instead of a no-op: every caller still locks + // *something*, but since it's thread-local, that lock never contends with + // whatever other thread holds the real mutex. + if (policy.capabilities.can_reenter_target_api_mutex) { + static thread_local std::recursive_mutex s_bypass_mutex; + return s_bypass_mutex; + } + if (policy.view == Policy::View::Private) return m_private_mutex; 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) { >From 1e5be1542a954c7af5afa4cad9d02cf4698c9bdf Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Sat, 11 Jul 2026 17:47:30 -0700 Subject: [PATCH 2/2] [lldb] Fix self-deadlock from frame identity aliasing in scripted frame providers ScriptedFrameProvider::GetFrameAtIndex reused the parent list's live StackFrame object directly whenever a provider's get_frame_at_index forwarded a frame under its own index (real_frame_index == idx), instead of wrapping it in a BorrowedStackFrame like every other forwarding case. SyntheticStackFrameList::FetchFramesUpTo unconditionally re-tags the returned frame's m_frame_list_id to this (child) list before caching it. Reusing the parent list's object means this re-tag corrupts the parent list's own cached frame to claim it belongs to the child list instead. Later, resolving that frame's identity back to a frame list (e.g. via ExecutionContextRef::GetFrameSP, used by SBFrame validity checks) follows the corrupted tag to the child list. If the resolving thread is the one already fetching frames on that child list -- holding its writer lock via GetFramesUpTo -- it self-deadlocks trying to take the (non-reentrant) reader lock on the same list. Always wrap in BorrowedStackFrame, even when the index is unchanged, so the object added to the child list is never the same object cached in the parent list. Adds a regression test using an identity-forwarding provider (mirroring the shape that surfaced this bug) to isolate it from the API-mutex deadlock fixed in the parent commit. Like that test, this is a best-effort race reproduction, not a guaranteed one. Signed-off-by: Med Ismail Bennani <[email protected]> --- .../ScriptedFrameProvider.cpp | 11 ++- .../Makefile | 3 + ...oviderRegisterCommandFrameAliasDeadlock.py | 94 +++++++++++++++++++ .../frame_provider.py | 30 ++++++ .../main.c | 7 ++ 5 files changed, 141 insertions(+), 4 deletions(-) create mode 100644 lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/Makefile create mode 100644 lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/TestFrameProviderRegisterCommandFrameAliasDeadlock.py create mode 100644 lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/frame_provider.py create mode 100644 lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/main.c diff --git a/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.cpp b/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.cpp index ab09d86b24d95..977ea5e0548e4 100644 --- a/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.cpp +++ b/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.cpp @@ -179,10 +179,13 @@ ScriptedFrameProvider::GetFrameAtIndex(uint32_t idx) { if (real_frame_index < m_input_frames->GetNumFrames()) { StackFrameSP real_frame_sp = m_input_frames->GetFrameAtIndex(real_frame_index); - synth_frame_sp = - (real_frame_index == idx) - ? real_frame_sp - : std::make_shared<BorrowedStackFrame>(real_frame_sp, idx); + // Always wrap in a BorrowedStackFrame, even when the index is + // unchanged. FetchFramesUpTo below unconditionally overwrites + // frame_sp->m_frame_list_id to tag the frame as belonging to this + // synthetic list; reusing real_frame_sp directly would corrupt the + // parent list's cached frame (still m_input_frames' object) to claim + // it belongs to this list instead. + synth_frame_sp = std::make_shared<BorrowedStackFrame>(real_frame_sp, idx); } } else if (StructuredData::Dictionary *dict = obj_sp->GetAsDictionary()) { // Check if it's a dictionary describing a frame. diff --git a/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/Makefile b/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/Makefile new file mode 100644 index 0000000000000..0b710c6e298ae --- /dev/null +++ b/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c +CFLAGS_EXTRAS := -std=c99 +include Makefile.rules diff --git a/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/TestFrameProviderRegisterCommandFrameAliasDeadlock.py b/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/TestFrameProviderRegisterCommandFrameAliasDeadlock.py new file mode 100644 index 0000000000000..dee8bf01c7195 --- /dev/null +++ b/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/TestFrameProviderRegisterCommandFrameAliasDeadlock.py @@ -0,0 +1,94 @@ +""" +Test that a scripted frame provider that forwards frames under their own +index (identity forwarding) does not deadlock when running `bt` from the +command interpreter. + +ScriptedFrameProvider::GetFrameAtIndex reused the parent list's live +StackFrame object directly whenever a provider forwarded a frame under +its own index, instead of wrapping it in a BorrowedStackFrame. Frame +construction unconditionally re-tags the returned frame as belonging to +the child list, corrupting the parent list's cached frame. Once that +frame's corrupted list identity is later resolved, it points back at +the (possibly still-being-built) child list, and a thread already +holding that list's writer lock can self-deadlock taking the reader +lock. + +The event-handler thread that can trigger this only runs when commands +are driven through SBDebugger.RunCommandInterpreter (what the lldb +driver itself uses), not 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 TestFrameProviderRegisterCommandFrameAliasDeadlock(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def test_register_command_then_bt_no_deadlock(self): + """ + Register a scripted frame provider that identity-forwards every + frame, 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.IdentityProvider" + ) + # 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 frame-aliasing self-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) + self.assertIn("frame3", output) + self.assertIn("frame2", output) + self.assertIn("frame1", output) diff --git a/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/frame_provider.py b/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/frame_provider.py new file mode 100644 index 0000000000000..e8ed77dc5a985 --- /dev/null +++ b/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/frame_provider.py @@ -0,0 +1,30 @@ +""" +Frame provider that forwards every frame under its own index while also +touching input_frames. + +Returning `index` (identity forwarding) means the provider reuses the +same StackFrame object that is cached in the parent (input) frame list +instead of wrapping it in a BorrowedStackFrame. Frame construction +unconditionally re-tags that frame as belonging to this (child) list, +corrupting the parent list's cached frame. When that corrupted frame is +later resolved back to a frame list, it resolves to this list -- which, +if the resolving thread is the one already fetching frames on this list, +self-deadlocks trying to take a reader lock on the writer lock it +already holds. +""" + +from lldb.plugins.scripted_frame_provider import ScriptedFrameProvider + + +class IdentityProvider(ScriptedFrameProvider): + @staticmethod + def get_description(): + return "Provider that forwards each frame under its own index" + + def get_frame_at_index(self, index): + if index < len(self.input_frames): + # __getitem__ calls SBFrame.IsValid() internally, which is what + # exercises GetStoppedExecutionContext. + self.input_frames[index] + return index + return None diff --git a/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/main.c b/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/main.c new file mode 100644 index 0000000000000..1aa56e3eddf7a --- /dev/null +++ b/lldb/test/API/functionalities/scripted_frame_provider/register_command_frame_alias_deadlock/main.c @@ -0,0 +1,7 @@ +int frame3() { return 3; } + +int frame2() { return frame3(); } + +int frame1() { return frame2(); } + +int main() { return frame1(); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
