[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
via lldb-commits
lldb-commits at lists.llvm.org
Thu May 16 19:21:47 PDT 2024
https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/90930
>From b72df8cf2a047ed731913609b58bdb4a3601e111 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Thu, 2 May 2024 18:12:04 -0700
Subject: [PATCH 1/4] 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 <jeffreytan at fb.com>
Date: Mon, 6 May 2024 11:14:01 -0700
Subject: [PATCH 2/4] 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 <jeffreytan at fb.com>
Date: Mon, 6 May 2024 11:24:39 -0700
Subject: [PATCH 3/4] 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 <jeffreytan at fb.com>
Date: Thu, 16 May 2024 19:21:21 -0700
Subject: [PATCH 4/4] 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;
}
More information about the lldb-commits
mailing list