Skip to content

[libc][cmake][linux] require new LLVM_LIBC_USE_HOST_KERNEL_HEADERS or LIBC_KERNEL_HEADERS #123820

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

Closed
wants to merge 6 commits into from
Closed

[libc][cmake][linux] require new LLVM_LIBC_USE_HOST_KERNEL_HEADERS or LIBC_KERNEL_HEADERS #123820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
456a4f5
[libc][cmake][linux] require new LLVM_LIBC_USE_HOST_KERNEL_HEADERS or…
nickdesaulniers Jan 21, 2025
d918f85
set LLVM_LIBC_USE_HOST_KERNEL_HEADERS for presubmit tests
nickdesaulniers Jan 21, 2025
e2c259d
drop LLVM_ prefix from option name
nickdesaulniers Jan 29, 2025
76e422c
switch set BOOL to option
nickdesaulniers Jan 29, 2025
d4c49a6
moar docs
nickdesaulniers Jan 29, 2025
db5b2f9
fixup cmake var name and path
nickdesaulniers Jan 29, 2025
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
15 changes: 8 additions & 7 deletions .github/workflows/libc-fullbuild-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
# cpp_compiler: g++
steps:
- uses: actions/checkout@v4

# Libc's build is relatively small comparing with other components of LLVM.
# A fresh fullbuild takes about 190MiB of uncompressed disk space, which can
# be compressed into ~40MiB. Limiting the cache size to 1G should be enough.
Expand All @@ -43,7 +43,7 @@ jobs:
max-size: 1G
key: libc_fullbuild_${{ matrix.c_compiler }}
variant: ${{ matrix.ccache-variant }}

# Notice:
# - MPFR is required by some of the mathlib tests.
# - Debian has a multilib setup, so we need to symlink the asm directory.
Expand All @@ -60,7 +60,7 @@ jobs:
run: |
echo "build-output-dir=${{ github.workspace }}/build" >> "$GITHUB_OUTPUT"
echo "build-install-dir=${{ github.workspace }}/install" >> "$GITHUB_OUTPUT"

# Configure libc fullbuild with scudo.
# Use MinSizeRel to reduce the size of the build.
- name: Configure CMake
Expand All @@ -73,6 +73,7 @@ jobs:
-DCMAKE_CXX_COMPILER_LAUNCHER=${{ matrix.ccache-variant }}
-DCMAKE_INSTALL_PREFIX=${{ steps.strings.outputs.build-install-dir }}
-DLLVM_ENABLE_RUNTIMES="libc;compiler-rt"
-DLIBC_USE_HOST_KERNEL_HEADERS=ON
-DLLVM_LIBC_FULL_BUILD=ON
-DLLVM_LIBC_INCLUDE_SCUDO=ON
-DCOMPILER_RT_BUILD_SCUDO_STANDALONE_WITH_LLVM_LIBC=ON
Expand All @@ -83,14 +84,14 @@ jobs:

- name: Build
run: >
cmake
--build ${{ steps.strings.outputs.build-output-dir }}
cmake
--build ${{ steps.strings.outputs.build-output-dir }}
--parallel
--target install

- name: Test
run: >
cmake
--build ${{ steps.strings.outputs.build-output-dir }}
cmake
--build ${{ steps.strings.outputs.build-output-dir }}
--parallel
--target check-libc
5 changes: 4 additions & 1 deletion libc/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ set(LIBC_BUILD_DIR ${CMAKE_CURRENT_BINARY_DIR})

set(LIBC_ENABLE_USE_BY_CLANG OFF CACHE BOOL "Whether or not to place libc in a build directory findable by a just built clang")

set(LIBC_KERNEL_HEADERS "/usr/include" CACHE STRING "Path to Linux kernel headers")
# LIBC_USE_HOST_KERNEL_HEADERS is brittle, and frowned upon (prefer setting
# LIBC_KERNEL_HEADERS), but is simpler from a hello world linux host build.
option(LIBC_USE_HOST_KERNEL_HEADERS "Add /usr/include to the header seach path" OFF)
set(LIBC_KERNEL_HEADERS "" CACHE STRING "Path to Linux kernel headers")
Copy link
Member Author

Choose a reason for hiding this comment

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

oh, I wonder if we should be using option instead? I see that used elsewhere in our cmake. I'm not familiar with option...

Copy link
Member

Choose a reason for hiding this comment

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

There are two ways to define a boolean flag in CMake:

set(FOO true CACHE BOOL "description")
option(FOO "description" ON)

The difference between the two is that the option command does not create a CACHE variable if a normal variable with the same name exists, but using set with CACHE variable named FOO when FOO exists as a normal variable will yield two copies of FOO, a normal variable and a CACHE one.

