Author: jimingham Date: 2026-03-05T08:17:16-08:00 New Revision: 97572c1860efeeb97b5940927cee72081b61810a
URL: https://github.com/llvm/llvm-project/commit/97572c1860efeeb97b5940927cee72081b61810a DIFF: https://github.com/llvm/llvm-project/commit/97572c1860efeeb97b5940927cee72081b61810a.diff LOG: Add the ability to "allow another thread to see the private state" mode. (#184272) When lldb stops to run a breakpoint condition or other callback that has to happen between the private stop and returning control to the user, it will run in the state where the public state is still "running". But if the callback needs to run lldb commands or python code, it needs to see the correct "stop" state. We used to handle that by switching the public state to stopped before running the callbacks. However, that opened a window where we are still handling the stop event and another thread would be allowed to continue the target or do other actions that can interfere with that orderly process. This patch adds the ability to designate a particular thread as "seeing the private state" while all other threads see the "public state". Then when we run a breakpoint callback, no threads but the one that is actually running the callback will see the state change until the event has been delivered to the primary state listener. It also adds a test that while a long-running breakpoint callback runs, another thread continues to see the state as running. Added: Modified: lldb/include/lldb/Host/ProcessRunLock.h lldb/include/lldb/Target/Process.h lldb/source/Target/Process.cpp lldb/source/Target/StopInfo.cpp lldb/test/API/python_api/run_locker/TestRunLocker.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/ProcessRunLock.h b/lldb/include/lldb/Host/ProcessRunLock.h index 27d10942ddb4c..c427023ade33f 100644 --- a/lldb/include/lldb/Host/ProcessRunLock.h +++ b/lldb/include/lldb/Host/ProcessRunLock.h @@ -34,6 +34,8 @@ class ProcessRunLock { /// Return false if the process was running. bool SetRunning(); + bool IsRunning() { return m_running; } + /// Set the process to stopped. Returns true if the process was running. /// Returns false if the process was stopped. bool SetStopped(); diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index b63fe73602cc8..871b533723fcd 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -14,6 +14,7 @@ #include <climits> #include <chrono> +#include <deque> #include <list> #include <memory> #include <mutex> @@ -3199,7 +3200,7 @@ void PruneThreadPlans(); // you won't be doing any debugging today. bool StartupThread(); - bool IsOnThread(const HostThread &thread) const; + bool IsOnThread(lldb::thread_t thread) const; bool IsJoinable() { return m_private_state_thread.IsJoinable(); } @@ -3218,7 +3219,11 @@ void PruneThreadPlans(); return m_private_state.GetValue(); } - lldb::StateType GetPublicState() const { return m_public_state.GetValue(); } + lldb::StateType GetPublicState() const { + if (UsesPrivateState()) + return m_private_state.GetValue(); + return m_public_state.GetValue(); + } void SetPublicState(lldb::StateType new_value) { m_public_state.SetValue(new_value); @@ -3257,12 +3262,33 @@ void PruneThreadPlans(); } ProcessRunLock &GetRunLock() { - if (IsOnThread(Host::GetCurrentThread())) + lldb::thread_t curr_thread = Host::GetCurrentThread(); + if (IsOnThread(curr_thread) || UsesPrivateState(curr_thread)) return m_private_run_lock; else return m_public_run_lock; } + void PushUsePrivateState(lldb::thread_t new_thread) { + m_use_private_state_stack.push_back(new_thread); + } + + void PopUsePrivateState() { + // Should we be permissive here? + if (!m_use_private_state_stack.empty()) + m_use_private_state_stack.pop_back(); + } + + bool UsesPrivateState() const { + return UsesPrivateState(Host::GetCurrentThread()); + } + + bool UsesPrivateState(lldb::thread_t thread) const { + if (m_use_private_state_stack.empty()) + return false; + return m_use_private_state_stack.back() == thread; + } + Process &m_process; ///< The process state that we show to client code. This will often diff er ///< from the actual process state, for instance when we've stopped in the @@ -3289,6 +3315,8 @@ void PruneThreadPlans(); ///< This will be the thread name given to the Private State HostThread when ///< it gets spun up. std::string m_thread_name; + + std::deque<lldb::thread_t> m_use_private_state_stack; }; bool SetPrivateRunLockToStopped() { diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index aa64c6fb4ebc1..32b6531785fc1 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -1282,7 +1282,10 @@ void Process::SetPublicState(StateType new_state, bool restarted) { Log *log(GetLog(LLDBLog::State | LLDBLog::Process)); LLDB_LOGF(log, "(plugin = %s, state = %s, restarted = %i)", GetPluginName().data(), StateAsCString(new_state), restarted); - const StateType old_state = GetPublicState(); + // This is one instance where we don't want to obey the "who sees what state" + // decisions, we just want to see what's in the public state field. + const StateType old_state = + m_current_private_state_thread->m_public_state.GetValue(); m_current_private_state_thread->SetPublicState(new_state); // On the transition from Run to Stopped, we unlock the writer end of the run @@ -1296,7 +1299,8 @@ void Process::SetPublicState(StateType new_state, bool restarted) { SetPublicRunLockToStopped(); } else { const bool old_state_is_stopped = StateIsStoppedState(old_state, false); - if ((old_state_is_stopped != new_state_is_stopped)) { + if ((old_state_is_stopped != new_state_is_stopped) || + (new_state_is_stopped && GetRunLock().IsRunning())) { if (new_state_is_stopped && !restarted) { LLDB_LOGF(log, "(plugin = %s, state = %s) -- unlocking run lock", GetPluginName().data(), StateAsCString(new_state)); @@ -3901,7 +3905,7 @@ bool Process::PrivateStateThread::StartupThread() { return true; } -bool Process::PrivateStateThread::IsOnThread(const HostThread &thread) const { +bool Process::PrivateStateThread::IsOnThread(lldb::thread_t thread) const { return m_private_state_thread.EqualsThread(thread); } @@ -4391,7 +4395,14 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr, this_thread_wants_to_stop = stop_info_sp->GetOverriddenShouldStopValue(); } else { + // PerformAction can call arbitrary Python code but we're still in + // ShouldStop so we haven't switched the public state yet. So we need + // to let this thread see the private state: + process_sp->m_current_private_state_thread->PushUsePrivateState( + Host::GetCurrentThread()); stop_info_sp->PerformAction(event_ptr); + process_sp->m_current_private_state_thread->PopUsePrivateState(); + // The stop action might restart the target. If it does, then we // want to mark that in the event so that whoever is receiving it // will know to wait for the running event and reflect that state @@ -4455,9 +4466,6 @@ void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) { if (m_update_state != 1) return; - process_sp->SetPublicState( - m_state, Process::ProcessEventData::GetRestartedFromEvent(event_ptr)); - if (m_state == eStateStopped && !m_restarted) { // Let process subclasses know we are about to do a public stop and do // anything they might need to in order to speed up register and memory @@ -4469,17 +4477,19 @@ void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) { // than a plain interrupt (e.g. we had already stopped for a breakpoint when // the halt request came through) don't do the StopInfo actions, as they may // end up restarting the process. - if (m_interrupted) - return; - - // If we're not stopped or have restarted, then skip the StopInfo actions: - if (m_state != eStateStopped || m_restarted) { + if (m_interrupted || m_state != eStateStopped || m_restarted) { + process_sp->SetPublicState( + m_state, Process::ProcessEventData::GetRestartedFromEvent(event_ptr)); return; } bool does_anybody_have_an_opinion = false; bool still_should_stop = ShouldStop(event_ptr, does_anybody_have_an_opinion); + // And now tell the world about the new state: + process_sp->SetPublicState( + m_state, Process::ProcessEventData::GetRestartedFromEvent(event_ptr)); + if (GetRestarted()) { return; } diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index e9e534a57973e..3affc30b5c338 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -600,7 +600,9 @@ class StopInfoBreakpoint : public StopInfo { Debugger &debugger = thread_sp->CalculateTarget()->GetDebugger(); bool old_async = debugger.GetAsyncExecution(); debugger.SetAsyncExecution(true); - + // The callback will run commands or SB API calls, which need to + // see the public state as stopped, but we haven't set the public + // state yet. callback_says_stop = bp_loc_sp->InvokeCallback(&context); debugger.SetAsyncExecution(old_async); diff --git a/lldb/test/API/python_api/run_locker/TestRunLocker.py b/lldb/test/API/python_api/run_locker/TestRunLocker.py index d525bbf6b406f..07347da4da638 100644 --- a/lldb/test/API/python_api/run_locker/TestRunLocker.py +++ b/lldb/test/API/python_api/run_locker/TestRunLocker.py @@ -1,8 +1,7 @@ """ Test that the run locker really does work to keep us from running SB API that should only be run -while stopped. This test is mostly concerned with -what happens between launch and first stop. +while stopped. """ import lldb @@ -30,21 +29,35 @@ def test_run_locker_stop_at_entry(self): self.build() self.runlocker_test(False) + def test_during_breakpoint_command(self): + """Test that other threads don't see the process as stopped + until we actually finish running the breakpoint callback.""" + self.build() + self.during_breakpoint_command() + def setUp(self): # Call super's setUp(). TestBase.setUp(self) self.main_source_file = lldb.SBFileSpec("main.c") - def runlocker_test(self, stop_at_entry): - """The code to stop at entry handles events slightly diff erently, so - we test both versions of process launch.""" + def start_process(self, flags, bkpt_text): + """Start up an async process, using flags for the launch flags + if it is not None. Also, set a breakpoint before running using + bkpt_text as the source regex. Returns the target, process, listener + and breakpoint.""" target = lldbutil.run_to_breakpoint_make_target(self) + bkpt = None + if bkpt_text: + bkpt = target.BreakpointCreateBySourceRegex( + bkpt_text, self.main_source_file + ) + launch_info = target.GetLaunchInfo() - if stop_at_entry: + if flags: flags = launch_info.GetFlags() - launch_info.SetFlags(flags | lldb.eLaunchFlagStopAtEntry) + launch_info.SetFlags(flags | flags) error = lldb.SBError() # We are trying to do things when the process is running, so @@ -70,20 +83,9 @@ def runlocker_test(self, stop_at_entry): state_type = lldb.SBProcess.GetStateFromEvent(event) self.assertState(state_type, lldb.eStateRunning, "Didn't get a running event") + return (target, process, listener, bkpt) - # We aren't checking the entry state, but just making sure - # the running state is set properly if we continue in this state. - - if stop_at_entry: - event_result = listener.WaitForEvent(10, event) - self.assertTrue(event_result, "Timed out waiting for stop at entry stop") - state_type = lldb.SBProcess.GetStateFromEvent(event) - self.assertState(state_type, eStateStopped, "Stop at entry stopped") - process.Continue() - - # Okay, now the process is running, make sure we can't do things - # you aren't supposed to do while running, and that we get some - # actual error: + def try_expression(self, target): val = target.EvaluateExpression("SomethingToCall()") # There was a bug [#93313] in the printing that would cause repr to crash, so I'm # testing that separately. @@ -110,3 +112,60 @@ def runlocker_test(self, stop_at_entry): self.assertIn( "can't evaluate expressions when the process is running", result.GetOutput() ) + + def runlocker_test(self, stop_at_entry): + """The code to stop at entry handles events slightly diff erently, so we test both versions of process launch.""" + flags = None + + if stop_at_entry: + flags = lldb.eLaunchFlagsStopAtEntry + + target, process, listener, _ = self.start_process(flags, None) + + # We aren't checking the entry state, but just making sure + # the running state is set properly if we continue in this state. + + if stop_at_entry: + event_result = listener.WaitForEvent(10, event) + self.assertTrue(event_result, "Timed out waiting for stop at entry stop") + state_type = lldb.SBProcess.GetStateFromEvent(event) + self.assertState(state_type, eStateStopped, "Stop at entry stopped") + process.Continue() + + # Okay, now the process is running, make sure we can't do things + # you aren't supposed to do while running, and that we get some + # actual error: + self.try_expression(target) + + def during_breakpoint_command(self): + target, process, listener, bkpt = self.start_process(None, "sleep.1.") + # The process should be stopped at our breakpoint, wait for that. + event = lldb.SBEvent() + result = listener.WaitForEvent(10, event) + self.assertTrue(event.IsValid(), "Didn't time out waiting for breakpoint") + state_type = lldb.SBProcess.GetStateFromEvent(event) + self.assertState(state_type, lldb.eStateStopped, "Didn't get a stopped event") + + # Now add a breakpoint callback that will stall for a while, and we'll wait a + # much shorter interval and each time we wake up ensure that we still see the + # process as running, and can't do things we aren't allowed to in that state. + commands = ( + "import time;print('About to sleep');time.sleep(20);print('Done sleeping')" + ) + + bkpt.SetScriptCallbackBody(commands) + + process.Continue() + result = listener.WaitForEvent(10, event) + state_type = lldb.SBProcess.GetStateFromEvent(event) + self.assertState(state_type, lldb.eStateRunning, "We started running") + counter = 0 + while state_type == lldb.eStateRunning: + print("About to wait") + result = listener.GetNextEvent(event) + counter += 1 + print(f"Woke up {counter} times") + if not result: + self.try_expression(target) + else: + state_type = lldb.SBProcess.GetStateFromEvent(event) _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
