[LSAN] Use ThreadArgRetval in LSAN

[vitalybuka]

Summary

Fixes false leaks on thread retval.

[vitalybuka]

update

[vitalybuka]

ping

[thurston]

compiler-rt/lib/lsan/lsan_thread.cpp

37

InitializeThreadRegistry/LockThreadRegistry/UnlockThreadRegistry should have a more generic name because they now deal with both ThreadRegistry and ThreadArgRetval

compiler-rt/lib/lsan/lsan_interceptors.cpp

 }
 #define LSAN_MAYBE_INTERCEPT_STRERROR INTERCEPT_FUNCTION(strerror)
 #else
 #define LSAN_MAYBE_INTERCEPT_STRERROR
 #endif
 
 #if SANITIZER_POSIX
 
-struct ThreadParam {
-  void *(*callback)(void *arg);
-  void *param;
-  atomic_uintptr_t tid;
-};
-
 extern "C" void *__lsan_thread_start_func(void *arg) {
-  ThreadParam *p = (ThreadParam*)arg;
-  void* (*callback)(void *arg) = p->callback;
-  void *param = p->param;
+  atomic_uintptr_t *atomic_tid = (atomic_uintptr_t *)arg;
   // Wait until the last iteration to maximize the chance that we are the last
   // destructor to run.
 #if !SANITIZER_NETBSD && !SANITIZER_FREEBSD
   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)
+  while ((tid = atomic_load(atomic_tid, memory_order_acquire)) == 0)
     internal_sched_yield();
   ThreadStart(tid, GetTid());