Which is one more appropriate really depends on your intention. There are places in LLVM build system where we intentionally rely on the set semantics (e.g. we would use the CACHE variable as the default value to initialize the normal variable).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, so it sounds like cmake has distinct "namespace-esque" differentiation between "normal variables" and "cache variables."

For cmake variables that we encourage folks to use to configure llvm-libc, I think we may want option. That seems very common in libcxx/CMakeLists.txt.

Copy link
Member Author

Choose a reason for hiding this comment

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

76e422c


# Defining a global namespace to enclose all libc functions.
set(default_namespace "__llvm_libc")
Expand Down
21 changes: 21 additions & 0 deletions libc/cmake/modules/LLVMLibCArchitectures.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,27 @@ if (LIBC_TARGET_OS_IS_WINDOWS AND LLVM_LIBC_FULL_BUILD)
message(FATAL_ERROR "Windows does not support full mode build.")
endif ()

if (LIBC_TARGET_OS_IS_LINUX)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to automatically set LLVM_LIBC_USE_HOST_KERNEL_HEADERS for overlay builds?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's worthwhile to consider this. Let me jot down my thoughts.

Doing so would effectively change the implicit default value of LIBC_USE_HOST_KERNEL_HEADERS based on whether we were doing full build (off) vs overlay build (on).

Two invariants I'd like to maintain:

  1. In the spirit of [libc][docs] reorder docs to be more beginner friendly #122376, I would like us to consider overlay builds the "more advanced" of the build configurations relative to full builds (due to the ABI compatibility concerns). So if we require folks doing overlay builds to add one more cmake flag, in this case -DLIBC_USE_HOST_KERNEL_HEADERS=ON, then I don't feel too bad.
  2. We encourage folks targeting linux to just build the kernel headers; it's pretty straightforward. If they can't do this, then forcing them to admit to bad behavior via another cmake var is somewhat part of the intent of this PR. Perhaps that's being malevolent. Implying LIBC_USE_HOST_KERNEL_HEADERS for overlay builds removes the point that folks own up to looking at /usr/include. (Besides, it hurts our hermeticity; once we add /usr/include to the directory search path, we MAY end up including other non-kernel headers since kernel and non-kernel headers are all mixed together in /usr/include. This alone gives me heartburn).

Thinking through the case of wanting to do an overlay build, is there ever a case where we'd want to use newer kernel headers (or perhaps older kernel headers) than what was available on the host? A hypothetical use case for newer headers: you want to build on one host (that has old headers) but support newer syscalls. The result can only run on other machines with newer kernels (when newer syscalls are made at runtime). A hypothetical use case for older headers: you want to build on one host (that has newer headers), but run the libc on another target that is running an older kernel. Both of those hypotheticals seem to be a stretch in my mind. Perhaps overlay mode should (or does) imply "literally the result will only run on my host machine+kernel?"

An inspiration for LIBC_USE_HOST_KERNEL_HEADERS is -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang which was a flag in clang that was necessary to use -ftrivial-auto-var-init= for some time. Basically, it was intended as a way to not outright block people from using -ftrivial-auto-var-init=, but make it such that there was one more hoop for them to jump through to use it.

In the same vein, I don't want to outright prevent people from looking at /usr/include for linux kernel headers (indeed, maybe on the buildbots we'll skip building kernel headers for each run), but I would like to gradually move us away from doing so.

By adding LIBC_USE_HOST_KERNEL_HEADERS, I think that strikes the right balance where folks (and build bots) can transition to LIBC_KERNEL_HEADERS over time, rather than forcing them to do so right now (and not providing any option otherwise).

So I guess, yes, it would make sense to automatically set it. But that betrays an intent in this PR to add one more hoop for folks who refuse to just build the kernel headers from source to sign on the X that they are potentially including more than just kernel headers when they add /usr/include to the compiler's header search path. Is that being too malicious?

Copy link
Contributor

Choose a reason for hiding this comment

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

My main concern is around default configurations. Currently, LLVM_LIBC_FULL_BUILD defaults to off, so overlay is the default. We should probably change that at some point, but that's a separate issue. The important thing is that our cmake shouldn't error if you don't specify any flags.

Currently, if you run cmake ../runtimes -G Ninja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_ENABLE_RUNTIMES="libc" you get an overlay mode build with no allocator. It's not a particularly useful configuration, but it will build and pass our tests. This is important for two reasons:

  1. First impressions are important, a build failure looks bad, and lots of people don't read the docs.
  2. A surprising number of people download the llvm-project repo, set the enabled projects/runtimes to "all", and run make install. They then file bugs when they run into a build error, even if it's for a project they didn't even plan to use. This is annoying and wastes both their time and ours, so I'd like to avoid it if possible.

