Author: jeffreytan81 Date: 2024-08-06T18:08:55-07:00 New Revision: 128ef9eb533afd00da2d3d2cfeab16de6abf2640
URL: https://github.com/llvm/llvm-project/commit/128ef9eb533afd00da2d3d2cfeab16de6abf2640 DIFF: https://github.com/llvm/llvm-project/commit/128ef9eb533afd00da2d3d2cfeab16de6abf2640.diff LOG: Fix ASAN failure in TestSingleThreadStepTimeout.py (#102208) This PR fixes the ASAN failure in https://github.com/llvm/llvm-project/pull/90930. The original PR made the assumption that parent `ThreadPlanStepOverRange`'s lifetime will always be longer than `ThreadPlanSingleThreadTimeout` leaf plan so it passes the `m_timeout_info` as reference to it. >From the ASAN failure, it seems that this assumption may not be true (likely the thread stack is holding a strong reference to the leaf plan). This PR fixes this lifetime issue by using shared pointer instead of passing by reference. --------- Co-authored-by: jeffreytan81 <jeffrey...@fb.com> Added: Modified: lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h lldb/include/lldb/Target/TimeoutResumeAll.h lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h index 1d88d6a51260b..5eff1186c6402 100644 --- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h +++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h @@ -20,6 +20,7 @@ namespace lldb_private { +class ThreadPlanSingleThreadTimeout; // // Thread plan used by single thread execution to issue timeout. This is useful // to detect potential deadlock in single thread execution. The timeout measures @@ -46,6 +47,8 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { bool m_isAlive = false; ThreadPlanSingleThreadTimeout::State m_last_state = State::WaitTimeout; }; + using TimeoutInfoSP = + std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo>; ~ThreadPlanSingleThreadTimeout() override; @@ -54,11 +57,11 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { // state. The reference of \param info is passed in so that when // ThreadPlanSingleThreadTimeout got popped its last state can be stored // in it for future resume. - static void PushNewWithTimeout(Thread &thread, TimeoutInfo &info); + static void PushNewWithTimeout(Thread &thread, TimeoutInfoSP &info); // Push a new ThreadPlanSingleThreadTimeout by restoring state from // input \param info and resume execution. - static void ResumeFromPrevState(Thread &thread, TimeoutInfo &info); + static void ResumeFromPrevState(Thread &thread, TimeoutInfoSP &info); void GetDescription(Stream *s, lldb::DescriptionLevel level) override; bool ValidatePlan(Stream *error) override { return true; } @@ -78,7 +81,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { bool StopOthers() override; private: - ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfo &info); + ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfoSP &info); bool IsTimeoutAsyncInterrupt(Event *event_ptr); bool HandleEvent(Event *event_ptr); @@ -91,7 +94,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { const ThreadPlanSingleThreadTimeout & operator=(const ThreadPlanSingleThreadTimeout &) = delete; - TimeoutInfo &m_info; // Reference to controlling ThreadPlan's TimeoutInfo. + TimeoutInfoSP 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 diff --git a/lldb/include/lldb/Target/TimeoutResumeAll.h b/lldb/include/lldb/Target/TimeoutResumeAll.h index d484ea02bcce9..9a1a6c46e53b5 100644 --- a/lldb/include/lldb/Target/TimeoutResumeAll.h +++ b/lldb/include/lldb/Target/TimeoutResumeAll.h @@ -19,7 +19,10 @@ namespace lldb_private { // ResumeWithTimeout() during DoWillResume(). class TimeoutResumeAll { public: - TimeoutResumeAll(Thread &thread) : m_thread(thread) {} + TimeoutResumeAll(Thread &thread) + : m_thread(thread), + m_timeout_info( + std::make_shared<ThreadPlanSingleThreadTimeout::TimeoutInfo>()) {} void PushNewTimeout() { ThreadPlanSingleThreadTimeout::PushNewWithTimeout(m_thread, m_timeout_info); @@ -32,7 +35,7 @@ class TimeoutResumeAll { private: Thread &m_thread; - ThreadPlanSingleThreadTimeout::TimeoutInfo m_timeout_info; + ThreadPlanSingleThreadTimeout::TimeoutInfoSP m_timeout_info; }; } // namespace lldb_private diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp index 9cc21bfbb1952..40a8af8e70343 100644 --- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -24,19 +24,19 @@ using namespace lldb_private; using namespace lldb; -ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread, - TimeoutInfo &info) +ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout( + Thread &thread, TimeoutInfoSP &info) : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout", thread, eVoteNo, eVoteNoOpinion), m_info(info), m_state(State::WaitTimeout) { // TODO: reuse m_timer_thread without recreation. m_timer_thread = std::thread(TimeoutThreadFunc, this); - m_info.m_isAlive = true; - m_state = m_info.m_last_state; + m_info->m_isAlive = true; + m_state = m_info->m_last_state; } ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() { - m_info.m_isAlive = false; + m_info->m_isAlive = false; } uint64_t ThreadPlanSingleThreadTimeout::GetRemainingTimeoutMilliSeconds() { @@ -66,7 +66,7 @@ std::string ThreadPlanSingleThreadTimeout::StateToString(State state) { } void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread, - TimeoutInfo &info) { + TimeoutInfoSP &info) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; @@ -88,13 +88,13 @@ void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread, } void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread, - TimeoutInfo &info) { + TimeoutInfoSP &info) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; // There is already an instance alive. - if (info.m_isAlive) + if (info->m_isAlive) return; // Do not create timeout if we are not stopping other threads. @@ -118,7 +118,7 @@ bool ThreadPlanSingleThreadTimeout::WillStop() { LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::WillStop()."); // Reset the state during stop. - m_info.m_last_state = State::WaitTimeout; + m_info->m_last_state = State::WaitTimeout; return true; } @@ -128,7 +128,7 @@ void ThreadPlanSingleThreadTimeout::DidPop() { std::lock_guard<std::mutex> lock(m_mutex); LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::DidPop()."); // Tell timer thread to exit. - m_info.m_isAlive = false; + m_info->m_isAlive = false; } m_wakeup_cv.notify_one(); // Wait for timer thread to exit. @@ -163,12 +163,12 @@ void ThreadPlanSingleThreadTimeout::TimeoutThreadFunc( " ms", timeout_in_ms); self->m_wakeup_cv.wait_for(lock, std::chrono::milliseconds(timeout_in_ms), - [self] { return !self->m_info.m_isAlive; }); + [self] { return !self->m_info->m_isAlive; }); LLDB_LOGF(log, "ThreadPlanSingleThreadTimeout::TimeoutThreadFunc() wake up with " "m_isAlive(%d).", - self->m_info.m_isAlive); - if (!self->m_info.m_isAlive) + self->m_info->m_isAlive); + if (!self->m_info->m_isAlive) return; self->HandleTimeout(); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
