EricWF added inline comments.
================
Comment at: cmake/config-ix.cmake:45
@@ -44,3 +44,3 @@
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 ""
----------------
compnerd wrote:
> EricWF wrote:
> > compnerd wrote:
> > > 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.
> > 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).
> 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).
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.
================
Comment at: src/CMakeLists.txt:37
@@ +36,3 @@
+
+remove_flags(-Wl,-z,defs)
+
----------------
compnerd wrote:
> EricWF wrote:
> > compnerd wrote:
> > > Do we need to worry about an alternative spelling of `-z defs`?
> > Not sure. I only considered the spellings used within the LLVM source tree
> > `llvm/cmake/modules/HandleLLVMOptions.cmake`.
> 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.
> 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?
================
Comment at: src/CMakeLists.txt:62
@@ -44,1 +61,3 @@
+# first and last library on the link line. This behavior mimics clang's
+# behavior.
set(libraries ${LIBCXXABI_CXX_ABI_LIBRARIES})
----------------
compnerd wrote:
> Why does gcc_s need to be first? Is there some symbol that needs to be
> resolved from it and not another provider?
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.
http://reviews.llvm.org/D15440
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
