[compiler-rt] [NFC][asan] Cleanup AsanThreadIdAndName ctor/init (PR #111923)
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 10 17:54:17 PDT 2024
https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/111923
>From 5e9272e50610634f1b0c42aaad47d0f90a8c9762 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 10 Oct 2024 16:38:23 -0700
Subject: [PATCH 1/2] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?=
=?UTF-8?q?anges=20to=20main=20this=20commit=20is=20based=20on?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
[skip ci]
---
compiler-rt/lib/asan/asan_descriptions.cpp | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_descriptions.cpp b/compiler-rt/lib/asan/asan_descriptions.cpp
index 1c2f20a76343bb..674fe9c1e90be0 100644
--- a/compiler-rt/lib/asan/asan_descriptions.cpp
+++ b/compiler-rt/lib/asan/asan_descriptions.cpp
@@ -48,9 +48,20 @@ void DescribeThread(AsanThreadContext *context) {
return;
}
context->announced = true;
+
+ AsanThreadContext *parent_context =
+ context->parent_tid == kInvalidTid
+ ? nullptr
+ : GetThreadContextByTidLocked(context->parent_tid);
+
+ // `context->parent_tid` may point to reused slot. Check `unique_id` which
+ // is always smaller for the parent, always greater for a new user.
+ if (context->unique_id <= parent_context->unique_id)
+ parent_context = nullptr;
+
InternalScopedString str;
str.AppendF("Thread %s", AsanThreadIdAndName(context).c_str());
- if (context->parent_tid == kInvalidTid) {
+ if (!parent_context) {
str.Append(" created by unknown thread\n");
Printf("%s", str.data());
return;
@@ -60,11 +71,8 @@ void DescribeThread(AsanThreadContext *context) {
Printf("%s", str.data());
StackDepotGet(context->stack_id).Print();
// Recursively described parent thread if needed.
- if (flags()->print_full_thread_history) {
- AsanThreadContext *parent_context =
- GetThreadContextByTidLocked(context->parent_tid);
+ if (flags()->print_full_thread_history)
DescribeThread(parent_context);
- }
}
// Shadow descriptions
>From c2994138bf793948350f273c4e448409caeb02e8 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 10 Oct 2024 17:54:06 -0700
Subject: [PATCH 2/2] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?=
=?UTF-8?q?anges=20introduced=20through=20rebase?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
[skip ci]
---
compiler-rt/lib/asan/asan_fuchsia.cpp | 3 +--
compiler-rt/lib/asan/asan_thread.cpp | 9 +++-----
compiler-rt/lib/asan/asan_thread.h | 11 +++------
compiler-rt/lib/memprof/memprof_thread.cpp | 9 +++-----
compiler-rt/lib/memprof/memprof_thread.h | 8 +------
.../sanitizer_thread_registry.cpp | 13 +++++++----
.../sanitizer_thread_registry.h | 9 ++++++--
.../tests/sanitizer_thread_registry_test.cpp | 23 +++++++++++++------
8 files changed, 43 insertions(+), 42 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fuchsia.cpp b/compiler-rt/lib/asan/asan_fuchsia.cpp
index dbc4342e83388c..96c41e9d42ba6a 100644
--- a/compiler-rt/lib/asan/asan_fuchsia.cpp
+++ b/compiler-rt/lib/asan/asan_fuchsia.cpp
@@ -121,8 +121,7 @@ static AsanThread *CreateAsanThread(StackTrace *stack, u32 parent_tid,
// In lieu of AsanThread::Create.
AsanThread *thread = (AsanThread *)MmapOrDie(AsanThreadMmapSize(), __func__);
- AsanThreadContext::CreateThreadContextArgs args = {thread, stack};
- u32 tid = asanThreadRegistry().CreateThread(0, detached, parent_tid, &args);
+ u32 tid = asanThreadRegistry().CreateThread(0, detached, parent_tid, thread);
asanThreadRegistry().SetThreadName(tid, name);
return thread;
diff --git a/compiler-rt/lib/asan/asan_thread.cpp b/compiler-rt/lib/asan/asan_thread.cpp
index c1a804b9fcccd3..0779daa107682b 100644
--- a/compiler-rt/lib/asan/asan_thread.cpp
+++ b/compiler-rt/lib/asan/asan_thread.cpp
@@ -28,10 +28,7 @@ namespace __asan {
// AsanThreadContext implementation.
void AsanThreadContext::OnCreated(void *arg) {
- CreateThreadContextArgs *args = static_cast<CreateThreadContextArgs *>(arg);
- if (args->stack)
- stack_id = StackDepotPut(*args->stack);
- thread = args->thread;
+ thread = static_cast<AsanThread *>(arg);
thread->set_context(this);
}
@@ -106,8 +103,8 @@ AsanThread *AsanThread::Create(const void *start_data, uptr data_size,
CHECK_LE(data_size, availible_size);
internal_memcpy(thread->start_data_, start_data, data_size);
}
- AsanThreadContext::CreateThreadContextArgs args = {thread, stack};
- asanThreadRegistry().CreateThread(0, detached, parent_tid, &args);
+ asanThreadRegistry().CreateThread(0, detached, parent_tid,
+ stack ? StackDepotPut(*stack) : 0, thread);
return thread;
}
diff --git a/compiler-rt/lib/asan/asan_thread.h b/compiler-rt/lib/asan/asan_thread.h
index 62f1b5337fe4bf..ad9e03d68fe96a 100644
--- a/compiler-rt/lib/asan/asan_thread.h
+++ b/compiler-rt/lib/asan/asan_thread.h
@@ -36,21 +36,16 @@ class AsanThread;
class AsanThreadContext final : public ThreadContextBase {
public:
explicit AsanThreadContext(int tid)
- : ThreadContextBase(tid), announced(false),
- destructor_iterations(GetPthreadDestructorIterations()), stack_id(0),
+ : ThreadContextBase(tid),
+ announced(false),
+ destructor_iterations(GetPthreadDestructorIterations()),
thread(nullptr) {}
bool announced;
u8 destructor_iterations;
- u32 stack_id;
AsanThread *thread;
void OnCreated(void *arg) override;
void OnFinished() override;
-
- struct CreateThreadContextArgs {
- AsanThread *thread;
- StackTrace *stack;
- };
};
// AsanThreadContext objects are never freed, so we need many of them.
diff --git a/compiler-rt/lib/memprof/memprof_thread.cpp b/compiler-rt/lib/memprof/memprof_thread.cpp
index 50072bb91ee74c..4b9665ffc3fcea 100644
--- a/compiler-rt/lib/memprof/memprof_thread.cpp
+++ b/compiler-rt/lib/memprof/memprof_thread.cpp
@@ -25,10 +25,7 @@ namespace __memprof {
// MemprofThreadContext implementation.
void MemprofThreadContext::OnCreated(void *arg) {
- CreateThreadContextArgs *args = static_cast<CreateThreadContextArgs *>(arg);
- if (args->stack)
- stack_id = StackDepotPut(*args->stack);
- thread = args->thread;
+ thread = static_cast<MemprofThread *>(arg);
thread->set_context(this);
}
@@ -79,8 +76,8 @@ MemprofThread *MemprofThread::Create(thread_callback_t start_routine, void *arg,
MemprofThread *thread = (MemprofThread *)MmapOrDie(size, __func__);
thread->start_routine_ = start_routine;
thread->arg_ = arg;
- MemprofThreadContext::CreateThreadContextArgs args = {thread, stack};
- memprofThreadRegistry().CreateThread(0, detached, parent_tid, &args);
+ memprofThreadRegistry().CreateThread(
+ 0, detached, parent_tid, stack ? StackDepotPut(*stack) : 0, thread);
return thread;
}
diff --git a/compiler-rt/lib/memprof/memprof_thread.h b/compiler-rt/lib/memprof/memprof_thread.h
index 4c9313fcb369eb..fb90dbf328a430 100644
--- a/compiler-rt/lib/memprof/memprof_thread.h
+++ b/compiler-rt/lib/memprof/memprof_thread.h
@@ -34,20 +34,14 @@ class MemprofThread;
struct MemprofThreadContext final : public ThreadContextBase {
explicit MemprofThreadContext(int tid)
: ThreadContextBase(tid), announced(false),
- destructor_iterations(GetPthreadDestructorIterations()), stack_id(0),
+ destructor_iterations(GetPthreadDestructorIterations()),
thread(nullptr) {}
bool announced;
u8 destructor_iterations;
- u32 stack_id;
MemprofThread *thread;
void OnCreated(void *arg) override;
void OnFinished() override;
-
- struct CreateThreadContextArgs {
- MemprofThread *thread;
- StackTrace *stack;
- };
};
// MemprofThreadContext objects are never freed, so we need many of them.
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
index df04822b28851c..cdc24f4a8869c7 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
@@ -27,6 +27,7 @@ ThreadContextBase::ThreadContextBase(u32 tid)
detached(false),
thread_type(ThreadType::Regular),
parent_tid(0),
+ stack_id(0),
next(0) {
name[0] = '\0';
atomic_store(&thread_destroyed, 0, memory_order_release);
@@ -88,14 +89,17 @@ void ThreadContextBase::SetStarted(tid_t _os_id, ThreadType _thread_type,
}
void ThreadContextBase::SetCreated(uptr _user_id, u64 _unique_id,
- bool _detached, u32 _parent_tid, void *arg) {
+ bool _detached, u32 _parent_tid,
+ u32 _stack_tid, void *arg) {
status = ThreadStatusCreated;
user_id = _user_id;
unique_id = _unique_id;
detached = _detached;
// Parent tid makes no sense for the main thread.
- if (tid != kMainTid)
+ if (tid != kMainTid) {
parent_tid = _parent_tid;
+ stack_id = _stack_tid;
+ }
OnCreated(arg);
}
@@ -143,7 +147,7 @@ uptr ThreadRegistry::GetMaxAliveThreads() {
}
u32 ThreadRegistry::CreateThread(uptr user_id, bool detached, u32 parent_tid,
- void *arg) {
+ u32 stack_tid, void *arg) {
ThreadRegistryLock l(this);
u32 tid = kInvalidTid;
ThreadContextBase *tctx = QuarantinePop();
@@ -181,7 +185,8 @@ u32 ThreadRegistry::CreateThread(uptr user_id, bool detached, u32 parent_tid,
// positives later (e.g. if we join a wrong thread).
CHECK(live_.try_emplace(user_id, tid).second);
}
- tctx->SetCreated(user_id, total_threads_++, detached, parent_tid, arg);
+ tctx->SetCreated(user_id, total_threads_++, detached, parent_tid, stack_tid,
+ arg);
return tid;
}
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
index 2c7e5c276fa1c7..f42859698ff98e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
@@ -52,6 +52,7 @@ class ThreadContextBase {
ThreadType thread_type;
u32 parent_tid;
+ u32 stack_id;
ThreadContextBase *next; // For storing thread contexts in a list.
atomic_uint32_t thread_destroyed; // To address race of Joined vs Finished
@@ -63,7 +64,7 @@ class ThreadContextBase {
void SetFinished();
void SetStarted(tid_t _os_id, ThreadType _thread_type, void *arg);
void SetCreated(uptr _user_id, u64 _unique_id, bool _detached,
- u32 _parent_tid, void *arg);
+ u32 _parent_tid, u32 _stack_tid, void *arg);
void Reset();
void SetDestroyed();
@@ -106,7 +107,11 @@ class SANITIZER_MUTEX ThreadRegistry {
u32 NumThreadsLocked() const { return threads_.size(); }
- u32 CreateThread(uptr user_id, bool detached, u32 parent_tid, void *arg);
+ u32 CreateThread(uptr user_id, bool detached, u32 parent_tid, u32 stack_tid,
+ void *arg);
+ u32 CreateThread(uptr user_id, bool detached, u32 parent_tid, void *arg) {
+ return CreateThread(user_id, detached, parent_tid, 0, arg);
+ }
typedef void (*ThreadCallback)(ThreadContextBase *tctx, void *arg);
// Invokes callback with a specified arg for each thread context.
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_thread_registry_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_thread_registry_test.cpp
index 8c4b4ba5c58f7d..c3cac707f0a0b3 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_thread_registry_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_thread_registry_test.cpp
@@ -64,11 +64,12 @@ static void MarkUidAsPresent(ThreadContextBase *tctx, void *arg) {
static void TestRegistry(ThreadRegistry *registry, bool has_quarantine) {
// Create and start a main thread.
- EXPECT_EQ(0U, registry->CreateThread(get_uid(0), true, -1, 0));
+ EXPECT_EQ(0U, registry->CreateThread(get_uid(0), true, -1, 0, nullptr));
registry->StartThread(0, 0, ThreadType::Regular, 0);
// Create a bunch of threads.
for (u32 i = 1; i <= 10; i++) {
- EXPECT_EQ(i, registry->CreateThread(get_uid(i), is_detached(i), 0, 0));
+ EXPECT_EQ(i, registry->CreateThread(get_uid(i), is_detached(i), 100 + i,
+ 200 + i, nullptr));
}
CheckThreadQuantity(registry, 11, 1, 11);
// Start some of them.
@@ -88,19 +89,27 @@ static void TestRegistry(ThreadRegistry *registry, bool has_quarantine) {
std::vector<u32> new_tids;
for (u32 i = 11; i <= 15; i++) {
new_tids.push_back(
- registry->CreateThread(get_uid(i), is_detached(i), 0, 0));
+ registry->CreateThread(get_uid(i), is_detached(i), 0, 0, nullptr));
}
ASSERT_LE(kRegistryQuarantine, 5U);
- u32 exp_total = 16 - (has_quarantine ? 5 - kRegistryQuarantine : 0);
+ u32 exp_total = 16 - (has_quarantine ? 5 - kRegistryQuarantine : 0);
CheckThreadQuantity(registry, exp_total, 6, 11);
// Test SetThreadName and FindThread.
registry->SetThreadName(6, "six");
registry->SetThreadName(7, "seven");
- EXPECT_EQ(7U, registry->FindThread(HasName, (void*)"seven"));
+ EXPECT_EQ(7U, registry->FindThread(HasName, (void *)"seven"));
EXPECT_EQ(kInvalidTid, registry->FindThread(HasName, (void *)"none"));
- EXPECT_EQ(0U, registry->FindThread(HasUid, (void*)get_uid(0)));
- EXPECT_EQ(10U, registry->FindThread(HasUid, (void*)get_uid(10)));
+ EXPECT_EQ(0U, registry->FindThread(HasUid, (void *)get_uid(0)));
+ EXPECT_EQ(10U, registry->FindThread(HasUid, (void *)get_uid(10)));
EXPECT_EQ(kInvalidTid, registry->FindThread(HasUid, (void *)0x1234));
+ EXPECT_EQ(7U,
+ registry->FindThread([](ThreadContextBase *tctx,
+ void *) { return tctx->parent_tid == 107; },
+ nullptr));
+ EXPECT_EQ(8U,
+ registry->FindThread([](ThreadContextBase *tctx,
+ void *) { return tctx->stack_id == 208; },
+ nullptr));
// Detach and finish and join remaining threads.
for (u32 i = 6; i <= 10; i++) {
registry->DetachThread(i, 0);
More information about the llvm-commits
mailing list