The other issue is that for overlay mode we do actually need to look at the other libc's headers. All of our type/macro headers in libc/hdr/ use the system's headers in overlay mode, and a couple places (specifically in printf, assert, and a few other places iirc) even call functions from the system's libc. Overlay mode can't be hermetic, and I don't think we can practically make it independent of the system it's targeting.

Copy link
Member Author

Choose a reason for hiding this comment

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

The other issue is that for overlay mode we do actually need to look at the other libc's headers.

D'oh, you're right!

My main concern is around default configurations.

This is something I hadn't even considered. I can get behind wanting an optionless configure to build out of the box. Let me think more about this over lunch.

Currently, LLVM_LIBC_FULL_BUILD defaults to off, so overlay is the default. We should probably change that at some point, but that's a separate issue.

I agree. Filed #124973.

if (NOT LIBC_USE_HOST_KERNEL_HEADERS AND "${LIBC_KERNEL_HEADERS}" STREQUAL "")
message(FATAL_ERROR "MUST specify either LIBC_KERNEL_HEADERS or LIBC_USE_HOST_KERNEL_HEADERS")
endif()
if (LIBC_USE_HOST_KERNEL_HEADERS AND NOT "${LIBC_KERNEL_HEADERS}" STREQUAL "")
message(FATAL_ERROR "LIBC_USE_HOST_KERNEL_HEADERS and LIBC_USE_HOST_KERNEL_HEADERS are mutually exclusive")
endif()
if(LIBC_TARGET_TRIPLE AND LIBC_USE_HOST_KERNEL_HEADERS)
# This is because the syscall numbers are frequently different between
# different target architectures. Code may compile, but you'll get spooky
# runtime failures.
message(FATAL_ERROR "LIBC_USE_HOST_KERNEL_HEADERS should not be set when using LIBC_TARGET_TRIPLE, set LIBC_KERNEL_HEADERS instead")
endif()
if (NOT "${LIBC_KERNEL_HEADERS}" STREQUAL "" AND NOT IS_DIRECTORY "${LIBC_KERNEL_HEADERS}")
message(FATAL_ERROR "LIBC_KERNEL_HEADERS should be a path to an existing directory")
endif()
if (LIBC_USE_HOST_KERNEL_HEADERS)
set(LIBC_KERNEL_HEADERS "/usr/include" CACHE STRING "Path to Linux kernel headers" FORCE)
endif()
endif()


