[PATCH] D51064: [tsan] Adjust setjmp/longjmp handling on Darwin for macOS Mojave
Kuba (Brecka) Mracek via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 22 16:40:02 PDT 2018
kubamracek added inline comments.
================
Comment at: lib/tsan/rtl/tsan_platform_mac.cc:260
+ __tsan_darwin_setjmp_xor_key =
+ (uptr)pthread_getspecific(kPthreadSetjmpXorKeySlot);
+ }
----------------
delcypher wrote:
> 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?
You're right, this is a bit weird, but I've confirmed with the Libc folks that currently, the value is process-global and doesn't change between threads. This is something that can potentially change in the future.
Adding the check sounds reasonable, I'll try to do that.
================
Comment at: lib/tsan/rtl/tsan_platform_mac.cc:261
+ (uptr)pthread_getspecific(kPthreadSetjmpXorKeySlot);
+ }
}
----------------
delcypher wrote:
> 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.
Hm, that seems superfluous. `__tsan_darwin_setjmp_xor_key` is initialized to zero a couple lines above.
================
Comment at: lib/tsan/rtl/tsan_rtl_amd64.S:199
mov %rdi, %rsi
+ xorq ___tsan_darwin_setjmp_xor_key(%rip), %rsi
#elif defined(__linux__)
----------------
delcypher wrote:
> 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?
Sure, this is to emit `%rip`-relative reference to `___tsan_darwin_setjmp_xor_key`. Position-independent code requires that, IMHO.
Repository:
rL LLVM
https://reviews.llvm.org/D51064
More information about the llvm-commits
mailing list