This is an archive of the discontinued LLVM Phabricator instance.

[libc++] add linker option "-Wl,-z,defs" in standalone build
ClosedPublic

Authored by zlei on Aug 31 2016, 8:20 PM.

Details

Summary

This option is used to avoid underlinking. It's available in LLVM's main tree, but not in a standalone build of libc++. This patch ensures the option is passed in whether libc++ is built in-tree or out-of-tree.

Diff Detail

Repository
rL LLVM

Event Timeline

zlei updated this revision to Diff 69944.Aug 31 2016, 8:20 PM
zlei retitled this revision from to [libc++] add linker option "-Wl,-z,defs" in standalone build.
zlei updated this object.
zlei added a reviewer: cfe-commits.
zlei edited reviewers, added: EricWF; removed: cfe-commits.Aug 31 2016, 11:04 PM
zlei added a subscriber: cfe-commits.
EricWF accepted this revision.Sep 26 2016, 7:16 PM
EricWF edited edge metadata.

LGTM. Thanks for the patch.

This revision is now accepted and ready to land.Sep 26 2016, 7:16 PM
mgorny added a subscriber: mgorny.Sep 26 2016, 11:54 PM
This revision was automatically updated to reflect the committed changes.
rmaprath added inline comments.
libcxx/trunk/CMakeLists.txt
329

Perhaps we should exclude LIBCXX_HAS_EXTERNAL_THREAD_API as well? Because there we explicitly strip off these flags.

zlei added inline comments.Sep 27 2016, 7:24 AM
libcxx/trunk/CMakeLists.txt
329

I agree. Could you prepare a patch for it?

BTW, I really don't like duplicating code snippet from llvm. Is there any better solution for that?

rmaprath added inline comments.Sep 27 2016, 7:48 AM
libcxx/trunk/CMakeLists.txt
329

Sure, I'll do a patch.

Btw, I think this configuration is better placed in HandleOutOfTreeLLVM.cmake module.

I'll see if I can find a better way to do this.

mgorny reopened this revision.Sep 27 2016, 11:56 AM

I'm starting to regret that I've committed this. It breaks horribly any pure-LLVM build, i.e. without linking to libgcc_s. It seems that the build system is completely unprepared to link to compiler-rt or libunwind when linking the shared library, and with -DLIBCXX_HAS_GCC_S_LIB=OFF, it fails due to a lot of missing symbols. I am going to revert this until we get the build system working completely.

This revision is now accepted and ready to land.Sep 27 2016, 11:56 AM

I'm starting to regret that I've committed this. It breaks horribly any pure-LLVM build, i.e. without linking to libgcc_s. It seems that the build system is completely unprepared to link to compiler-rt or libunwind when linking the shared library, and with -DLIBCXX_HAS_GCC_S_LIB=OFF, it fails due to a lot of missing symbols. I am going to revert this until we get the build system working completely.

Doesn't -DLIBCXXABI_USE_LLVM_UNWINDER=ON make it possible to build without libgcc_s? Or is this something else?

Doesn't -DLIBCXXABI_USE_LLVM_UNWINDER=ON make it possible to build without libgcc_s? Or is this something else?

It's supposed to but it doesn't add the necessary libraries when linking libc++.so.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2019, 6:45 AM