Page MenuHomePhabricator

[libc++abi] Add LIBCXXABI_ENABLE_PIC cmake option
ClosedPublic

Authored by sbc100 on Mar 31 2019, 12:32 PM.

Details

Summary

This is on by default, since on some platforms and configurations
libc++abi.a gets statically linked into shared libraries and/or
PIE executables.

This change is a followup to https://reviews.llvm.org/D60005 which
allows us to default to PIC code, but disable this if needed (for
example on WebAssembly where PIC code its currently incompatible with
static linking).

Diff Detail

Event Timeline

sbc100 created this revision.Mar 31 2019, 12:32 PM
sbc100 retitled this revision from Add LIBCXXABI_ENABLE_PIC cmake option to [libc++abi]Add LIBCXXABI_ENABLE_PIC cmake option.Mar 31 2019, 12:33 PM
sbc100 added reviewers: jiangyi, phosek.
sbc100 retitled this revision from [libc++abi]Add LIBCXXABI_ENABLE_PIC cmake option to [libc++abi] Add LIBCXXABI_ENABLE_PIC cmake option.Mar 31 2019, 12:33 PM
jiangyi accepted this revision.Apr 1 2019, 4:54 AM

Looks good to me.

This revision is now accepted and ready to land.Apr 1 2019, 4:54 AM
ldionne requested changes to this revision.Apr 1 2019, 7:48 AM
ldionne added a subscriber: ldionne.

I would much rather let people use CMAKE_POSITION_INDEPENDENT_CODE instead. Why is that not a viable approach?

This revision now requires changes to proceed.Apr 1 2019, 7:48 AM
sbc100 added a comment.Apr 1 2019, 9:45 AM

I would much rather let people use CMAKE_POSITION_INDEPENDENT_CODE instead. Why is that not a viable approach?

I think the reason is that we want more fine grained control here. My understanding is that CMAKE_POSITION_INDEPENDENT_CODE effects all the targets in the entire cmake build which in the case of llvm might include llvm, clang, lld, libc++, libc++abi, etc. i could be wrong though.

Also we already have LLVM_ENABLE_PIC option for the wider project so this naming made sense. @phosek might have more background here?

I would much rather let people use CMAKE_POSITION_INDEPENDENT_CODE instead. Why is that not a viable approach?

I think the reason is that we want more fine grained control here. My understanding is that CMAKE_POSITION_INDEPENDENT_CODE effects all the targets in the entire cmake build which in the case of llvm might include llvm, clang, lld, libc++, libc++abi, etc. i could be wrong though.

Yes, it does. I'm not a big fan of duplicating logic that CMake already has builtin, but OK. However, why doesn't this control whether -fPIC is being used for the shared library AND the static library?

sbc100 added a comment.Apr 1 2019, 2:24 PM

My understanding is that POSITION_INDEPENDENT_CODE should always be on when building shared libraries. What would it mean to build a non-PIC shared library?

The motivation for this change is allow me to disable PIC code in non-shared libraries. For WebAssembly we currently disable shared libraries so increasing the reach of this option wouldn't break us, but I'm not sure I see any reason to do that.

sbc100 edited the summary of this revision. (Show Details)Apr 1 2019, 2:25 PM
brzycki added a subscriber: brzycki.EditedApr 2 2019, 10:03 AM

Hello @sbc100, I'm seeing a build break caused by D60005 when I attempt to do the following:

  1. compile llvm for aarch64 with a gnu toolchain sysroot
  2. use the freshly built llvm aarch64 compiler to build libcxxabi aarch64 before building libcxx aarch64

I see the following ninja error when attempting to create the libc++abi.so.1.0. I do not see this issue with the immediately previous SHA commit of 5f0c4c67bbff53.

I also tried applying the patch in this ticket to tip and recompiled and I still see the same error.

Please let me know if you need further build details.

