"clang++ hello.cc --rtlib=compiler-rt"
now works without specifying additional unwind or exception
handling libraries.
"clang++ hello.cc --rtlib=compiler-rt"
now works without specifying additional unwind or exception
handling libraries.
Buildable 27330 | |
Build 27329: arc lint + arc unit |
Looks good to me, thanks! Let's try this and see if it shakes out any problems.
We really need to give LLVM's unwinder a better name.
clang/include/clang/Driver/Options.td | ||
---|---|---|
2572 | I think libgcc_eh here should be just libgcc, and compiler-rt-unwind should be just compiler-rt, right? |
clang/include/clang/Driver/Options.td | ||
---|---|---|
2572 | Good catch. My original design had their own names, but in my own testing I found it awkward so switched out and forgot to update these. |
This has broken all our toolchain builders (https://luci-milo.appspot.com/p/fuchsia/g/clang/console). These are using the runtimes build https://github.com/llvm/llvm-project/blob/master/llvm/runtimes/CMakeLists.txt which can be used to build compiler runtimes using the just-built Clang. The way that works is runtimes build first builds builtins for all specified targets (by invoking builtins build directly in https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/builtins/CMakeLists.txt), and then as second step builds remaining runtimes (libunwind, libc++abi, libc++, and the rest of compiler-rt). Now, with this change this breaks the second phase because where before Clang has been only looking for compiler-rt builtins (which have been built in the first phase) now Clang is also looking for -luwind which hasn't been built yet. There's actually a cycle because CMake checks ran by libunwind build are looking for libunwind (I see a lot of ld.lld: error: unable to find library -lunwind errors in the CMake log).
I think we'll probably need to update the CMake build to pass -nostdlib when running the checks in llvm/cmake/modules/HandleLLVMOptions.cmake (that's invoked from https://github.com/llvm/llvm-project/blob/master/llvm/runtimes/CMakeLists.txt#L117). However, it'll take a me while to prepare that change and test it, is there any chance we could revert this in the meantime?
One more thing, shouldn't libunwind be only included for C++ (since it's the internal dependency of libc++abi)? There shouldn't be any need to include it for C.
There's some dark magic done by glibc and thread cancellation, in absence of libgcc_s I think it may weakly link against symbols that would be provided by any unwinder library. I think it not being exposed may break glibc if using thread cancellation (not sure if that's used by a lot) without libgcc_s and without libc++abi loaded (to pull in the unwinder).
I think most just call it libunwind_llvm?
FWIW, this also broke my bootstraps of mingw-w64 environments/toolchains. After building compiler-rt builtins, before having any libunwind/libcxx built, I previously regarded my toolchain as complete for building and testing C apps and libraries, but that fails now.
Would it be possible to add a third alternative, --unwindlib=none, to signal that while I'm using --rtlib=compiler-rt, I don't want to link to any unwinder? (In my case, I'm injecting libunwind in libc++.a so it only gets added when linking C++ code.) Or at least make it possible to only add this linker flag when linking C++? Alternatively I'll need to provide a dummy libunwind.a until the real one has been built.
+1 on this, although consolidated libc++/libc++abi/libunwind_llvm have been supported "experimentally" for at least two years now, if this somehow breaks it, I think a lot of users would be frustrated.
(Also is it time to make merging libc++.a and libc++abi.a not-an-experimental (in CMake) feature anymore?).
+2 this is how we're shipping static libunwind in our (Fuchsia) toolchain for all platforms, we merge it (together with libc++abi.a) into libc++.a, so beyond the bootstrap issues, this change is going to break our build unless we provide dummy libunwind.a. Ideally there should be a vendor flag (e.g. a CMake option) to set this as default when building Clang (akin to CLANG_DEFAULT_CXX_STDLIB and CLANG_DEFAULT_RTLIB), having to manually always set --unwindlib=none for every compiler invocation is going to be incredibly annoying.
We're having issues with this as well, we don't have any libunwind so this change causes
/usr/bin/ld: cannot find -lunwind
Sorry for the breakage everyone, I'm preparing a rollback now.
I think this shows that everyone using rtlib had their own method of getting the unwind symbols. We ought to figure out a standardized method.
glibc's pthread cancel does assume that the unwind symbols would be available. The --as-needed flags should prevent it from being a hard dependency in plain C, but that doesn't solve the "no file present" problem.
The biggest issue with setting it separately is that would require a second command-line argument every time. Need to think about how to handle that case.