Page MenuHomePhabricator

[libcxx] Make libc++.so a linker script by default on most platforms.
ClosedPublic

Authored by EricWF on Oct 14 2015, 1:26 PM.

Details

Summary

This patch turns on LIBCXX_ENABLE_ABI_LINKER_SCRIPT by default whenever LLVM_HAVE_LINK_VERSION_SCRIPT is ON. This turns out to be whenever:

  1. WIN32 is not defined.

2 UNIX is defined.

  1. APPLE is not defined.

While LLVM_HAVE_LINK_VERSION_SCRIPT is meant to reflect exactly what we are asking I think it's close enough.

After committing this patch Linux users will no longer have to use "-lc++abi" explicitly!

Diff Detail

Event Timeline

EricWF updated this revision to Diff 37384.Oct 14 2015, 1:26 PM
EricWF retitled this revision from to [libcxx] Make libc++.so a linker script by default on most platforms..
EricWF updated this object.

‎And does this hardcode the abi and break cxxrt? It's not just me who uses that 

  Original Message  
From: Eric Fiselier
Sent: Thursday, October 15, 2015 03:26
To: eric@efcs.ca; mclow.lists@gmail.com; jonathan@codesourcery.com; danalbert@google.com; compnerd@compnerd.org
Reply To: reviews+D13739+public+524ebe3e6873e8d5@reviews.llvm.org
Cc: emaste@freebsd.org; renato.golin@linaro.org; cbergstrom@pathscale.com; cfe-commits@lists.llvm.org
Subject: [PATCH] D13739: [libcxx] Make libc++.so a linker script by default on most platforms.

EricWF created this revision.
EricWF added reviewers: mclow.lists, jroelofs, danalbert, compnerd.
EricWF added subscribers: cfe-commits, cbergstrom, rengolin, emaste.

This patch turns on LIBCXX_ENABLE_ABI_LINKER_SCRIPT by default whenever LLVM_HAVE_LINK_VERSION_SCRIPT is ON. This turns out to be whenever:

  1. WIN32 is not defined.

2 UNIX is defined.

  1. APPLE is not defined.

While LLVM_HAVE_LINK_VERSION_SCRIPT is meant to reflect exactly what we are asking I think it's close enough.

After committing this patch Linux users will no longer have to use "-lc++abi" explicitly!

http://reviews.llvm.org/D13739

Files:
CMakeLists.txt
cmake/Modules/HandleOutOfTreeLLVM.cmake
docs/BuildingLibcxx.rst
docs/UsingLibcxx.rst

‎And does this hardcode the abi and break cxxrt? It's not just me who uses that 

No it does not hard code the ABI. The linker script is generated based on the name of the ABI library selected at build time. For libcxxrt the linker script would be "INPUT(libc++.so.1 -lcxxrt)".
This shouldn't make it any harder to use libcxxrt.

@cbergstrom After installing libcxxrt I ran the libc++ configuration with -DLIBCXX_CXX_ABI=libcxxrt and everything worked perfectly. It generated libc++.so as INPUT(libc++.so.1 -lcxxrt) and after running it against the test suite everything works great.

jroelofs added inline comments.Oct 14 2015, 2:42 PM
CMakeLists.txt
71

Should probably make this a cmake error, not just have it do something other than what you wanted.

76

s/On/on/ ?

78

DEFAULT_VALUE seems too generic a name for this... or is this a common CMake pattern that I'm unaware of?

cmake/Modules/HandleOutOfTreeLLVM.cmake
116

Where are LLVM_ON_WIN32 and LLVM_ON_UNIX used?

docs/BuildingLibcxx.rst
176

I find this sentence hard to parse. Maybe phrase it the same way you did in the comment:

"ON by default on UNIX platforms other than Apple unless 'LIBCXX_ENABLE_STATIC_ABI_LIBRARY' is ON"

docs/UsingLibcxx.rst
59

Looks like an inconsistent use of " vs ' for '-stdlib=libc++' and "-lc++abi". I don't have good RST-fu, but that seems fishy.

EricWF updated this revision to Diff 37393.Oct 14 2015, 2:56 PM
EricWF marked 5 inline comments as done.

Address review comments.

CMakeLists.txt
71

It already is :-). Theres some logic near line #179.

78

Nope, I was just being lazy with the name because it's not meant to be a global name but instead a scratch variable. I'll fix it anyway.

cmake/Modules/HandleOutOfTreeLLVM.cmake
116

Nowhere yet but they will be used to validate sanitizer flags and other UNIX specific behavior. The whole chunk is copy and pasted from HandleLLVMOptions.cmake

jroelofs accepted this revision.Oct 14 2015, 2:59 PM
jroelofs edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 14 2015, 2:59 PM

Thanks for the review. I'm going to wait a little to see if any objections crop up before committing.

EricWF closed this revision.Oct 15 2015, 3:43 PM
vkalintiris added inline comments.
cmake/Modules/HandleOutOfTreeLLVM.cmake
123–133

I've created the review request D13025, that checks whether version scripts are supported in non-APPLE Unix platforms. As far as I know, LLD doesn't support version scripts at the moment. Would it make sense to check this in a similar fashion for out of tree LLVM?

EricWF added inline comments.Oct 16 2015, 1:35 PM
cmake/Modules/HandleOutOfTreeLLVM.cmake
123–133

Thank's for the heads up. I'm actually using this incorrectly to mean "linker script" and not specifically a version script. After grepping the lld sources it seems that it does support linker scripts with the "INPUT" command in them, but I haven't tested it.

Really I should just change my uses of "LLVM_HAVE_LINK_VERSION_SCRIPT" to "LLVM_UNIX AND NOT APPLE" which is really what I'm trying to say. After doing that I can probably just remove the "LLVM_HAVE_LINK_VERSION_SCRIPT" from libc++ entirely.

vasich added a subscriber: vasich.May 2 2018, 7:34 AM