message(STATUS
"Building libc for ${LIBC_TARGET_ARCHITECTURE} on ${LIBC_TARGET_OS} with
Expand Down
7 changes: 7 additions & 0 deletions libc/docs/full_cross_build.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,15 @@ Below is the CMake command to configure the standalone crossbuild of the libc.
$> cd build
$> C_COMPILER=<C compiler> # For example "clang"
$> CXX_COMPILER=<C++ compiler> # For example "clang++"
$> KERNEL_HEADERS=<path/to/built/linux/kernel/headers>
$> cmake ../runtimes \
-G Ninja \
-DLLVM_ENABLE_RUNTIMES=libc \
-DCMAKE_C_COMPILER=$C_COMPILER \
-DCMAKE_CXX_COMPILER=$CXX_COMPILER \
-DLLVM_LIBC_FULL_BUILD=ON \
-DLIBC_TARGET_TRIPLE=<Your target triple> \
-DLIBC_KERNEL_HEADERS=$KERNEL_HEADERS \
-DCMAKE_BUILD_TYPE=<Release|Debug>

We will go over the special options passed to the ``cmake`` command above.
Expand All @@ -77,6 +79,10 @@ We will go over the special options passed to the ``cmake`` command above.
* **The target triple** - This is the target triple of the target for which
we are building the libc. For example, for a Linux 32-bit Arm target,
one can specify it as ``arm-linux-eabi``.
* **The path to the kernel headers** - (Optional) Necessary when targeting
Linux. The Linux kernel headers are architecture specific and should be built
from source. See :ref:`linux_headers` section for how to build the Linux
kernel headers from source.

Build step
----------
Expand Down Expand Up @@ -113,6 +119,7 @@ CMake configure step
-G Ninja \
-DCMAKE_C_COMPILER=$C_COMPILER \
-DCMAKE_CXX_COMPILER=$CXX_COMPILER \
-DLIBC_KERNEL_HEADERS=$KERNEL_HEADERS \
-DLLVM_ENABLE_PROJECTS=clang \
-DLLVM_ENABLE_RUNTIMES=libc \
-DLLVM_LIBC_FULL_BUILD=ON \
Expand Down
16 changes: 13 additions & 3 deletions libc/docs/full_host_build.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ development. In this we've set the Ninja generator, set the build type to
"Debug", and enabled the Scudo allocator. This build also enables generating the
documentation and verbose cmake logging, which are useful development features.

If targeting Linux, see :ref:`linux_headers` for how to build the Linux kernel
headers from source, which llvm-libc will depend on. If not targeting Linux,
the below cmake variable ``LIBC_KERNEL_HEADERS`` should be omitted.

.. note::
if your build fails with an error saying the compiler can't find
``<asm/unistd.h>`` or similar then you're probably missing the symlink from
Expand All @@ -44,11 +48,12 @@ documentation and verbose cmake logging, which are useful development features.
$> cd build
$> cmake ../runtimes \
-G Ninja \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_C_COMPILER=clang \
-DLLVM_ENABLE_RUNTIMES="libc;compiler-rt" \
-DLIBC_KERNEL_HEADERS=/path/to/sysroot/ \
-DLLVM_LIBC_FULL_BUILD=ON \
-DCMAKE_BUILD_TYPE=Debug \
-DLLVM_LIBC_INCLUDE_SCUDO=ON \
-DCOMPILER_RT_BUILD_SCUDO_STANDALONE_WITH_LLVM_LIBC=ON \
-DCOMPILER_RT_BUILD_GWP_ASAN=OFF \
Expand Down Expand Up @@ -131,6 +136,7 @@ allocator for LLVM-libc.
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DLIBC_KERNEL_HEADERS=/path/to/sysroot/ \
-DLLVM_LIBC_FULL_BUILD=ON \
-DLLVM_LIBC_INCLUDE_SCUDO=ON \
-DCOMPILER_RT_BUILD_SCUDO_STANDALONE_WITH_LLVM_LIBC=ON \
Expand Down Expand Up @@ -171,6 +177,8 @@ toolchain with which you can build practical/real-world C applications. See
`<https://github.com/llvm/llvm-project/tree/main/libc/examples>`_ for examples
of how to start using this new toolchain.

.. _linux_headers:

Linux Headers
=============

Expand All @@ -183,7 +191,9 @@ Linux headers in your sysroot. Let's build them from source.
$> make LLVM=1 INSTALL_HDR_PATH=/path/to/sysroot -C /tmp/linux headers_install

The headers can be built to target non-host architectures by adding the
``ARCH={arm|arm64|i386}`` to the above invocation of ``make``.
``ARCH={arm|arm64|i386}`` to the above invocation of ``make``. Then you should
set the cmake variable `-DLIBC_KERNEL_HEADERS=/path/to/sysroot` when
Comment on lines 191 to +195
Copy link
Contributor

Choose a reason for hiding this comment

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

for consistency, would it make sense to use /path/to/kernel/headers/ instead of /path/to/sysroot here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, you're right about the inconsistency. I think I will resolve it the other way, since existing docs talk about setting

$> SYSROOT=/path/to/sysroot # Remember to set this!

above. So I will use /path/to/sysroot/ more consistently and get rid of /path/to/kernel/headers.

Done in db5b2f9.

configuring llvm-libc.

Using your newly built libc
===========================
Expand Down
1 change: 1 addition & 0 deletions libc/docs/getting_started.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Install dependencies first:
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_C_COMPILER=clang \
-DLIBC_USE_HOST_KERNEL_HEADERS=ON \
-DLLVM_LIBC_FULL_BUILD=ON \
-DLLVM_LIBC_INCLUDE_SCUDO=ON \
-DCOMPILER_RT_BUILD_SCUDO_STANDALONE_WITH_LLVM_LIBC=ON \
Expand Down
2 changes: 2 additions & 0 deletions libc/docs/overlay_mode.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ building it with the following cmake command:
$> mkdir build
$> cd build
$> cmake ../runtimes -G Ninja -DLLVM_ENABLE_RUNTIMES="libc" \
-LIBC_USE_HOST_KERNEL_HEADERS=ON \
-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_BUILD_TYPE=<Debug|Release> \ # Select build type
-DCMAKE_INSTALL_PREFIX=<Your prefix of choice> # Optional
Expand Down Expand Up @@ -78,6 +79,7 @@ performance possible.

$> cmake ../llvm -G Ninja -DLLVM_ENABLE_PROJECTS="clang" \
-DLLVM_ENABLE_RUNTIMES="libc" \ # libc is listed as runtime and not as a project
-DLIBC_USE_HOST_KERNEL_HEADERS=ON \
-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_BUILD_TYPE=<Debug|Release> \ # Select build type
-DCMAKE_INSTALL_PREFIX=<Your prefix of choice> # Optional
Expand Down
Loading