EricWF added inline comments.
================
Comment at: src/CMakeLists.txt:37
@@ +36,3 @@
+
+remove_flags(-Wl,-z,defs)
+
----------------
compnerd wrote:
> EricWF wrote:
> > 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?
> 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.
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.
================
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:
> EricWF wrote:
> > 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.
> 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.
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
http://reviews.llvm.org/D15440
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
