[lsan] Factor pthread-specific assumptions out of thread tracking code

[mcgrathr]

Summary

This is a small refactoring to prepare for porting LSan to Fuchsia.
Factor out parts of lsan_thread.{cpp,h} that don't apply to Fuchsia.
Since existing supported systems are POSIX-based, the affected code
is moved to lsan_posix.{cpp.h}.

[vitalybuka]

compiler-rt/lib/lsan/lsan_posix.cpp

31

can you make it more consistent?
type name1;
type name2;
...

65

is this clang-formated?

compiler-rt/lib/lsan/lsan_posix.h

39

same about one per line

compiler-rt/lib/lsan/lsan_thread.h

35

maybe here and else
just
uptr stack_begin_ = 0;

[mcgrathr]

Style changes to copied code, clang-format.

[mcgrathr]

I've made the style changes you asked for and clang-format'd the files I touched. But note that all of these changes are to existing code that I just moved and left exactly as it had been and not anything I'd added. I erred on the side of leaving things alone rather than cleaning them up. I've now done the opposite as you asked, though it's not entirely consistent with the style and formatting of the existing code in the same directory. I'm happy to use whatever style choices you prefer, but it's easiest to do that when the codebase is consistent with those choices already.

compiler-rt/lib/lsan/lsan_posix.cpp

31

Consistent with what?
I'm happy to use whatever style rules you prefer.
But there's no sign that one-declaration-per-line is the prevailing sign in the existing codebase.

I'm changing this as you asked, but it was verbatim moved from the existing code in the other file and all the style choices were not mine.

65

It was moved verbatim from the old code.

compiler-rt/lib/lsan/CMakeLists.txt

 include_directories(..)
 
 set(LSAN_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF LSAN_CFLAGS)
 
 set(LSAN_COMMON_SOURCES
   lsan_common.cpp
   lsan_common_linux.cpp
   lsan_common_mac.cpp
   )
 
 set(LSAN_SOURCES
   lsan.cpp
   lsan_allocator.cpp
-  lsan_linux.cpp
   lsan_interceptors.cpp
+  lsan_linux.cpp
   lsan_mac.cpp
   lsan_malloc_mac.cpp
+  lsan_posix.cpp
   lsan_preinit.cpp
   lsan_thread.cpp
   )
 
 set(LSAN_HEADERS
   lsan.h
   lsan_allocator.h
   lsan_common.h

compiler-rt/lib/lsan/lsan.h

 //=-- lsan.h --------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is a part of LeakSanitizer.
 // Private header for standalone LSan RTL.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lsan_thread.h"
+#if SANITIZER_POSIX
+#include "lsan_posix.h"
+#endif
 #include "sanitizer_common/sanitizer_flags.h"
 #include "sanitizer_common/sanitizer_stacktrace.h"
 
 #define GET_STACK_TRACE(max_size, fast)                       \
   __sanitizer::BufferedStackTrace stack;                      \
   stack.Unwind(StackTrace::GetCurrentPc(),                    \
                GET_CURRENT_FRAME(), nullptr, fast, max_size);
 

compiler-rt/lib/lsan/lsan.cpp

extern "C" void __lsan_init() {
   InitializeFlags();
   InitCommonLsan();
   InitializeAllocator();
   ReplaceSystemMalloc();
   InitTlsSize();
   InitializeInterceptors();
   InitializeThreadRegistry();
   InstallDeadlySignalHandlers(LsanOnDeadlySignal);
-  u32 tid = ThreadCreate(0, 0, true);
-  CHECK_EQ(tid, 0);
-  ThreadStart(tid, GetTid());
-  SetCurrentThread(tid);
+  InitializeMainThread();
 
   if (common_flags()->detect_leaks && common_flags()->leak_check_at_exit)
     Atexit(DoLeakCheck);
 
   InitializeCoverage(common_flags()->coverage, common_flags()->coverage_dir);
 
   lsan_inited = true;
   lsan_init_is_running = false;
 }
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 void __sanitizer_print_stack_trace() {
   GET_STACK_TRACE_FATAL;
   stack.Print();
 }

compiler-rt/lib/lsan/lsan_interceptors.cpp

 #include "sanitizer_common/sanitizer_atomic.h"
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_flags.h"
 #include "sanitizer_common/sanitizer_internal_defs.h"
 #include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_platform_interceptors.h"
 #include "sanitizer_common/sanitizer_platform_limits_netbsd.h"
 #include "sanitizer_common/sanitizer_platform_limits_posix.h"
+#if SANITIZER_POSIX
 #include "sanitizer_common/sanitizer_posix.h"
+#endif
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 #include "lsan.h"
 #include "lsan_allocator.h"
 #include "lsan_common.h"
 #include "lsan_thread.h"
 
 #include <stddef.h>
 
if (pthread_setspecific(g_thread_finalize_key,
                           (void*)GetPthreadDestructorIterations())) {
     Report("LeakSanitizer: failed to set thread key.\n");
     Die();
   }
 #endif
   int tid = 0;
   while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0)
     internal_sched_yield();
