[libc++abi] Use libgcc and libgcc_s to provide _Unwind symbols instead of libgcc_eh.a

[EricWF]

Summary

libgcc_eh.a cannot be used when building libc++abi as a shared library (the default configuration). See this post for some more discussion: https://gcc.gnu.org/ml/gcc/2012-03/msg00104.html

This patch reverts back to using libgcc_s when linking libc++abi.so.

[compnerd]

cmake/config-ix.cmake

45

Might be nice to extend this further to allow building against clang_rt.builtins. We could of course do that as a follow up if you prefer.

src/CMakeLists.txt

37

Do we need to worry about an alternative spelling of -z defs?

[EricWF]

cmake/config-ix.cmake

45

I don't think the clang_rt.builtin libraries are *ever* along the library search path. Maybe we could detect if clang accepts --rtlib <name>? (I don't know the flag off hand).

src/CMakeLists.txt

37

Not sure. I only considered the spellings used within the LLVM source tree llvm/cmake/modules/HandleLLVMOptions.cmake.

[compnerd]

cmake/config-ix.cmake

45

Pretty close with the flag: it is --rtlib= if you like the 2 dash version. Hmm, though the problem with the symbol we are using for gcc_s here is the crus of the issue I think. The problem is that if clang_rt.builtins is used, __gcc_personality_v0 will be defined since the symbol is the C personality routine. Perhaps a compromise may be to change the symbol we use to detect gcc_s. However, given that this was the original behavior, Im not as concerned, since it has worked for most people so far (and its easy to modify).

src/CMakeLists.txt

37

This is a change in the original behavior. I think it may be safer to add the -z defs and -zdefs spellings as well if you want the no undefined symbols behavior. At least on solaris, I believe that -no-undefined is also going to cause this to be emitted.

62

Why does gcc_s need to be first? Is there some symbol that needs to be resolved from it and not another provider?

[EricWF]

cmake/config-ix.cmake

45

Another problem is that --rtlib=compiler-rt doesn't work when -nodefaultlibs is given. We can try and manually find libclang_rt.builtins-<target>.a but that requires us to 1) find clangs library directory 2) Explicitly deduce the name of the actual library file based on the target triple.

I've done this in libc++ for some more extreme cases but it is not fun.

I would like to support using clang_rt.builtins but we might need to add some compiler support to help us find it.

src/CMakeLists.txt

37

This is a change in the original behavior.

Yes it is. libc++abi.so used to resolve the missing _Unwind symbols in libgcc_eh. However I'm hesitant to use remove_flags more than we need to because it's really dumb. For example calling remove_flags(-pedantic) on "-Wno-pedantic -pedantic-errors -pedantic" will result in the string "-Wno- -errors". For this reason I think its safest to only handle the spelling LLVM uses.

Users shouldn't be passing any spelling of "-Wl,-zdefs" to the libc++abi build. I feel like this is one of those instances of "Doctor it hurts when I do this!".

Has this swayed your opinion at all?

62

Woops. It doesn't need to be first, but it needs to come before the builtin C libraries. In particular libpthread and libc. This is because libc (and maybe pthread) can have their own definitions for some of the _Unwind functions found in libgcc_s. Because we want ld to always choose the version in libgcc_s we need to put it first.

Here is a quote from a bug report I found:

If by some circumstance (use of -Bdirect, -z lazyload, maybe others)
libc.so.1 happens to be searched by ld.so.1 before libgcc_s.so.1 and
some library (e.g. libstdc++.so.6) uses functions both from GCC_3.0
(then resolved from libc.so.1) and others (resolved from libgcc_s.so.1),
crashes result due to mixing those different implementations with
different internal data structures.

http://patchwork.ozlabs.org/patch/312087/

I'm not sure if we will run into any of these cases when building libc++abi but it seems best to mimic what the linux linker does with libgcc_s.

[compnerd]

cmake/config-ix.cmake

45

We should definitely fix that: gcc -print-file-name=libgcc.a should be rewritten to clang -print-file-name=libclang_rt.buitlins-${ARCH}.a and we can use that :-). Ill see if I can fix that.

src/CMakeLists.txt

37

