[Lldb-commits] [lldb] Fix ASAN failure in TestSingleThreadStepTimeout.py (PR #102208)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 6 13:54:08 PDT 2024
https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/102208
>From 03e22727aa470b60762baa25e20a36f875d451c3 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at 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 <jeffreytan at 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;
More information about the lldb-commits
mailing list