Add --unwindlib=[libgcc|compiler-rt] to parallel --rtlib=

[saugustine]

Summary

"clang++ hello.cc --rtlib=compiler-rt"

now works without specifying additional unwind or exception
handling libraries.

[rsmith]

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?

[saugustine]

• Correct help text for --unwindlib options.

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.

[phosek]

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?

[phosek]

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.

[kristina]

In D57128#1374928, @phosek wrote:

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).

In D57128#1370712, @rsmith wrote:

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.

I think most just call it libunwind_llvm?

[mstorsjo]

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.

[kristina]

In D57128#1375040, @mstorsjo wrote:

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?).

[phosek]

In D57128#1375049, @kristina wrote:

In D57128#1375040, @mstorsjo wrote:

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.

[uabelho]

We're having issues with this as well, we don't have any libunwind so this change causes
/usr/bin/ld: cannot find -lunwind

[saugustine]

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.

[saugustine]

This was rolled back with r352524. I'll work out something better.