[LSAN][NFC] Eliminated GetThreadRegistryLocked from the LSAN interface to avoid the need to implement it in HWASAN.

[kstoimenov]

Some updates.

[vitalybuka]

compiler-rt/lib/lsan/lsan_posix.cpp

65

Here we need to get into specific lsan/asan or hwasan thread registry
with this patch it's always lsan

you probably need lsan::FindThreadContextByOsIDLocked then?

compiler-rt/lib/lsan/lsan_thread.h

48

Why do you need this in header?
Maybe just static in cpp file?

[kstoimenov]

Adde GetLsanThreadRegistryLocked back.

[kstoimenov]

compiler-rt/lib/lsan/lsan_posix.cpp

65

Actually it should be the lsan specific "GetTreadRegistry" because GetThreadRangesLocked is part of the lsan link-time interface. In HWASAN we might not even use TreadRegistry at all.

compiler-rt/lib/lsan/lsan_thread.h

48

It is used by lsan_posix.cc.

[vitalybuka]

compiler-rt/lib/lsan/lsan_posix.cpp

65

Yes, but with asan we have no GetLsanThreadRegistryLocked, FindThreadContextByOsIDLocked should look into asan thread registry

[kstoimenov]

compiler-rt/lib/lsan/lsan_posix.cpp

65

Well, asan has it's own implementation of GetThreadRangesLocked: https://tinyurl.com/4hf7a5cx and it is not using the TreadRegistry at all.

[kstoimenov]

compiler-rt/lib/lsan/lsan_posix.cpp

65

If we have to be pedantic __asan::GetAsanThreadByOsIDLocked uses the registry so technically it is using it indirectly through another function. But still we don't need to provide generic implementation for GetThreadRegistryLocked which is the point of this patch.

compiler-rt/lib/asan/asan_thread.cpp

 } // namespace __asan
 
 // --- Implementation of LSan-specific functions --- {{{1
 namespace __lsan {
 void LockThreadRegistry() { __asan::asanThreadRegistry().Lock(); }
 
 void UnlockThreadRegistry() { __asan::asanThreadRegistry().Unlock(); }
 
-ThreadRegistry *GetThreadRegistryLocked() {
+static ThreadRegistry *GetAsanThreadRegistryLocked() {
   __asan::asanThreadRegistry().CheckLocked();
   return &__asan::asanThreadRegistry();
 }
 
 void EnsureMainThreadIDIsCorrect() { __asan::EnsureMainThreadIDIsCorrect(); }
 
 bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
                            uptr *tls_begin, uptr *tls_end, uptr *cache_begin,
void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
   if (!t)
     return;
   __asan::FakeStack *fake_stack = t->get_fake_stack();
   if (!fake_stack)
     return;
   fake_stack->ForEachFakeFrame(callback, arg);
 }
 
+void RunCallbackForEachThreadLocked(__sanitizer::ThreadRegistry::ThreadCallback cb,
+                                    void *arg) {
+  GetAsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(cb, arg);
+}
+
+void FinishThreadLocked(u32 tid) {
+  GetAsanThreadRegistryLocked()->FinishThread(tid);
+}
+
 } // namespace __lsan
 
 // ---------------------- Interface ---------------- {{{1
 using namespace __asan;
 
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE
 void __sanitizer_start_switch_fiber(void **fakestacksave, const void *bottom,

compiler-rt/lib/lsan/lsan_common.h

 
 #include "sanitizer_common/sanitizer_allocator.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_internal_defs.h"
 #include "sanitizer_common/sanitizer_platform.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
 #include "sanitizer_common/sanitizer_stoptheworld.h"
 #include "sanitizer_common/sanitizer_symbolizer.h"
+#include "sanitizer_common/sanitizer_thread_registry.h"
 
 // LeakSanitizer relies on some Glibc's internals (e.g. TLS machinery) on Linux.
 // Also, LSan doesn't like 32 bit architectures
 // because of "small" (4 bytes) pointer size that leads to high false negative
 // ratio on large leaks. But we still want to have it for some 32 bit arches
 // (e.g. x86), see https://github.com/google/sanitizers/issues/403.
 // To enable LeakSanitizer on a new architecture, one needs to implement the
 // internal_clone function as well as (probably) adjust the TLS machinery for

 
 //// --------------------------------------------------------------------------
 //// Thread prototypes.
 //// --------------------------------------------------------------------------
 
 // Wrappers for ThreadRegistry access.
 void LockThreadRegistry() SANITIZER_NO_THREAD_SAFETY_ANALYSIS;
 void UnlockThreadRegistry() SANITIZER_NO_THREAD_SAFETY_ANALYSIS;
-ThreadRegistry *GetThreadRegistryLocked();
 // If called from the main thread, updates the main thread's TID in the thread
 // registry. We need this to handle processes that fork() without a subsequent
 // exec(), which invalidates the recorded TID. To update it, we must call
 // gettid() from the main thread. Our solution is to call this function before
 // leak checking and also before every call to pthread_create() (to handle cases
 // where leak checking is initiated from a non-main thread).
 void EnsureMainThreadIDIsCorrect();
 
 bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
                            uptr *tls_begin, uptr *tls_end, uptr *cache_begin,
                            uptr *cache_end, DTLS **dtls);
 void GetAllThreadAllocatorCachesLocked(InternalMmapVector<uptr> *caches);
 void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
                             void *arg);
 