It has, only in a slightly different direction. Why not use --allow-shlib-undefined instead? That way we can insert the flag, which would actually mean that we wouldn't need to filter, and by adding it to the end, we don't need to worry about the flags from LLVM/users.

62

Because they may statically link against gcc_eh, I assume, which when not done with a rebuild of the system, you get this "beautiful" behavior. Ill just throw in an "ugh" and look the other way. However, I don't think that you will be able to inject the link request in the right location since the -lc is inserted by the driver, and the request that you put in will not be hoisted before that -lc. So, you may still end up with cross-library references. Im not sure what the best option here is.

[EricWF]

src/CMakeLists.txt

37

That seems fairly reasonable. I didn't think of using --allow-shlib-undefined since that was already the default behavior. However so long as it actually works it seems like a good solution.

62

libgcc_eh is only provided as a static library for static linking, That's the intent of the library [1][2].

When we are building libc++abi we link with -nodefaultlib so we don't need to worry about the driver messing with the link line.

More importantly, the link line I create here is meant to mimic what clang already does. On linux both GCC and clang will make sure "-lgcc_s" appears after "-lc++" but before "-lc" and "-lpthread" (as well at at the end of the link line).

I think this is needed because libc and libpthread contain their own definitions of the _Unwinding symbols in order to support C++ destructors in pthread_exit and pthread_cancel.
It appears that libc's definition of _Unwind_resume attempts to dispatch to libgcc_s via dlopen. [3]

I suspect this is the reason for the deliberate placement and double linking "-lgcc_s".

I think putting "-lgcc_s" before "-lc" is needed to ensure that libraries like libc++abi resolve their _Unwind symbols in libgcc and not libc.

[1] https://gcc.gnu.org/ml/gcc-patches/2005-02/msg00541.html
[2] https://gcc.gnu.org/ml/gcc/2012-03/msg00104.html
[3] http://osxr.org/glibc/source/sysdeps/gnu/unwind-resume.c?v=glibc-2.18

[EricWF]

src/CMakeLists.txt

37

After looking at documentation it would appear that --allow-shlib-undefined doesn't do what we though.

http://stackoverflow.com/questions/2356168/force-gcc-to-notify-about-undefined-references-in-shared-libraries

[EricWF]

This patch is updated in the following ways:

1. No longer try and remove -Wl,-z,defs from the link flags. I think I was mistaken that this caused an issue.
2. No longer try and link libgcc_s before and after libc. It is only linked after libc now. I don't think this will cause issues when building a shared library.
3. Reorder pthread before libc.

cmake/config-ix.cmake

 check_library_exists(c fopen "" LIBCXXABI_HAS_C_LIB)
 check_library_exists(dl dladdr "" LIBCXXABI_HAS_DL_LIB)
 check_library_exists(pthread pthread_once "" LIBCXXABI_HAS_PTHREAD_LIB)
-check_library_exists(gcc_eh _Unwind_GetRegionStart "" LIBCXXABI_HAS_GCC_EH_LIB)
+check_library_exists(gcc_s __gcc_personality_v0 "" LIBCXXABI_HAS_GCC_S_LIB)
 check_library_exists(c __cxa_thread_atexit_impl ""
   LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)

src/CMakeLists.txt

   add_definitions(-DHAVE___CXA_THREAD_ATEXIT_IMPL)
 endif()
 
-# Generate library list.
+# Generate library list
 set(libraries ${LIBCXXABI_CXX_ABI_LIBRARIES})
-append_if(libraries LIBCXXABI_HAS_C_LIB c)
+
 if (LIBCXXABI_ENABLE_THREADS)
   append_if(libraries LIBCXXABI_HAS_PTHREAD_LIB pthread)
 endif()
 
+append_if(libraries LIBCXXABI_HAS_C_LIB c)
+
 if (LIBCXXABI_USE_LLVM_UNWINDER)
   list(APPEND libraries unwind)
 else()
-  append_if(libraries LIBCXXABI_HAS_GCC_EH_LIB gcc_eh)
+  append_if(libraries LIBCXXABI_HAS_GCC_S_LIB gcc_s)
 endif()
 
 # Setup flags.