[compiler-rt] 8e489b4 - sanitizer_common: add simpler ThreadRegistry ctor

Dmitry Vyukov via llvm-commits llvm-commits at lists.llvm.org
Sun Jul 11 03:36:34 PDT 2021


Author: Dmitry Vyukov
Date: 2021-07-11T12:36:28+02:00
New Revision: 8e489b4b96e31cfb004e03cfa1393c425c504013

URL: https://github.com/llvm/llvm-project/commit/8e489b4b96e31cfb004e03cfa1393c425c504013
DIFF: https://github.com/llvm/llvm-project/commit/8e489b4b96e31cfb004e03cfa1393c425c504013.diff

LOG: sanitizer_common: add simpler ThreadRegistry ctor

Currently ThreadRegistry is overcomplicated because of tsan,
it needs tid quarantine and reuse counters. Other sanitizers
don't need that. It also seems that no other sanitizer now
needs max number of threads. Asan used to need 2^24 limit,
but it does not seem to be needed now. Other sanitizers blindly
copy-pasted that without reasons. Lsan also uses quarantine,
but I don't see why that may be potentially needed.

Add a ThreadRegistry ctor that does not require any sizes
and use it in all sanitizers except for tsan.
In preparation for new tsan runtime, which won't need
any of these parameters as well.

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D105713

Added: 
    

Modified: 
    compiler-rt/lib/asan/asan_thread.cpp
    compiler-rt/lib/asan/asan_thread.h
    compiler-rt/lib/lsan/lsan_thread.cpp
    compiler-rt/lib/memprof/memprof_thread.cpp
    compiler-rt/lib/memprof/memprof_thread.h
    compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/asan/asan_thread.cpp b/compiler-rt/lib/asan/asan_thread.cpp
index 7099850254875..35d4467e7b53a 100644
--- a/compiler-rt/lib/asan/asan_thread.cpp
+++ b/compiler-rt/lib/asan/asan_thread.cpp
@@ -60,8 +60,8 @@ ThreadRegistry &asanThreadRegistry() {
     // in TSD and can't reliably tell when no more TSD destructors will
     // be called. It would be wrong to reuse AsanThreadContext for another
     // thread before all TSD destructors will be called for it.
-    asan_thread_registry = new(thread_registry_placeholder) ThreadRegistry(
-        GetAsanThreadContext, kMaxNumberOfThreads, kMaxNumberOfThreads);
+    asan_thread_registry =
+        new (thread_registry_placeholder) ThreadRegistry(GetAsanThreadContext);
     initialized = true;
   }
   return *asan_thread_registry;

