This is an archive of the discontinued LLVM Phabricator instance.

[tsan] Adjust setjmp/longjmp handling on Darwin for macOS Mojave
ClosedPublic

Authored by kubamracek on Aug 21 2018, 2:07 PM.

Details

Summary

On macOS Mojave, the OS started using the XOR-by-a-secret-key scheme (same as glibc is alread doing) for storing the SP value in setjmp environment. We need to adjust for that to keep supporting setjmp/longjmp on latest Darwin. The patch is basically doing the same what we're already doing for glibc.

rdar://problem/43542596

Diff Detail

Repository
rL LLVM

Event Timeline

kubamracek created this revision.Aug 21 2018, 2:07 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
kubamracek edited the summary of this revision. (Show Details)Aug 21 2018, 2:08 PM
george.karpenkov accepted this revision.Aug 21 2018, 2:52 PM
This revision is now accepted and ready to land.Aug 21 2018, 2:52 PM

I'm going to land this patch now, even though it's only been in review for a couple hours as there currently tests failing and known crashes (basically anything that does setjmp + longjmp) on macOS. I'll be happy to address post-commit comments.

lib/sanitizer_common/sanitizer_mac.cc
530 ↗(On Diff #161813)

I'm a bit confused how this bit is relevant to your patch. Probably should be removed.

Removing the check for '2x' versions, as that's not really relevant here.

This revision was automatically updated to reflect the committed changes.
delcypher added inline comments.Aug 22 2018, 4:29 PM
lib/tsan/rtl/tsan_platform_mac.cc
260 ↗(On Diff #161813)

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?

261 ↗(On Diff #161813)

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.

lib/tsan/rtl/tsan_rtl_amd64.S
199 ↗(On Diff #161813)

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?

kubamracek added inline comments.Aug 22 2018, 4:39 PM
lib/tsan/rtl/tsan_platform_mac.cc
260 ↗(On Diff #161813)

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.

261 ↗(On Diff #161813)

Hm, that seems superfluous. __tsan_darwin_setjmp_xor_key is initialized to zero a couple lines above.

lib/tsan/rtl/tsan_rtl_amd64.S
199 ↗(On Diff #161813)

Sure, this is to emit %rip-relative reference to ___tsan_darwin_setjmp_xor_key. Position-independent code requires that, IMHO.