Page MenuHomePhabricator

[sanitizer] On OS X, verify that interceptors work and abort if not
ClosedPublic

Authored by kubamracek on Mar 12 2016, 10:53 AM.

Details

Summary

On OS X 10.11+, we have "automatic interceptors", so we don't need to use DYLD_INSERT_LIBRARIES when launching instrumented programs. However, non-instrumented programs that load TSan late (e.g. via dlopen) are currently broken, as TSan will still try to initialize, but the program will crash/hang at random places (because the interceptors don't work). This patch adds an explicit check that interceptors are working, and if not, it aborts and prints out an error message suggesting to explicitly use DYLD_INSERT_LIBRARIES.

Diff Detail

Repository
rL LLVM

Event Timeline

kubamracek retitled this revision from to [sanitizer] On OS X, verify that interceptors work and abort if not.
kubamracek updated this object.
kubamracek added reviewers: samsonov, kcc, glider, dvyukov.
kubamracek added subscribers: llvm-commits, zaks.anna.

Thanks for adding the check this is very useful!

Looks like this would work for all sanitizers, including ASan. Is that correct?

Thanks for adding the check this is very useful!

Looks like this would work for all sanitizers, including ASan. Is that correct?

Yes, this should work for all sanitizers.

filcab added a subscriber: filcab.Mar 12 2016, 9:58 PM
filcab added inline comments.
lib/sanitizer_common/sanitizer_mac.cc
636 ↗(On Diff #50527)

Wouldn't if (info.dli_fbase != info_pthread_create.dli_fbase) work as well? Seems easier to read.

test/tsan/Darwin/dlopen.cc
12 ↗(On Diff #50527)

I really think this would be better in the lit.cfg files (and then dealt with via REQUIRES/UNSUPPORTED), instead of having tests depend on more and more available executables/specific shells.
Having said that, I don't see it as a very big problem because it's a Darwin-specific test (Still hinders to do that, anyway :-) )

kubamracek added inline comments.Mar 12 2016, 11:45 PM
lib/sanitizer_common/sanitizer_mac.cc
636 ↗(On Diff #50527)

It seems to work, but I don't think it's guaranteed by the API. Since we're doing this once only, it's not a big deal performance-wise, and I think readability is fine here – we *are* actually comparing two strings.

glider accepted this revision.Mar 14 2016, 2:16 AM
glider edited edge metadata.

LGTM

test/tsan/Darwin/dlopen.cc
29 ↗(On Diff #50527)

I'm curious how does the dynamic linker behave in the presence of another main().
Maybe rename it to foo() just to be sure?

This revision is now accepted and ready to land.Mar 14 2016, 2:16 AM
kubamracek updated this revision to Diff 50571.Mar 14 2016, 3:34 AM
kubamracek edited edge metadata.

Changed the test to detect OS X version in lit.cfg instead, and to produce a regular shared library instead of two executables with two main functions.

I'm curious how does the dynamic linker behave in the presence of another main().
Maybe rename it to foo() just to be sure?

I believe this is supposed to work fine because of OS X two-level namespaces, and because I'm performing the dlopen/dladdr lookup in a single module (so there is no clash between the symbols). Anyway, I changed the test to generate a regular shared library with a foo symbol.

kubamracek requested a review of this revision.Mar 14 2016, 3:34 AM
kubamracek edited edge metadata.
samsonov accepted this revision.Mar 14 2016, 10:30 AM
samsonov edited edge metadata.

LGTM

This revision is now accepted and ready to land.Mar 14 2016, 10:30 AM
This revision was automatically updated to reflect the committed changes.

I had to revert this and make more changes, but I can't reopen this revision, so I instead submitted a new patch to http://reviews.llvm.org/D18212.