https://github.com/jeffreytan81 created https://github.com/llvm/llvm-project/pull/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. >From 03e22727aa470b60762baa25e20a36f875d451c3 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Tue, 6 Aug 2024 12:47:50 -0700 Subject: [PATCH] Fix ASAN failure in TestSingleThreadStepTimeout.py --- .../Target/ThreadPlanSingleThreadTimeout.h | 15 ++++++--- lldb/include/lldb/Target/TimeoutResumeAll.h | 7 ++-- .../Target/ThreadPlanSingleThreadTimeout.cpp | 33 ++++++++++--------- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h index 1d88d6a51260b..87e6ba6d76695 100644 --- a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h +++ b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h @@ -54,11 +54,15 @@ 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, + std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &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, + std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &info); void GetDescription(Stream *s, lldb::DescriptionLevel level) override; bool ValidatePlan(Stream *error) override { return true; } @@ -78,7 +82,9 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { bool StopOthers() override; private: - ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfo &info); + ThreadPlanSingleThreadTimeout( + Thread &thread, + std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &info); bool IsTimeoutAsyncInterrupt(Event *event_ptr); bool HandleEvent(Event *event_ptr); @@ -91,7 +97,8 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { const ThreadPlanSingleThreadTimeout & operator=(const ThreadPlanSingleThreadTimeout &) = delete; - TimeoutInfo &m_info; // Reference to controlling ThreadPlan's TimeoutInfo. + std::shared_ptr<ThreadPlanSingleThreadTimeout::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 diff --git a/lldb/include/lldb/Target/TimeoutResumeAll.h b/lldb/include/lldb/Target/TimeoutResumeAll.h index d484ea02bcce9..30e923f30602a 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; + std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> m_timeout_info; }; } // namespace lldb_private diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp index 9cc21bfbb1952..17e6befb7d388 100644 --- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -24,19 +24,20 @@ using namespace lldb_private; using namespace lldb; -ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(Thread &thread, - TimeoutInfo &info) +ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout( + Thread &thread, + std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &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() { @@ -65,8 +66,9 @@ std::string ThreadPlanSingleThreadTimeout::StateToString(State state) { } } -void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread, - TimeoutInfo &info) { +void ThreadPlanSingleThreadTimeout::PushNewWithTimeout( + Thread &thread, + std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &info) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; @@ -87,14 +89,15 @@ void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread, timeout_in_ms); } -void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread, - TimeoutInfo &info) { +void ThreadPlanSingleThreadTimeout::ResumeFromPrevState( + Thread &thread, + std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &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 +121,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 +131,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 +166,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