diff  --git a/compiler-rt/lib/asan/asan_thread.h b/compiler-rt/lib/asan/asan_thread.h
index faa25d6964838..801a3960ec6cb 100644
--- a/compiler-rt/lib/asan/asan_thread.h
+++ b/compiler-rt/lib/asan/asan_thread.h
@@ -28,8 +28,6 @@ struct DTLS;
 
 namespace __asan {
 
-const u32 kMaxNumberOfThreads = (1 << 22);  // 4M
-
 class AsanThread;
 
 // These objects are created for every thread and are never deleted,

diff  --git a/compiler-rt/lib/lsan/lsan_thread.cpp b/compiler-rt/lib/lsan/lsan_thread.cpp
index 9003620dd8fb1..1d224ebca693b 100644
--- a/compiler-rt/lib/lsan/lsan_thread.cpp
+++ b/compiler-rt/lib/lsan/lsan_thread.cpp
@@ -30,13 +30,10 @@ static ThreadContextBase *CreateThreadContext(u32 tid) {
   return new (mem) ThreadContext(tid);
 }
 
-static const uptr kMaxThreads = 1 << 22;  // 4M
-static const uptr kThreadQuarantineSize = 64;
-
 void InitializeThreadRegistry() {
   static ALIGNED(64) char thread_registry_placeholder[sizeof(ThreadRegistry)];
-  thread_registry = new (thread_registry_placeholder)
-      ThreadRegistry(CreateThreadContext, kMaxThreads, kThreadQuarantineSize);
+  thread_registry =
+      new (thread_registry_placeholder) ThreadRegistry(CreateThreadContext);
 }
 
 ThreadContextLsanBase::ThreadContextLsanBase(int tid)

diff  --git a/compiler-rt/lib/memprof/memprof_thread.cpp b/compiler-rt/lib/memprof/memprof_thread.cpp
index 0da811901f708..5ae7a2ee85b99 100644
--- a/compiler-rt/lib/memprof/memprof_thread.cpp
+++ b/compiler-rt/lib/memprof/memprof_thread.cpp
@@ -57,8 +57,8 @@ ThreadRegistry &memprofThreadRegistry() {
     // in TSD and can't reliably tell when no more TSD destructors will
     // be called. It would be wrong to reuse MemprofThreadContext for another
     // thread before all TSD destructors will be called for it.
-    memprof_thread_registry = new (thread_registry_placeholder) ThreadRegistry(
-        GetMemprofThreadContext, kMaxNumberOfThreads, kMaxNumberOfThreads);
+    memprof_thread_registry = new (thread_registry_placeholder)
+        ThreadRegistry(GetMemprofThreadContext);
     initialized = true;
   }
   return *memprof_thread_registry;

diff  --git a/compiler-rt/lib/memprof/memprof_thread.h b/compiler-rt/lib/memprof/memprof_thread.h
index a191b96b5b82a..4c9313fcb369e 100644
--- a/compiler-rt/lib/memprof/memprof_thread.h
+++ b/compiler-rt/lib/memprof/memprof_thread.h
@@ -27,8 +27,6 @@ struct DTLS;
 
 namespace __memprof {
 
-const u32 kMaxNumberOfThreads = (1 << 22); // 4M
-
 class MemprofThread;
 
 // These objects are created for every thread and are never deleted,

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
index 3273da38bfd6c..745fbf76b01f6 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp
@@ -99,6 +99,9 @@ void ThreadContextBase::Reset() {
 
 // ThreadRegistry implementation.
 
+ThreadRegistry::ThreadRegistry(ThreadContextFactory factory)
+    : ThreadRegistry(factory, UINT32_MAX, UINT32_MAX, 0) {}
+
 ThreadRegistry::ThreadRegistry(ThreadContextFactory factory, u32 max_threads,
                                u32 thread_quarantine_size, u32 max_reuse)
     : context_factory_(factory),
@@ -106,13 +109,10 @@ ThreadRegistry::ThreadRegistry(ThreadContextFactory factory, u32 max_threads,
       thread_quarantine_size_(thread_quarantine_size),
       max_reuse_(max_reuse),
       mtx_(),
-      n_contexts_(0),
       total_threads_(0),
       alive_threads_(0),
       max_alive_threads_(0),
       running_threads_(0) {
-  threads_ = (ThreadContextBase **)MmapOrDie(max_threads_ * sizeof(threads_[0]),
-                                             "ThreadRegistry");
   dead_threads_.clear();
   invalid_threads_.clear();
 }
@@ -120,7 +120,8 @@ ThreadRegistry::ThreadRegistry(ThreadContextFactory factory, u32 max_threads,
 void ThreadRegistry::GetNumberOfThreads(uptr *total, uptr *running,
                                         uptr *alive) {
   BlockingMutexLock l(&mtx_);
-  if (total) *total = n_contexts_;
+  if (total)
+    *total = threads_.size();
   if (running) *running = running_threads_;
   if (alive) *alive = alive_threads_;
 }
@@ -137,11 +138,11 @@ u32 ThreadRegistry::CreateThread(uptr user_id, bool detached, u32 parent_tid,
   ThreadContextBase *tctx = QuarantinePop();
   if (tctx) {
     tid = tctx->tid;
-  } else if (n_contexts_ < max_threads_) {
+  } else if (threads_.size() < max_threads_) {
     // Allocate new thread context and tid.
-    tid = n_contexts_++;
+    tid = threads_.size();
     tctx = context_factory_(tid);
-    threads_[tid] = tctx;
+    threads_.push_back(tctx);
   } else {
 #if !SANITIZER_GO
     Report("%s: Thread limit (%u threads) exceeded. Dying.\n",
@@ -169,7 +170,7 @@ u32 ThreadRegistry::CreateThread(uptr user_id, bool detached, u32 parent_tid,
 void ThreadRegistry::RunCallbackForEachThreadLocked(ThreadCallback cb,
                                                     void *arg) {
   CheckLocked();
-  for (u32 tid = 0; tid < n_contexts_; tid++) {
+  for (u32 tid = 0; tid < threads_.size(); tid++) {
     ThreadContextBase *tctx = threads_[tid];
     if (tctx == 0)
       continue;
@@ -179,7 +180,7 @@ void ThreadRegistry::RunCallbackForEachThreadLocked(ThreadCallback cb,
 
 u32 ThreadRegistry::FindThread(FindThreadCallback cb, void *arg) {
   BlockingMutexLock l(&mtx_);
-  for (u32 tid = 0; tid < n_contexts_; tid++) {
+  for (u32 tid = 0; tid < threads_.size(); tid++) {
     ThreadContextBase *tctx = threads_[tid];
     if (tctx != 0 && cb(tctx, arg))
       return tctx->tid;
@@ -190,7 +191,7 @@ u32 ThreadRegistry::FindThread(FindThreadCallback cb, void *arg) {
 ThreadContextBase *
 ThreadRegistry::FindThreadContextLocked(FindThreadCallback cb, void *arg) {
   CheckLocked();
-  for (u32 tid = 0; tid < n_contexts_; tid++) {
+  for (u32 tid = 0; tid < threads_.size(); tid++) {
     ThreadContextBase *tctx = threads_[tid];
     if (tctx != 0 && cb(tctx, arg))
       return tctx;
@@ -211,7 +212,6 @@ ThreadContextBase *ThreadRegistry::FindThreadContextByOsIDLocked(tid_t os_id) {
 
 void ThreadRegistry::SetThreadName(u32 tid, const char *name) {
   BlockingMutexLock l(&mtx_);
-  CHECK_LT(tid, n_contexts_);
   ThreadContextBase *tctx = threads_[tid];
   CHECK_NE(tctx, 0);
   CHECK_EQ(SANITIZER_FUCHSIA ? ThreadStatusCreated : ThreadStatusRunning,
@@ -221,7 +221,7 @@ void ThreadRegistry::SetThreadName(u32 tid, const char *name) {
 
 void ThreadRegistry::SetThreadNameByUserId(uptr user_id, const char *name) {
   BlockingMutexLock l(&mtx_);
-  for (u32 tid = 0; tid < n_contexts_; tid++) {
+  for (u32 tid = 0; tid < threads_.size(); tid++) {
     ThreadContextBase *tctx = threads_[tid];
     if (tctx != 0 && tctx->user_id == user_id &&
         tctx->status != ThreadStatusInvalid) {
@@ -233,7 +233,6 @@ void ThreadRegistry::SetThreadNameByUserId(uptr user_id, const char *name) {
 
 void ThreadRegistry::DetachThread(u32 tid, void *arg) {
   BlockingMutexLock l(&mtx_);
-  CHECK_LT(tid, n_contexts_);
   ThreadContextBase *tctx = threads_[tid];
   CHECK_NE(tctx, 0);
   if (tctx->status == ThreadStatusInvalid) {
@@ -254,7 +253,6 @@ void ThreadRegistry::JoinThread(u32 tid, void *arg) {
   do {
     {
       BlockingMutexLock l(&mtx_);
-      CHECK_LT(tid, n_contexts_);
       ThreadContextBase *tctx = threads_[tid];
       CHECK_NE(tctx, 0);
       if (tctx->status == ThreadStatusInvalid) {
@@ -280,7 +278,6 @@ ThreadStatus ThreadRegistry::FinishThread(u32 tid) {
   BlockingMutexLock l(&mtx_);
   CHECK_GT(alive_threads_, 0);
   alive_threads_--;
-  CHECK_LT(tid, n_contexts_);
   ThreadContextBase *tctx = threads_[tid];
   CHECK_NE(tctx, 0);
   bool dead = tctx->detached;
@@ -306,7 +303,6 @@ void ThreadRegistry::StartThread(u32 tid, tid_t os_id, ThreadType thread_type,
                                  void *arg) {
   BlockingMutexLock l(&mtx_);
   running_threads_++;
-  CHECK_LT(tid, n_contexts_);
   ThreadContextBase *tctx = threads_[tid];
   CHECK_NE(tctx, 0);
   CHECK_EQ(ThreadStatusCreated, tctx->status);
@@ -339,7 +335,6 @@ ThreadContextBase *ThreadRegistry::QuarantinePop() {
 
 void ThreadRegistry::SetThreadUserId(u32 tid, uptr user_id) {
   BlockingMutexLock l(&mtx_);
-  CHECK_LT(tid, n_contexts_);
   ThreadContextBase *tctx = threads_[tid];
   CHECK_NE(tctx, 0);
   CHECK_NE(tctx->status, ThreadStatusInvalid);

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
index dcd445c28ae92..fc9135f9e2d90 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
@@ -87,8 +87,9 @@ typedef ThreadContextBase* (*ThreadContextFactory)(u32 tid);
 
 class ThreadRegistry {
  public:
+  ThreadRegistry(ThreadContextFactory factory);
   ThreadRegistry(ThreadContextFactory factory, u32 max_threads,
-                 u32 thread_quarantine_size, u32 max_reuse = 0);
+                 u32 thread_quarantine_size, u32 max_reuse);
   void GetNumberOfThreads(uptr *total = nullptr, uptr *running = nullptr,
                           uptr *alive = nullptr);
   uptr GetMaxAliveThreads();
@@ -99,7 +100,6 @@ class ThreadRegistry {
 
   // Should be guarded by ThreadRegistryLock.
   ThreadContextBase *GetThreadLocked(u32 tid) {
-    DCHECK_LT(tid, n_contexts_);
     return threads_[tid];
   }
 
@@ -137,15 +137,13 @@ class ThreadRegistry {
 
   BlockingMutex mtx_;
 
-  u32 n_contexts_;      // Number of created thread contexts,
-                        // at most max_threads_.
   u64 total_threads_;   // Total number of created threads. May be greater than
                         // max_threads_ if contexts were reused.
   uptr alive_threads_;  // Created or running.
   uptr max_alive_threads_;
   uptr running_threads_;
 
-  ThreadContextBase **threads_;  // Array of thread contexts is leaked.
+  InternalMmapVector<ThreadContextBase *> threads_;
   IntrusiveList<ThreadContextBase> dead_threads_;
   IntrusiveList<ThreadContextBase> invalid_threads_;
 


        


More information about the llvm-commits mailing list