diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp index a34b8c15aa5b01..2e1cd0238812b0 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cpp @@ -13,6 +13,8 @@ #include "sanitizer_thread_registry.h" +#include "sanitizer_placement_new.h" + namespace __sanitizer { ThreadContextBase::ThreadContextBase(u32 tid) @@ -162,6 +164,12 @@ u32 ThreadRegistry::CreateThread(uptr user_id, bool detached, u32 parent_tid, max_alive_threads_++; CHECK_EQ(alive_threads_, max_alive_threads_); } + if (user_id) { + // Ensure that user_id is unique. If it's not the case we are screwed. + // Ignoring this situation may lead to very hard to debug false + // 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); return tid; @@ -221,14 +229,8 @@ void ThreadRegistry::SetThreadName(u32 tid, const char *name) { void ThreadRegistry::SetThreadNameByUserId(uptr user_id, const char *name) { ThreadRegistryLock l(this); - for (u32 tid = 0; tid < threads_.size(); tid++) { - ThreadContextBase *tctx = threads_[tid]; - if (tctx != 0 && tctx->user_id == user_id && - tctx->status != ThreadStatusInvalid) { - tctx->SetName(name); - return; - } - } + if (const auto *tid = live_.find(user_id)) + threads_[tid->second]->SetName(name); } void ThreadRegistry::DetachThread(u32 tid, void *arg) { @@ -241,6 +243,8 @@ void ThreadRegistry::DetachThread(u32 tid, void *arg) { } tctx->OnDetached(arg); if (tctx->status == ThreadStatusFinished) { + if (tctx->user_id) + live_.erase(tctx->user_id); tctx->SetDead(); QuarantinePush(tctx); } else { @@ -260,6 +264,8 @@ void ThreadRegistry::JoinThread(u32 tid, void *arg) { return; } if ((destroyed = tctx->GetDestroyed())) { + if (tctx->user_id) + live_.erase(tctx->user_id); tctx->SetJoined(arg); QuarantinePush(tctx); } @@ -292,6 +298,8 @@ ThreadStatus ThreadRegistry::FinishThread(u32 tid) { } tctx->SetFinished(); if (dead) { + if (tctx->user_id) + live_.erase(tctx->user_id); tctx->SetDead(); QuarantinePush(tctx); } @@ -333,6 +341,19 @@ ThreadContextBase *ThreadRegistry::QuarantinePop() { return tctx; } +u32 ThreadRegistry::ConsumeThreadUserId(uptr user_id) { + ThreadRegistryLock l(this); + u32 tid; + auto *t = live_.find(user_id); + CHECK(t); + tid = t->second; + live_.erase(t); + auto *tctx = threads_[tid]; + CHECK_EQ(tctx->user_id, user_id); + tctx->user_id = 0; + return tid; +} + void ThreadRegistry::SetThreadUserId(u32 tid, uptr user_id) { ThreadRegistryLock l(this); ThreadContextBase *tctx = threads_[tid]; @@ -341,6 +362,7 @@ void ThreadRegistry::SetThreadUserId(u32 tid, uptr user_id) { CHECK_NE(tctx->status, ThreadStatusDead); CHECK_EQ(tctx->user_id, 0); tctx->user_id = user_id; + CHECK(live_.try_emplace(user_id, tctx->tid).second); } } // namespace __sanitizer diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h index a8a4d4d86a03eb..a259b324220f37 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h @@ -15,6 +15,7 @@ #define SANITIZER_THREAD_REGISTRY_H #include "sanitizer_common.h" +#include "sanitizer_dense_map.h" #include "sanitizer_list.h" #include "sanitizer_mutex.h" @@ -127,6 +128,7 @@ class MUTEX ThreadRegistry { // Finishes thread and returns previous status. ThreadStatus FinishThread(u32 tid); void StartThread(u32 tid, tid_t os_id, ThreadType thread_type, void *arg); + u32 ConsumeThreadUserId(uptr user_id); void SetThreadUserId(u32 tid, uptr user_id); private: @@ -146,6 +148,7 @@ class MUTEX ThreadRegistry { InternalMmapVector threads_; IntrusiveList dead_threads_; IntrusiveList invalid_threads_; + DenseMap live_; void QuarantinePush(ThreadContextBase *tctx); ThreadContextBase *QuarantinePop(); diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp index dfead437f467b5..c8f7124c009d6e 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp @@ -270,29 +270,8 @@ struct ConsumeThreadContext { ThreadContextBase *tctx; }; -static bool ConsumeThreadByUid(ThreadContextBase *tctx, void *arg) { - ConsumeThreadContext *findCtx = (ConsumeThreadContext *)arg; - if (tctx->user_id == findCtx->uid && tctx->status != ThreadStatusInvalid) { - if (findCtx->tctx) { - // Ensure that user_id is unique. If it's not the case we are screwed. - // Something went wrong before, but now there is no way to recover. - // Returning a wrong thread is not an option, it may lead to very hard - // to debug false positives (e.g. if we join a wrong thread). - Report("ThreadSanitizer: dup thread with used id 0x%zx\n", findCtx->uid); - Die(); - } - findCtx->tctx = tctx; - tctx->user_id = 0; - } - return false; -} - Tid ThreadConsumeTid(ThreadState *thr, uptr pc, uptr uid) { - ConsumeThreadContext findCtx = {uid, nullptr}; - ctx->thread_registry.FindThread(ConsumeThreadByUid, &findCtx); - Tid tid = findCtx.tctx ? findCtx.tctx->tid : kInvalidTid; - DPrintf("#%d: ThreadTid uid=%zu tid=%d\n", thr->tid, uid, tid); - return tid; + return ctx->thread_registry.ConsumeThreadUserId(uid); } void ThreadJoin(ThreadState *thr, uptr pc, Tid tid) {