Page MenuHomePhabricator

[TSan] Add CMake check for minimal SDK requirements on Darwin
ClosedPublic

Authored by yln on Feb 12 2020, 1:08 PM.

Details

Summary

Fails with the following message in the error case:

CMake Error at /path/to/llvm-project/compiler-rt/lib/tsan/CMakeLists.txt:119 (message):
  Building the TSan runtime requires at least macOS SDK 10.12

Fixes #44682.
https://bugs.llvm.org/show_bug.cgi?id=44682

Diff Detail

Event Timeline

yln created this revision.Feb 12 2020, 1:08 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 12 2020, 1:08 PM
Herald added subscribers: llvm-commits, Restricted Project, Charusso, mgorny. · View Herald Transcript
yln added a comment.Feb 17 2020, 9:05 AM

@dmajor: are you okay with this?

dmajor accepted this revision.Feb 17 2020, 1:22 PM
In D74501#1879277, @yln wrote:

@dmajor: are you okay with this?

This patch is fine from my side. I was hoping that someone more involved with this code would do the formal signoff, but it seems nobody is speaking up...

This revision is now accepted and ready to land.Feb 17 2020, 1:22 PM
delcypher requested changes to this revision.Feb 17 2020, 6:45 PM
delcypher added inline comments.
compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
128 ↗(On Diff #244256)

@yln Why are you changing this? This is a different piece of functionality and this verbose-ness is also deliberate. Older versions of CMake do not support VERSION_GREATER_EQUAL and so this change will break the build for older CMake versions.

compiler-rt/lib/tsan/CMakeLists.txt
117

You need to guard this check on osx being present in ${TSAN_SUPPORTED_OS}. It's possible to have compiler-rt not target osx so we shouldn't fatal error when we aren't actually building for macOS.

This revision now requires changes to proceed.Feb 17 2020, 6:45 PM
yln marked 3 inline comments as done.Feb 18 2020, 9:35 AM
yln added inline comments.
compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
128 ↗(On Diff #244256)

Ahh, I didn't realize VERSION_GREATER_EQUAL isn't available in older versions of CMake. Good catch!

compiler-rt/lib/tsan/CMakeLists.txt
117

I would like to fail the build for "macOS SDK < 10.12 or aligned SDK versions on other platforms". Do you know how to best express that?

yln marked an inline comment as done.Feb 18 2020, 9:35 AM
yln updated this revision to Diff 246787.Feb 26 2020, 10:47 AM

Add comment explaining that we use macOS to check aligned SDK versions. Improve error message.

This revision is now accepted and ready to land.Feb 26 2020, 10:53 AM
This revision was automatically updated to reflect the committed changes.