Skip to content

[libc][libcxx] Support for building libc++ against LLVM libc #99287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 19, 2024
Merged

[libc][libcxx] Support for building libc++ against LLVM libc #99287

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7d7a788
[libc][libcxx] Support for building libc++ against LLVM libc
petrhosek Jul 17, 2024
76e5e02
Introduce the LIBCXX_LIBC option
petrhosek Jul 17, 2024
c6ca8cf
Review feedback
petrhosek Jul 18, 2024
1418ea8
Review feedback
petrhosek Jul 18, 2024
a27eaa4
Remove unnecessary changes
petrhosek Jul 18, 2024
1777997
Change dependency visibility
petrhosek Jul 18, 2024
9f90269
Update formatting
petrhosek Jul 18, 2024
0cafc79
Merge remote-tracking branch 'origin/main' into libcxx-use-libc
petrhosek Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/cmake/caches/Fuchsia-stage2.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ foreach(target armv6m-unknown-eabi;armv7m-unknown-eabi;armv8m.main-unknown-eabi)
set(RUNTIMES_${target}_LIBCXX_CXX_ABI none CACHE STRING "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC ON CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_USE_LLVM_LIBC ON CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_LOCALIZATION OFF CACHE BOOL "")
Expand Down Expand Up @@ -388,6 +389,7 @@ foreach(target riscv32-unknown-elf)
set(RUNTIMES_${target}_LIBCXX_CXX_ABI none CACHE STRING "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC ON CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_USE_LLVM_LIBC ON CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_RANDOM_DEVICE OFF CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_ENABLE_LOCALIZATION OFF CACHE BOOL "")
Expand Down
3 changes: 3 additions & 0 deletions libc/cmake/modules/CheckCompilerFeatures.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,6 @@ check_cxx_compiler_flag("-nostdlib++" LIBC_CC_SUPPORTS_NOSTDLIBPP)

# clang-3.0+
check_cxx_compiler_flag("-nostdlibinc" LIBC_CC_SUPPORTS_NOSTDLIBINC)

# clang-3.0+
check_cxx_compiler_flag("-nolibc" LIBC_CC_SUPPORTS_NOLIBC)
3 changes: 3 additions & 0 deletions libc/include/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,9 @@ get_all_install_header_targets(all_install_header_targets ${TARGET_PUBLIC_HEADER
add_library(libc-headers INTERFACE)
add_dependencies(libc-headers ${all_install_header_targets})
target_include_directories(libc-headers SYSTEM INTERFACE ${LIBC_INCLUDE_DIR})
if(LIBC_CC_SUPPORTS_NOSTDLIBINC)
target_compile_options(libc-headers INTERFACE "-nostdlibinc")
endif()

foreach(target IN LISTS all_install_header_targets)
get_target_property(header_file ${target} HEADER_FILE_PATH)
Expand Down
5 changes: 4 additions & 1 deletion libc/lib/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ foreach(archive IN ZIP_LISTS
ARCHIVE_OUTPUT_NAME ${archive_0}
)
if(LLVM_LIBC_FULL_BUILD)
target_link_libraries(${archive_1} PUBLIC libc-headers)
target_link_libraries(${archive_1} INTERFACE libc-headers)
if(LIBC_CC_SUPPORTS_NOLIBC)
target_link_options(${archive_1} INTERFACE "-nolibc")
endif()
if(TARGET libc-startup)
add_dependencies(${archive_1} libc-startup)
endif()
Expand Down
1 change: 1 addition & 0 deletions libcxx/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ option(LIBCXX_ENABLE_THREADS "Build libc++ with support for threads." ON)
option(LIBCXX_ENABLE_MONOTONIC_CLOCK
"Build libc++ with support for a monotonic clock.
This option may only be set to OFF when LIBCXX_ENABLE_THREADS=OFF." ON)
option(LIBCXX_USE_LLVM_LIBC "Build libc++ against LLVM libc." OFF)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to avoid introducing a CMake setting like this. Instead, we should do something similar to the way we handle ABI choices and provide a pseudo-target that represents the libc we're building against. We can then have a multi-valued option like LIBCXX_LIBC="llvm-libc|system-libc|musl|etc...".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my original thought, but I don't think the different C libraries have enough granularity for this to be anything but a binary flag in practice. What this flag truly means is whether or not to use the LLVM libc built in-tree or not. It's basically the exact same use-case as the already present LIBCXXABI_USE_LLVM_UNWINDER. If someone installed the LLVM libc globally, then the existing logic would just pick it up as the libc without this flag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benefit of that approach is that we can define how to build against arbitrary new C stdlibs very easily, and we avoid adding more logic than there already is in the CMakeLists.txt. I don't mind if we start with only two configurations for this flag, what matters to me is that we use the right approach to introduce this new functionality.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I don't think that config easily exposes what we're trying to express, since you could say llvm-libc and it wouldn't disambiguate between llvm-libc installed on the system globally and llvm-libc that exists as a target inside the current CMake project. So, we'd still need a flag in that case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a solution to the same problem for selecting the ABI library: set(LIBCXX_SUPPORTED_ABI_LIBRARIES none libcxxabi system-libcxxabi libcxxrt libstdc++ libsupc++ vcruntime).

There's libcxxabi (in-tree) and system-libcxxabi (whatever libc++abi is installed on the system).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, then we should have system system-llvm-libc llvm-libc, where system is the current default (Just use whatever you find in the default include path).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how system-llvm-libc would differ from system, but yeah that sounds about right.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary for now, was just thinking how if we started adding new ones like musl or glibc then we'd need to disambiguate. Right now we probably just want system as the default and llvm-libc which mirrors this patch's binary selection well enough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced LIBCXX_LIBC which currently accepts system or llvm-libc. system is currently no-op but that can be expanded later. In the future we may also want to introduce additional options e.g. musl replacing the existing LIBCXX_HAS_MUSL_LIBC option but I'd prefer to do it in a separate change.

I'm not sure if it's better to set the -nostdlibinc and -nolibc on the libc side (as interface flags on the targets) or on the libc++ side (as interface flags on the proxy targets), I can see an argument for doing either way. Right now I set those on the libc side which has the upside of not needing to duplicate those flags in libunwind and libc++abi which should eventually support LLVM libc as well.

In the future we'll probably want to move HandleLibC.cmake to the top-level cmake/Modules directory so they can be shared by libc++abi and libunwind, although we'll need to rethink the target and variables names because the LIBCXX_ and libcxx- prefix is undesirable (maybe we can use RUNTIMES_ and runtimes-)?

option(LIBCXX_HAS_MUSL_LIBC "Build libc++ with support for the Musl C library" OFF)
option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)
option(LIBCXX_HAS_WIN32_THREAD_API "Ignore auto-detection and force use of win32 thread API" OFF)
Expand Down
3 changes: 3 additions & 0 deletions libcxx/include/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,9 @@ add_custom_target(generate-cxx-headers ALL DEPENDS ${_all_includes})

add_library(cxx-headers INTERFACE)
target_link_libraries(cxx-headers INTERFACE libcxx-abi-headers)
if (LIBCXX_USE_LLVM_LIBC)
target_link_libraries(cxx-headers INTERFACE libc-headers)
endif()
add_dependencies(cxx-headers generate-cxx-headers)
# It's important that the arch directory be included first so that its header files
# which interpose on the default include dir be included instead of the default ones.
Expand Down
10 changes: 10 additions & 0 deletions libcxx/src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ if (LIBCXX_ENABLE_SHARED)
target_link_libraries(cxx_shared PUBLIC libcxx-abi-shared)
endif()

# Link against LLVM libc.
if (LIBCXX_USE_LLVM_LIBC)
target_link_libraries(cxx_shared PUBLIC libc libm)
endif()

# Maybe re-export symbols from libc++abi
# In particular, we don't re-export the symbols if libc++abi is merged statically
# into libc++ because in that case there's no dylib to re-export from.
Expand Down Expand Up @@ -324,6 +329,11 @@ if (LIBCXX_ENABLE_STATIC)
if (LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY)
target_link_libraries(cxx_static PRIVATE libcxx-abi-static-objects)
endif()

# Link against LLVM libc.
if (LIBCXX_USE_LLVM_LIBC)
target_link_libraries(cxx_static PUBLIC libc libm)
endif()
endif()

# Add a meta-target for both libraries.
Expand Down
Loading