[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