[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Aug 5 10:28:38 PDT 2024
https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/90930
>From 87469906264665702868ec91a08e82343d362785 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Thu, 2 May 2024 18:12:04 -0700
Subject: [PATCH 01/10] 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 7d2e3bddcd4e6..42c97c1c5e280 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1320,11 +1320,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
@@ -3159,6 +3161,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 2ff1f50d497e2..aacc59c292ec7 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -58,6 +58,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 74ff458bf87de..7bfde8b9de127 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 55688c9cfa4f1..786f62bd66d52 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 fc07168b6c0ac..71c928ec811fc 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2513,7 +2513,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 08d5f5039d516..a0b08a219ae14 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 604c92369e9a2..6f9c2cc1e4b4e 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 b44ffefcd0d36..d8057c58171fc 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 8186ccbea27d4..a9975b9c7f966 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 f841c0cfcf8fc..a674b4c1ae7dd 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -473,7 +473,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(),
@@ -895,6 +896,7 @@ bool Process::HandleProcessStateChangedEvent(
case eStopReasonThreadExiting:
case eStopReasonInstrumentation:
case eStopReasonProcessorTrace:
+ case eStopReasonInterrupt:
if (!other_thread)
other_thread = thread;
break;
@@ -3873,7 +3875,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);
@@ -4099,9 +4107,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 4404a45492254..ff1858793673d 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -313,6 +313,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 abe4d34bd32c2..1e3803dd9e4c8 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.
@@ -414,6 +426,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 801856bd5419b..1e85c2d7f3d85 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -347,7 +347,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 =
@@ -381,7 +383,9 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
return true;
} else
return false;
- }
+ } else
+ LLDB_LOGF(log, "ThreadPlanStepRange::SetNextBranchBreakpoint - skipping "
+ "invalid run_to_address");
}
return false;
}
@@ -418,7 +422,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 544e9ffb172bf..a8b85f55939e1 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -923,6 +923,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 8a8ea121d0fd3433888fdf44454f4284f4b055f4 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Mon, 6 May 2024 11:14:01 -0700
Subject: [PATCH 02/10] 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 ff1858793673d..ef538678670fe 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -316,7 +316,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 1e3803dd9e4c8..6c251f7c6a8f0 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
@@ -349,6 +347,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
@@ -426,7 +429,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 1e85c2d7f3d85..3c825058b8c37 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -293,6 +293,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);
@@ -305,6 +319,11 @@ void ThreadPlanStepRange::ClearNextBranchBreakpoint() {
}
}
+void ThreadPlanStepRange::ClearNextBranchBreakpointExplainedStop() {
+ if (IsNextBranchBreakpointStop(GetPrivateStopInfo()))
+ ClearNextBranchBreakpoint();
+}
+
bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
if (m_next_branch_bp_sp)
return true;
@@ -392,8 +411,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();
@@ -401,29 +419,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 a42d7d1fca8f872e0c211d75d5235191facbd81f Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Mon, 6 May 2024 11:24:39 -0700
Subject: [PATCH 03/10] 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 9d1b108fc24df8e5e9c12058b884f11f101dedb4 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Thu, 16 May 2024 19:21:21 -0700
Subject: [PATCH 04/10] 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 42c97c1c5e280..e0c633dd478ae 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1322,10 +1322,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 a674b4c1ae7dd..8b33127c2582a 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3875,8 +3875,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 6c251f7c6a8f0..5468eee656c12 100644
--- a/lldb/source/Target/ThreadPlanStepOverRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp
@@ -349,7 +349,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) {
@@ -430,6 +430,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 0e90389b55f9916aa2cc9a936d04536912f7c8e2 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Tue, 28 May 2024 12:46:03 -0700
Subject: [PATCH 05/10] 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 e0c633dd478ae..38c9ab69c3acf 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1324,11 +1324,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.
@@ -2879,6 +2879,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 d8057c58171fc..2492795851388 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 8b33127c2582a..3c9247fdbbbc9 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4105,7 +4105,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 5468eee656c12..381d4ae5de05d 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);
}
@@ -349,7 +349,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) {
@@ -430,7 +430,6 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType resume_state,
}
}
if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan())
- ThreadPlanSingleThreadTimeout::ResetFromPrevState(GetThread(),
- m_timeout_info);
+ ResumeWithTimeout();
return true;
}
>From 56e99e412db1715859e856bf489c35bcc43c4541 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Tue, 9 Jul 2024 20:36:36 -0700
Subject: [PATCH 06/10] Address review comments by adding more testcases
---
lldb/include/lldb/Target/Process.h | 3 +
lldb/include/lldb/Target/ThreadPlan.h | 5 +
.../Target/ThreadPlanSingleThreadTimeout.h | 19 +--
.../include/lldb/Target/ThreadPlanStepRange.h | 2 +
lldb/include/lldb/Target/TimeoutResumeAll.h | 4 +-
lldb/source/Target/Thread.cpp | 6 +
.../Target/ThreadPlanSingleThreadTimeout.cpp | 87 ++++++++-----
.../TestSingleThreadStepTimeout.py | 117 ++++++++++++++++++
.../single-thread-step/main.cpp | 12 +-
9 files changed, 214 insertions(+), 41 deletions(-)
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 38c9ab69c3acf..9cf6f50558570 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2882,6 +2882,9 @@ void PruneThreadPlans();
/// Handle thread specific async interrupt and return the original thread
/// that requested the async interrupt. It can be null if original thread
/// has exited.
+ ///
+ /// \param[in] description
+ /// Returns the stop reason description of the async interrupt.
virtual lldb::ThreadSP
HandleThreadAsyncInterrupt(uint8_t signo, const std::string &description) {
return lldb::ThreadSP();
diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h
index 5b639e7b6609a..ed7f582df54f1 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -396,6 +396,11 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
bool IsControllingPlan() { return m_is_controlling_plan; }
+ // Returns true if this plan is a leaf plan, meaning the plan will be popped
+ // during each stop and re-pushed before resuming to stay at the top of the
+ // stack.
+ virtual bool IsLeafPlan() { return false; }
+
bool SetIsControllingPlan(bool value) {
bool old_value = m_is_controlling_plan;
m_is_controlling_plan = value;
diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
index deaaa49b9bc59..781fd88b4d863 100644
--- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
+++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
@@ -15,6 +15,7 @@
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/State.h"
+#include <chrono>
#include <thread>
namespace lldb_private {
@@ -27,9 +28,9 @@ namespace lldb_private {
// 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
-// issuing a thread specific async interrupt to enter stop state, then all
-// threads execution are resumed to resolve the potential deadlock.
+// When a timeout happens, the thread plan resolves the potential deadlock by
+// issuing a thread specific async interrupt to enter stop state, then execution
+// is resumed with all threads running to resolve the potential deadlock
//
class ThreadPlanSingleThreadTimeout : public ThreadPlan {
enum class State {
@@ -39,8 +40,10 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
};
public:
+ // TODO: allow timeout to be set on per thread plan basis.
struct TimeoutInfo {
- ThreadPlanSingleThreadTimeout *m_instance = nullptr;
+ // Whether there is a ThreadPlanSingleThreadTimeout instance alive.
+ bool m_isAlive;
ThreadPlanSingleThreadTimeout::State m_last_state = State::WaitTimeout;
};
@@ -49,7 +52,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
// 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
+ // ThreadPlanSingleThreadTimeout got popped its last state can be stored
// in it for future resume.
static void PushNewWithTimeout(Thread &thread, TimeoutInfo &info);
@@ -62,6 +65,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
bool WillStop() override;
void DidPop() override;
+ bool IsLeafPlan() override { return true; }
bool DoPlanExplainsStop(Event *event_ptr) override;
lldb::StateType GetPlanRunState() override;
@@ -76,8 +80,10 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
private:
ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfo &info);
+ bool IsTimeoutAsyncInterrupt(Event *event_ptr);
bool HandleEvent(Event *event_ptr);
void HandleTimeout();
+ uint64_t GetRemainingTimeoutMicroSeconds();
static std::string StateToString(State state);
@@ -92,9 +98,8 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
// thread
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;
+ std::chrono::steady_clock::time_point m_timeout_start;
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Target/ThreadPlanStepRange.h b/lldb/include/lldb/Target/ThreadPlanStepRange.h
index ced84c03e83cf..001f3d9fa8957 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepRange.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepRange.h
@@ -59,6 +59,8 @@ class ThreadPlanStepRange : public ThreadPlan {
bool SetNextBranchBreakpoint();
// Whether the input stop info is caused by the next branch breakpoint.
+ // Note: this does not check if branch breakpoint site is shared by other
+ // breakpoints or not.
bool IsNextBranchBreakpointStop(lldb::StopInfoSP stop_info_sp);
void ClearNextBranchBreakpoint();
diff --git a/lldb/include/lldb/Target/TimeoutResumeAll.h b/lldb/include/lldb/Target/TimeoutResumeAll.h
index 4c2f368877fca..d484ea02bcce9 100644
--- a/lldb/include/lldb/Target/TimeoutResumeAll.h
+++ b/lldb/include/lldb/Target/TimeoutResumeAll.h
@@ -13,8 +13,8 @@
namespace lldb_private {
-// Mixin class that provides capability for ThreadPlan that supports single
-// thread execution to resume all threads after a timeout.
+// Mixin class that provides the capability for ThreadPlan to support single
+// thread execution that resumes all threads after a timeout.
// Opt-in thread plan should call PushNewTimeout() in its DidPush() and
// ResumeWithTimeout() during DoWillResume().
class TimeoutResumeAll {
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 7b4988f795504..1733252697acf 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -824,6 +824,12 @@ bool Thread::ShouldStop(Event *event_ptr) {
done_processing_current_plan = true;
should_stop = false;
} else {
+ // Leaf plan that does not explain the stop should be popped.
+ // The plan should be push itself later again before resuming to stay
+ // as leaf.
+ if (current_plan->IsLeafPlan() && current_plan->MischiefManaged())
+ PopPlan();
+
// If the current plan doesn't explain the stop, then find one that does
// and let it handle the situation.
ThreadPlan *plan_ptr = current_plan;
diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
index a4e842105f2c5..6a1da216da0ef 100644
--- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
+++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
@@ -28,21 +28,30 @@ ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread,
TimeoutInfo &info)
: ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout",
thread, eVoteNo, eVoteNoOpinion),
- m_info(info), m_state(State::WaitTimeout), m_exit_flag(false) {
+ m_info(info), m_state(State::WaitTimeout) {
+ // TODO: reuse m_timer_thread without recreation.
m_timer_thread = std::thread(TimeoutThreadFunc, this);
- m_info.m_instance = this;
+ m_info.m_isAlive = true;
m_state = m_info.m_last_state;
}
ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() {
- m_info.m_instance = nullptr;
- if (m_state == State::Done)
- m_state = State::WaitTimeout;
+ m_info.m_isAlive = false;
+}
+
+uint64_t ThreadPlanSingleThreadTimeout::GetRemainingTimeoutMicroSeconds() {
+ uint64_t timeout_in_ms = GetThread().GetSingleThreadPlanTimeout();
+ std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();
+ std::chrono::microseconds duration_us =
+ std::chrono::duration_cast<std::chrono::microseconds>(now -
+ m_timeout_start);
+ return timeout_in_ms * 1000 - duration_us.count();
}
void ThreadPlanSingleThreadTimeout::GetDescription(
Stream *s, lldb::DescriptionLevel level) {
- s->Printf("Single thread timeout, state(%s)", StateToString(m_state).c_str());
+ s->Printf("Single thread timeout, state(%s), remaining %lld us",
+ StateToString(m_state).c_str(), GetRemainingTimeoutMicroSeconds());
}
std::string ThreadPlanSingleThreadTimeout::StateToString(State state) {
@@ -71,7 +80,9 @@ void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread,
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");
+ LLDB_LOGF(
+ log, "ThreadPlanSingleThreadTimeout pushing a brand new one with %llu ms",
+ timeout_in_ms);
}
void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread,
@@ -80,7 +91,8 @@ void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread,
if (timeout_in_ms == 0)
return;
- if (info.m_instance != nullptr)
+ // There is already an instance alive.
+ if (info.m_isAlive)
return;
// Do not create timeout if we are not stopping other threads.
@@ -92,7 +104,10 @@ void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread,
auto status = thread.QueueThreadPlan(thread_plan_sp,
/*abort_other_plans*/ false);
Log *log = GetLog(LLDBLog::Step);
- LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout reset from previous state");
+ LLDB_LOGF(
+ log,
+ "ThreadPlanSingleThreadTimeout reset from previous state with %llu ms",
+ timeout_in_ms);
}
bool ThreadPlanSingleThreadTimeout::WillStop() {
@@ -101,7 +116,6 @@ bool ThreadPlanSingleThreadTimeout::WillStop() {
// Reset the state during stop.
m_info.m_last_state = State::WaitTimeout;
- m_info.m_instance = this;
return true;
}
@@ -111,7 +125,7 @@ void ThreadPlanSingleThreadTimeout::DidPop() {
std::lock_guard<std::mutex> lock(m_mutex);
LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::DidPop().");
// Tell timer thread to exit.
- m_exit_flag = true;
+ m_info.m_isAlive = false;
}
m_wakeup_cv.notify_one();
// Wait for timer thread to exit.
@@ -119,14 +133,13 @@ void ThreadPlanSingleThreadTimeout::DidPop() {
}
bool ThreadPlanSingleThreadTimeout::DoPlanExplainsStop(Event *event_ptr) {
- lldb::StateType stop_state =
- Process::ProcessEventData::GetStateFromEvent(event_ptr);
+ bool is_timeout_interrupt = IsTimeoutAsyncInterrupt(event_ptr);
Log *log = GetLog(LLDBLog::Step);
- LLDB_LOGF(
- log,
- "ThreadPlanSingleThreadTimeout::DoPlanExplainsStop(): got event: %s.",
- StateAsCString(stop_state));
- return true;
+ LLDB_LOGF(log,
+ "ThreadPlanSingleThreadTimeout::DoPlanExplainsStop() returns %d. "
+ "%llu us remaining.",
+ is_timeout_interrupt, GetRemainingTimeoutMicroSeconds());
+ return is_timeout_interrupt;
}
lldb::StateType ThreadPlanSingleThreadTimeout::GetPlanRunState() {
@@ -137,15 +150,21 @@ 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; });
-
+ // The thread should wakeup either when timeout or
+ // ThreadPlanSingleThreadTimeout has been popped (not alive).
Log *log = GetLog(LLDBLog::Step);
+ self->m_timeout_start = std::chrono::steady_clock::now();
+ LLDB_LOGF(
+ log,
+ "ThreadPlanSingleThreadTimeout::TimeoutThreadFunc(), wait for %llu ms",
+ timeout_in_ms);
+ self->m_wakeup_cv.wait_for(lock, std::chrono::milliseconds(timeout_in_ms),
+ [self] { return !self->m_info.m_isAlive; });
LLDB_LOGF(log,
- "ThreadPlanSingleThreadTimeout::TimeoutThreadFunc() called with "
- "m_exit_flag(%d).",
- self->m_exit_flag);
- if (self->m_exit_flag)
+ "ThreadPlanSingleThreadTimeout::TimeoutThreadFunc() wake up with "
+ "m_isAlive(%d).",
+ self->m_info.m_isAlive);
+ if (!self->m_info.m_isAlive)
return;
self->HandleTimeout();
@@ -163,6 +182,8 @@ bool ThreadPlanSingleThreadTimeout::ShouldStop(Event *event_ptr) {
}
void ThreadPlanSingleThreadTimeout::SetStopOthers(bool new_value) {
+ // Note: this assumes that the SingleThreadTimeout plan is always going to be
+ // pushed on behalf of the plan directly above it.
GetPreviousPlan()->SetStopOthers(new_value);
}
@@ -173,16 +194,24 @@ bool ThreadPlanSingleThreadTimeout::StopOthers() {
return GetPreviousPlan()->StopOthers();
}
-bool ThreadPlanSingleThreadTimeout::HandleEvent(Event *event_ptr) {
+bool ThreadPlanSingleThreadTimeout::IsTimeoutAsyncInterrupt(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.",
+ LLDB_LOGF(log,
+ "ThreadPlanSingleThreadTimeout::IsTimeoutAsyncInterrupt(): 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) {
+ return (m_state == State::AsyncInterrupt &&
+ stop_state == lldb::eStateStopped && stop_info &&
+ stop_info->GetStopReason() == lldb::eStopReasonInterrupt);
+}
+
+bool ThreadPlanSingleThreadTimeout::HandleEvent(Event *event_ptr) {
+ if (IsTimeoutAsyncInterrupt(event_ptr)) {
+ Log *log = GetLog(LLDBLog::Step);
if (Process::ProcessEventData::GetRestartedFromEvent(event_ptr)) {
// If we were restarted, we just need to go back up to fetch
// another event.
diff --git a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
index f3370d2d82447..4ce3b443d8ee4 100644
--- a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
+++ b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
@@ -121,3 +121,120 @@ def test_step_over_multi_calls_large_timeout_slow_stepping(self):
)
self.dbg.HandleCommand("settings set target.use-fast-stepping false")
self.step_over_multi_calls_helper()
+
+ @skipIfWindows
+ def test_step_over_deadlock_with_inner_breakpoint_continue(self):
+ """Test step over deadlock function with inner breakpoint will trigger the breakpoint
+ and later continue will finish the stepping.
+ """
+ self.dbg.HandleCommand(
+ "settings set target.process.thread.single-thread-plan-timeout 2000"
+ )
+ (target, process, 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")
+
+ # Set breakpoint on inner function call
+ inner_breakpoint = target.BreakpointCreateByLocation(
+ lldb.SBFileSpec(self.main_source), line_number("main.cpp", "// Set interrupt breakpoint here"),
+ 0, 0, lldb.SBFileSpecList(), False
+ )
+
+ # Step over will hit the inner breakpoint and stop
+ self.thread.StepOver(lldb.eOnlyThisThread)
+ self.assertStopReason(self.thread.GetStopReason(), lldb.eStopReasonBreakpoint)
+ thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(
+ process, inner_breakpoint
+ )
+ self.assertTrue(thread1.IsValid(), "We are indeed stopped at inner breakpoint inside deadlock_func")
+
+ # Continue the process should complete the step-over
+ process.Continue()
+ self.assertState(process.GetState(), lldb.eStateStopped)
+ self.assertStopReason(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete)
+
+ self.verify_hit_correct_line("// Finish step-over from breakpoint1")
+
+ @skipIfWindows
+ def test_step_over_deadlock_with_inner_breakpoint_step(self):
+ """Test step over deadlock function with inner breakpoint will trigger the breakpoint
+ and later step still works
+ """
+ self.dbg.HandleCommand(
+ "settings set target.process.thread.single-thread-plan-timeout 2000"
+ )
+ (target, process, 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")
+
+ # Set breakpoint on inner function call
+ inner_breakpoint = target.BreakpointCreateByLocation(
+ lldb.SBFileSpec(self.main_source), line_number("main.cpp", "// Set interrupt breakpoint here"),
+ 0, 0, lldb.SBFileSpecList(), False
+ )
+
+ # Step over will hit the inner breakpoint and stop
+ self.thread.StepOver(lldb.eOnlyThisThread)
+ self.assertStopReason(self.thread.GetStopReason(), lldb.eStopReasonBreakpoint)
+ thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(
+ process, inner_breakpoint
+ )
+ self.assertTrue(thread1.IsValid(), "We are indeed stopped at inner breakpoint inside deadlock_func")
+
+ # Step still works
+ self.thread.StepOver(lldb.eOnlyThisThread)
+ self.assertState(process.GetState(), lldb.eStateStopped)
+ self.assertStopReason(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete)
+
+ self.verify_hit_correct_line("// Finish step-over from inner breakpoint")
+
+ @skipIfWindows
+ def test_step_over_deadlock_with_user_async_interrupt(self):
+ """Test step over deadlock function without large timeout then async interrupt will result
+ Correct stop reason
+ """
+
+ self.dbg.HandleCommand(
+ "settings set target.process.thread.single-thread-plan-timeout 2000000"
+ )
+
+ (target, process, 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.dbg.SetAsync(True)
+
+ # This stepping should block due to large timeout and should be interrupted by the
+ # async interrupt from the worker thread
+ self.thread.StepOver(lldb.eOnlyThisThread)
+ time.sleep(1)
+
+ listener = self.dbg.GetListener()
+ lldbutil.expect_state_changes(self, listener, process, [lldb.eStateRunning])
+ self.dbg.SetAsync(False)
+
+ process.SendAsyncInterrupt()
+
+ lldbutil.expect_state_changes(self, listener, process, [lldb.eStateStopped])
+ self.assertStopReason(self.thread.GetStopReason(), lldb.eStopReasonSignal)
diff --git a/lldb/test/API/functionalities/single-thread-step/main.cpp b/lldb/test/API/functionalities/single-thread-step/main.cpp
index 16356a08375b8..fe4fb11468d98 100644
--- a/lldb/test/API/functionalities/single-thread-step/main.cpp
+++ b/lldb/test/API/functionalities/single-thread-step/main.cpp
@@ -27,13 +27,19 @@ void worker(int id) {
std::cout << "Worker " << id << " finished." << std::endl;
}
+void deadlock_func(std::unique_lock<std::mutex> &lock) {
+ int i = 10;
+ ++i; // Set interrupt breakpoint here
+ printf("%d", i); // Finish step-over from inner breakpoint
+ auto func = [] { return ready_thread_id == 1; };
+ cv.wait(lock, func);
+}
+
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
+ deadlock_func(lock); // Set breakpoint1 here
std::thread t2(worker, 2); // Finish step-over from breakpoint1
>From 92cb3ab7a724cc8ffdfca75e3ad7a0b4b0b58c62 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Wed, 10 Jul 2024 13:04:22 -0700
Subject: [PATCH 07/10] Fix python test formatter
---
.../TestSingleThreadStepTimeout.py | 64 +++++++++++--------
1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
index 4ce3b443d8ee4..71aba07b63520 100644
--- a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
+++ b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
@@ -121,11 +121,11 @@ def test_step_over_multi_calls_large_timeout_slow_stepping(self):
)
self.dbg.HandleCommand("settings set target.use-fast-stepping false")
self.step_over_multi_calls_helper()
-
+
@skipIfWindows
def test_step_over_deadlock_with_inner_breakpoint_continue(self):
"""Test step over deadlock function with inner breakpoint will trigger the breakpoint
- and later continue will finish the stepping.
+ and later continue will finish the stepping.
"""
self.dbg.HandleCommand(
"settings set target.process.thread.single-thread-plan-timeout 2000"
@@ -140,11 +140,15 @@ def test_step_over_deadlock_with_inner_breakpoint_continue(self):
# 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")
-
+
# Set breakpoint on inner function call
inner_breakpoint = target.BreakpointCreateByLocation(
- lldb.SBFileSpec(self.main_source), line_number("main.cpp", "// Set interrupt breakpoint here"),
- 0, 0, lldb.SBFileSpecList(), False
+ lldb.SBFileSpec(self.main_source),
+ line_number("main.cpp", "// Set interrupt breakpoint here"),
+ 0,
+ 0,
+ lldb.SBFileSpecList(),
+ False,
)
# Step over will hit the inner breakpoint and stop
@@ -153,19 +157,22 @@ def test_step_over_deadlock_with_inner_breakpoint_continue(self):
thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(
process, inner_breakpoint
)
- self.assertTrue(thread1.IsValid(), "We are indeed stopped at inner breakpoint inside deadlock_func")
-
+ self.assertTrue(
+ thread1.IsValid(),
+ "We are indeed stopped at inner breakpoint inside deadlock_func",
+ )
+
# Continue the process should complete the step-over
process.Continue()
self.assertState(process.GetState(), lldb.eStateStopped)
self.assertStopReason(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete)
-
- self.verify_hit_correct_line("// Finish step-over from breakpoint1")
-
+
+ self.verify_hit_correct_line("// Finish step-over from breakpoint1")
+
@skipIfWindows
def test_step_over_deadlock_with_inner_breakpoint_step(self):
"""Test step over deadlock function with inner breakpoint will trigger the breakpoint
- and later step still works
+ and later step still works
"""
self.dbg.HandleCommand(
"settings set target.process.thread.single-thread-plan-timeout 2000"
@@ -180,11 +187,15 @@ def test_step_over_deadlock_with_inner_breakpoint_step(self):
# 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")
-
+
# Set breakpoint on inner function call
inner_breakpoint = target.BreakpointCreateByLocation(
- lldb.SBFileSpec(self.main_source), line_number("main.cpp", "// Set interrupt breakpoint here"),
- 0, 0, lldb.SBFileSpecList(), False
+ lldb.SBFileSpec(self.main_source),
+ line_number("main.cpp", "// Set interrupt breakpoint here"),
+ 0,
+ 0,
+ lldb.SBFileSpecList(),
+ False,
)
# Step over will hit the inner breakpoint and stop
@@ -193,25 +204,28 @@ def test_step_over_deadlock_with_inner_breakpoint_step(self):
thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(
process, inner_breakpoint
)
- self.assertTrue(thread1.IsValid(), "We are indeed stopped at inner breakpoint inside deadlock_func")
-
+ self.assertTrue(
+ thread1.IsValid(),
+ "We are indeed stopped at inner breakpoint inside deadlock_func",
+ )
+
# Step still works
self.thread.StepOver(lldb.eOnlyThisThread)
self.assertState(process.GetState(), lldb.eStateStopped)
self.assertStopReason(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete)
-
- self.verify_hit_correct_line("// Finish step-over from inner breakpoint")
+
+ self.verify_hit_correct_line("// Finish step-over from inner breakpoint")
@skipIfWindows
def test_step_over_deadlock_with_user_async_interrupt(self):
"""Test step over deadlock function without large timeout then async interrupt will result
Correct stop reason
"""
-
+
self.dbg.HandleCommand(
"settings set target.process.thread.single-thread-plan-timeout 2000000"
)
-
+
(target, process, self.thread, _) = lldbutil.run_to_source_breakpoint(
self, "// Set breakpoint1 here", lldb.SBFileSpec(self.main_source)
)
@@ -222,18 +236,18 @@ def test_step_over_deadlock_with_user_async_interrupt(self):
# 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.dbg.SetAsync(True)
-
- # This stepping should block due to large timeout and should be interrupted by the
+
+ # This stepping should block due to large timeout and should be interrupted by the
# async interrupt from the worker thread
self.thread.StepOver(lldb.eOnlyThisThread)
time.sleep(1)
-
+
listener = self.dbg.GetListener()
lldbutil.expect_state_changes(self, listener, process, [lldb.eStateRunning])
self.dbg.SetAsync(False)
-
+
process.SendAsyncInterrupt()
lldbutil.expect_state_changes(self, listener, process, [lldb.eStateStopped])
>From 261d77da1dc0cc9f7dcb5241c23367847369f315 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Wed, 10 Jul 2024 19:00:50 -0700
Subject: [PATCH 08/10] Some more changes
---
.../lldb/Target/ThreadPlanSingleThreadTimeout.h | 4 ++--
.../Target/ThreadPlanSingleThreadTimeout.cpp | 16 ++++++++--------
.../TestSingleThreadStepTimeout.py | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
index 781fd88b4d863..1d88d6a51260b 100644
--- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
+++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h
@@ -43,7 +43,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
// TODO: allow timeout to be set on per thread plan basis.
struct TimeoutInfo {
// Whether there is a ThreadPlanSingleThreadTimeout instance alive.
- bool m_isAlive;
+ bool m_isAlive = false;
ThreadPlanSingleThreadTimeout::State m_last_state = State::WaitTimeout;
};
@@ -83,7 +83,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan {
bool IsTimeoutAsyncInterrupt(Event *event_ptr);
bool HandleEvent(Event *event_ptr);
void HandleTimeout();
- uint64_t GetRemainingTimeoutMicroSeconds();
+ uint64_t GetRemainingTimeoutMilliSeconds();
static std::string StateToString(State state);
diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
index 6a1da216da0ef..663d885222cc6 100644
--- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
+++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
@@ -39,19 +39,19 @@ ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() {
m_info.m_isAlive = false;
}
-uint64_t ThreadPlanSingleThreadTimeout::GetRemainingTimeoutMicroSeconds() {
+uint64_t ThreadPlanSingleThreadTimeout::GetRemainingTimeoutMilliSeconds() {
uint64_t timeout_in_ms = GetThread().GetSingleThreadPlanTimeout();
std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();
- std::chrono::microseconds duration_us =
- std::chrono::duration_cast<std::chrono::microseconds>(now -
+ std::chrono::milliseconds duration_ms =
+ std::chrono::duration_cast<std::chrono::milliseconds>(now -
m_timeout_start);
- return timeout_in_ms * 1000 - duration_us.count();
+ return timeout_in_ms - duration_ms.count();
}
void ThreadPlanSingleThreadTimeout::GetDescription(
Stream *s, lldb::DescriptionLevel level) {
- s->Printf("Single thread timeout, state(%s), remaining %lld us",
- StateToString(m_state).c_str(), GetRemainingTimeoutMicroSeconds());
+ s->Printf("Single thread timeout, state(%s), remaining %lld ms",
+ StateToString(m_state).c_str(), GetRemainingTimeoutMilliSeconds());
}
std::string ThreadPlanSingleThreadTimeout::StateToString(State state) {
@@ -137,8 +137,8 @@ bool ThreadPlanSingleThreadTimeout::DoPlanExplainsStop(Event *event_ptr) {
Log *log = GetLog(LLDBLog::Step);
LLDB_LOGF(log,
"ThreadPlanSingleThreadTimeout::DoPlanExplainsStop() returns %d. "
- "%llu us remaining.",
- is_timeout_interrupt, GetRemainingTimeoutMicroSeconds());
+ "%llu ms remaining.",
+ is_timeout_interrupt, GetRemainingTimeoutMilliSeconds());
return is_timeout_interrupt;
}
diff --git a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
index 71aba07b63520..d677ed301c6f2 100644
--- a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
+++ b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
@@ -218,8 +218,8 @@ def test_step_over_deadlock_with_inner_breakpoint_step(self):
@skipIfWindows
def test_step_over_deadlock_with_user_async_interrupt(self):
- """Test step over deadlock function without large timeout then async interrupt will result
- Correct stop reason
+ """Test step over deadlock function with large timeout then send async interrupt
+ should report correct stop reason
"""
self.dbg.HandleCommand(
>From 2d6e565a63038501818f4fc262f08d8c64310dca Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Fri, 12 Jul 2024 09:31:54 -0700
Subject: [PATCH 09/10] Fix format specifier and python formatting
---
.../Target/ThreadPlanSingleThreadTimeout.cpp | 14 +++++++++-----
.../TestSingleThreadStepTimeout.py | 2 +-
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
index 663d885222cc6..9cc21bfbb1952 100644
--- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
+++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
@@ -50,7 +50,7 @@ uint64_t ThreadPlanSingleThreadTimeout::GetRemainingTimeoutMilliSeconds() {
void ThreadPlanSingleThreadTimeout::GetDescription(
Stream *s, lldb::DescriptionLevel level) {
- s->Printf("Single thread timeout, state(%s), remaining %lld ms",
+ s->Printf("Single thread timeout, state(%s), remaining %" PRIu64 " ms",
StateToString(m_state).c_str(), GetRemainingTimeoutMilliSeconds());
}
@@ -81,7 +81,9 @@ void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread,
/*abort_other_plans*/ false);
Log *log = GetLog(LLDBLog::Step);
LLDB_LOGF(
- log, "ThreadPlanSingleThreadTimeout pushing a brand new one with %llu ms",
+ log,
+ "ThreadPlanSingleThreadTimeout pushing a brand new one with %" PRIu64
+ " ms",
timeout_in_ms);
}
@@ -106,7 +108,8 @@ void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread,
Log *log = GetLog(LLDBLog::Step);
LLDB_LOGF(
log,
- "ThreadPlanSingleThreadTimeout reset from previous state with %llu ms",
+ "ThreadPlanSingleThreadTimeout reset from previous state with %" PRIu64
+ " ms",
timeout_in_ms);
}
@@ -137,7 +140,7 @@ bool ThreadPlanSingleThreadTimeout::DoPlanExplainsStop(Event *event_ptr) {
Log *log = GetLog(LLDBLog::Step);
LLDB_LOGF(log,
"ThreadPlanSingleThreadTimeout::DoPlanExplainsStop() returns %d. "
- "%llu ms remaining.",
+ "%" PRIu64 " ms remaining.",
is_timeout_interrupt, GetRemainingTimeoutMilliSeconds());
return is_timeout_interrupt;
}
@@ -156,7 +159,8 @@ void ThreadPlanSingleThreadTimeout::TimeoutThreadFunc(
self->m_timeout_start = std::chrono::steady_clock::now();
LLDB_LOGF(
log,
- "ThreadPlanSingleThreadTimeout::TimeoutThreadFunc(), wait for %llu ms",
+ "ThreadPlanSingleThreadTimeout::TimeoutThreadFunc(), wait for %" PRIu64
+ " ms",
timeout_in_ms);
self->m_wakeup_cv.wait_for(lock, std::chrono::milliseconds(timeout_in_ms),
[self] { return !self->m_info.m_isAlive; });
diff --git a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
index d677ed301c6f2..214a2fb6cb87b 100644
--- a/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
+++ b/lldb/test/API/functionalities/single-thread-step/TestSingleThreadStepTimeout.py
@@ -218,7 +218,7 @@ def test_step_over_deadlock_with_inner_breakpoint_step(self):
@skipIfWindows
def test_step_over_deadlock_with_user_async_interrupt(self):
- """Test step over deadlock function with large timeout then send async interrupt
+ """Test step over deadlock function with large timeout then send async interrupt
should report correct stop reason
"""
>From 82e1237b487a414a157757373887b5e3381ba4e6 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Mon, 5 Aug 2024 10:24:22 -0700
Subject: [PATCH 10/10] Fix failing test
---
lldb/source/Target/ThreadPlanStepOverRange.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp
index 381d4ae5de05d..934f23b3b21a2 100644
--- a/lldb/source/Target/ThreadPlanStepOverRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp
@@ -348,6 +348,7 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) {
}
void ThreadPlanStepOverRange::DidPush() {
+ ThreadPlanStepRange::DidPush();
if (m_run_mode == lldb::eOnlyThisThread && IsControllingPlan())
PushNewTimeout();
}
More information about the lldb-commits
mailing list