-  atomic_store(&p->tid, 0, memory_order_release);
-  return callback(param);
+  atomic_store(atomic_tid, 0, memory_order_release);
+  auto self = GetThreadSelf();
+  auto args = GetThreadArgRetval().GetArgs(self);
+  void *retval = (*args.routine)(args.arg_retval);
+  GetThreadArgRetval().Finish(self, retval);
+  return retval;
 }
 
 INTERCEPTOR(int, pthread_create, void *th, void *attr,
             void *(*callback)(void *), void *param) {
   ENSURE_LSAN_INITED;
   EnsureMainThreadIDIsCorrect();
+  bool detached = [attr]() {
+    int d = 0;
+    return attr && !pthread_attr_getdetachstate(attr, &d) && IsStateDetached(d);
+  }();
   __sanitizer_pthread_attr_t myattr;
   if (!attr) {
     pthread_attr_init(&myattr);
     attr = &myattr;
   }
   AdjustStackSize(attr);
-  int detached = 0;
-  pthread_attr_getdetachstate(attr, &detached);
-  ThreadParam p;
-  p.callback = callback;
-  p.param = param;
-  atomic_store(&p.tid, 0, memory_order_relaxed);
-  int res;
+  atomic_uintptr_t atomic_tid = {};
+  int result;
   {
     // Ignore all allocations made by pthread_create: thread stack/TLS may be
     // stored by pthread for future reuse even after thread destruction, and
     // the linked list it's stored in doesn't even hold valid pointers to the
     // objects, the latter are calculated by obscure pointer arithmetic.
     ScopedInterceptorDisabler disabler;
-    res = REAL(pthread_create)(th, attr, __lsan_thread_start_func, &p);
+    GetThreadArgRetval().Create(detached, {callback, param}, [&]() -> uptr {
+      result =
+          REAL(pthread_create)(th, attr, __lsan_thread_start_func, &atomic_tid);
+      return result ? 0 : *(uptr *)(th);
+    });
   }
-  if (res == 0) {
-    int tid = ThreadCreate(GetCurrentThreadId(), IsStateDetached(detached));
+  if (result == 0) {
+    int tid = ThreadCreate(GetCurrentThreadId(), detached);
     CHECK_NE(tid, kMainTid);
-    atomic_store(&p.tid, tid, memory_order_release);
-    while (atomic_load(&p.tid, memory_order_acquire) != 0)
+    atomic_store(&atomic_tid, tid, memory_order_release);
+    while (atomic_load(&atomic_tid, memory_order_acquire) != 0)
       internal_sched_yield();
   }
   if (attr == &myattr)
     pthread_attr_destroy(&myattr);
-  return res;
+  return result;
 }
 
 INTERCEPTOR(int, pthread_join, void *thread, void **retval) {
-  return REAL(pthread_join)(thread, retval);
+  int result;
+  GetThreadArgRetval().Join((uptr)thread, [&]() {
+    result = REAL(pthread_join)(thread, retval);
+    return !result;
+  });
+  return result;
 }
 
 INTERCEPTOR(int, pthread_detach, void *thread) {
-  return REAL(pthread_detach)(thread);
+  int result;
+  GetThreadArgRetval().Detach((uptr)thread, [&]() {
+    result = REAL(pthread_detach)(thread);
+    return !result;
+  });
+  return result;
 }
 
 INTERCEPTOR(int, pthread_exit, void *retval) {
+  GetThreadArgRetval().Finish(GetThreadSelf(), retval);
   return REAL(pthread_exit)(retval);
 }
 
 #  if SANITIZER_INTERCEPT_TRYJOIN
 INTERCEPTOR(int, pthread_tryjoin_np, void *thread, void **ret) {
-  return REAL(pthread_tryjoin_np)(thread, ret);
+  int result;
+  GetThreadArgRetval().Join((uptr)thread, [&]() {
+    result = REAL(pthread_tryjoin_np)(thread, ret);
+    return !result;
+  });
+  return result;
 }
 #    define LSAN_MAYBE_INTERCEPT_TRYJOIN INTERCEPT_FUNCTION(pthread_tryjoin_np)
 #  else
 #    define LSAN_MAYBE_INTERCEPT_TRYJOIN
 #  endif  // SANITIZER_INTERCEPT_TRYJOIN
 
 #  if SANITIZER_INTERCEPT_TIMEDJOIN
 INTERCEPTOR(int, pthread_timedjoin_np, void *thread, void **ret,
             const struct timespec *abstime) {
-  return REAL(pthread_timedjoin_np)(thread, ret, abstime);
+  int result;
+  GetThreadArgRetval().Join((uptr)thread, [&]() {
+    result = REAL(pthread_timedjoin_np)(thread, ret, abstime);
+    return !result;
+  });
+  return result;
 }
 #    define LSAN_MAYBE_INTERCEPT_TIMEDJOIN \
       INTERCEPT_FUNCTION(pthread_timedjoin_np)
 #  else
 #    define LSAN_MAYBE_INTERCEPT_TIMEDJOIN
 #  endif  // SANITIZER_INTERCEPT_TIMEDJOIN
 
 DEFINE_REAL_PTHREAD_FUNCTIONS

compiler-rt/lib/lsan/lsan_thread.h

 //=-- lsan_thread.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.
 // Thread registry for standalone LSan.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LSAN_THREAD_H
 #define LSAN_THREAD_H
 
+#include "sanitizer_common/sanitizer_thread_arg_retval.h"
 #include "sanitizer_common/sanitizer_thread_registry.h"
 
 namespace __lsan {
 
 class ThreadContextLsanBase : public ThreadContextBase {
  public:
   explicit ThreadContextLsanBase(int tid);
   void OnStarted(void *arg) override;

 
 // This subclass of ThreadContextLsanBase is declared in an OS-specific header.
 class ThreadContext;
 
 void InitializeThreadRegistry();
 void InitializeMainThread();
 
 ThreadRegistry *GetLsanThreadRegistryLocked();
+ThreadArgRetval &GetThreadArgRetval();
 
 u32 ThreadCreate(u32 tid, bool detached, void *arg = nullptr);
 void ThreadFinish();
 
 ThreadContextLsanBase *GetCurrentThread();
 inline u32 GetCurrentThreadId() {
   ThreadContextLsanBase *ctx = GetCurrentThread();
   return ctx ? ctx->tid : kInvalidTid;
 }
 void SetCurrentThread(ThreadContextLsanBase *tctx);
 void EnsureMainThreadIDIsCorrect();
 
 }  // namespace __lsan
 
 #endif  // LSAN_THREAD_H

compiler-rt/lib/lsan/lsan_thread.cpp

 #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"
 
 namespace __lsan {
 
 static ThreadRegistry *thread_registry;
+static ThreadArgRetval *thread_arg_retval;
 
 static Mutex mu_for_thread_context;
 static LowLevelAllocator allocator_for_thread_context;
 
 static ThreadContextBase *CreateThreadContext(u32 tid) {
   Lock lock(&mu_for_thread_context);
   return new (allocator_for_thread_context) ThreadContext(tid);
 }
 
 void InitializeThreadRegistry() {
-  static ALIGNED(64) char thread_registry_placeholder[sizeof(ThreadRegistry)];
+  static ALIGNED(alignof(
+      ThreadRegistry)) char thread_registry_placeholder[sizeof(ThreadRegistry)];
   thread_registry =
       new (thread_registry_placeholder) ThreadRegistry(CreateThreadContext);
+
+  static ALIGNED(alignof(ThreadArgRetval)) char
+      thread_arg_retval_placeholder[sizeof(ThreadArgRetval)];
+  thread_arg_retval = new (thread_arg_retval_placeholder) ThreadArgRetval();
 }
 
+ThreadArgRetval &GetThreadArgRetval() { return *thread_arg_retval; }
+
 ThreadContextLsanBase::ThreadContextLsanBase(int tid)
     : ThreadContextBase(tid) {}
 
 void ThreadContextLsanBase::OnStarted(void *arg) { SetCurrentThread(this); }
 
 void ThreadContextLsanBase::OnFinished() {
   AllocatorThreadFinish();
   DTLS_Destroy();

 }
 
 ///// Interface to the common LSan module. /////
 
 void GetThreadExtraStackRangesLocked(tid_t os_id,
                                      InternalMmapVector<Range> *ranges) {}
 void GetThreadExtraStackRangesLocked(InternalMmapVector<Range> *ranges) {}
 
-void LockThreadRegistry() { thread_registry->Lock(); }
+void LockThreadRegistry() {
+  thread_registry->Lock();
+  thread_arg_retval->Lock();
+}
 
-void UnlockThreadRegistry() { thread_registry->Unlock(); }
+void UnlockThreadRegistry() {
+  thread_arg_retval->Unlock();
+  thread_registry->Unlock();
+}
 
 ThreadRegistry *GetLsanThreadRegistryLocked() {
   thread_registry->CheckLocked();
   return thread_registry;
 }
 
 void GetRunningThreadsLocked(InternalMmapVector<tid_t> *threads) {
   GetLsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
       [](ThreadContextBase *tctx, void *threads) {
         if (tctx->status == ThreadStatusRunning) {
           reinterpret_cast<InternalMmapVector<tid_t> *>(threads)->push_back(
               tctx->os_id);
         }
       },
       threads);
 }
 
 void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs) {
-  // This function can be used to treat memory reachable from `tctx` as live.
-  // This is useful for threads that have been created but not yet started.
-
-  // This is currently a no-op because the LSan `pthread_create()` interceptor
-  // blocks until the child thread starts which keeps the thread's `arg` pointer
-  // live.
+  GetThreadArgRetval().GetAllPtrsLocked(ptrs);
 }
 
 }  // namespace __lsan

compiler-rt/test/lsan/TestCases/create_thread_leak.cpp

 // Test handling of arg and retval of child thread.
 
 // RUN: %clangxx_lsan -pthread %s -o %t
 // RUN: %run not %t 10 1 0 0 2>&1 | FileCheck %s --check-prefixes=LEAK,LEAK123
 // RUN: %run not %t 10 0 1 0 2>&1 | FileCheck %s --check-prefixes=LEAK,LEAK234
 // RUN: %run not %t 10 0 0 1 2>&1 | FileCheck %s --check-prefixes=LEAK,LEAK234
-
-// FIXME: Remove "not". There is no leak.
-// False LEAK234 is broken for LSAN.
-// RUN: %run %if lsan-standalone %{ not %} %t 10 0 0 0
+// RUN: %run %t 10 0 0 0
 
 #include <pthread.h>
 #include <stdlib.h>
 
 #include <sanitizer/lsan_interface.h>
 
 int detach;
 int leak_arg;