Page MenuHomePhabricator

[Driver] Make -static-libgcc imply static libunwind
ClosedPublic

Authored by jkz on Nov 18 2019, 6:21 PM.

Details

Summary

In the GNU toolchain, -static-libgcc implies that the unwindlib will be linked statically. However, when --unwindlib=libunwind, this flag is ignored, and a bare -lunwind is added to the linker args. Unfortunately, this means that if both libunwind.so, and libunwind.a are present in the library path, libunwind.so will be chosen in all cases where -static is not set.

This change makes -static-libgcc affect the -l flag produced by --unwindlib=libunwind. After this patch, providing -static-libgcc --unwindlib=libunwind will cause the driver to explicitly emit -l:libunwind.a to statically link libunwind. For all other cases it will emit -l:libunwind.so matching current behavior with a more explicit link line.

Diff Detail

Event Timeline

jkz created this revision.Nov 18 2019, 6:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 18 2019, 6:21 PM
joerg added a subscriber: joerg.Nov 18 2019, 9:56 PM

This is normally done by using -Bstatic/-Bdynamic around the library. See tools::addOpenMPRuntime.

This is normally done by using -Bstatic/-Bdynamic around the library. See tools::addOpenMPRuntime.

Historically, the unwind library is a little different. Gcc uses -llibgcc_eh for static links (which is only resolved by libgcc_eh.a) and libgcc_s.so for dynamic links (which is only resolved by libgcc_s.so, which in turn pulls in libgcc.a for certain things).

sivachandra accepted this revision.Nov 19 2019, 9:20 AM

Please wait for saugustine's acceptance.

This revision is now accepted and ready to land.Nov 19 2019, 9:20 AM
jkz added a comment.Nov 20 2019, 12:31 PM

This is normally done by using -Bstatic/-Bdynamic around the library. See tools::addOpenMPRuntime.

-Bstatic/-Bdynamic wrapping does seem to be more common, but that will change the linkage for libraries that are added after the run-time libraries (e.g., -lpthread). We would need something like --push-state -Bstatic -lunwind --pop-state to preserve the "mode" of the link. (IMO, the existing usages of -Bstatic, -Bdynamic wrapping are wrong). The current approach is simpler.

Additionally, I think saugustine's reasoning makes sense here, and we should match the libgcc behavior of requiring a .a or .so depending on the "mode". I'd prefer to keep this patch the way it is.

saugustine accepted this revision.Nov 22 2019, 10:41 AM
MaskRay closed this revision.Apr 15 2020, 11:29 AM
MaskRay added a subscriber: MaskRay.

Closed by 6551ac7489fe070a2edcfac88f68c93321cba9a9

The commit does not have Differential Revision: ... so the differential is not closed automatically.

This fails to account for whether you actually have the shared and/or static version of the library.

clang/lib/Driver/ToolChains/CommonArgs.cpp
1215

If you are building with -DLIBUNWIND_ENABLE_SHARED:OFF -DLIBUNWIND_ENABLE_STATIC:ON, there's no shared version available. And vice versa. This doesn't account for that.

saugustine added inline comments.Apr 23 2020, 4:29 PM
clang/lib/Driver/ToolChains/CommonArgs.cpp
1215

Although true, I don't believe this is any different from the parallel libgcc situation. libgcc_eh is the static version, and will never be satisfied by the dynamic version. libgcc_s is the dynamic version, and can never satisfy the static version. See lines 1204-1208 above.

The case where you ask for a static compiler-rt but still want a dynamic unwind library is new. I'm not opposed to supporting it, but "what gcc does" isn't a terrible standard.

I only build libunwind as a static library because it's (so far) the approach that gets me the furthest in avoiding all of the trouble that comes from having multiple libunwind (on Linux). I'm also running with the patch from D26244 (now abandoned unfortunately).

This approach works quite well. It doesn't solve everything tough.

I can use the (default) static version of the sanitizer runtimes as long as the project being built doesn't itself use another libuwnind. If it does, I can use the shared sanitizer runtimes since they are linked against the static LLVM libunwind and therefore self-contained. Obviously, this only works for sanitiziers that have shared runtimes. Those that don't can't be used in this case. Well, as far as I've been able to work out at least.

I planned on experimenting with merging libunwind into the shared version of libc++abi (i.e. the whole libunwind archive) to see if that could solve it, but haven't gotten around to testing it yet.

clang/lib/Driver/ToolChains/CommonArgs.cpp
1215

I haven't really requested anything specific one way or the other (I think).

I'm guessing getLibGccType() returns LibGccType::SharedLibGcc because CCCIsCXX() returns true.

Even if it returned LibGccType::UnspecifiedLibGcc, the shared version would still be used though.