Page MenuHomePhabricator

[compiler-rt] Fix ASan's Noinst unit tests
ClosedPublic

Authored by kubamracek on Feb 8 2015, 1:28 PM.

Details

Reviewers
samsonov
Summary

I noticed that on OS X, we currently skip all "Noinst" unit tests, see http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA_check/1384/consoleFull and search for "Asan-i386-inline-Noinst-Test" (they're not being run at all). The reason is that in MaybeReexec() we always try to set DYLD_INSERT_LIBRARIES to the module where we find the __asan_init symbol. However, the non-instrumented unit tests link the runtime statically, so this symbol is in the executable itself.

This was probably caused when we removed the "allow_reexec" flag. This patch adds a check into MaybeReexec such that if the module that contains the __asan_init symbol is not a dylib, we will not try to re-execute at all. This should only affect the statically-linked unit tests, all other uses on OS X should use the dynamic library.

Diff Detail

Event Timeline

kubamracek updated this revision to Diff 19554.Feb 8 2015, 1:28 PM
kubamracek retitled this revision from to [compiler-rt] Fix ASan's Noinst unit tests.
kubamracek updated this object.
kubamracek edited the test plan for this revision. (Show Details)
kubamracek added subscribers: Unknown Object (MLST), zaks.anna, glider and 2 others.

I think we should fix the tests instead: build dynamic version of RTAsanTest library on OS X, and link with it dynamically.

I don't think it's a good idea to make a dynamic library that actually isn't a runtime library and use it as a runtime library.
Also, to create this new dynamic library we'll have to expose additional interface functions which the noinst tests are supposed to test.

Maybe we can just add an internal function that disables re-execution?

kubamracek updated this revision to Diff 19682.Feb 10 2015, 8:03 AM

Updating the patch to use an internal function that disable re-execution.

samsonov accepted this revision.Feb 10 2015, 12:05 PM
samsonov edited edge metadata.

LGTM

This revision is now accepted and ready to land.Feb 10 2015, 12:05 PM
kubamracek closed this revision.Feb 10 2015, 12:40 PM

Landed in r228740.