[PATCH] D51064: [tsan] Adjust setjmp/longjmp handling on Darwin for macOS Mojave

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.
Herald added a subscriber: jfb.


================
Comment at: lib/tsan/rtl/tsan_platform_mac.cc:260
+    __tsan_darwin_setjmp_xor_key =
+        (uptr)pthread_getspecific(kPthreadSetjmpXorKeySlot);
+  }
----------------
This seems slightly odd. Using `pthread_getspecific` suggests the value you're requested could be different between threads. However the value you read here is global is being written to a global in the TSan runtime and thus will be shared by all threads.

At this point during init there is only one thread so there's only one value you can retrieve this early on but I do wonder if you'd get a different value on another thread. Perhaps this key should be stored per thread instead and initialized on thread creation?

If we expect the key to not change between threads perhaps we should add a `CHECK_EQ()` call that on thread creation checks that the key retrieved for the thread matches the global value stored during init?


================
Comment at: lib/tsan/rtl/tsan_platform_mac.cc:261
+        (uptr)pthread_getspecific(kPthreadSetjmpXorKeySlot);
+  }
 }
----------------
On the else branch here we could add `CHECK_EQ(__tsan_darwin_setjmp_xor_key, 0);` because we require this value for the key so that the xor operation on older OS versions is a no-op.


================
Comment at: lib/tsan/rtl/tsan_rtl_amd64.S:199
   mov %rdi, %rsi
+  xorq ___tsan_darwin_setjmp_xor_key(%rip), %rsi
 #elif defined(__linux__)
----------------
Could you jog my memory. Why is `%rip` involved in the xor calculation? `___tsan_darwin_setjmp_xor_key` is being used as an offset here so presumably the offset is instruction relative?


Repository:
  rL LLVM

https://reviews.llvm.org/D51064