[asan] Improve thread lifetime tracking on POSIX systems.

[earthdok]

Summary

Call AsanThread::Destroy() from a late-running TSD destructor.
Previously we called it before any user-registered TSD destructors, which caused
false positives in LeakSanitizer.

[samsonov]

I'm OK with this change, but let's wait for Kostya's opinion as well.

lib/asan/asan_thread.cc

168

I prefer

if (!SANITIZER_POSIX)

Elaborate whan "unhappy" means (thread will be treated as dead too early, so we'll fail to see live memory from this thread while it's still running TSD destructors).

lib/asan/asan_thread.h

133

Consider moving this to asan_internal.h next to other TSD-related functions.

[earthdok]

• address samsonov's comments

[kcc]

lib/asan/asan_thread.cc

173

I don't like this 'if'. What will happen on Windos? Won't PlatformTSDDtor be called?

[samsonov]

lib/asan/asan_thread.cc

173

It won't be called on Windows:

void AsanTSDInit(void (*destructor)(void *tsd)) {
  // FIXME: we're ignoring the destructor for now.
  tsd_key_inited = true;
}

It shouldn't break in a bad way if PlatformTSDDtor will be called on Windows: if thread is already destroyed, AsanThread::Destroy() is a no-op.

[kcc]

LGTM
I still don't like this 'if' but see no better way

[earthdok]

Committed r192585

lib/asan/asan_internal.h

 void ReadContextStack(void *context, uptr *stack, uptr *ssize);
 void AsanPlatformThreadInit();
 void StopInitOrderChecking();
 
 // Wrapper for TLS/TSD.
 void AsanTSDInit(void (*destructor)(void *tsd));
 void *AsanTSDGet();
 void AsanTSDSet(void *tsd);
+void PlatformTSDDtor(void *tsd);
 
 void AppendToErrorMessageBuffer(const char *buffer);
 
 // Platfrom-specific options.
 #if SANITIZER_MAC
 bool PlatformHasDifferentMemcpyAndMemmove();
 # define PLATFORM_HAS_DIFFERENT_MEMCPY_AND_MEMMOVE \
     (PlatformHasDifferentMemcpyAndMemmove())

lib/asan/asan_posix.cc

-//===-- asan_linux.cc -----------------------------------------------------===//
+//===-- asan_posix.cc -----------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
void *AsanTSDGet() {
   return pthread_getspecific(tsd_key);
 }
 
 void AsanTSDSet(void *tsd) {
   CHECK(tsd_key_inited);
   pthread_setspecific(tsd_key, tsd);
 }
 
+void PlatformTSDDtor(void *tsd) {
+  AsanThreadContext *context = (AsanThreadContext*)tsd;
+  if (context->destructor_iterations > 1) {
+    context->destructor_iterations--;
+    CHECK_EQ(0, pthread_setspecific(tsd_key, tsd));
+    return;
+  }
+  AsanThread::TSDDtor(tsd);
+}
 }  // namespace __asan
 
 #endif  // SANITIZER_LINUX || SANITIZER_MAC

lib/asan/asan_rtl.cc

if (full_shadow_is_available) {
     Report("Shadow memory range interleaves with an existing memory mapping. "
            "ASan cannot proceed correctly. ABORTING.\n");
     DumpProcessMap();
     Die();
   }
 
   InstallSignalHandlers();
 
