[PATCH] D14777: [tsan] For OS X thread finalization, remove g_thread_finalize_key in favor of libpthread hooks

[Kuba Brecka via llvm-commits]

kubabrecka created this revision.
kubabrecka added reviewers: dvyukov, samsonov, glider, kcc.
kubabrecka added subscribers: llvm-commits, zaks.anna, ismailp.

On OS X, the thread finalization is fragile due to thread-local variables destruction order.  I've seen cases where the we destroy the ThreadState too early and subsequent thread-local values' destructors call interceptors again.  Let's replace the TLV-based thread finalization method with libpthread hooks.  The notification PTHREAD_INTROSPECTION_THREAD_TERMINATE is called *after* all TLVs have been destroyed.

http://reviews.llvm.org/D14777

Files:
  lib/tsan/rtl/tsan_interceptors.cc
  lib/tsan/rtl/tsan_platform_mac.cc

Index: lib/tsan/rtl/tsan_platform_mac.cc
===================================================================
--- lib/tsan/rtl/tsan_platform_mac.cc
+++ lib/tsan/rtl/tsan_platform_mac.cc
@@ -124,7 +124,7 @@
 extern "C" pthread_introspection_hook_t pthread_introspection_hook_install(
     pthread_introspection_hook_t hook);
 static const uptr PTHREAD_INTROSPECTION_THREAD_CREATE = 1;
-static const uptr PTHREAD_INTROSPECTION_THREAD_DESTROY = 4;
+static const uptr PTHREAD_INTROSPECTION_THREAD_TERMINATE = 3;
 static pthread_introspection_hook_t prev_pthread_introspection_hook;
 static void my_pthread_introspection_hook(unsigned int event, pthread_t thread,
                                           void *addr, size_t size) {
@@ -137,10 +137,12 @@
       ThreadState *thr = cur_thread();
       ThreadStart(thr, tid, GetTid());
     }
-  } else if (event == PTHREAD_INTROSPECTION_THREAD_DESTROY) {
-    ThreadState *thr = cur_thread();
-    if (thr->tctx && thr->tctx->parent_tid == kInvalidTid) {
-      DestroyThreadState();
+  } else if (event == PTHREAD_INTROSPECTION_THREAD_TERMINATE) {
+    if (thread == pthread_self()) {
+      ThreadState *thr = cur_thread();
+      if (thr->tctx) {
+        DestroyThreadState();
+      }
     }
   }
 
Index: lib/tsan/rtl/tsan_interceptors.cc
===================================================================
--- lib/tsan/rtl/tsan_interceptors.cc
+++ lib/tsan/rtl/tsan_interceptors.cc
@@ -233,7 +233,9 @@
   return ctx;
 }
 
+#if !SANITIZER_MAC
 static unsigned g_thread_finalize_key;
+#endif
 
 ScopedInterceptor::ScopedInterceptor(ThreadState *thr, const char *fname,
                                      uptr pc)
@@ -836,6 +838,7 @@
 }
 }  // namespace __tsan
 
+#if !SANITIZER_MAC
 static void thread_finalize(void *v) {
   uptr iter = (uptr)v;
   if (iter > 1) {
@@ -847,6 +850,7 @@
   }
   DestroyThreadState();
 }
+#endif
 
 
 struct ThreadParam {
@@ -864,13 +868,15 @@
     ThreadState *thr = cur_thread();
     // Thread-local state is not initialized yet.
     ScopedIgnoreInterceptors ignore;
+#if !SANITIZER_MAC
     ThreadIgnoreBegin(thr, 0);
     if (pthread_setspecific(g_thread_finalize_key,
                             (void *)GetPthreadDestructorIterations())) {
       Printf("ThreadSanitizer: failed to set thread key\n");
       Die();
     }
     ThreadIgnoreEnd(thr, 0);
+#endif
     while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0)
       internal_sched_yield();
     ThreadStart(thr, tid, GetTid());
@@ -2651,10 +2657,12 @@
     Die();
   }
 
+#if !SANITIZER_MAC
   if (pthread_key_create(&g_thread_finalize_key, &thread_finalize)) {
     Printf("ThreadSanitizer: failed to create thread key\n");
     Die();
   }
+#endif
 
   FdInit();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D14777.40510.patch
Type: text/x-patch
Size: 2731 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151118/2f9d6127/attachment.bin>