This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Add an explicit option to build against system-libcxxabi
ClosedPublic

Authored by mgorny on Feb 11 2022, 3:59 AM.

Details

Summary

Add an explicit LIBCXX_CXX_ABI=system-libcxxabi option for linking to
system-installed libc++abi. This fixes the ability to link against one
when building libcxx via the runtimes build, as otherwise the build
system insists on linking into in-tree targets.

Diff Detail

Event Timeline

mgorny requested review of this revision.Feb 11 2022, 3:59 AM
mgorny created this revision.
ldionne added inline comments.Feb 14 2022, 3:15 PM
libcxx/cmake/Modules/HandleLibCXXABI.cmake
113

I think this doesn't work because cxxabi_shared and cxxabi_static might not be known by the time we get here. What is the behavior you'd like to see here? Are you attempting to link against the system libc++abi.so?

mgorny added inline comments.Feb 14 2022, 3:39 PM
libcxx/cmake/Modules/HandleLibCXXABI.cmake
113

Yes, exactly that.

ldionne added inline comments.Feb 17 2022, 6:43 AM
libcxx/cmake/Modules/HandleLibCXXABI.cmake
113

I think what we want instead is to have another choice for the ABI library, something like system-libcxxabi. When that ABI library is selected, we'd add -lc++abi and the right include path.

mgorny added inline comments.Feb 17 2022, 11:12 AM
libcxx/cmake/Modules/HandleLibCXXABI.cmake
113

Ok, I'll update the patch in a few hours.

mgorny updated this revision to Diff 409728.Feb 17 2022, 11:28 AM
mgorny retitled this revision from [libcxx] [cmake] Fix linking to installed libc++abi in runtimes build to [libcxx] Add an explicit option to build against system-libcxxabi.
mgorny edited the summary of this revision. (Show Details)

Add another LIBCXX_CXX_ABI variant instead, as requested.

Ping. I'd like to try to backport the fixes to 14.x, so would be nice to get it sorted out before it goes out.

ldionne accepted this revision.Mar 1 2022, 6:47 AM

Sorry, I went OOO. I've actually been working on a more holistic refactoring of this, but I think this patch is fine as a first step. I'll get my refactoring out soon.

This revision is now accepted and ready to land.Mar 1 2022, 6:47 AM

My proposed change is up as https://reviews.llvm.org/D120727.

Let's land this one and cherry-pick to release/14.x. Then, I can rebase my D120727 on it.

Could you push it for me, please? My PC died yesterday and I won't have it fixed in the next few days.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2022, 10:45 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Done, also cherry-picked onto release/14.x