cmake -G Ninja -D CMAKE_INSTALL_PREFIX=/work/b.rzycki/4/aarch64-sarc-linux-gnu/sysroot -D LLVM_PATH=/tmp/tmp.N01uFPofsv/src/llvm -D LIBCXXABI_LIBCXX_PATH=/tmp/tmp.N01uFPofsv/src/libcxx -D CMAKE_C_FLAGS= -D CMAKE_CXX_FLAGS= /tmp/tmp.N01uFPofsv/src/libcxxabi
Re-run cmake no build system arguments
-- The CXX compiler identification is Clang 9.0.0
-- The C compiler identification is Clang 9.0.0
-- Check for working CXX compiler: /work/b.rzycki/4/bin/aarch64-linux-clang++
-- Check for working CXX compiler: /work/b.rzycki/4/bin/aarch64-linux-clang++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Check for working C compiler: /work/b.rzycki/4/bin/aarch64-linux-clang
-- Check for working C compiler: /work/b.rzycki/4/bin/aarch64-linux-clang -- works
...

ninja -v -j 64
...
FAILED: lib/libc++abi.so.1.0
: && /work/b.rzycki/4/bin/aarch64-linux-clang++ -fPIC   -nodefaultlibs -shared -Wl,-soname,libc++abi.so.1 -o lib/libc++abi.so.1.0 src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_demangle.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_exception_storage.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_guard.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_handlers.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_unexpected.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_vector.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_virtual.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/stdlib_exception.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/stdlib_stdexcept.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/stdlib_typeinfo.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/abort_message.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/fallback_malloc.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/private_typeinfo.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/stdlib_new_delete.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_exception.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_personality.cpp.o src/CMakeFiles/cxxabi_shared_objects.dir/cxa_thread_atexit.cpp.o  -Wl,-rpath,"\$ORIGIN/../lib" -lpthread -lc -lgcc_s && :
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `__cxa_unexpected_handler' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: in function `std::set_unexpected(void (*)())':
cxa_default_handlers.cpp:(.text+0x18): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `__cxa_terminate_handler' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: in function `std::set_terminate(void (*)())':
cxa_default_handlers.cpp:(.text+0x234): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `_ZTISt9exception' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: in function `demangling_terminate_handler()':
cxa_default_handlers.cpp:(.text+0x2c0): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `__cxa_terminate_handler' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: in function `__cxx_global_var_init':
cxa_default_handlers.cpp:(.text.startup+0x8): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `__cxa_unexpected_handler' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_default_handlers.cpp.o: in function `__cxx_global_var_init.1':
cxa_default_handlers.cpp:(.text.startup+0x24): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `_ZTISt8bad_cast' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o: in function `__cxa_bad_cast':
cxa_aux_runtime.cpp:(.text+0x10): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `_ZNSt8bad_castD1Ev' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: cxa_aux_runtime.cpp:(.text+0x18): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `_ZTISt10bad_typeid' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o: in function `__cxa_bad_typeid':
cxa_aux_runtime.cpp:(.text+0x54): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `_ZNSt10bad_typeidD1Ev' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: cxa_aux_runtime.cpp:(.text+0x5c): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `_ZTISt20bad_array_new_length' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o: in function `__cxa_throw_bad_array_new_length':
cxa_aux_runtime.cpp:(.text+0x98): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_aux_runtime.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `_ZNSt20bad_array_new_lengthD1Ev' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: cxa_aux_runtime.cpp:(.text+0xa0): dangerous relocation: unsupported relocation
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_demangle.cpp.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `stderr@@GLIBC_2.17' which may bind externally can not be used when making a shared object; recompile with -fPIC
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: src/CMakeFiles/cxxabi_shared_objects.dir/cxa_demangle.cpp.o(.text+0x17c8): unresolvable R_AARCH64_ADR_PREL_PG_HI21 relocation against symbol `stderr@@GLIBC_2.17'
/work/b.rzycki/4/bin/aarch64-sarc-linux-gnu-ld: final link failed: bad value
clang-9: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.
sbc100 added a comment.EditedApr 2 2019, 3:16 PM

Oops. Sorry that is a different issue. Fixed in https://reviews.llvm.org/D60158

Hello @sbc100, I'm seeing a build break caused by D60005 when I attempt to do the following:

  1. compile llvm for aarch64 with a gnu toolchain sysroot
  2. use the freshly built llvm aarch64 compiler to build libcxxabi aarch64 before building libcxx aarch64

My understanding is that POSITION_INDEPENDENT_CODE should always be on when building shared libraries. What would it mean to build a non-PIC shared library?

The motivation for this change is allow me to disable PIC code in non-shared libraries. For WebAssembly we currently disable shared libraries so increasing the reach of this option wouldn't break us, but I'm not sure I see any reason to do that.

For example, Apple's libc++.dylib is not built with -fPIC. I believe this is because of the shared linker cache but I should investigate TBH. But regardless, it does make sense not to force shared libraries to be built with -fPIC if the user doesn't want that.

However, there's a twist here. We're using an object library to create the shared library so the CMake default for enabling -fPIC (on for shared libraries, off for static libraries) is not going to trigger, and the shared library will never be built with -fPIC. We need a bit of refactoring for this to be possible.

sbc100 updated this revision to Diff 193383.Apr 2 2019, 3:52 PM
  • control pic for shared library too
sbc100 added a comment.Apr 2 2019, 3:54 PM

OK I made LIBCXXABI_ENABLE_PIC effect both shared and non-shared versions of the library.

OK I made LIBCXXABI_ENABLE_PIC effect both shared and non-shared versions of the library.

Let's wait for https://reviews.llvm.org/D60166 to land, and then you can rebase on top of it. This is going to fix the issue I mentioned.

libcxxabi/src/CMakeLists.txt
183–184

This doesn't work because it's an object library. Now libc++abi.dylib will never be built with -fPIC.

sbc100 added a comment.Apr 2 2019, 5:01 PM

OK I made LIBCXXABI_ENABLE_PIC effect both shared and non-shared versions of the library.

Let's wait for https://reviews.llvm.org/D60166 to land, and then you can rebase on top of it. This is going to fix the issue I mentioned.

OK I made LIBCXXABI_ENABLE_PIC effect both shared and non-shared versions of the library.

Let's wait for https://reviews.llvm.org/D60166 to land, and then you can rebase on top of it. This is going to fix the issue I mentioned.

IIUC this change will still be required with way since we want to default to using PIC in the static library. In which case if you don't mind I'd rather land mine first since its smaller and ready to go. In other words I don't see any reason to block this change on your refactor, unless I'm missing something.

sbc100 added a comment.EditedApr 2 2019, 5:04 PM

Let me re-phrase that. I feel like I broke stuff in https://reviews.llvm.org/D60005 and I want to get it fixed. Since the current state is broken I'd rather not refactor without first with fixing (via this change) or reverting https://reviews.llvm.org/D60005.

sbc100 marked an inline comment as done.Apr 2 2019, 5:12 PM
sbc100 added inline comments.
libcxxabi/src/CMakeLists.txt
183–184

I have another minor fix to this line as a separate change: https://reviews.llvm.org/D60158. It does seem seem to work and builds the library code with fPIC.

ldionne accepted this revision.Apr 2 2019, 5:20 PM
ldionne added inline comments.
libcxxabi/src/CMakeLists.txt
183–184

Hmm yes, indeed my last comment doesn't make sense since you explicitly set POSITION_INDEPENDENT_CODE ON on the object library for the shared library. So this will work and my refactoring is not necessary prior to this change. We can land this and then I'll rebase my refactoring on top.

This revision is now accepted and ready to land.Apr 2 2019, 5:20 PM
sbc100 updated this revision to Diff 193401.Apr 2 2019, 5:32 PM
  • rebase
This revision was automatically updated to reflect the committed changes.