https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/90930
>From b72df8cf2a047ed731913609b58bdb4a3601e111 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Thu, 2 May 2024 18:12:04 -0700 Subject: [PATCH 1/5] Single thread timeout feature --- lldb/include/lldb/Target/Process.h | 11 +- lldb/include/lldb/Target/StopInfo.h | 4 + lldb/include/lldb/Target/Thread.h | 2 + lldb/include/lldb/Target/ThreadPlan.h | 8 +- .../Target/ThreadPlanSingleThreadTimeout.h | 89 ++++++++ lldb/include/lldb/Target/ThreadPlanStepOut.h | 1 + .../lldb/Target/ThreadPlanStepOverRange.h | 5 + lldb/include/lldb/lldb-enumerations.h | 1 + lldb/source/API/SBThread.cpp | 6 + .../source/Interpreter/CommandInterpreter.cpp | 2 +- .../GDBRemoteCommunicationServerLLGS.cpp | 2 + .../Process/gdb-remote/ProcessGDBRemote.cpp | 61 +++++- .../Process/gdb-remote/ProcessGDBRemote.h | 6 + lldb/source/Target/CMakeLists.txt | 1 + lldb/source/Target/Process.cpp | 23 +- lldb/source/Target/StopInfo.cpp | 37 ++++ lldb/source/Target/TargetProperties.td | 4 + lldb/source/Target/Thread.cpp | 9 +- lldb/source/Target/ThreadPlan.cpp | 1 + .../Target/ThreadPlanSingleThreadTimeout.cpp | 205 ++++++++++++++++++ lldb/source/Target/ThreadPlanStepInRange.cpp | 2 + .../source/Target/ThreadPlanStepOverRange.cpp | 17 +- lldb/source/Target/ThreadPlanStepRange.cpp | 9 +- .../single-thread-step/Makefile | 4 + .../TestSingleThreadStepTimeout.py | 123 +++++++++++ .../single-thread-step/main.cpp | 62 ++++++ lldb/tools/lldb-dap/JSONUtils.cpp | 3 + lldb/tools/lldb-dap/LLDBUtils.cpp | 1 + 28 files changed, 671 insertions(+), 28 deletions(-) create mode 100644 lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h create mode 100644 lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp create mode 100644 lldb/test/API/functionalities/single-thread-step/Makefile create mode 100644 lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py create mode 100644 lldb/test/API/functionalities/single-thread-step/main.cpp diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index aac0cf51680a9..7e758dbb9f645 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1312,11 +1312,13 @@ class Process : public std::enable_shared_from_this<Process>, size_t GetThreadStatus(Stream &ostrm, bool only_threads_with_stop_reason, uint32_t start_frame, uint32_t num_frames, - uint32_t num_frames_with_source, - bool stop_format); + uint32_t num_frames_with_source, bool stop_format); void SendAsyncInterrupt(); + // Send an async interrupt and receive stop from a specific /p thread. + void SendAsyncInterrupt(Thread *thread); + // Notify this process class that modules got loaded. // // If subclasses override this method, they must call this version before @@ -3102,6 +3104,11 @@ void PruneThreadPlans(); // Resume will only request a resume, using this // flag to check. + lldb::tid_t m_interrupt_tid; /// The tid of the thread that issued the async + /// interrupt, used by thread plan timeout. It + /// can be LLDB_INVALID_THREAD_ID to indicate + /// user level async interrupt. + /// This is set at the beginning of Process::Finalize() to stop functions /// from looking up or creating things during or after a finalize call. std::atomic<bool> m_finalizing; diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h index d1848fcbbbdb1..fae90364deaf0 100644 --- a/lldb/include/lldb/Target/StopInfo.h +++ b/lldb/include/lldb/Target/StopInfo.h @@ -123,6 +123,10 @@ class StopInfo : public std::enable_shared_from_this<StopInfo> { const char *description = nullptr, std::optional<int> code = std::nullopt); + static lldb::StopInfoSP + CreateStopReasonWithInterrupt(Thread &thread, int signo, + const char *description); + static lldb::StopInfoSP CreateStopReasonToTrace(Thread &thread); static lldb::StopInfoSP diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..584093348b4c6 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -57,6 +57,8 @@ class ThreadProperties : public Properties { bool GetStepOutAvoidsNoDebug() const; uint64_t GetMaxBacktraceDepth() const; + + uint64_t GetSingleThreadPlanTimeout() const; }; class Thread : public std::enable_shared_from_this<Thread>, diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h index bf68a42e54d18..640e997caf7b3 100644 --- a/lldb/include/lldb/Target/ThreadPlan.h +++ b/lldb/include/lldb/Target/ThreadPlan.h @@ -302,7 +302,8 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>, eKindStepInRange, eKindRunToAddress, eKindStepThrough, - eKindStepUntil + eKindStepUntil, + eKindSingleThreadTimeout, }; virtual ~ThreadPlan(); @@ -483,6 +484,8 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>, return m_takes_iteration_count; } + virtual lldb::StateType GetPlanRunState() = 0; + protected: // Constructors and Destructors ThreadPlan(ThreadPlanKind kind, const char *name, Thread &thread, @@ -522,8 +525,6 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>, GetThread().SetStopInfo(stop_reason_sp); } - virtual lldb::StateType GetPlanRunState() = 0; - bool IsUsuallyUnexplainedStopReason(lldb::StopReason); Status m_status; @@ -590,6 +591,7 @@ class ThreadPlanNull : public ThreadPlan { const Status &GetStatus() { return m_status; } protected: + friend class ThreadPlanSingleThreadTimeout; bool DoPlanExplainsStop(Event *event_ptr) override; lldb::StateType GetPlanRunState() override; diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h new file mode 100644 index 0000000000000..777ed55fbae26 --- /dev/null +++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h @@ -0,0 +1,89 @@ +//===-- ThreadPlanSingleThreadTimeout.h -------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TARGET_THREADPLANSINGLETHREADTIMEOUT_H +#define LLDB_TARGET_THREADPLANSINGLETHREADTIMEOUT_H + +#include "lldb/Target/Thread.h" +#include "lldb/Target/ThreadPlan.h" +#include "lldb/Utility/Event.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/State.h" + +#include <thread> + +namespace lldb_private { + +// +// Thread plan used by single thread execution to issue timeout. This is useful +// to detect potential deadlock in single thread execution. The timeout measures +// the elapsed time from the last internal stop and got reset by each internal +// stops to ensure we are accurately detecting execution not moving forward. +// This means this thread plan can be created/destroyed multiple times by the +// parent execution plan. +// +// When timeout happens, the thread plan resolves the potential deadlock by +// issuing a thread specific async interrupt to enter stop state, then all +// threads execution are resumed to resolve the potential deadlock. +// +class ThreadPlanSingleThreadTimeout : public ThreadPlan { +public: + ~ThreadPlanSingleThreadTimeout() override; + + static void ResetIfNeeded(Thread &thread); + + void GetDescription(Stream *s, lldb::DescriptionLevel level) override; + bool ValidatePlan(Stream *error) override { return true; } + bool WillStop() override; + void DidPop() override; + + bool DoPlanExplainsStop(Event *event_ptr) override; + + lldb::StateType GetPlanRunState() override; + static void TimeoutThreadFunc(ThreadPlanSingleThreadTimeout *self); + + bool MischiefManaged() override; + + bool ShouldStop(Event *event_ptr) override; + void SetStopOthers(bool new_value) override; + bool StopOthers() override; + +private: + ThreadPlanSingleThreadTimeout(Thread &thread); + + static bool IsAlive() { return s_instance != nullptr; } + + enum class State { + WaitTimeout, // Waiting for timeout. + AsyncInterrupt, // Async interrupt has been issued. + Done, // Finished resume all threads. + }; + + static ThreadPlanSingleThreadTimeout *s_instance; + static State s_prev_state; + + bool HandleEvent(Event *event_ptr); + void HandleTimeout(); + + static std::string StateToString(State state); + + ThreadPlanSingleThreadTimeout(const ThreadPlanSingleThreadTimeout &) = delete; + const ThreadPlanSingleThreadTimeout & + operator=(const ThreadPlanSingleThreadTimeout &) = delete; + + State m_state; + std::mutex m_mutex; + std::condition_variable m_wakeup_cv; + // Whether the timer thread should exit or not. + bool m_exit_flag; + std::thread m_timer_thread; +}; + +} // namespace lldb_private + +#endif // LLDB_TARGET_THREADPLANSINGLETHREADTIMEOUT_H diff --git a/lldb/include/lldb/Target/ThreadPlanStepOut.h b/lldb/include/lldb/Target/ThreadPlanStepOut.h index b1d8769f7c546..013c675afc33d 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOut.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOut.h @@ -30,6 +30,7 @@ class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere { bool ValidatePlan(Stream *error) override; bool ShouldStop(Event *event_ptr) override; bool StopOthers() override; + void SetStopOthers(bool new_value) override { m_stop_others = new_value; } lldb::StateType GetPlanRunState() override; bool WillStop() override; bool MischiefManaged() override; diff --git a/lldb/include/lldb/Target/ThreadPlanStepOverRange.h b/lldb/include/lldb/Target/ThreadPlanStepOverRange.h index 8585ac62f09b3..6b9fdadf0cbd9 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOverRange.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOverRange.h @@ -27,6 +27,7 @@ class ThreadPlanStepOverRange : public ThreadPlanStepRange, ~ThreadPlanStepOverRange() override; void GetDescription(Stream *s, lldb::DescriptionLevel level) override; + void SetStopOthers(bool new_value) override; bool ShouldStop(Event *event_ptr) override; protected: @@ -42,8 +43,12 @@ class ThreadPlanStepOverRange : public ThreadPlanStepRange, void SetupAvoidNoDebug(LazyBool step_out_avoids_code_without_debug_info); bool IsEquivalentContext(const SymbolContext &context); + // Clear and create a new ThreadPlanSingleThreadTimeout to detect if program + // is moving forward. + void ResetSingleThreadTimeout(); bool m_first_resume; + lldb::RunMode m_run_mode; ThreadPlanStepOverRange(const ThreadPlanStepOverRange &) = delete; const ThreadPlanStepOverRange & diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index 15e4585718609..2000931913c81 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -253,6 +253,7 @@ enum StopReason { eStopReasonFork, eStopReasonVFork, eStopReasonVForkDone, + eStopReasonInterrupt, ///< Thread requested interrupt }; /// Command Return Status Types. diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp index ac3e2cd25daa9..c6925096e3cb1 100644 --- a/lldb/source/API/SBThread.cpp +++ b/lldb/source/API/SBThread.cpp @@ -192,6 +192,9 @@ size_t SBThread::GetStopReasonDataCount() { case eStopReasonSignal: return 1; + case eStopReasonInterrupt: + return 1; + case eStopReasonException: return 1; @@ -261,6 +264,9 @@ uint64_t SBThread::GetStopReasonDataAtIndex(uint32_t idx) { case eStopReasonSignal: return stop_info_sp->GetValue(); + case eStopReasonInterrupt: + return stop_info_sp->GetValue(); + case eStopReasonException: return stop_info_sp->GetValue(); diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 4c58ecc3c1848..d327822db7bd3 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -2500,7 +2500,7 @@ bool CommandInterpreter::DidProcessStopAbnormally() const { const StopReason reason = stop_info->GetStopReason(); if (reason == eStopReasonException || reason == eStopReasonInstrumentation || - reason == eStopReasonProcessorTrace) + reason == eStopReasonProcessorTrace || reason == eStopReasonInterrupt) return true; if (reason == eStopReasonSignal) { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index ae1a77e5be832..b3c0359b7dcf7 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -714,6 +714,8 @@ static const char *GetStopReasonString(StopReason stop_reason) { return "vfork"; case eStopReasonVForkDone: return "vforkdone"; + case eStopReasonInterrupt: + return "async interrupt"; case eStopReasonInstrumentation: case eStopReasonInvalid: case eStopReasonPlanComplete: diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 871683a605686..b93aad8808f5b 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1730,14 +1730,24 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo( thread_sp = memory_thread_sp; if (exc_type != 0) { - const size_t exc_data_size = exc_data.size(); - - thread_sp->SetStopInfo( - StopInfoMachException::CreateStopReasonWithMachException( - *thread_sp, exc_type, exc_data_size, - exc_data_size >= 1 ? exc_data[0] : 0, - exc_data_size >= 2 ? exc_data[1] : 0, - exc_data_size >= 3 ? exc_data[2] : 0)); + // For thread plan async interrupt, creating stop info on the + // original async interrupt request thread instead. If interrupt thread + // does not exist anymore we fallback to current signal receiving thread + // instead. + ThreadSP interrupt_thread; + if (m_interrupt_tid != LLDB_INVALID_THREAD_ID) + interrupt_thread = HandleThreadAsyncInterrupt(signo, description); + if (interrupt_thread) + thread_sp = interrupt_thread; + else { + const size_t exc_data_size = exc_data.size(); + thread_sp->SetStopInfo( + StopInfoMachException::CreateStopReasonWithMachException( + *thread_sp, exc_type, exc_data_size, + exc_data_size >= 1 ? exc_data[0] : 0, + exc_data_size >= 2 ? exc_data[1] : 0, + exc_data_size >= 3 ? exc_data[2] : 0)); + } } else { bool handled = false; bool did_exec = false; @@ -1936,9 +1946,20 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo( *thread_sp, signo, description.c_str())); } } - if (!handled) - thread_sp->SetStopInfo(StopInfo::CreateStopReasonWithSignal( - *thread_sp, signo, description.c_str())); + if (!handled) { + // For thread plan async interrupt, creating stop info on the + // original async interrupt request thread instead. If interrupt + // thread does not exist anymore we fallback to current signal + // receiving thread instead. + ThreadSP interrupt_thread; + if (m_interrupt_tid != LLDB_INVALID_THREAD_ID) + interrupt_thread = HandleThreadAsyncInterrupt(signo, description); + if (interrupt_thread) + thread_sp = interrupt_thread; + else + thread_sp->SetStopInfo(StopInfo::CreateStopReasonWithSignal( + *thread_sp, signo, description.c_str())); + } } if (!description.empty()) { @@ -1957,6 +1978,24 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo( return thread_sp; } +ThreadSP +ProcessGDBRemote::HandleThreadAsyncInterrupt(uint8_t signo, + const std::string &description) { + ThreadSP thread_sp; + { + std::lock_guard<std::recursive_mutex> guard(m_thread_list_real.GetMutex()); + thread_sp = m_thread_list_real.FindThreadByProtocolID(m_interrupt_tid, + /*can_update=*/false); + } + if (thread_sp) + thread_sp->SetStopInfo(StopInfo::CreateStopReasonWithInterrupt( + *thread_sp, signo, description.c_str())); + // Clear m_interrupt_tid regardless we can find original interrupt thread or + // not. + m_interrupt_tid = LLDB_INVALID_THREAD_ID; + return thread_sp; +} + lldb::ThreadSP ProcessGDBRemote::SetThreadStopInfo(StructuredData::Dictionary *thread_dict) { static constexpr llvm::StringLiteral g_key_tid("tid"); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 610a1ee0b34d2..615831dd7e6f6 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -440,6 +440,12 @@ class ProcessGDBRemote : public Process, void HandleStopReply() override; void HandleAsyncStructuredDataPacket(llvm::StringRef data) override; + // Handle thread specific async interrupt and return the original thread + // that requested the async interrupt. It can be null if original thread + // has exited. + lldb::ThreadSP HandleThreadAsyncInterrupt(uint8_t signo, + const std::string &description); + void SetThreadPc(const lldb::ThreadSP &thread_sp, uint64_t index); using ModuleCacheKey = std::pair<std::string, std::string>; // KeyInfo for the cached module spec DenseMap. diff --git a/lldb/source/Target/CMakeLists.txt b/lldb/source/Target/CMakeLists.txt index cf4818eae3eb8..d1209d29245d5 100644 --- a/lldb/source/Target/CMakeLists.txt +++ b/lldb/source/Target/CMakeLists.txt @@ -59,6 +59,7 @@ add_lldb_library(lldbTarget ThreadPlanCallUserExpression.cpp ThreadPlanPython.cpp ThreadPlanRunToAddress.cpp + ThreadPlanSingleThreadTimeout.cpp ThreadPlanShouldStopHere.cpp ThreadPlanStepInRange.cpp ThreadPlanStepInstruction.cpp diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 30c240b064b59..3e5e4fe58b4b3 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -446,7 +446,8 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP listener_sp, m_memory_cache(*this), m_allocated_memory_cache(*this), m_should_detach(false), m_next_event_action_up(), m_public_run_lock(), m_private_run_lock(), m_currently_handling_do_on_removals(false), - m_resume_requested(false), m_finalizing(false), m_destructing(false), + m_resume_requested(false), m_interrupt_tid(LLDB_INVALID_THREAD_ID), + m_finalizing(false), m_destructing(false), m_clear_thread_plans_on_stop(false), m_force_next_event_delivery(false), m_last_broadcast_state(eStateInvalid), m_destroy_in_process(false), m_can_interpret_function_calls(false), m_run_thread_plan_lock(), @@ -863,6 +864,7 @@ bool Process::HandleProcessStateChangedEvent( case eStopReasonThreadExiting: case eStopReasonInstrumentation: case eStopReasonProcessorTrace: + case eStopReasonInterrupt: if (!other_thread) other_thread = thread; break; @@ -3679,7 +3681,13 @@ void Process::ControlPrivateStateThread(uint32_t signal) { } } -void Process::SendAsyncInterrupt() { +void Process::SendAsyncInterrupt() { SendAsyncInterrupt(nullptr); } + +void Process::SendAsyncInterrupt(Thread *thread) { + if (thread != nullptr) + m_interrupt_tid = thread->GetProtocolID(); + else + m_interrupt_tid = LLDB_INVALID_THREAD_ID; if (PrivateStateThreadIsValid()) m_private_state_broadcaster.BroadcastEvent(Process::eBroadcastBitInterrupt, nullptr); @@ -3905,9 +3913,14 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { if (interrupt_requested) { if (StateIsStoppedState(internal_state, true)) { - // We requested the interrupt, so mark this as such in the stop event - // so clients can tell an interrupted process from a natural stop - ProcessEventData::SetInterruptedInEvent(event_sp.get(), true); + // Oly mark interrupt event if it is not thread specific async + // interrupt. + if (m_interrupt_tid == LLDB_INVALID_THREAD_ID) { + // We requested the interrupt, so mark this as such in the stop + // event so clients can tell an interrupted process from a natural + // stop + ProcessEventData::SetInterruptedInEvent(event_sp.get(), true); + } interrupt_requested = false; } else if (log) { LLDB_LOGF(log, diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index 95f78056b1644..936d49cc312d7 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -1125,6 +1125,38 @@ class StopInfoUnixSignal : public StopInfo { std::optional<int> m_code; }; +// StopInfoInterrupt + +class StopInfoInterrupt : public StopInfo { +public: + StopInfoInterrupt(Thread &thread, int signo, const char *description) + : StopInfo(thread, signo) { + SetDescription(description); + } + + ~StopInfoInterrupt() override = default; + + StopReason GetStopReason() const override { + return lldb::eStopReasonInterrupt; + } + + bool ShouldStopSynchronous(Event *event_ptr) override { return true; } + + bool ShouldStop(Event *event_ptr) override { + ThreadSP thread_sp(m_thread_wp.lock()); + if (thread_sp) + return thread_sp->GetProcess()->GetUnixSignals()->GetShouldStop(m_value); + return false; + } + + const char *GetDescription() override { + if (m_description.empty()) { + m_description = "async interrupt"; + } + return m_description.c_str(); + } +}; + // StopInfoTrace class StopInfoTrace : public StopInfo { @@ -1390,6 +1422,11 @@ StopInfoSP StopInfo::CreateStopReasonWithSignal(Thread &thread, int signo, return StopInfoSP(new StopInfoUnixSignal(thread, signo, description, code)); } +StopInfoSP StopInfo::CreateStopReasonWithInterrupt(Thread &thread, int signo, + const char *description) { + return StopInfoSP(new StopInfoInterrupt(thread, signo, description)); +} + StopInfoSP StopInfo::CreateStopReasonToTrace(Thread &thread) { return StopInfoSP(new StopInfoTrace(thread)); } diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td index 7f79218e0a6a4..1e08d352a6e1c 100644 --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -310,6 +310,10 @@ let Definition = "thread" in { def MaxBacktraceDepth: Property<"max-backtrace-depth", "UInt64">, DefaultUnsignedValue<600000>, Desc<"Maximum number of frames to backtrace.">; + def SingleThreadPlanTimeout: Property<"single-thread-plan-timeout", "UInt64">, + Global, + DefaultUnsignedValue<1000>, + Desc<"The time in milliseconds to wait for single thread ThreadPlan to move forward before resuming all threads to resolve any potential deadlock.">; } let Definition = "language" in { diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index e75f5a356cec2..7b4988f795504 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -143,6 +143,12 @@ uint64_t ThreadProperties::GetMaxBacktraceDepth() const { idx, g_thread_properties[idx].default_uint_value); } +uint64_t ThreadProperties::GetSingleThreadPlanTimeout() const { + const uint32_t idx = ePropertySingleThreadPlanTimeout; + return GetPropertyAtIndexAs<uint64_t>( + idx, g_thread_properties[idx].default_uint_value); +} + // Thread Event Data llvm::StringRef Thread::ThreadEventData::GetFlavorString() { @@ -813,7 +819,6 @@ bool Thread::ShouldStop(Event *event_ptr) { // decide whether they still need to do more work. bool done_processing_current_plan = false; - if (!current_plan->PlanExplainsStop(event_ptr)) { if (current_plan->TracerExplainsStop()) { done_processing_current_plan = true; @@ -1715,6 +1720,8 @@ std::string Thread::StopReasonAsString(lldb::StopReason reason) { return "instrumentation break"; case eStopReasonProcessorTrace: return "processor trace"; + case eStopReasonInterrupt: + return "async interrupt"; } return "StopReason = " + std::to_string(reason); diff --git a/lldb/source/Target/ThreadPlan.cpp b/lldb/source/Target/ThreadPlan.cpp index 7927fc3140145..f05e1faf343a6 100644 --- a/lldb/source/Target/ThreadPlan.cpp +++ b/lldb/source/Target/ThreadPlan.cpp @@ -174,6 +174,7 @@ bool ThreadPlan::IsUsuallyUnexplainedStopReason(lldb::StopReason reason) { case eStopReasonFork: case eStopReasonVFork: case eStopReasonVForkDone: + case eStopReasonInterrupt: return true; default: return false; diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp new file mode 100644 index 0000000000000..11575d347105e --- /dev/null +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -0,0 +1,205 @@ +//===-- ThreadPlanStepOverRange.cpp ---------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Target/ThreadPlanSingleThreadTimeout.h" +#include "lldb/Symbol/Block.h" +#include "lldb/Symbol/CompileUnit.h" +#include "lldb/Symbol/Function.h" +#include "lldb/Symbol/LineTable.h" +#include "lldb/Target/Process.h" +#include "lldb/Target/RegisterContext.h" +#include "lldb/Target/Target.h" +#include "lldb/Target/Thread.h" +#include "lldb/Target/ThreadPlanStepOut.h" +#include "lldb/Target/ThreadPlanStepThrough.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" +#include "lldb/Utility/Stream.h" + +using namespace lldb_private; +using namespace lldb; + +ThreadPlanSingleThreadTimeout *ThreadPlanSingleThreadTimeout::s_instance = + nullptr; +ThreadPlanSingleThreadTimeout::State + ThreadPlanSingleThreadTimeout::s_prev_state = State::WaitTimeout; + +ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread) + : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout", + thread, eVoteNo, eVoteNoOpinion), + m_state(State::WaitTimeout), m_exit_flag(false) { + m_timer_thread = std::thread(TimeoutThreadFunc, this); + s_instance = this; + m_state = s_prev_state; +} + +ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() { + std::lock_guard<std::mutex> lock(m_mutex); + s_instance = nullptr; + if (m_state == State::Done) + m_state = State::WaitTimeout; + s_prev_state = m_state; +} + +void ThreadPlanSingleThreadTimeout::GetDescription( + Stream *s, lldb::DescriptionLevel level) { + s->Printf("Single thread timeout, state(%s)", StateToString(m_state).c_str()); +} + +std::string ThreadPlanSingleThreadTimeout::StateToString(State state) { + switch (state) { + case State::WaitTimeout: + return "WaitTimeout"; + case State::AsyncInterrupt: + return "AsyncInterrupt"; + case State::Done: + return "Done"; + } +} + +void ThreadPlanSingleThreadTimeout::ResetIfNeeded(Thread &thread) { + uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); + if (timeout_in_ms == 0) + return; + + // TODO: mutex? + if (ThreadPlanSingleThreadTimeout::IsAlive()) + return; + + // Do not create timeout if we are not stopping other threads. + if (!thread.GetCurrentPlan()->StopOthers()) + return; + + auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread); + ThreadPlanSP thread_plan_sp(timeout_plan); + auto status = thread.QueueThreadPlan(thread_plan_sp, + /*abort_other_plans*/ false); + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout pushing a new one"); +} + +bool ThreadPlanSingleThreadTimeout::WillStop() { + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::WillStop()."); + + // Reset the state during stop. + std::lock_guard<std::mutex> lock(m_mutex); + s_prev_state = State::WaitTimeout; + return true; +} + +void ThreadPlanSingleThreadTimeout::DidPop() { + Log *log = GetLog(LLDBLog::Step); + { + std::lock_guard<std::mutex> lock(m_mutex); + LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::DidPop()."); + // Tell timer thread to exit. + m_exit_flag = true; + } + m_wakeup_cv.notify_one(); + // Wait for timer thread to exit. + m_timer_thread.join(); +} + +bool ThreadPlanSingleThreadTimeout::DoPlanExplainsStop(Event *event_ptr) { + lldb::StateType stop_state = + Process::ProcessEventData::GetStateFromEvent(event_ptr); + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF( + log, + "ThreadPlanSingleThreadTimeout::DoPlanExplainsStop(): got event: %s.", + StateAsCString(stop_state)); + return true; +} + +lldb::StateType ThreadPlanSingleThreadTimeout::GetPlanRunState() { + return GetPreviousPlan()->GetPlanRunState(); +} + +void ThreadPlanSingleThreadTimeout::TimeoutThreadFunc( + ThreadPlanSingleThreadTimeout *self) { + std::unique_lock<std::mutex> lock(self->m_mutex); + uint64_t timeout_in_ms = self->GetThread().GetSingleThreadPlanTimeout(); + self->m_wakeup_cv.wait_for(lock, std::chrono::milliseconds(timeout_in_ms), + [self] { return self->m_exit_flag; }); + + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF(log, + "ThreadPlanSingleThreadTimeout::TimeoutThreadFunc() called with " + "m_exit_flag(%d).", + self->m_exit_flag); + if (self->m_exit_flag) + return; + + self->HandleTimeout(); +} + +bool ThreadPlanSingleThreadTimeout::MischiefManaged() { + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::MischiefManaged() called."); + // Need to reset timer on each internal stop/execution progress. + return true; +} + +bool ThreadPlanSingleThreadTimeout::ShouldStop(Event *event_ptr) { + return HandleEvent(event_ptr); +} + +void ThreadPlanSingleThreadTimeout::SetStopOthers(bool new_value) { + GetPreviousPlan()->SetStopOthers(new_value); +} + +bool ThreadPlanSingleThreadTimeout::StopOthers() { + if (m_state == State::Done) + return false; + else + return GetPreviousPlan()->StopOthers(); +} + +bool ThreadPlanSingleThreadTimeout::HandleEvent(Event *event_ptr) { + lldb::StateType stop_state = + Process::ProcessEventData::GetStateFromEvent(event_ptr); + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::HandleEvent(): got event: %s.", + StateAsCString(stop_state)); + + lldb::StopInfoSP stop_info = GetThread().GetStopInfo(); + if (m_state == State::AsyncInterrupt && stop_state == lldb::eStateStopped && + stop_info && stop_info->GetStopReason() == lldb::eStopReasonInterrupt) { + if (Process::ProcessEventData::GetRestartedFromEvent(event_ptr)) { + // If we were restarted, we just need to go back up to fetch + // another event. + LLDB_LOGF(log, + "ThreadPlanSingleThreadTimeout::HandleEvent(): Got a stop and " + "restart, so we'll continue waiting."); + + } else { + LLDB_LOGF( + log, + "ThreadPlanSingleThreadTimeout::HandleEvent(): Got async interrupt " + ", so we will resume all threads."); + GetThread().GetCurrentPlan()->SetStopOthers(false); + GetPreviousPlan()->SetStopOthers(false); + m_state = State::Done; + } + } + // Should not report stop. + return false; +} + +void ThreadPlanSingleThreadTimeout::HandleTimeout() { + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF( + log, + "ThreadPlanSingleThreadTimeout::HandleTimeout() send async interrupt."); + m_state = State::AsyncInterrupt; + + // Private state thread will only send async interrupt + // in running state so no need to check state here. + m_process.SendAsyncInterrupt(&GetThread()); +} diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp index 17f2100b804fd..0c8b92f415420 100644 --- a/lldb/source/Target/ThreadPlanStepInRange.cpp +++ b/lldb/source/Target/ThreadPlanStepInRange.cpp @@ -134,6 +134,8 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) { GetTarget().GetArchitecture().GetAddressByteSize()); LLDB_LOGF(log, "ThreadPlanStepInRange reached %s.", s.GetData()); } + if (DoPlanExplainsStop(event_ptr)) + ClearNextBranchBreakpoint(); if (IsPlanComplete()) return true; diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp index 84f282f1de520..6d24b5d9d7f36 100644 --- a/lldb/source/Target/ThreadPlanStepOverRange.cpp +++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp @@ -15,6 +15,7 @@ #include "lldb/Target/RegisterContext.h" #include "lldb/Target/Target.h" #include "lldb/Target/Thread.h" +#include "lldb/Target/ThreadPlanSingleThreadTimeout.h" #include "lldb/Target/ThreadPlanStepOut.h" #include "lldb/Target/ThreadPlanStepThrough.h" #include "lldb/Utility/LLDBLog.h" @@ -36,7 +37,8 @@ ThreadPlanStepOverRange::ThreadPlanStepOverRange( : ThreadPlanStepRange(ThreadPlan::eKindStepOverRange, "Step range stepping over", thread, range, addr_context, stop_others), - ThreadPlanShouldStopHere(this), m_first_resume(true) { + ThreadPlanShouldStopHere(this), m_first_resume(true), + m_run_mode(stop_others) { SetFlagsToDefault(); SetupAvoidNoDebug(step_out_avoids_code_without_debug_info); } @@ -124,6 +126,11 @@ bool ThreadPlanStepOverRange::IsEquivalentContext( return m_addr_context.symbol && m_addr_context.symbol == context.symbol; } +void ThreadPlanStepOverRange::SetStopOthers(bool stop_others) { + if (!stop_others) + m_stop_others = RunMode::eAllThreads; +} + bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { Log *log = GetLog(LLDBLog::Step); Thread &thread = GetThread(); @@ -135,12 +142,17 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { LLDB_LOGF(log, "ThreadPlanStepOverRange reached %s.", s.GetData()); } + if (DoPlanExplainsStop(event_ptr)) + ClearNextBranchBreakpoint(); + // If we're out of the range but in the same frame or in our caller's frame // then we should stop. When stepping out we only stop others if we are // forcing running one thread. bool stop_others = (m_stop_others == lldb::eOnlyThisThread); ThreadPlanSP new_plan_sp; FrameComparison frame_order = CompareCurrentFrameToStartFrame(); + LLDB_LOGF(log, "ThreadPlanStepOverRange compare frame result: %d.", + frame_order); if (frame_order == eFrameCompareOlder) { // If we're in an older frame then we should stop. @@ -411,6 +423,7 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType resume_state, } } } - + if (m_run_mode == lldb::eOnlyThisThread) + ThreadPlanSingleThreadTimeout::ResetIfNeeded(GetThread()); return true; } diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp index 998e76cb65d13..ef66bf2ff2514 100644 --- a/lldb/source/Target/ThreadPlanStepRange.cpp +++ b/lldb/source/Target/ThreadPlanStepRange.cpp @@ -346,7 +346,9 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() { run_to_address = instructions->GetInstructionAtIndex(branch_index)->GetAddress(); } - + if (branch_index == pc_index) + LLDB_LOGF(log, "ThreadPlanStepRange::SetNextBranchBreakpoint - skipping " + "because current is branch instruction"); if (run_to_address.IsValid()) { const bool is_internal = true; m_next_branch_bp_sp = @@ -380,7 +382,9 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() { return true; } else return false; - } + } else + LLDB_LOGF(log, "ThreadPlanStepRange::SetNextBranchBreakpoint - skipping " + "invalid run_to_address"); } return false; } @@ -417,7 +421,6 @@ bool ThreadPlanStepRange::NextRangeBreakpointExplainsStop( "next range breakpoint which has %" PRIu64 " constituents - explains stop: %u.", (uint64_t)num_constituents, explains_stop); - ClearNextBranchBreakpoint(); return explains_stop; } } diff --git a/lldb/test/API/functionalities/single-thread-step/Makefile b/lldb/test/API/functionalities/single-thread-step/Makefile new file mode 100644 index 0000000000000..de4ec12b13cbc --- /dev/null +++ b/lldb/test/API/functionalities/single-thread-step/Makefile @@ -0,0 +1,4 @@ +ENABLE_THREADS := YES +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py new file mode 100644 index 0000000000000..f734b4bbf116e --- /dev/null +++ b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py @@ -0,0 +1,123 @@ +""" +Test that single thread step over deadlock issue can be resolved +after timeout. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class SingleThreadStepTimeoutTestCase(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def setUp(self): + TestBase.setUp(self) + self.main_source = "main.cpp" + self.build() + + def verify_hit_correct_line(self, pattern): + target_line = line_number(self.main_source, pattern) + self.assertNotEqual(target_line, 0, "Could not find source pattern " + pattern) + cur_line = self.thread.frames[0].GetLineEntry().GetLine() + self.assertEqual( + cur_line, + target_line, + "Stepped to line %d instead of expected %d with pattern '%s'." + % (cur_line, target_line, pattern), + ) + + def step_over_deadlock_helper(self): + (target, _, self.thread, _) = lldbutil.run_to_source_breakpoint( + self, "// Set breakpoint1 here", lldb.SBFileSpec(self.main_source) + ) + + signal_main_thread_value = target.FindFirstGlobalVariable("signal_main_thread") + self.assertTrue(signal_main_thread_value.IsValid()) + + # Change signal_main_thread global variable to 1 so that worker thread loop can + # terminate and move forward to signal main thread + signal_main_thread_value.SetValueFromCString("1") + + self.thread.StepOver(lldb.eOnlyThisThread) + self.verify_hit_correct_line("// Finish step-over from breakpoint1") + + @skipIfWindows + def test_step_over_deadlock_small_timeout_fast_stepping(self): + """Test single thread step over deadlock on other threads can be resolved after timeout with small timeout and fast stepping.""" + self.dbg.HandleCommand( + "settings set target.process.thread.single-thread-plan-timeout 10" + ) + self.dbg.HandleCommand("settings set target.use-fast-stepping true") + self.step_over_deadlock_helper() + + @skipIfWindows + def test_step_over_deadlock_small_timeout_slow_stepping(self): + """Test single thread step over deadlock on other threads can be resolved after timeout with small timeout and slow stepping.""" + self.dbg.HandleCommand( + "settings set target.process.thread.single-thread-plan-timeout 10" + ) + self.dbg.HandleCommand("settings set target.use-fast-stepping false") + self.step_over_deadlock_helper() + + @skipIfWindows + def test_step_over_deadlock_large_timeout_fast_stepping(self): + """Test single thread step over deadlock on other threads can be resolved after timeout with large timeout and fast stepping.""" + self.dbg.HandleCommand( + "settings set target.process.thread.single-thread-plan-timeout 2000" + ) + self.dbg.HandleCommand("settings set target.use-fast-stepping true") + self.step_over_deadlock_helper() + + @skipIfWindows + def test_step_over_deadlock_large_timeout_slow_stepping(self): + """Test single thread step over deadlock on other threads can be resolved after timeout with large timeout and slow stepping.""" + self.dbg.HandleCommand( + "settings set target.process.thread.single-thread-plan-timeout 2000" + ) + self.dbg.HandleCommand("settings set target.use-fast-stepping false") + self.step_over_deadlock_helper() + + def step_over_multi_calls_helper(self): + (target, _, self.thread, _) = lldbutil.run_to_source_breakpoint( + self, "// Set breakpoint2 here", lldb.SBFileSpec(self.main_source) + ) + self.thread.StepOver(lldb.eOnlyThisThread) + self.verify_hit_correct_line("// Finish step-over from breakpoint2") + + @skipIfWindows + def test_step_over_deadlock_small_timeout_fast_stepping(self): + """Test step over source line with multiple call instructions works fine with small timeout and fast stepping.""" + self.dbg.HandleCommand( + "settings set target.process.thread.single-thread-plan-timeout 10" + ) + self.dbg.HandleCommand("settings set target.use-fast-stepping true") + self.step_over_multi_calls_helper() + + @skipIfWindows + def test_step_over_deadlock_small_timeout_slow_stepping(self): + """Test step over source line with multiple call instructions works fine with small timeout and slow stepping.""" + self.dbg.HandleCommand( + "settings set target.process.thread.single-thread-plan-timeout 10" + ) + self.dbg.HandleCommand("settings set target.use-fast-stepping false") + self.step_over_multi_calls_helper() + + @skipIfWindows + def test_step_over_deadlock_large_timeout_fast_stepping(self): + """Test step over source line with multiple call instructions works fine with large timeout and fast stepping.""" + self.dbg.HandleCommand( + "settings set target.process.thread.single-thread-plan-timeout 2000" + ) + self.dbg.HandleCommand("settings set target.use-fast-stepping true") + self.step_over_multi_calls_helper() + + @skipIfWindows + def test_step_over_deadlock_large_timeout_slow_stepping(self): + """Test step over source line with multiple call instructions works fine with large timeout and slow stepping.""" + self.dbg.HandleCommand( + "settings set target.process.thread.single-thread-plan-timeout 2000" + ) + self.dbg.HandleCommand("settings set target.use-fast-stepping false") + self.step_over_multi_calls_helper() diff --git a/lldb/test/API/functionalities/single-thread-step/main.cpp b/lldb/test/API/functionalities/single-thread-step/main.cpp new file mode 100644 index 0000000000000..16356a08375b8 --- /dev/null +++ b/lldb/test/API/functionalities/single-thread-step/main.cpp @@ -0,0 +1,62 @@ +#include <condition_variable> +#include <iostream> +#include <mutex> +#include <thread> + +std::mutex mtx; +std::condition_variable cv; +int ready_thread_id = 0; +int signal_main_thread = 0; + +void worker(int id) { + std::cout << "Worker " << id << " executing..." << std::endl; + + // lldb test should change signal_main_thread to true to break the loop. + while (!signal_main_thread) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + + // Signal the main thread to continue main thread + { + std::lock_guard<std::mutex> lock(mtx); + ready_thread_id = id; // break worker thread here + } + cv.notify_one(); + + std::this_thread::sleep_for(std::chrono::seconds(1)); + std::cout << "Worker " << id << " finished." << std::endl; +} + +int simulate_thread() { + std::thread t1(worker, 1); + + // Wait until signaled by the first thread + std::unique_lock<std::mutex> lock(mtx); + auto func = [] { return ready_thread_id == 1; }; + cv.wait(lock, func); // Set breakpoint1 here + + std::thread t2(worker, 2); // Finish step-over from breakpoint1 + + cv.wait(lock, [] { return ready_thread_id == 2; }); + + t1.join(); + t2.join(); + + std::cout << "Main thread continues..." << std::endl; + + return 0; +} + +int bar() { return 54; } + +int foo(const std::string p1, int extra) { return p1.size() + extra; } + +int main(int argc, char *argv[]) { + std::string ss = "this is a string for testing", + ls = "this is a long string for testing"; + foo(ss.size() % 2 == 0 ? ss : ls, bar()); // Set breakpoint2 here + + simulate_thread(); // Finish step-over from breakpoint2 + + return 0; +} diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index b4a2718bbb096..5ff0196619829 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -924,6 +924,9 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread &thread, case lldb::eStopReasonVForkDone: body.try_emplace("reason", "vforkdone"); break; + case lldb::eStopReasonInterrupt: + body.try_emplace("reason", "async interrupt"); + break; case lldb::eStopReasonThreadExiting: case lldb::eStopReasonInvalid: case lldb::eStopReasonNone: diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp index a91cc6718f4df..2da107887604b 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.cpp +++ b/lldb/tools/lldb-dap/LLDBUtils.cpp @@ -110,6 +110,7 @@ bool ThreadHasStopReason(lldb::SBThread &thread) { case lldb::eStopReasonFork: case lldb::eStopReasonVFork: case lldb::eStopReasonVForkDone: + case lldb::eStopReasonInterrupt: return true; case lldb::eStopReasonThreadExiting: case lldb::eStopReasonInvalid: >From d125c5a761a70b024afb16d22b4326df4071e04b Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 6 May 2024 11:14:01 -0700 Subject: [PATCH 2/5] Refinement --- .../Target/ThreadPlanSingleThreadTimeout.h | 8 ++- .../lldb/Target/ThreadPlanStepOverRange.h | 1 + .../include/lldb/Target/ThreadPlanStepRange.h | 5 ++ lldb/source/Target/TargetProperties.td | 2 +- .../Target/ThreadPlanSingleThreadTimeout.cpp | 39 ++++++++++-- lldb/source/Target/ThreadPlanStepInRange.cpp | 3 +- .../source/Target/ThreadPlanStepOverRange.cpp | 13 ++-- lldb/source/Target/ThreadPlanStepRange.cpp | 62 ++++++++++++------- .../TestSingleThreadStepTimeout.py | 8 +-- 9 files changed, 99 insertions(+), 42 deletions(-) diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h index 777ed55fbae26..89db966b29bff 100644 --- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h +++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h @@ -35,7 +35,10 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { public: ~ThreadPlanSingleThreadTimeout() override; - static void ResetIfNeeded(Thread &thread); + // Create a new instance from fresh new state. + static void CreateNew(Thread &thread); + // Reset and create a new instance from the previous state. + static void ResetFromPrevState(Thread &thread); void GetDescription(Stream *s, lldb::DescriptionLevel level) override; bool ValidatePlan(Stream *error) override { return true; } @@ -56,7 +59,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { private: ThreadPlanSingleThreadTimeout(Thread &thread); - static bool IsAlive() { return s_instance != nullptr; } + static bool IsAlive(); enum class State { WaitTimeout, // Waiting for timeout. @@ -64,6 +67,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { Done, // Finished resume all threads. }; + static std::mutex s_mutex; static ThreadPlanSingleThreadTimeout *s_instance; static State s_prev_state; diff --git a/lldb/include/lldb/Target/ThreadPlanStepOverRange.h b/lldb/include/lldb/Target/ThreadPlanStepOverRange.h index 6b9fdadf0cbd9..faa0ab596a283 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOverRange.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOverRange.h @@ -29,6 +29,7 @@ class ThreadPlanStepOverRange : public ThreadPlanStepRange, void GetDescription(Stream *s, lldb::DescriptionLevel level) override; void SetStopOthers(bool new_value) override; bool ShouldStop(Event *event_ptr) override; + void DidPush() override; protected: bool DoPlanExplainsStop(Event *event_ptr) override; diff --git a/lldb/include/lldb/Target/ThreadPlanStepRange.h b/lldb/include/lldb/Target/ThreadPlanStepRange.h index 2fe8852771000..ced84c03e83cf 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepRange.h +++ b/lldb/include/lldb/Target/ThreadPlanStepRange.h @@ -58,8 +58,13 @@ class ThreadPlanStepRange : public ThreadPlan { // run' plan, then just single step. bool SetNextBranchBreakpoint(); + // Whether the input stop info is caused by the next branch breakpoint. + bool IsNextBranchBreakpointStop(lldb::StopInfoSP stop_info_sp); + void ClearNextBranchBreakpoint(); + void ClearNextBranchBreakpointExplainedStop(); + bool NextRangeBreakpointExplainsStop(lldb::StopInfoSP stop_info_sp); SymbolContext m_addr_context; diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td index 1e08d352a6e1c..87d7e64198501 100644 --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -313,7 +313,7 @@ let Definition = "thread" in { def SingleThreadPlanTimeout: Property<"single-thread-plan-timeout", "UInt64">, Global, DefaultUnsignedValue<1000>, - Desc<"The time in milliseconds to wait for single thread ThreadPlan to move forward before resuming all threads to resolve any potential deadlock.">; + Desc<"The time in milliseconds to wait for single thread ThreadPlan to move forward before resuming all threads to resolve any potential deadlock. Specify value 0 to disable timeout.">; } let Definition = "language" in { diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp index 11575d347105e..ce214af2337bf 100644 --- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -24,6 +24,7 @@ using namespace lldb_private; using namespace lldb; +std::mutex ThreadPlanSingleThreadTimeout::s_mutex; ThreadPlanSingleThreadTimeout *ThreadPlanSingleThreadTimeout::s_instance = nullptr; ThreadPlanSingleThreadTimeout::State @@ -33,13 +34,14 @@ ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread) : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout", thread, eVoteNo, eVoteNoOpinion), m_state(State::WaitTimeout), m_exit_flag(false) { + std::lock_guard<std::mutex> lock(s_mutex); m_timer_thread = std::thread(TimeoutThreadFunc, this); s_instance = this; m_state = s_prev_state; } ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() { - std::lock_guard<std::mutex> lock(m_mutex); + std::lock_guard<std::mutex> lock(s_mutex); s_instance = nullptr; if (m_state == State::Done) m_state = State::WaitTimeout; @@ -62,12 +64,34 @@ std::string ThreadPlanSingleThreadTimeout::StateToString(State state) { } } -void ThreadPlanSingleThreadTimeout::ResetIfNeeded(Thread &thread) { +void ThreadPlanSingleThreadTimeout::CreateNew(Thread &thread) { + uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); + if (timeout_in_ms == 0) + return; + + // Do not create timeout if we are not stopping other threads. + if (!thread.GetCurrentPlan()->StopOthers()) + return; + + if (ThreadPlanSingleThreadTimeout::IsAlive()) + return; + { + + s_prev_state = State::WaitTimeout; + } + auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread); + ThreadPlanSP thread_plan_sp(timeout_plan); + auto status = thread.QueueThreadPlan(thread_plan_sp, + /*abort_other_plans*/ false); + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout pushing a brand new one"); +} + +void ThreadPlanSingleThreadTimeout::ResetFromPrevState(Thread &thread) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; - // TODO: mutex? if (ThreadPlanSingleThreadTimeout::IsAlive()) return; @@ -80,7 +104,7 @@ void ThreadPlanSingleThreadTimeout::ResetIfNeeded(Thread &thread) { auto status = thread.QueueThreadPlan(thread_plan_sp, /*abort_other_plans*/ false); Log *log = GetLog(LLDBLog::Step); - LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout pushing a new one"); + LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout reset from previous state"); } bool ThreadPlanSingleThreadTimeout::WillStop() { @@ -88,7 +112,7 @@ bool ThreadPlanSingleThreadTimeout::WillStop() { LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::WillStop()."); // Reset the state during stop. - std::lock_guard<std::mutex> lock(m_mutex); + std::lock_guard<std::mutex> lock(s_mutex); s_prev_state = State::WaitTimeout; return true; } @@ -203,3 +227,8 @@ void ThreadPlanSingleThreadTimeout::HandleTimeout() { // in running state so no need to check state here. m_process.SendAsyncInterrupt(&GetThread()); } + +bool ThreadPlanSingleThreadTimeout::IsAlive() { + std::lock_guard<std::mutex> lock(s_mutex); + return s_instance != nullptr; +} diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp index 0c8b92f415420..567dcc26d0d37 100644 --- a/lldb/source/Target/ThreadPlanStepInRange.cpp +++ b/lldb/source/Target/ThreadPlanStepInRange.cpp @@ -134,8 +134,7 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) { GetTarget().GetArchitecture().GetAddressByteSize()); LLDB_LOGF(log, "ThreadPlanStepInRange reached %s.", s.GetData()); } - if (DoPlanExplainsStop(event_ptr)) - ClearNextBranchBreakpoint(); + ClearNextBranchBreakpointExplainedStop(); if (IsPlanComplete()) return true; diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp index 6d24b5d9d7f36..d3e6f4d79e040 100644 --- a/lldb/source/Target/ThreadPlanStepOverRange.cpp +++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp @@ -141,9 +141,7 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { GetTarget().GetArchitecture().GetAddressByteSize()); LLDB_LOGF(log, "ThreadPlanStepOverRange reached %s.", s.GetData()); } - - if (DoPlanExplainsStop(event_ptr)) - ClearNextBranchBreakpoint(); + ClearNextBranchBreakpointExplainedStop(); // If we're out of the range but in the same frame or in our caller's frame // then we should stop. When stepping out we only stop others if we are @@ -346,6 +344,11 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { return false; } +void ThreadPlanStepOverRange::DidPush() { + if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan()) + ThreadPlanSingleThreadTimeout::CreateNew(GetThread()); +} + bool ThreadPlanStepOverRange::DoPlanExplainsStop(Event *event_ptr) { // For crashes, breakpoint hits, signals, etc, let the base plan (or some // plan above us) handle the stop. That way the user can see the stop, step @@ -423,7 +426,7 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType resume_state, } } } - if (m_run_mode == lldb::eOnlyThisThread) - ThreadPlanSingleThreadTimeout::ResetIfNeeded(GetThread()); + if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan()) + ThreadPlanSingleThreadTimeout::ResetFromPrevState(GetThread()); return true; } diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp index ef66bf2ff2514..d9384160720d6 100644 --- a/lldb/source/Target/ThreadPlanStepRange.cpp +++ b/lldb/source/Target/ThreadPlanStepRange.cpp @@ -292,6 +292,20 @@ InstructionList *ThreadPlanStepRange::GetInstructionsForAddress( return nullptr; } +bool ThreadPlanStepRange::IsNextBranchBreakpointStop(StopInfoSP stop_info_sp) { + if (!m_next_branch_bp_sp) + return false; + + break_id_t bp_site_id = stop_info_sp->GetValue(); + BreakpointSiteSP bp_site_sp = + m_process.GetBreakpointSiteList().FindByID(bp_site_id); + if (!bp_site_sp) + return false; + else if (!bp_site_sp->IsBreakpointAtThisSite(m_next_branch_bp_sp->GetID())) + return false; + return true; +} + void ThreadPlanStepRange::ClearNextBranchBreakpoint() { if (m_next_branch_bp_sp) { Log *log = GetLog(LLDBLog::Step); @@ -304,6 +318,11 @@ void ThreadPlanStepRange::ClearNextBranchBreakpoint() { } } +void ThreadPlanStepRange::ClearNextBranchBreakpointExplainedStop() { + if (IsNextBranchBreakpointStop(GetPrivateStopInfo())) + ClearNextBranchBreakpoint(); +} + bool ThreadPlanStepRange::SetNextBranchBreakpoint() { if (m_next_branch_bp_sp) return true; @@ -391,8 +410,7 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() { bool ThreadPlanStepRange::NextRangeBreakpointExplainsStop( lldb::StopInfoSP stop_info_sp) { - Log *log = GetLog(LLDBLog::Step); - if (!m_next_branch_bp_sp) + if (!IsNextBranchBreakpointStop(stop_info_sp)) return false; break_id_t bp_site_id = stop_info_sp->GetValue(); @@ -400,29 +418,27 @@ bool ThreadPlanStepRange::NextRangeBreakpointExplainsStop( m_process.GetBreakpointSiteList().FindByID(bp_site_id); if (!bp_site_sp) return false; - else if (!bp_site_sp->IsBreakpointAtThisSite(m_next_branch_bp_sp->GetID())) - return false; - else { - // If we've hit the next branch breakpoint, then clear it. - size_t num_constituents = bp_site_sp->GetNumberOfConstituents(); - bool explains_stop = true; - // If all the constituents are internal, then we are probably just stepping - // over this range from multiple threads, or multiple frames, so we want to - // continue. If one is not internal, then we should not explain the stop, - // and let the user breakpoint handle the stop. - for (size_t i = 0; i < num_constituents; i++) { - if (!bp_site_sp->GetConstituentAtIndex(i)->GetBreakpoint().IsInternal()) { - explains_stop = false; - break; - } + + // If we've hit the next branch breakpoint, then clear it. + size_t num_constituents = bp_site_sp->GetNumberOfConstituents(); + bool explains_stop = true; + // If all the constituents are internal, then we are probably just stepping + // over this range from multiple threads, or multiple frames, so we want to + // continue. If one is not internal, then we should not explain the stop, + // and let the user breakpoint handle the stop. + for (size_t i = 0; i < num_constituents; i++) { + if (!bp_site_sp->GetConstituentAtIndex(i)->GetBreakpoint().IsInternal()) { + explains_stop = false; + break; } - LLDB_LOGF(log, - "ThreadPlanStepRange::NextRangeBreakpointExplainsStop - Hit " - "next range breakpoint which has %" PRIu64 - " constituents - explains stop: %u.", - (uint64_t)num_constituents, explains_stop); - return explains_stop; } + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF(log, + "ThreadPlanStepRange::NextRangeBreakpointExplainsStop - Hit " + "next range breakpoint which has %" PRIu64 + " constituents - explains stop: %u.", + (uint64_t)num_constituents, explains_stop); + return explains_stop; } bool ThreadPlanStepRange::WillStop() { return true; } diff --git a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py index f734b4bbf116e..f3370d2d82447 100644 --- a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py +++ b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py @@ -87,7 +87,7 @@ def step_over_multi_calls_helper(self): self.verify_hit_correct_line("// Finish step-over from breakpoint2") @skipIfWindows - def test_step_over_deadlock_small_timeout_fast_stepping(self): + def test_step_over_multi_calls_small_timeout_fast_stepping(self): """Test step over source line with multiple call instructions works fine with small timeout and fast stepping.""" self.dbg.HandleCommand( "settings set target.process.thread.single-thread-plan-timeout 10" @@ -96,7 +96,7 @@ def test_step_over_deadlock_small_timeout_fast_stepping(self): self.step_over_multi_calls_helper() @skipIfWindows - def test_step_over_deadlock_small_timeout_slow_stepping(self): + def test_step_over_multi_calls_small_timeout_slow_stepping(self): """Test step over source line with multiple call instructions works fine with small timeout and slow stepping.""" self.dbg.HandleCommand( "settings set target.process.thread.single-thread-plan-timeout 10" @@ -105,7 +105,7 @@ def test_step_over_deadlock_small_timeout_slow_stepping(self): self.step_over_multi_calls_helper() @skipIfWindows - def test_step_over_deadlock_large_timeout_fast_stepping(self): + def test_step_over_multi_calls_large_timeout_fast_stepping(self): """Test step over source line with multiple call instructions works fine with large timeout and fast stepping.""" self.dbg.HandleCommand( "settings set target.process.thread.single-thread-plan-timeout 2000" @@ -114,7 +114,7 @@ def test_step_over_deadlock_large_timeout_fast_stepping(self): self.step_over_multi_calls_helper() @skipIfWindows - def test_step_over_deadlock_large_timeout_slow_stepping(self): + def test_step_over_multi_calls_large_timeout_slow_stepping(self): """Test step over source line with multiple call instructions works fine with large timeout and slow stepping.""" self.dbg.HandleCommand( "settings set target.process.thread.single-thread-plan-timeout 2000" >From 53d3a4bea16ddc5d80bb72fbe54ba401c173b1ef Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Mon, 6 May 2024 11:24:39 -0700 Subject: [PATCH 3/5] Add missing statement --- lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp index ce214af2337bf..f5dccbd4500fe 100644 --- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -76,7 +76,7 @@ void ThreadPlanSingleThreadTimeout::CreateNew(Thread &thread) { if (ThreadPlanSingleThreadTimeout::IsAlive()) return; { - + std::lock_guard<std::mutex> lock(s_mutex); s_prev_state = State::WaitTimeout; } auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread); @@ -230,5 +230,5 @@ void ThreadPlanSingleThreadTimeout::HandleTimeout() { bool ThreadPlanSingleThreadTimeout::IsAlive() { std::lock_guard<std::mutex> lock(s_mutex); - return s_instance != nullptr; + return s_instance != nullptr; } >From 1a38d9080c62e86253df9960434515d02a9f0da6 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Thu, 16 May 2024 19:21:21 -0700 Subject: [PATCH 4/5] Address review comments --- lldb/include/lldb/Target/Process.h | 12 +++-- lldb/include/lldb/Target/ThreadPlan.h | 1 - .../Target/ThreadPlanSingleThreadTimeout.h | 39 ++++++++------- .../lldb/Target/ThreadPlanStepOverRange.h | 2 + lldb/source/Target/Process.cpp | 2 - .../Target/ThreadPlanSingleThreadTimeout.cpp | 50 +++++++------------ .../source/Target/ThreadPlanStepOverRange.cpp | 5 +- 7 files changed, 52 insertions(+), 59 deletions(-) diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 7e758dbb9f645..4dbe742bac547 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1314,10 +1314,14 @@ class Process : public std::enable_shared_from_this<Process>, uint32_t start_frame, uint32_t num_frames, uint32_t num_frames_with_source, bool stop_format); - void SendAsyncInterrupt(); - - // Send an async interrupt and receive stop from a specific /p thread. - void SendAsyncInterrupt(Thread *thread); + /// Send an async interrupt request. + /// + /// If \a thread is specified the async interrupt stop will be attributed the + /// specified thread. + /// + /// \param[in] thread + /// The thread from which to attribute the async interrupt stop to. + void SendAsyncInterrupt(Thread *thread = nullptr); // Notify this process class that modules got loaded. // diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h index 640e997caf7b3..5b639e7b6609a 100644 --- a/lldb/include/lldb/Target/ThreadPlan.h +++ b/lldb/include/lldb/Target/ThreadPlan.h @@ -591,7 +591,6 @@ class ThreadPlanNull : public ThreadPlan { const Status &GetStatus() { return m_status; } protected: - friend class ThreadPlanSingleThreadTimeout; bool DoPlanExplainsStop(Event *event_ptr) override; lldb::StateType GetPlanRunState() override; diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h index 89db966b29bff..3cbd6a5a61599 100644 --- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h +++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h @@ -22,9 +22,9 @@ namespace lldb_private { // // Thread plan used by single thread execution to issue timeout. This is useful // to detect potential deadlock in single thread execution. The timeout measures -// the elapsed time from the last internal stop and got reset by each internal -// stops to ensure we are accurately detecting execution not moving forward. -// This means this thread plan can be created/destroyed multiple times by the +// the elapsed time from the last internal stop and gets reset by each internal +// stop to ensure we are accurately detecting execution not moving forward. +// This means this thread plan may be created/destroyed multiple times by the // parent execution plan. // // When timeout happens, the thread plan resolves the potential deadlock by @@ -32,13 +32,24 @@ namespace lldb_private { // threads execution are resumed to resolve the potential deadlock. // class ThreadPlanSingleThreadTimeout : public ThreadPlan { + enum class State { + WaitTimeout, // Waiting for timeout. + AsyncInterrupt, // Async interrupt has been issued. + Done, // Finished resume all threads. + }; + public: + struct TimeoutInfo { + ThreadPlanSingleThreadTimeout *m_instance = nullptr; + ThreadPlanSingleThreadTimeout::State m_last_state = State::WaitTimeout; + }; + ~ThreadPlanSingleThreadTimeout() override; // Create a new instance from fresh new state. - static void CreateNew(Thread &thread); + static void CreateNew(Thread &thread, TimeoutInfo &info); // Reset and create a new instance from the previous state. - static void ResetFromPrevState(Thread &thread); + static void ResetFromPrevState(Thread &thread, TimeoutInfo &info); void GetDescription(Stream *s, lldb::DescriptionLevel level) override; bool ValidatePlan(Stream *error) override { return true; } @@ -57,19 +68,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { bool StopOthers() override; private: - ThreadPlanSingleThreadTimeout(Thread &thread); - - static bool IsAlive(); - - enum class State { - WaitTimeout, // Waiting for timeout. - AsyncInterrupt, // Async interrupt has been issued. - Done, // Finished resume all threads. - }; - - static std::mutex s_mutex; - static ThreadPlanSingleThreadTimeout *s_instance; - static State s_prev_state; + ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfo &info); bool HandleEvent(Event *event_ptr); void HandleTimeout(); @@ -80,7 +79,11 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { const ThreadPlanSingleThreadTimeout & operator=(const ThreadPlanSingleThreadTimeout &) = delete; + TimeoutInfo &m_info; // Reference to controlling ThreadPlan's TimeoutInfo. State m_state; + + // Lock for m_wakeup_cv and m_exit_flag between thread plan thread and timer + // thread std::mutex m_mutex; std::condition_variable m_wakeup_cv; // Whether the timer thread should exit or not. diff --git a/lldb/include/lldb/Target/ThreadPlanStepOverRange.h b/lldb/include/lldb/Target/ThreadPlanStepOverRange.h index faa0ab596a283..9f104f6b5654b 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOverRange.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOverRange.h @@ -12,6 +12,7 @@ #include "lldb/Core/AddressRange.h" #include "lldb/Target/StackID.h" #include "lldb/Target/Thread.h" +#include "lldb/Target/ThreadPlanSingleThreadTimeout.h" #include "lldb/Target/ThreadPlanStepRange.h" namespace lldb_private { @@ -50,6 +51,7 @@ class ThreadPlanStepOverRange : public ThreadPlanStepRange, bool m_first_resume; lldb::RunMode m_run_mode; + ThreadPlanSingleThreadTimeout::TimeoutInfo m_timeout_info; ThreadPlanStepOverRange(const ThreadPlanStepOverRange &) = delete; const ThreadPlanStepOverRange & diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 3e5e4fe58b4b3..19093c86121f7 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3681,8 +3681,6 @@ void Process::ControlPrivateStateThread(uint32_t signal) { } } -void Process::SendAsyncInterrupt() { SendAsyncInterrupt(nullptr); } - void Process::SendAsyncInterrupt(Thread *thread) { if (thread != nullptr) m_interrupt_tid = thread->GetProtocolID(); diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp index f5dccbd4500fe..a2a5211f5102e 100644 --- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -24,28 +24,20 @@ using namespace lldb_private; using namespace lldb; -std::mutex ThreadPlanSingleThreadTimeout::s_mutex; -ThreadPlanSingleThreadTimeout *ThreadPlanSingleThreadTimeout::s_instance = - nullptr; -ThreadPlanSingleThreadTimeout::State - ThreadPlanSingleThreadTimeout::s_prev_state = State::WaitTimeout; - -ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread) +ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread, + TimeoutInfo &info) : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout", thread, eVoteNo, eVoteNoOpinion), - m_state(State::WaitTimeout), m_exit_flag(false) { - std::lock_guard<std::mutex> lock(s_mutex); + m_info(info), m_state(State::WaitTimeout), m_exit_flag(false) { m_timer_thread = std::thread(TimeoutThreadFunc, this); - s_instance = this; - m_state = s_prev_state; + m_info.m_instance = this; + m_state = m_info.m_last_state; } ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() { - std::lock_guard<std::mutex> lock(s_mutex); - s_instance = nullptr; + m_info.m_instance = nullptr; if (m_state == State::Done) m_state = State::WaitTimeout; - s_prev_state = m_state; } void ThreadPlanSingleThreadTimeout::GetDescription( @@ -64,22 +56,20 @@ std::string ThreadPlanSingleThreadTimeout::StateToString(State state) { } } -void ThreadPlanSingleThreadTimeout::CreateNew(Thread &thread) { +void ThreadPlanSingleThreadTimeout::CreateNew(Thread &thread, + TimeoutInfo &info) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; + if (info.m_instance != nullptr) + return; + // Do not create timeout if we are not stopping other threads. if (!thread.GetCurrentPlan()->StopOthers()) return; - if (ThreadPlanSingleThreadTimeout::IsAlive()) - return; - { - std::lock_guard<std::mutex> lock(s_mutex); - s_prev_state = State::WaitTimeout; - } - auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread); + auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread, info); ThreadPlanSP thread_plan_sp(timeout_plan); auto status = thread.QueueThreadPlan(thread_plan_sp, /*abort_other_plans*/ false); @@ -87,19 +77,20 @@ void ThreadPlanSingleThreadTimeout::CreateNew(Thread &thread) { LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout pushing a brand new one"); } -void ThreadPlanSingleThreadTimeout::ResetFromPrevState(Thread &thread) { +void ThreadPlanSingleThreadTimeout::ResetFromPrevState(Thread &thread, + TimeoutInfo &info) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; - if (ThreadPlanSingleThreadTimeout::IsAlive()) + if (info.m_instance != nullptr) return; // Do not create timeout if we are not stopping other threads. if (!thread.GetCurrentPlan()->StopOthers()) return; - auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread); + auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread, info); ThreadPlanSP thread_plan_sp(timeout_plan); auto status = thread.QueueThreadPlan(thread_plan_sp, /*abort_other_plans*/ false); @@ -112,8 +103,8 @@ bool ThreadPlanSingleThreadTimeout::WillStop() { LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::WillStop()."); // Reset the state during stop. - std::lock_guard<std::mutex> lock(s_mutex); - s_prev_state = State::WaitTimeout; + m_info.m_last_state = State::WaitTimeout; + m_info.m_instance = this; return true; } @@ -227,8 +218,3 @@ void ThreadPlanSingleThreadTimeout::HandleTimeout() { // in running state so no need to check state here. m_process.SendAsyncInterrupt(&GetThread()); } - -bool ThreadPlanSingleThreadTimeout::IsAlive() { - std::lock_guard<std::mutex> lock(s_mutex); - return s_instance != nullptr; -} diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp index d3e6f4d79e040..31dfb5083e495 100644 --- a/lldb/source/Target/ThreadPlanStepOverRange.cpp +++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp @@ -346,7 +346,7 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { void ThreadPlanStepOverRange::DidPush() { if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan()) - ThreadPlanSingleThreadTimeout::CreateNew(GetThread()); + ThreadPlanSingleThreadTimeout::CreateNew(GetThread(), m_timeout_info); } bool ThreadPlanStepOverRange::DoPlanExplainsStop(Event *event_ptr) { @@ -427,6 +427,7 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType resume_state, } } if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan()) - ThreadPlanSingleThreadTimeout::ResetFromPrevState(GetThread()); + ThreadPlanSingleThreadTimeout::ResetFromPrevState(GetThread(), + m_timeout_info); return true; } >From a423f34a6fbff413ebb0b33e2543595debe6f9ae Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Tue, 28 May 2024 12:46:03 -0700 Subject: [PATCH 5/5] Address review comments with mixin class --- lldb/include/lldb/Target/Process.h | 14 +++++-- .../Target/ThreadPlanSingleThreadTimeout.h | 14 +++++-- .../lldb/Target/ThreadPlanStepOverRange.h | 9 ++--- lldb/include/lldb/Target/TimeoutResumeAll.h | 40 +++++++++++++++++++ .../Process/gdb-remote/ProcessGDBRemote.h | 8 ++-- lldb/source/Target/Process.cpp | 2 +- lldb/source/Target/StopInfo.cpp | 9 ----- .../Target/ThreadPlanSingleThreadTimeout.cpp | 11 ++--- .../source/Target/ThreadPlanStepOverRange.cpp | 9 ++--- 9 files changed, 76 insertions(+), 40 deletions(-) create mode 100644 lldb/include/lldb/Target/TimeoutResumeAll.h diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 4dbe742bac547..9aaa1e8fcde78 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1316,11 +1316,11 @@ class Process : public std::enable_shared_from_this<Process>, /// Send an async interrupt request. /// - /// If \a thread is specified the async interrupt stop will be attributed the - /// specified thread. + /// If \a thread is specified the async interrupt stop will be attributed to + /// the specified thread. /// /// \param[in] thread - /// The thread from which to attribute the async interrupt stop to. + /// The thread the async interrupt will be attributed to. void SendAsyncInterrupt(Thread *thread = nullptr); // Notify this process class that modules got loaded. @@ -2822,6 +2822,14 @@ void PruneThreadPlans(); return std::nullopt; } + /// Handle thread specific async interrupt and return the original thread + /// that requested the async interrupt. It can be null if original thread + /// has exited. + virtual lldb::ThreadSP + HandleThreadAsyncInterrupt(uint8_t signo, const std::string &description) { + return lldb::ThreadSP(); + } + lldb::StateType GetPrivateState(); /// The "private" side of resuming a process. This doesn't alter the state diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h index 3cbd6a5a61599..deaaa49b9bc59 100644 --- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h +++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h @@ -46,10 +46,16 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { ~ThreadPlanSingleThreadTimeout() override; - // Create a new instance from fresh new state. - static void CreateNew(Thread &thread, TimeoutInfo &info); - // Reset and create a new instance from the previous state. - static void ResetFromPrevState(Thread &thread, TimeoutInfo &info); + // If input \param thread is running in single thread mode, push a + // new ThreadPlanSingleThreadTimeout based on timeout setting from fresh new + // state. The reference of \param info is passed in so that when + // ThreadPlanSingleThreadTimeout got popped out its last state can be stored + // in it for future resume. + static void PushNewWithTimeout(Thread &thread, TimeoutInfo &info); + + // Push a new ThreadPlanSingleThreadTimeout by restoring state from + // input \param info and resume execution. + static void ResumeFromPrevState(Thread &thread, TimeoutInfo &info); void GetDescription(Stream *s, lldb::DescriptionLevel level) override; bool ValidatePlan(Stream *error) override { return true; } diff --git a/lldb/include/lldb/Target/ThreadPlanStepOverRange.h b/lldb/include/lldb/Target/ThreadPlanStepOverRange.h index 9f104f6b5654b..af29d1877dc71 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOverRange.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOverRange.h @@ -12,13 +12,14 @@ #include "lldb/Core/AddressRange.h" #include "lldb/Target/StackID.h" #include "lldb/Target/Thread.h" -#include "lldb/Target/ThreadPlanSingleThreadTimeout.h" #include "lldb/Target/ThreadPlanStepRange.h" +#include "lldb/Target/TimeoutResumeAll.h" namespace lldb_private { class ThreadPlanStepOverRange : public ThreadPlanStepRange, - ThreadPlanShouldStopHere { + ThreadPlanShouldStopHere, + TimeoutResumeAll { public: ThreadPlanStepOverRange(Thread &thread, const AddressRange &range, const SymbolContext &addr_context, @@ -45,13 +46,9 @@ class ThreadPlanStepOverRange : public ThreadPlanStepRange, void SetupAvoidNoDebug(LazyBool step_out_avoids_code_without_debug_info); bool IsEquivalentContext(const SymbolContext &context); - // Clear and create a new ThreadPlanSingleThreadTimeout to detect if program - // is moving forward. - void ResetSingleThreadTimeout(); bool m_first_resume; lldb::RunMode m_run_mode; - ThreadPlanSingleThreadTimeout::TimeoutInfo m_timeout_info; ThreadPlanStepOverRange(const ThreadPlanStepOverRange &) = delete; const ThreadPlanStepOverRange & diff --git a/lldb/include/lldb/Target/TimeoutResumeAll.h b/lldb/include/lldb/Target/TimeoutResumeAll.h new file mode 100644 index 0000000000000..4c2f368877fca --- /dev/null +++ b/lldb/include/lldb/Target/TimeoutResumeAll.h @@ -0,0 +1,40 @@ +//===-- TimeoutResumeAll.h -------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TARGET_TIMEOUTRESUMEALL_H +#define LLDB_TARGET_TIMEOUTRESUMEALL_H + +#include "lldb/Target/ThreadPlanSingleThreadTimeout.h" + +namespace lldb_private { + +// Mixin class that provides capability for ThreadPlan that supports single +// thread execution to resume all threads after a timeout. +// Opt-in thread plan should call PushNewTimeout() in its DidPush() and +// ResumeWithTimeout() during DoWillResume(). +class TimeoutResumeAll { +public: + TimeoutResumeAll(Thread &thread) : m_thread(thread) {} + + void PushNewTimeout() { + ThreadPlanSingleThreadTimeout::PushNewWithTimeout(m_thread, m_timeout_info); + } + + void ResumeWithTimeout() { + ThreadPlanSingleThreadTimeout::ResumeFromPrevState(m_thread, + m_timeout_info); + } + +private: + Thread &m_thread; + ThreadPlanSingleThreadTimeout::TimeoutInfo m_timeout_info; +}; + +} // namespace lldb_private + +#endif // LLDB_TARGET_TIMEOUTRESUMEALL_H diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 615831dd7e6f6..b4942091fb01d 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -440,11 +440,9 @@ class ProcessGDBRemote : public Process, void HandleStopReply() override; void HandleAsyncStructuredDataPacket(llvm::StringRef data) override; - // Handle thread specific async interrupt and return the original thread - // that requested the async interrupt. It can be null if original thread - // has exited. - lldb::ThreadSP HandleThreadAsyncInterrupt(uint8_t signo, - const std::string &description); + lldb::ThreadSP + HandleThreadAsyncInterrupt(uint8_t signo, + const std::string &description) override; void SetThreadPc(const lldb::ThreadSP &thread_sp, uint64_t index); using ModuleCacheKey = std::pair<std::string, std::string>; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 19093c86121f7..b6d42e1c97a55 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3911,7 +3911,7 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { if (interrupt_requested) { if (StateIsStoppedState(internal_state, true)) { - // Oly mark interrupt event if it is not thread specific async + // Only mark interrupt event if it is not thread specific async // interrupt. if (m_interrupt_tid == LLDB_INVALID_THREAD_ID) { // We requested the interrupt, so mark this as such in the stop diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index 936d49cc312d7..bd7032b803df9 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -1140,15 +1140,6 @@ class StopInfoInterrupt : public StopInfo { return lldb::eStopReasonInterrupt; } - bool ShouldStopSynchronous(Event *event_ptr) override { return true; } - - bool ShouldStop(Event *event_ptr) override { - ThreadSP thread_sp(m_thread_wp.lock()); - if (thread_sp) - return thread_sp->GetProcess()->GetUnixSignals()->GetShouldStop(m_value); - return false; - } - const char *GetDescription() override { if (m_description.empty()) { m_description = "async interrupt"; diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp index a2a5211f5102e..a4e842105f2c5 100644 --- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -56,15 +56,12 @@ std::string ThreadPlanSingleThreadTimeout::StateToString(State state) { } } -void ThreadPlanSingleThreadTimeout::CreateNew(Thread &thread, - TimeoutInfo &info) { +void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread, + TimeoutInfo &info) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; - if (info.m_instance != nullptr) - return; - // Do not create timeout if we are not stopping other threads. if (!thread.GetCurrentPlan()->StopOthers()) return; @@ -77,8 +74,8 @@ void ThreadPlanSingleThreadTimeout::CreateNew(Thread &thread, LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout pushing a brand new one"); } -void ThreadPlanSingleThreadTimeout::ResetFromPrevState(Thread &thread, - TimeoutInfo &info) { +void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread, + TimeoutInfo &info) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp index 31dfb5083e495..24443c54f5106 100644 --- a/lldb/source/Target/ThreadPlanStepOverRange.cpp +++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp @@ -37,8 +37,8 @@ ThreadPlanStepOverRange::ThreadPlanStepOverRange( : ThreadPlanStepRange(ThreadPlan::eKindStepOverRange, "Step range stepping over", thread, range, addr_context, stop_others), - ThreadPlanShouldStopHere(this), m_first_resume(true), - m_run_mode(stop_others) { + ThreadPlanShouldStopHere(this), TimeoutResumeAll(thread), + m_first_resume(true), m_run_mode(stop_others) { SetFlagsToDefault(); SetupAvoidNoDebug(step_out_avoids_code_without_debug_info); } @@ -346,7 +346,7 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { void ThreadPlanStepOverRange::DidPush() { if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan()) - ThreadPlanSingleThreadTimeout::CreateNew(GetThread(), m_timeout_info); + PushNewTimeout(); } bool ThreadPlanStepOverRange::DoPlanExplainsStop(Event *event_ptr) { @@ -427,7 +427,6 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType resume_state, } } if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan()) - ThreadPlanSingleThreadTimeout::ResetFromPrevState(GetThread(), - m_timeout_info); + ResumeWithTimeout(); return true; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits