[PATCH] [asan] Improve thread lifetime tracking on POSIX systems.

[Sergey Matveev]

Hi samsonov, kcc,

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.

http://llvm-reviews.chandlerc.com/D1896

Files:
  lib/asan/asan_posix.cc
  lib/asan/asan_rtl.cc
  lib/asan/asan_thread.cc
  lib/asan/asan_thread.h
  lib/asan/asan_win.cc
  lib/lsan/lit_tests/TestCases/cleanup_in_tsd_destructor.cc

Index: lib/asan/asan_posix.cc
===================================================================
--- lib/asan/asan_posix.cc
+++ lib/asan/asan_posix.cc
@@ -1,4 +1,4 @@
-//===-- asan_linux.cc -----------------------------------------------------===//
+//===-- asan_posix.cc -----------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -116,6 +116,15 @@
   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
Index: lib/asan/asan_rtl.cc
===================================================================
--- lib/asan/asan_rtl.cc
+++ lib/asan/asan_rtl.cc
@@ -535,7 +535,7 @@
 
   InstallSignalHandlers();
 
-  AsanTSDInit(AsanThread::TSDDtor);
+  AsanTSDInit(PlatformTSDDtor);
   // Allocator should be initialized before starting external symbolizer, as
   // fork() on Mac locks the allocator.
   InitializeAllocator();
Index: lib/asan/asan_thread.cc
===================================================================
--- lib/asan/asan_thread.cc
+++ lib/asan/asan_thread.cc
@@ -165,7 +165,11 @@
   malloc_storage().CommitBack();
   if (flags()->use_sigaltstack) UnsetAlternateSignalStack();
 
+#if !SANITIZER_POSIX
+  // On POSIX systems the TSD destructor will take care of this later on. Doing
+  // this too early would make LeakSanitizer unhappy.
   this->Destroy();
+#endif
 
   return res;
 }
Index: lib/asan/asan_thread.h
===================================================================
--- lib/asan/asan_thread.h
+++ lib/asan/asan_thread.h
@@ -19,6 +19,7 @@
 #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"
 
@@ -36,10 +37,12 @@
   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;
 
@@ -127,6 +130,8 @@
   bool unwinding;
 };
 
+void PlatformTSDDtor(void *tsd);
+
 // ScopedUnwinding is a scope for stacktracing member of a context
 class ScopedUnwinding {
  public:
Index: lib/asan/asan_win.cc
===================================================================
--- lib/asan/asan_win.cc
+++ lib/asan/asan_win.cc
@@ -60,6 +60,9 @@
   fake_tsd = tsd;
 }
 
+void PlatformTSDDtor(void *tsd) {
+  AsanThread::TSDDtor(tsd);
+}
 // ---------------------- Various stuff ---------------- {{{1
 void MaybeReexec() {
   // No need to re-exec on Windows.
Index: lib/lsan/lit_tests/TestCases/cleanup_in_tsd_destructor.cc
===================================================================
--- /dev/null
+++ lib/lsan/lit_tests/TestCases/cleanup_in_tsd_destructor.cc
@@ -0,0 +1,45 @@
+// 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]]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1896.1.patch
Type: text/x-patch
Size: 4631 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131011/bde49de0/attachment.bin>