[PATCH] D98023: [clang] Don't default to a specifically shared libunwind on mingw with a g++ driver

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

In D98023#2607944 <https://reviews.llvm.org/D98023#2607944>, @mstorsjo wrote:

> In D98023#2607854 <https://reviews.llvm.org/D98023#2607854>, @MaskRay wrote:
>
>> I have tested the following configurations on x86_64-linux-gnu and I don't find a difference.
>>
>>   CC=/tmp/RelA/bin/clang
>>   CXX=/tmp/RelA/bin/clang++
>>   $CC -c a.c
>>   $CC a.o '-###' &> a/c.raw.txt
>>   $CC a.o --rtlib=compiler-rt --unwindlib=none '-###' &> a/c.rt.none.txt
>>   $CC a.o --rtlib=compiler-rt --unwindlib=libgcc '-###' &> a/c.rt.libgcc.txt
>>   $CC a.o --rtlib=compiler-rt --unwindlib=libunwind '-###' &> a/c.rt.libunwind.txt
>>   $CXX a.o '-###' &> a/cc.raw.txt
>>   $CCX a.o --rtlib=compiler-rt --unwindlib=none '-###' &> a/cc.rt.none.txt
>>   $CCX a.o --rtlib=compiler-rt --unwindlib=libgcc '-###' &> a/cc.rt.libgcc.txt
>>   $CCX a.o --rtlib=compiler-rt --unwindlib=libunwind '-###' &> a/cc.rt.libunwind.txt
>
> That's strange; if I do `clang++ a.o -### -rtlib=compiler-rt -unwindlib=libunwind` I get `--as-needed` around the `-l:libunwind.so` with this patch, which wasn't there before.

Perhaps my tests were faulty... I do not check it again.

>> I'd hope we reduce the usage of `UnspecifiedLibGcc` as it has the tricky `--as-needed` implication. I think the idea is that for C (no exceptions, so no `_Unwind_Resume` at the end of non-catch handlers), `libgcc_s.so.1` is usually unneeded, so `--as-needed` can likely result in one fewer DT_NEEDED... This optimization probably does not have much value.
>
> Tangentially related though, I'm running into a similar issue with linking C, when experimenting with taking `-unwindlib` into use - right now, my toolchain is complete to build C applications once I've built compiler-rt builtins. But if I'm adding `-unwindlib=libunwind`, linking of all C programs fail until I've built libunwind - and the fact that linking of C programs fails messes up the cmake setup for building libunwind. So that essentially forces adding an extra `-unwindlib=none` throughout the bootstrap process until libunwind has been built, which is a bit inelegant. (The alternative, creating a dummy empty `libunwind.a` until the real deal is built isn't very elegant either.)
>
> So due to that, I'm pondering whether it'd make sense to propose a patch to only add the `-lunwind` into the link if actually linking with the g++ driver. I'm sure there's some C programs somewhere that wants to call the unwinder though (maybe for calling `_Unwind_Backtrace`?), which that'd break. (That would be broken in my current toolchains anyway, though.)

Omitting the unwind library can be problematic if the C program does use such symbols. It is also difficult to fix because `-lstdc++` is placed before the unwind library.
So user `-lunwind` will have a wrong linking position.

>> So if you want to add a condition, please test mingw triple instead of testing `RLT == ToolChain::RLT_Libgcc`
>
> Ok, I guess that'd be more straightforward...




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98023/new/

https://reviews.llvm.org/D98023