-  SetCurrentThread(tid);
   ThreadStart(tid, GetTid());
   atomic_store(&p->tid, 0, memory_order_release);
   return callback(param);
 }
 
 INTERCEPTOR(int, pthread_create, void *th, void *attr,
             void *(*callback)(void *), void *param) {
   ENSURE_LSAN_INITED;

compiler-rt/lib/lsan/lsan_posix.h

+//=-- lsan_posix.h -----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+//
+// This file is a part of LeakSanitizer.
+// Standalone LSan RTL code common to POSIX-like systems.
+//
+//===---------------------------------------------------------------------===//
+
+#ifndef LSAN_POSIX_H
+#define LSAN_POSIX_H
+
+#include "sanitizer_common/sanitizer_platform.h"
+#include "lsan_thread.h"
+
+#if !SANITIZER_POSIX
+#error "lsan_posix.h is used only on POSIX-like systems (SANITIZER_POSIX)"
+#endif
+
+namespace __sanitizer {
+struct DTLS;
+}
+
+namespace __lsan {
+
+class ThreadContext : public ThreadContextLsanBase {
+ public:
+  explicit ThreadContext(int tid);
+  void OnStarted(void *arg) override;
+  uptr tls_begin() { return tls_begin_; }
+  uptr tls_end() { return tls_end_; }
+  DTLS *dtls() { return dtls_; }
+
+ private:
+  uptr tls_begin_, tls_end_;
+  DTLS *dtls_;
+};
+
+void ThreadStart(u32 tid, tid_t os_id,
+                 ThreadType thread_type = ThreadType::Regular);
+
+}  // namespace __lsan
+
+#endif  // LSAN_POSIX_H

compiler-rt/lib/lsan/lsan_posix.cpp

+//=-- lsan_posix.cpp -----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+//
+// This file is a part of LeakSanitizer.
+// Standalone LSan RTL code common to POSIX-like systems.
+//
+//===---------------------------------------------------------------------===//
+
+#include "sanitizer_common/sanitizer_platform.h"
+
+#if SANITIZER_POSIX
+#include "lsan.h"
+#include "lsan_allocator.h"
+#include "sanitizer_common/sanitizer_stacktrace.h"
+#include "sanitizer_common/sanitizer_tls_get_addr.h"
+
+namespace __lsan {
+
+ThreadContext::ThreadContext(int tid)
+    : ThreadContextLsanBase(tid),
+      tls_begin_(0),
+      tls_end_(0),
+      dtls_(nullptr) {}
+
+struct OnStartedArgs {
+  uptr stack_begin, stack_end,
+       cache_begin, cache_end,
+       tls_begin, tls_end;
+  DTLS *dtls;
+};
+
+void ThreadContext::OnStarted(void *arg) {
+  auto args = reinterpret_cast<const OnStartedArgs *>(arg);
+  stack_begin_ = args->stack_begin;
+  stack_end_ = args->stack_end;
+  tls_begin_ = args->tls_begin;
+  tls_end_ = args->tls_end;
+  cache_begin_ = args->cache_begin;
+  cache_end_ = args->cache_end;
+  dtls_ = args->dtls;
+}
+
+void ThreadStart(u32 tid, tid_t os_id, ThreadType thread_type) {
+  OnStartedArgs args;
+  uptr stack_size = 0;
+  uptr tls_size = 0;
+  GetThreadStackAndTls(tid == 0, &args.stack_begin, &stack_size,
+                       &args.tls_begin, &tls_size);
+  args.stack_end = args.stack_begin + stack_size;
+  args.tls_end = args.tls_begin + tls_size;
+  GetAllocatorCacheRange(&args.cache_begin, &args.cache_end);
+  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));
+  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();
+  *dtls = context->dtls();
+  return true;
+}
+
+void InitializeMainThread() {
+  u32 tid = ThreadCreate(0, 0, true);
+  CHECK_EQ(tid, 0);
+  ThreadStart(tid, GetTid());
+}
+
+}  // namespace __lsan
+
+#endif  // SANITIZER_POSIX

compiler-rt/lib/lsan/lsan_thread.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LSAN_THREAD_H
 #define LSAN_THREAD_H
 
 #include "sanitizer_common/sanitizer_thread_registry.h"
 
