https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/102208
>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 1/2] 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 1d88d6a51260b1..87e6ba6d766955 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 d484ea02bcce94..30e923f30602a3 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 9cc21bfbb1952d..17e6befb7d388c 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(); >From 91a23953eb6c1ed840d7c24199cd7b4b612b61dd Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Tue, 6 Aug 2024 13:52:30 -0700 Subject: [PATCH 2/2] Using type aliasing for shared_ptr --- .../Target/ThreadPlanSingleThreadTimeout.h | 18 +++++++----------- lldb/include/lldb/Target/TimeoutResumeAll.h | 2 +- .../Target/ThreadPlanSingleThreadTimeout.cpp | 13 +++++-------- 3 files changed, 13 insertions(+), 20 deletions(-) diff --git a/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h b/lldb/include/lldb/Target/ThreadPlanSingleThreadTimeout.h index 87e6ba6d766955..5eff1186c6402d 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,15 +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, - std::shared_ptr<ThreadPlanSingleThreadTimeout::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, - std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &info); + static void ResumeFromPrevState(Thread &thread, TimeoutInfoSP &info); void GetDescription(Stream *s, lldb::DescriptionLevel level) override; bool ValidatePlan(Stream *error) override { return true; } @@ -82,9 +81,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { bool StopOthers() override; private: - ThreadPlanSingleThreadTimeout( - Thread &thread, - std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &info); + ThreadPlanSingleThreadTimeout(Thread &thread, TimeoutInfoSP &info); bool IsTimeoutAsyncInterrupt(Event *event_ptr); bool HandleEvent(Event *event_ptr); @@ -97,8 +94,7 @@ class ThreadPlanSingleThreadTimeout : public ThreadPlan { const ThreadPlanSingleThreadTimeout & operator=(const ThreadPlanSingleThreadTimeout &) = delete; - std::shared_ptr<ThreadPlanSingleThreadTimeout::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 30e923f30602a3..9a1a6c46e53b54 100644 --- a/lldb/include/lldb/Target/TimeoutResumeAll.h +++ b/lldb/include/lldb/Target/TimeoutResumeAll.h @@ -35,7 +35,7 @@ class TimeoutResumeAll { private: Thread &m_thread; - std::shared_ptr<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 17e6befb7d388c..40a8af8e703432 100644 --- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp +++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp @@ -25,8 +25,7 @@ using namespace lldb_private; using namespace lldb; ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout( - Thread &thread, - std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &info) + Thread &thread, TimeoutInfoSP &info) : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout", thread, eVoteNo, eVoteNoOpinion), m_info(info), m_state(State::WaitTimeout) { @@ -66,9 +65,8 @@ std::string ThreadPlanSingleThreadTimeout::StateToString(State state) { } } -void ThreadPlanSingleThreadTimeout::PushNewWithTimeout( - Thread &thread, - std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &info) { +void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread, + TimeoutInfoSP &info) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; @@ -89,9 +87,8 @@ void ThreadPlanSingleThreadTimeout::PushNewWithTimeout( timeout_in_ms); } -void ThreadPlanSingleThreadTimeout::ResumeFromPrevState( - Thread &thread, - std::shared_ptr<ThreadPlanSingleThreadTimeout::TimeoutInfo> &info) { +void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread, + TimeoutInfoSP &info) { uint64_t timeout_in_ms = thread.GetSingleThreadPlanTimeout(); if (timeout_in_ms == 0) return; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits