This is an archive of the discontinued LLVM Phabricator instance.

[tsan] Filter OS X architectures for unit testing
ClosedPublic

Authored by kubamracek on Nov 12 2015, 1:18 AM.

Details

Summary

The TSan unit test build currently fails if we're also building the iOS parts of compiler-rt, because TSAN_SUPPORTED_ARCH contains ARM64. For unit tests, we need to filter this only to host architecture(s).

Diff Detail

Repository
rL LLVM

Event Timeline

kubamracek updated this revision to Diff 40015.Nov 12 2015, 1:18 AM
kubamracek retitled this revision from to [tsan] Filter OS X architectures for unit testing.
kubamracek updated this object.
kubamracek added reviewers: dvyukov, kcc, samsonov, glider.
glider added inline comments.Nov 12 2015, 1:49 AM
lib/tsan/tests/CMakeLists.txt
38 ↗(On Diff #40015)

I'm not familiar with CMake infrastructure enough but I think darwin_filter_host_archs isn't the right way to say "remove ARM64 from the list".
Can we just explicitly set TSAN_TEST_ARCH to x86_64 on Darwin?

kubamracek added inline comments.Nov 12 2015, 1:56 AM
lib/tsan/tests/CMakeLists.txt
38 ↗(On Diff #40015)

darwin_filter_host_archs is already used for this purpose in asan/tests/ and sanitizer_common/tests/. It's actually doing a little more work, namely keeping both x86_64 and x86_64h in case your system is a Haswell+ machine.

But I'm actually completely fine with just doing set(TSAN_TEST_ARCH x86_64) here, if you still want me to.

glider added inline comments.Nov 12 2015, 2:43 AM
lib/tsan/tests/CMakeLists.txt
38 ↗(On Diff #40015)

Sorry, I cannot comment on whether we want x86_64h or not.
Does that mean that we'll be running the tests twice on the Haswell+ machines?
I think it's good to be consistent with the rest of compiler-rt, but I don't understand why this is done there in the first place.
If you know this is needed, I'm fine with keeping darwin_filter_host_archs

kubamracek added inline comments.Nov 12 2015, 2:49 AM
lib/tsan/tests/CMakeLists.txt
38 ↗(On Diff #40015)

Yes, this is consistent with the rest of compiler-rt. We're already running both lit tests and unit tests of ASan and sanitizer-common for x86_64 and x86_64h, if your system is post-Haswell. That's because we also *build* two slices into the ASan dylib...

$ lipo -info ./lib/clang/3.8.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib 
Architectures in the fat file: ./lib/clang/3.8.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib are: i386 x86_64 x86_64h

...so we want to test them both.

glider accepted this revision.Nov 12 2015, 2:52 AM
glider edited edge metadata.

I see, thank you for explaining!
The change looks good to me.

This revision is now accepted and ready to land.Nov 12 2015, 2:52 AM
This revision was automatically updated to reflect the committed changes.