-namespace __sanitizer {
-struct DTLS;
-}
-
 namespace __lsan {
 
-class ThreadContext : public ThreadContextBase {
+class ThreadContextLsanBase : public ThreadContextBase {
  public:
-  explicit ThreadContext(int tid);
-  void OnStarted(void *arg) override;
+  explicit ThreadContextLsanBase(int tid);
   void OnFinished() override;
   uptr stack_begin() { return stack_begin_; }
   uptr stack_end() { return stack_end_; }
-  uptr tls_begin() { return tls_begin_; }
-  uptr tls_end() { return tls_end_; }
   uptr cache_begin() { return cache_begin_; }
   uptr cache_end() { return cache_end_; }
-  DTLS *dtls() { return dtls_; }
 
- private:
+  // The argument is passed on to the subclass's OnStarted member function.
+  static void ThreadStart(u32 tid, tid_t os_id, ThreadType thread_type,
+                          void *onstarted_arg);
+
+ protected:
   uptr stack_begin_, stack_end_,
-       cache_begin_, cache_end_,
-       tls_begin_, tls_end_;
-  DTLS *dtls_;
+       cache_begin_, cache_end_;
 };
 
+// This subclass of ThreadContextLsanBase is declared in an OS-specific header.
+class ThreadContext;
+
 void InitializeThreadRegistry();
+void InitializeMainThread();
 
-void ThreadStart(u32 tid, tid_t os_id,
-                 ThreadType thread_type = ThreadType::Regular);
+u32 ThreadCreate(u32 tid, uptr uid, bool detached, void *arg = nullptr);
 void ThreadFinish();
-u32 ThreadCreate(u32 tid, uptr uid, bool detached);
 void ThreadJoin(u32 tid);
 u32 ThreadTid(uptr uid);
 
 u32 GetCurrentThread();
 void SetCurrentThread(u32 tid);
 ThreadContext *CurrentThreadContext();
 void EnsureMainThreadIDIsCorrect();
+
 }  // namespace __lsan
 
 #endif  // LSAN_THREAD_H

compiler-rt/lib/lsan/lsan_thread.cpp

 //===----------------------------------------------------------------------===//
 
 #include "lsan_thread.h"
 
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_thread_registry.h"
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
+#include "lsan.h"
 #include "lsan_allocator.h"
 #include "lsan_common.h"
 
 namespace __lsan {
 
 static ThreadRegistry *thread_registry;
 
 static ThreadContextBase *CreateThreadContext(u32 tid) {
   void *mem = MmapOrDie(sizeof(ThreadContext), "ThreadContext");
   return new(mem) ThreadContext(tid);
 }
 
 static const uptr kMaxThreads = 1 << 13;
 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);
 }
 
-ThreadContext::ThreadContext(int tid)
+ThreadContextLsanBase::ThreadContextLsanBase(int tid)
     : ThreadContextBase(tid),
       stack_begin_(0),
       stack_end_(0),
       cache_begin_(0),
-      cache_end_(0),
-      tls_begin_(0),
-      tls_end_(0),
-      dtls_(nullptr) {}
-
-struct OnStartedArgs {
-  uptr stack_begin, stack_end,
-       cache_begin, cache_end,
-       tls_begin, tls_end;
-  DTLS *dtls;
-};
-
-void ThreadContext::OnStarted(void *arg) {
-  OnStartedArgs *args = reinterpret_cast<OnStartedArgs *>(arg);
-  stack_begin_ = args->stack_begin;
-  stack_end_ = args->stack_end;
-  tls_begin_ = args->tls_begin;
-  tls_end_ = args->tls_end;
-  cache_begin_ = args->cache_begin;
-  cache_end_ = args->cache_end;
-  dtls_ = args->dtls;
-}
+      cache_end_(0) {}
 
-void ThreadContext::OnFinished() {
+void ThreadContextLsanBase::OnFinished() {
   AllocatorThreadFinish();
   DTLS_Destroy();
 }
 
-u32 ThreadCreate(u32 parent_tid, uptr user_id, bool detached) {
-  return thread_registry->CreateThread(user_id, detached, parent_tid,
-                                       /* arg */ nullptr);
-}
-
-void ThreadStart(u32 tid, tid_t os_id, ThreadType thread_type) {
-  OnStartedArgs args;
-  uptr stack_size = 0;
-  uptr tls_size = 0;
-  GetThreadStackAndTls(tid == 0, &args.stack_begin, &stack_size,
-                       &args.tls_begin, &tls_size);
-  args.stack_end = args.stack_begin + stack_size;
-  args.tls_end = args.tls_begin + tls_size;
-  GetAllocatorCacheRange(&args.cache_begin, &args.cache_end);
-  args.dtls = DTLS_Get();
-  thread_registry->StartThread(tid, os_id, thread_type, &args);
+u32 ThreadCreate(u32 parent_tid, uptr user_id, bool detached, void *arg) {
+  return thread_registry->CreateThread(user_id, detached, parent_tid, arg);
+}
+
+void ThreadContextLsanBase::ThreadStart(u32 tid, tid_t os_id,
+                                        ThreadType thread_type, void *arg) {
+  thread_registry->StartThread(tid, os_id, thread_type, arg);
+  SetCurrentThread(tid);
 }
 
 void ThreadFinish() {
   thread_registry->FinishThread(GetCurrentThread());
   SetCurrentThread(kInvalidTid);
 }
 
 ThreadContext *CurrentThreadContext() {

 
 void EnsureMainThreadIDIsCorrect() {
   if (GetCurrentThread() == 0)
     CurrentThreadContext()->os_id = GetTid();
 }
 
 ///// Interface to the common LSan module. /////
 
-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 *>(
-      thread_registry->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();
-  *dtls = context->dtls();
-  return true;
-}
-
 void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
                             void *arg) {
 }
 
 void LockThreadRegistry() {
   thread_registry->Lock();
 }