+void RunCallbackForEachThreadLocked(__sanitizer::ThreadRegistry::ThreadCallback cb,
+                                    void *arg);
+void FinishThreadLocked(u32 tid);
+
 //// --------------------------------------------------------------------------
 //// Allocator prototypes.
 //// --------------------------------------------------------------------------
 
 // Wrappers for allocator's ForceLock()/ForceUnlock().
 void LockAllocator();
 void UnlockAllocator();
 

compiler-rt/lib/lsan/lsan_common.cpp

 #    if SANITIZER_ANDROID
 // FIXME: Move this out into *libcdep.cpp
 extern "C" SANITIZER_WEAK_ATTRIBUTE void __libc_iterate_dynamic_tls(
     pid_t, void (*cb)(void *, void *, uptr, void *), void *);
 #    endif
 
 static void ProcessThreadRegistry(Frontier *frontier) {
   InternalMmapVector<uptr> ptrs;
-  GetThreadRegistryLocked()->RunCallbackForEachThreadLocked(
-      GetAdditionalThreadContextPtrs, &ptrs);
+  RunCallbackForEachThreadLocked(GetAdditionalThreadContextPtrs, &ptrs);
 
   for (uptr i = 0; i < ptrs.size(); ++i) {
     void *ptr = reinterpret_cast<void *>(ptrs[i]);
     uptr chunk = PointsIntoChunk(ptr);
     if (!chunk)
       continue;
     LsanMetadata m(chunk);
     if (!m.allocated())

 static void ReportUnsuspendedThreads(
     const SuspendedThreadsList &suspended_threads) {
   InternalMmapVector<tid_t> threads(suspended_threads.ThreadCount());
   for (uptr i = 0; i < suspended_threads.ThreadCount(); ++i)
     threads[i] = suspended_threads.GetThreadID(i);
 
   Sort(threads.data(), threads.size());
 
-  GetThreadRegistryLocked()->RunCallbackForEachThreadLocked(
-      &ReportIfNotSuspended, &threads);
+  RunCallbackForEachThreadLocked(&ReportIfNotSuspended, &threads);
 }
 
 #  endif  // !SANITIZER_FUCHSIA
 
 static void CheckForLeaksCallback(const SuspendedThreadsList &suspended_threads,
                                   void *arg) {
   CheckForLeaksParam *param = reinterpret_cast<CheckForLeaksParam *>(arg);
   CHECK(param);

compiler-rt/lib/lsan/lsan_common_fuchsia.cpp

__sanitizer_memory_snapshot(
       [](zx_status_t, void *data) {
         auto params = static_cast<const Params *>(data);
 
         // We don't use the thread registry at all for enumerating the threads
         // and their stacks, registers, and TLS regions.  So use it separately
         // just for the allocator cache, and to call ForEachExtraStackRange,
         // which ASan needs.
         if (flags()->use_stacks) {
-          GetThreadRegistryLocked()->RunCallbackForEachThreadLocked(
+          RunCallbackForEachThreadLocked(
               [](ThreadContextBase *tctx, void *arg) {
                 ForEachExtraStackRange(tctx->os_id, ForEachExtraStackRangeCb,
                                        arg);
               },
               &params->argument->frontier);
         }
 
         params->callback(SuspendedThreadsListFuchsia(), params->argument);

compiler-rt/lib/lsan/lsan_fuchsia.cpp

void InitializeMainThread() {
   __sanitizer::GetThreadStackTopAndBottom(true, &args.stack_end,
                                           &args.stack_begin);
   u32 tid = ThreadCreate(kMainTid, true, &args);
   CHECK_EQ(tid, 0);
   ThreadStart(tid);
 }
 
 void GetAllThreadAllocatorCachesLocked(InternalMmapVector<uptr> *caches) {
-  GetThreadRegistryLocked()->RunCallbackForEachThreadLocked(
+  RunCallbackForEachThreadLocked(
       [](ThreadContextBase *tctx, void *arg) {
         auto ctx = static_cast<ThreadContext *>(tctx);
         static_cast<decltype(caches)>(arg)->push_back(ctx->cache_begin());
       },
       caches);
 }
 
 // On Fuchsia, leak detection is done by a special hook after atexit hooks.

 
 // This is called after creating a new thread (in the creating thread),
 // with the pointer returned by __sanitizer_before_thread_create_hook (above).
 void __sanitizer_thread_create_hook(void *hook, thrd_t thread, int error) {
   u32 tid = static_cast<u32>(reinterpret_cast<uptr>(hook));
   // On success, there is nothing to do here.
   if (error != thrd_success) {
     // Clean up the thread registry for the thread creation that didn't happen.
-    GetThreadRegistryLocked()->FinishThread(tid);
+    FinishThreadLocked(tid);
   }
 }
 
 // This is called in the newly-created thread before it runs anything else,
 // with the pointer returned by __sanitizer_before_thread_create_hook (above).
 void __sanitizer_thread_start_hook(void *hook, thrd_t self) {
   u32 tid = static_cast<u32>(reinterpret_cast<uptr>(hook));
   ThreadStart(tid);
 }
 
 // Each thread runs this just before it exits,
 // with the pointer returned by BeforeThreadCreateHook (above).
 // All per-thread destructors have already been called.
 void __sanitizer_thread_exit_hook(void *hook, thrd_t self) { ThreadFinish(); }
 
 #endif  // SANITIZER_FUCHSIA

compiler-rt/lib/lsan/lsan_posix.cpp

 //
 //===---------------------------------------------------------------------===//
 
 #include "sanitizer_common/sanitizer_platform.h"
 
 #if SANITIZER_POSIX
 #include "lsan.h"
 #include "lsan_allocator.h"
+#include "lsan_thread.h"
 #include "sanitizer_common/sanitizer_stacktrace.h"
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 
 namespace __lsan {
 
 ThreadContext::ThreadContext(int tid) : ThreadContextLsanBase(tid) {}
 
 struct OnStartedArgs {
void ThreadStart(u32 tid, tid_t os_id, ThreadType thread_type) {
   args.dtls = DTLS_Get();
   ThreadContextLsanBase::ThreadStart(tid, os_id, thread_type, &args);
 }
 
 bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
                            uptr *tls_begin, uptr *tls_end, uptr *cache_begin,
                            uptr *cache_end, DTLS **dtls) {
   ThreadContext *context = static_cast<ThreadContext *>(
-      GetThreadRegistryLocked()->FindThreadContextByOsIDLocked(os_id));
+      GetLsanThreadRegistryLocked()->FindThreadContextByOsIDLocked(os_id));
   if (!context)
     return false;
   *stack_begin = context->stack_begin();
   *stack_end = context->stack_end();
   *tls_begin = context->tls_begin();
   *tls_end = context->tls_end();
   *cache_begin = context->cache_begin();
   *cache_end = context->cache_end();

compiler-rt/lib/lsan/lsan_thread.h

 };
 
 // This subclass of ThreadContextLsanBase is declared in an OS-specific header.
 class ThreadContext;
 
 void InitializeThreadRegistry();
 void InitializeMainThread();
 
+ThreadRegistry *GetLsanThreadRegistryLocked();
+
 u32 ThreadCreate(u32 tid, bool detached, void *arg = nullptr);
 void ThreadFinish();
 
 u32 GetCurrentThread();
 void SetCurrentThread(u32 tid);
 ThreadContext *CurrentThreadContext();
 void EnsureMainThreadIDIsCorrect();
 
 }  // namespace __lsan
 
 #endif  // LSAN_THREAD_H

compiler-rt/lib/lsan/lsan_thread.cpp

 
 void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
                             void *arg) {}
 
 void LockThreadRegistry() { thread_registry->Lock(); }
 
 void UnlockThreadRegistry() { thread_registry->Unlock(); }
 
-ThreadRegistry *GetThreadRegistryLocked() {
+ThreadRegistry *GetLsanThreadRegistryLocked() {
   thread_registry->CheckLocked();
   return thread_registry;
 }
 
+void RunCallbackForEachThreadLocked(
+    __sanitizer::ThreadRegistry::ThreadCallback cb, void *arg) {
+  GetLsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(cb, arg);
+}
+
+void FinishThreadLocked(u32 tid) {
+  GetLsanThreadRegistryLocked()->FinishThread(tid);
+}
+
 }  // namespace __lsan