Page MenuHomePhabricator

[CMake] Detect the compiler runtime and standard library
ClosedPublic

Authored by phosek on May 14 2018, 4:39 PM.

Details

Summary

Rather then requiring the user to specify runtime the compiler
runtime and C++ standard library, or trying to guess them which is
error-prone, use auto-detection by parsing the compiler link output.

Diff Detail

Event Timeline

phosek created this revision.May 14 2018, 4:39 PM
Herald added subscribers: Restricted Project, llvm-commits, mgorny. · View Herald TranscriptMay 14 2018, 4:39 PM
dim added subscribers: emaste, dim.May 14 2018, 4:56 PM

Do we still leave the defaults for particular OSes here? I'm asking because for example in FreeBSD, we use libcxxrt, but you *won't* find it in linker output, since our libc++.so is a linker script:

$ clang++ /usr/local/share/cmake/Modules/DummyCXXFile.cxx -###
FreeBSD clang version 6.0.0 (tags/RELEASE_600/final 326565) (based on LLVM 6.0.0)
Target: x86_64-unknown-freebsd11.2
Thread model: posix
InstalledDir: /usr/bin
 "/usr/bin/clang++" "-cc1" "-triple" "x86_64-unknown-freebsd11.2" "-emit-obj" "-mrelax-all" "-disable-free" "-disable-llvm-verifier" "-discard-value-names" "-main-file-name" "DummyCXXFile.cxx" "-mrelocation-model" "static" "-mthread-model" "posix" "-mdisable-fp-elim" "-masm-verbose" "-mconstructor-aliases" "-munwind-tables" "-target-cpu" "x86-64" "-dwarf-column-info" "-debugger-tuning=gdb" "-resource-dir" "/usr/lib/clang/6.0.0" "-internal-isystem" "/usr/include/c++/v1" "-fdeprecated-macro" "-fdebug-compilation-dir" "/home/dim/src/llvm/trunk" "-ferror-limit" "19" "-fmessage-length" "160" "-fobjc-runtime=gnustep" "-fcxx-exceptions" "-fexceptions" "-fdiagnostics-show-option" "-fcolor-diagnostics" "-o" "/tmp/DummyCXXFile-5ffab3.o" "-x" "c++" "/usr/local/share/cmake/Modules/DummyCXXFile.cxx"
 "/usr/bin/ld" "--eh-frame-hdr" "-dynamic-linker" "/libexec/ld-elf.so.1" "--hash-style=both" "--enable-new-dtags" "-o" "a.out" "/usr/lib/crt1.o" "/usr/lib/crti.o" "/usr/lib/crtbegin.o" "-L/usr/lib" "/tmp/DummyCXXFile-5ffab3.o" "-lc++" "-lm" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "/usr/lib/crtend.o" "/usr/lib/crtn.o"

E.g., you see -lc++, and our /usr/lib/libc++.so contains:

$ cat /usr/lib/libc++.so
/* $FreeBSD: stable/11/lib/libc++/libc++.ldscript 253917 2013-08-03 16:23:43Z dim $ */
GROUP ( /usr/lib/libc++.so.1 /usr/lib/libcxxrt.so )

So what would check_link_libraries find in this case?

check_link_libraries would find c++ and set -lc++ as SANITIZER_CXX_ABI_LIBRARY which is presumably what it does today since I don't see libcxxrt in the CXXABIS list, so there should be no functional difference. The autodetection logic is only used when SANITIZER_CXX_ABI is set to "default", so you can still override it through CMake same as today.

One of the positive side-effects of this change is that it also avoids passing unnecessary --rtlib= and --stdlib= flags which eliminates spurious warnings in Fuchsia build that forced us to disable -Werror, this would affect any platform that uses non-default --rtlib= or --stdlib= values.

vitalybuka added inline comments.May 15 2018, 5:35 PM
compiler-rt/CMakeLists.txt
157 ↗(On Diff #146721)

What does happen with the unwind?

compiler-rt/cmake/Modules/CompilerRTUtils.cmake
155 ↗(On Diff #146721)

Should this message be in some if() statement?

329 ↗(On Diff #146721)

why not to use try_compile?

354 ↗(On Diff #146721)

Should this variable names be upper case?

phosek updated this revision to Diff 146977.May 15 2018, 6:07 PM
phosek marked 3 inline comments as done.
phosek added inline comments.
compiler-rt/CMakeLists.txt
157 ↗(On Diff #146721)

That's a great question, this is relying on libc++ being a linker script that pulls in all the additional dependencies such as libunwind which is how libc++ is normally installed on platforms that support linker scripts, except for macOS where they use different mechanism but this should still work there. It'd only break on platforms where libunwind has to be linked explicitly, but those are probably broken in current setup as well and I don't know how to easily handle those.

compiler-rt/cmake/Modules/CompilerRTUtils.cmake
155 ↗(On Diff #146721)

This is a leftover debugging print, removed.

329 ↗(On Diff #146721)

In this case I'm only using --print-libgcc-file-name to find out what the runtime is so we don't need to compile anything. I could possibly completely drop this and just use check_link_libraries below and then see if output contains either libclang_rt.builtins or libgcc, the downside is that I'd need to iterate over all libraries in that list and try to MATCH each one of them against the regex which might be slower than actually invoking the compiler again. WDYT?

beanz accepted this revision.May 17 2018, 9:52 AM

This all looks reasonable to me. I had a few small comments below. There are a few isolated bits of code cleanup that you included in this patch, and I'd prefer to see those land separately.

compiler-rt/cmake/Modules/CompilerRTUtils.cmake
329 ↗(On Diff #146721)

--print-libgcc-file-name is broken on Darwin (it returns an incorrect result). I suspect that this is actually alright because you're not actually using the file, but we should probably look into fixing that.

compiler-rt/cmake/Modules/HandleCompilerRT.cmake
1 ↗(On Diff #146977)

This just looks like some generally good code cleanup. Can you put it in a separate patch? No review necessary.

compiler-rt/cmake/config-ix.cmake
129 ↗(On Diff #146977)

This is also just a generally good fix. Can you also commit this separately?

This revision is now accepted and ready to land.May 17 2018, 9:52 AM
vitalybuka accepted this revision.May 17 2018, 11:05 AM

please move unrelated change into separate patches

phosek marked an inline comment as done.May 17 2018, 2:30 PM
phosek added inline comments.
compiler-rt/cmake/config-ix.cmake
129 ↗(On Diff #146977)
phosek updated this revision to Diff 147399.May 17 2018, 3:16 PM
This revision was automatically updated to reflect the committed changes.
rnk added a subscriber: rnk.May 18 2018, 11:43 AM

This is causing warnings on Windows:

-- Targeting WebAssembly
clang-cl.exe: warning: unknown argument ignored in clang-cl: '--print-libgcc-file-name' [-Wunknown-argument]
clang-cl.exe: error: no input files
-- Compiler-RT supported architectures: x86_64

The fact that you are blindly passing --print-libgcc-file-name suggests that this code doesn't work with MSVC or any other compiler that has a non-GCC command line interface. Things might work, but I'd prefer it if it was relanded after proactively considering those cases, rather than patching a new pile of conditionals.

Looks like MSVC also warns:
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/28806/steps/cmake/logs/stdio

-- Targeting XCore
DummyCXXFile.cxx
Microsoft (R) Incremental Linker Version 14.00.24215.1
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:DummyCXXFile.exe 
DummyCXXFile.obj 
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24215.1 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

cl : Command line warning D9002 : ignoring unknown option '--print-libgcc-file-name'
cl : Command line error D8003 : missing source filename