This is an archive of the discontinued LLVM Phabricator instance.

libcxx: Fix libcxx test on aarch64 with libunwind
ClosedPublic

Authored by zatrazz on Aug 11 2016, 1:00 PM.

Details

Summary

Some tests uses 'long double' to/from conversions and for some targets
they are provided by compiler runtime (either compiler-rt or libgcc).
However when building libcxx with linunwinder current test configuration
at target_info.py do not include the required libraries, as:

not llvm_unwinder:
  [-lgcc_s] [-lgcc] [...] [-lgcc_s] [-lgcc]

llvm_unwinder
  [-lunwind] [-ldl]

This causes some tests build issues with missing symbols on aarch64,
for instance, where 'long double' is a binary float with 128-bits with
mostly of internal operations being provided by software routines.

This patch changes how to define the default linker flags by:

not llvm_unwinder:
  [-lgcc_s] [-lgcc]

llvm_unwinder
  [-lunwind] [-ldl] [-lgcc_s] [-lgcc]

I checked and aarch64 and x86_64 with libcxx and libunwind (with and without
LIBCXXABI_USE_LLVM_UNWINDER).

Diff Detail

Event Timeline

zatrazz updated this revision to Diff 67732.Aug 11 2016, 1:00 PM
zatrazz retitled this revision from to libcxx: Fix libcxx test on aarch64 with libunwind.
zatrazz updated this object.
zatrazz added reviewers: jroelofs, danalbert.
zatrazz added subscribers: rengolin, cfe-commits.

I too noticed this very recently (on our non-bare-metal builds), and wondered why these dependencies are not included by default. Thanks for looking into it. I'll let someone familiar with the details approve.

EricWF added a subscriber: EricWF.

Urg... Not this again. This link order is important. I can't remember *why*, but libgcc has to appear both before and after pthread and libc IIRC. Either way the intention here is to mimic the output of Clangs driver. For example the order Clang generates on Linux is:

// clang++ -### -pthread -stdlib=libc++ -xc++ - < /dev/null
"-lc++" "-lm" "-lgcc_s" "-lgcc" "-lpthread" "-lc" "-lgcc_s" "-lgcc"

What link order does the compiler generate on your platform?

I tried to find why exactly libgcc has to appear before and after but I couldn't get any information from commit history. On my system (aarch64/linux) I am also getting "-lc++" "-lm" "-lgcc_s" "-lgcc" "-lpthread" "-lc" "-lgcc_s" "-lgcc".

I think we can got by the safe side and just add the final libgcc at the end for the soft-fp symbols.

zatrazz updated this revision to Diff 67835.Aug 12 2016, 7:13 AM

What about this version? The only difference is for libunwind libgcc is still included.

jroelofs edited edge metadata.Aug 12 2016, 7:28 AM

Doesn't libgcc_s contain bits of gcc's unwinder?

Yes, but my understaning is the proposed link order will force libc++ to link against _Unwind* symbols from libunwind (this is what I am observing also).

This breaks the ODR... the behavior under those circumstances is undefined.

Yes, although in pratice for shared libraries this is not an issue (at least on Linux with current linker strategies). And I open for suggestion on how to proceed in this case since we have some other options:

  1. Add the required soft-sp implementations when building for Linux (this might happen not only on aarch64, but any other ABI that defines long double using fallback soft-fp)
  2. Remove the possible soft-fp usages on all the tests. However this will lead to possible remove *all* the FP cases if libcxx should be used in a pure soft-fp platform
  3. Only allows the libcxx + linunwind to be built against compiler-rt

Yes, although in pratice for shared libraries this is not an issue (at least on Linux with current linker strategies). And I open for suggestion on how to proceed in this case since we have some other options:

  1. Add the required soft-sp implementations when building for Linux (this might happen not only on aarch64, but any other ABI that defines long double using fallback soft-fp)

Are the softfp symbols you need not contained in libgcc.a?

  1. Remove the possible soft-fp usages on all the tests. However this will lead to possible remove *all* the FP cases if libcxx should be used in a pure soft-fp platform
  2. Only allows the libcxx + linunwind to be built against compiler-rt

Yes, although in pratice for shared libraries this is not an issue (at least on Linux with current linker strategies). And I open for suggestion on how to proceed in this case since we have some other options:

  1. Add the required soft-sp implementations when building for Linux (this might happen not only on aarch64, but any other ABI that defines long double using fallback soft-fp)

Are the softfp symbols you need not contained in libgcc.a?

Good call, I think just using 'lgcc' should be suffice. I will change the patch.

zatrazz updated this revision to Diff 67850.Aug 12 2016, 10:02 AM
zatrazz edited edge metadata.

I think patch should be safe now.

jroelofs accepted this revision.Aug 22 2016, 7:02 AM
jroelofs edited edge metadata.

LGTM

This revision is now accepted and ready to land.Aug 22 2016, 7:02 AM
zatrazz closed this revision.Aug 23 2016, 12:33 PM