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.
Details
Diff Detail
- Repository
- rCRT Compiler Runtime
Event Timeline
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.
compiler-rt/CMakeLists.txt | ||
---|---|---|
157 | 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 | This is a leftover debugging print, removed. | |
329 | 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? |
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 | --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 | 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 | This is also just a generally good fix. Can you also commit this separately? |
compiler-rt/cmake/config-ix.cmake | ||
---|---|---|
129 |
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
What does happen with the unwind?