[TSan] The "thread_terminate" event is delivered on the terminating thread

[kubamracek]

I've mentioned this to Julian in person: The code change look good to me, but any changes in this piece of code need a high level of manual testing as this code is very critical and has potential for breaking something even with innocent changes like this.

[delcypher]

@yln The description of this change needs improving. The title probably ought to be "[TSan] The "thread_terminate" event is always delivered on the terminating thread" and then that should be followed with a brief justification along with a radar number (if there is one).

I don't know if the invariant you're adding is actually true so I can't really comment on that.

[delcypher]

compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp

197

Does this comment need updating? The comment and your change together imply that the PTHREAD_INTROSPECTION_THREAD_TERMINATE event only happens on GCD worker threads. Is that right?

[yln]

Superseded by D72549.

compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp

 
 #if !SANITIZER_GO
 void InitializeShadowMemoryPlatform() { }
 
 // On OS X, GCD worker threads are created without a call to pthread_create. We
 // need to properly register these threads with ThreadCreate and ThreadStart.
 // These threads don't have a parent thread, as they are created "spuriously".
 // We're using a libpthread API that notifies us about a newly created thread.
 // The `thread == pthread_self()` check indicates this is actually a worker
 // thread. If it's just a regular thread, this hook is called on the parent
 // thread.
 typedef void (*pthread_introspection_hook_t)(unsigned int event,
                                              pthread_t thread, void *addr,
                                              size_t size);
 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_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) {
   if (event == PTHREAD_INTROSPECTION_THREAD_CREATE) {
     if (thread == pthread_self()) {
       // The current thread is a newly created GCD worker thread.
       ThreadState *thr = cur_thread();
       Processor *proc = ProcCreate();
       ProcWire(proc, thr);
       ThreadState *parent_thread_state = nullptr;  // No parent.
       int tid = ThreadCreate(parent_thread_state, 0, (uptr)thread, true);
       CHECK_NE(tid, 0);
       ThreadStart(thr, tid, GetTid(), ThreadType::Worker);
     }
   } else if (event == PTHREAD_INTROSPECTION_THREAD_TERMINATE) {
-    if (thread == pthread_self()) {
-      ThreadState *thr = cur_thread();
-      if (thr->tctx) {
-        DestroyThreadState();
-      }
-    }
-  }
+    CHECK(thread == pthread_self());
 
   if (prev_pthread_introspection_hook != nullptr)
     prev_pthread_introspection_hook(event, thread, addr, size);
 }
 #endif
 
 void InitializePlatformEarly() {
 #if defined(__aarch64__)