-  AsanTSDInit(AsanThread::TSDDtor);
+  AsanTSDInit(PlatformTSDDtor);
   // Allocator should be initialized before starting external symbolizer, as
   // fork() on Mac locks the allocator.
   InitializeAllocator();
 
   // Start symbolizer process if necessary.
   if (common_flags()->symbolize && &getSymbolizer) {
     getSymbolizer()
         ->InitializeExternal(common_flags()->external_symbolizer_path);

lib/asan/asan_thread.h

 #ifndef ASAN_THREAD_H
 #define ASAN_THREAD_H
 
 #include "asan_allocator.h"
 #include "asan_internal.h"
 #include "asan_fake_stack.h"
 #include "asan_stack.h"
 #include "asan_stats.h"
+#include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_thread_registry.h"
 
 namespace __asan {
 
 const u32 kInvalidTid = 0xffffff;  // Must fit into 24 bits.
 const u32 kMaxNumberOfThreads = (1 << 22);  // 4M
 
 class AsanThread;
 
 // These objects are created for every thread and are never deleted,
 // so we can find them by tid even if the thread is long dead.
 class AsanThreadContext : public ThreadContextBase {
  public:
   explicit AsanThreadContext(int tid)
       : ThreadContextBase(tid),
         announced(false),
+        destructor_iterations(kPthreadDestructorIterations),
         thread(0) {
     internal_memset(&stack, 0, sizeof(stack));
   }
   bool announced;
+  int destructor_iterations;
   StackTrace stack;
   AsanThread *thread;
 
   void OnCreated(void *arg);
   void OnFinished();
 };
 
 // AsanThreadContext objects are never freed, so we need many of them.
private:
   uptr tls_end_;
 
   FakeStack *fake_stack_;
   AsanThreadLocalMallocStorage malloc_storage_;
   AsanStats stats_;
   bool unwinding;
 };
 
 // ScopedUnwinding is a scope for stacktracing member of a context
 class ScopedUnwinding {
  public:
   explicit ScopedUnwinding(AsanThread *t) : thread(t) {
     t->setUnwinding(true);
   }
   ~ScopedUnwinding() { thread->setUnwinding(false); }
 
  private:

lib/asan/asan_thread.cc

if (!start_routine_) {
     CHECK_EQ(tid(), 0);
     return 0;
   }
 
   thread_return_t res = start_routine_(arg_);
   malloc_storage().CommitBack();
   if (flags()->use_sigaltstack) UnsetAlternateSignalStack();
 
+  // On POSIX systems we defer this to the TSD destructor. LSan will consider
+  // the thread's memory as non-live from the moment we call Destroy(), even
+  // though that memory might contain pointers to heap objects which will be
+  // cleaned up by a user-defined TSD destructor. Thus, calling Destroy() before
+  // the TSD destructors have run might cause false positives in LSan.
+  if (!SANITIZER_POSIX)
     this->Destroy();
 
   return res;
 }
 
 void AsanThread::SetThreadStackAndTls() {
   uptr tls_size = 0;
   GetThreadStackAndTls(tid() == 0, &stack_bottom_, &stack_size_, &tls_begin_,
                        &tls_size);

lib/asan/asan_win.cc

void *AsanTSDGet() {
   return fake_tsd;
 }
 
 void AsanTSDSet(void *tsd) {
   CHECK(tsd_key_inited);
   fake_tsd = tsd;
 }
 
+void PlatformTSDDtor(void *tsd) {
+  AsanThread::TSDDtor(tsd);
+}
 // ---------------------- Various stuff ---------------- {{{1
 void MaybeReexec() {
   // No need to re-exec on Windows.
 }
 
 void *AsanDoesNotSupportStaticLinkage() {
 #if defined(_DEBUG)
 #error Please build the runtime with a non-debug CRT: /MD or /MT

lib/lsan/lit_tests/TestCases/cleanup_in_tsd_destructor.cc

+// Regression test for thread lifetime tracking. Thread data should be
+// considered live during the thread's termination, at least until the
+// user-installed TSD destructors have finished running (since they may contain
+// additional cleanup tasks). LSan doesn't actually meet that goal 100%, but it
+// makes its best effort.
+// RUN: LSAN_BASE="report_objects=1:use_registers=0:use_stacks=0:use_globals=0"
+// RUN: %clangxx_lsan %s -o %t
+// RUN: LSAN_OPTIONS=$LSAN_BASE:use_tls=1 %t
+// RUN: LSAN_OPTIONS=$LSAN_BASE:use_tls=0 not %t 2>&1 | FileCheck %s
+
+#include <assert.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "sanitizer/lsan_interface.h"
+
+pthread_key_t key;
+__thread void *p;
+
+void key_destructor(void *arg) {
+  // Generally this may happen on a different thread.
+  __lsan_do_leak_check();
+}
+
+void *thread_func(void *arg) {
+  p = malloc(1337);
+  fprintf(stderr, "Test alloc: %p.\n", p);
+  int res = pthread_setspecific(key, (void*)1);
+  assert(res == 0);
+  return 0;
+}
+
+int main() {
+  int res = pthread_key_create(&key, &key_destructor);
+  assert(res == 0);
+  pthread_t thread_id;
+  res = pthread_create(&thread_id, 0, thread_func, 0);
+  assert(res == 0);
+  res = pthread_join(thread_id, 0);
+  assert(res == 0);
+  return 0;
+}
+// CHECK: Test alloc: [[ADDR:.*]].
+// CHECK: leaked 1337 byte object at [[ADDR]]