Page MenuHomePhabricator

[TSan] Register threads created via pthread_create_from_mach_thread()

Authored by yln on Jan 10 2020, 6:04 PM.



On Darwin, TSan was crashing on the first call into the runtime from
any thread created via pthread_create_from_mach_thread() SPI. These
threads were uninitialized, because TSan could not observe their
creation. This meant that TSan crashed for Xcode's SwiftUI preview
(which uses remote code injection).

Threads created via pthread_create_from_mach_thread() bypass the
pthread_create() interceptor / __tsan_thread_start_func() thread
function wrapper that usually ensures that threads are properly
registered. In addition, we don't receive THREAD_CREATE/DESTROY events
in our pthread introspection hooks. The reason for this is that those
would run in the context of the parent mach thread which makes them not
very useful.

Adding an interceptor for this SPI is not sufficient; it is typically
looked up in the parent process and then marshalled into the
instrumented process.

This change uses the following strategy to register these threads. When
observing the THREAD_CREATE event, we attach enough information to the
thread that is being created (using the new
pthread_introspection_setspecific_np() API) so that in THREAD_START
we can identify how the thread that we are on was actually created. For
regular threads and GCD workers there will be identifying information,
for "special" threads (threads for which we are unable to observe
creation, e.g., threads created via pthread_create_from_mach_thread()
from a Mach thread) the absence of this information lets us infer that
they are special.

If pthread_introspection_setspecific_np() is not available (running on
older OS), we fallback to the previous behavior: registering GCD threads
during THREAD_CREATE and doing nothing for THREAD_START.


Diff Detail

Event Timeline

yln created this revision.Jan 10 2020, 6:04 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 10 2020, 6:04 PM
Herald added subscribers: llvm-commits, Restricted Project, jfb. · View Herald Transcript
MadCoder accepted this revision.Jan 11 2020, 10:48 AM
MadCoder added inline comments.

nit: it should be void * not const void * ;) but it really doesn't matter for you

This revision is now accepted and ready to land.Jan 11 2020, 10:48 AM
yln marked 2 inline comments as done.Jan 13 2020, 9:55 AM
yln added inline comments.

Thanks! Fixed.

yln updated this revision to Diff 237719.Jan 13 2020, 10:03 AM
yln marked an inline comment as done.

Fixed signature of pthread_setspecific_introspection_np and added
CHECK_EQ(res, 0); to ensure the call to it succeeds.

yln updated this revision to Diff 336990.Apr 12 2021, 4:35 PM

Replace usage of magic constants with actual API.

yln retitled this revision from [TSan] Register threads created via pthread_create_from_mach_thread to [TSan] Register threads created via pthread_create_from_mach_thread().Apr 12 2021, 4:36 PM
yln edited the summary of this revision. (Show Details)
yln added a reviewer: aralisza.
yln updated this revision to Diff 336992.Apr 12 2021, 4:56 PM

Skipping test when running on old OS.

dvyukov accepted this revision.Apr 13 2021, 11:42 PM
yln updated this revision to Diff 339069.Apr 20 2021, 6:35 PM

Remove default: UNREACHABLE("unknown event"); switch case to avoid crashes in case enum gets extended in the future.

kubamracek accepted this revision.Apr 20 2021, 8:31 PM

LGTM, thanks!