Skip to content

Commit 0cf9c77

Browse files
nickdesaulniersnickdesaulniers
committed
[RFC][libc] Codify header inclusion policy
This RFC is meant to spur discussion; it's not yet clear if this policy is best (or even makes sense). When supporting "overlay" vs "fullbuild" modes, "what ABI are you using?" becomes a fundamental question to have concrete answers for. Overlay mode MUST match the ABI of the system being overlayed onto; fullbuild more flexible (the only system ABI relevant is the OS kernel). When implementing llvm-libc we generally prefer the include-what-you use style of avoiding transitive dependencies (since that makes refactoring headers more painful, and slows down build times). So what header do you include for any given type or function declaration? For any given userspace program, the answer is straightforward. But for llvm-libc which is trying to support multiple ABIs (at least one per configuration), the answer is perhaps less clear. This proposal seeks to add one layer of indirection relative to what's being done today. It then converts users of sigset_t and struct epoll_event over to this convention as an example.
1 parent 7789ec0 commit 0cf9c77

31 files changed

+189
-75
lines changed

‎libc/cmake/modules/LLVMLibCCompileOptionRules.cmake

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ function(_get_common_compile_options output_var flags)
4343
list(APPEND compile_options "-fpie")
4444

4545
if(LLVM_LIBC_FULL_BUILD)
46+
list(APPEND compile_options "-DLIBC_FULL_BUILD")
4647
# Only add -ffreestanding flag in full build mode.
4748
list(APPEND compile_options "-ffreestanding")
4849
endif()
@@ -117,6 +118,7 @@ function(_get_common_test_compile_options output_var c_test flags)
117118
list(APPEND compile_options "-fpie")
118119

119120
if(LLVM_LIBC_FULL_BUILD)
121+
list(APPEND compile_options "-DLIBC_FULL_BUILD")
120122
# Only add -ffreestanding flag in full build mode.
121123
list(APPEND compile_options "-ffreestanding")
122124
list(APPEND compile_options "-fno-exceptions")

‎libc/config/linux/x86_64/entrypoints.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ set(TARGET_LIBC_ENTRYPOINTS
219219
libc.src.sys.epoll.epoll_pwait
220220
# TODO: Need to check if pwait2 is available before providing.
221221
# https://github.com/llvm/llvm-project/issues/80060
222-
# libc.src.sys.epoll.epoll_pwait2
222+
libc.src.sys.epoll.epoll_pwait2
223223

224224
# sys/mman.h entrypoints
225225
libc.src.sys.mman.madvise

‎libc/docs/dev/code_style.rst

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,30 @@ We expect contributions to be free of warnings from the `minimum supported
186186
compiler versions`__ (and newer).
187187

188188
.. __: https://libc.llvm.org/compiler_support.html#minimum-supported-versions
189+
190+
Header Inclusion Policy
191+
=======================
192+
193+
Because llvm-libc supports
194+
`Overlay Mode <https://libc.llvm.org/overlay_mode.html>`__ and
195+
`Fullbuild Mode <https://libc.llvm.org/fullbuild_mode.html>`__ care must be
196+
taken when ``#include``'ing certain headers.
197+
198+
The ``include/`` directory contains public facing headers that users must
199+
consume for fullbuild mode. As such, types defined here will have ABI
200+
implications as these definitions may differ from the underlying system for
201+
overlay mode and are NEVER appropriate to include in ``libc/src/`` without
202+
preprocessor guards for ``LLVM_LIBC_FULL_BUILD``.
203+
204+
Consider the case where an implementation in ``libc/src/`` may wish to refer to
205+
a ``sigset_t``, what header should be included? ``<signal.h>``, ``<spawn.h>``,
206+
``<sys/select.h>``?
207+
208+
None of the above. Instead, code under ``src/`` should ``#include
209+
"src/signal/sigset_t.h"`` which contains preprocessor guards on
210+
``LLVM_LIBC_FULL_BUILD`` to either include the public type (fullbuild mode) or
211+
the underlying system header (overlay mode).
212+
213+
Implementations in ``libc/src/`` should NOT be ``#include``'ing using ``<>`` or
214+
``"include/*``, except for these "proxy" headers that first check for
215+
``LLVM_LIBC_FULL_BUILD``.

‎libc/docs/usage_modes.rst

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ The libc can used in two different modes:
66

77
#. The **overlay** mode: In this mode, the link order semantics are exploited
88
to overlay implementations from LLVM's libc over the system libc. See
9-
:ref:`overlay_mode` for more information about this mode.
9+
:ref:`overlay_mode` for more information about this mode. In this mode, libc
10+
uses the ABI of the system it's being overlayed onto. Headers are NOT
11+
generated. libllvmlibc.a is the only build artifact.
1012
#. The **fullbuild** mode: In this mode, LLVM's libc is used as the only libc
1113
for the binary. See :ref:`fullbuild_mode` for information about this mode.
14+
In this mode, libc uses its own ABI. Headers are generated along with a
15+
libc.a.

‎libc/src/signal/CMakeLists.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,12 @@ add_entrypoint_object(
7171
DEPENDS
7272
.${LIBC_TARGET_OS}.sigdelset
7373
)
74+
75+
add_header_library(
76+
sigset_t
77+
HDRS
78+
sigset_t.h
79+
DEPENDS
80+
# TODO: this dependency is only for fullbuild mode...
81+
libc.include.llvm-libc-types.sigset_t
82+
)

‎libc/src/signal/linux/CMakeLists.txt

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ add_header_library(
33
HDRS
44
signal_utils.h
55
DEPENDS
6+
libc.include.signal
67
libc.include.sys_syscall
78
libc.src.__support.OSUtil.osutil
9+
libc.src.signal.sigset_t
810
)
911

1012
add_entrypoint_object(
@@ -28,9 +30,9 @@ add_entrypoint_object(
2830
../raise.h
2931
DEPENDS
3032
.signal_utils
31-
libc.include.signal
3233
libc.include.sys_syscall
3334
libc.src.__support.OSUtil.osutil
35+
libc.src.signal.sigset_t
3436
)
3537

3638
add_object_library(
@@ -57,10 +59,10 @@ add_entrypoint_object(
5759
../sigaction.h
5860
DEPENDS
5961
.__restore
60-
libc.include.signal
6162
libc.include.sys_syscall
6263
libc.src.__support.OSUtil.osutil
6364
libc.src.errno.errno
65+
libc.src.signal.sigset_t
6466
)
6567

6668
add_entrypoint_object(
@@ -84,10 +86,10 @@ add_entrypoint_object(
8486
../sigprocmask.h
8587
DEPENDS
8688
.signal_utils
87-
libc.include.signal
8889
libc.include.sys_syscall
8990
libc.src.__support.OSUtil.osutil
9091
libc.src.errno.errno
92+
libc.src.signal.sigset_t
9193
)
9294

9395
add_entrypoint_object(
@@ -98,8 +100,8 @@ add_entrypoint_object(
98100
../sigemptyset.h
99101
DEPENDS
100102
.signal_utils
101-
libc.include.signal
102103
libc.src.errno.errno
104+
libc.src.signal.sigset_t
103105
)
104106

105107
add_entrypoint_object(
@@ -110,8 +112,8 @@ add_entrypoint_object(
110112
../sigaddset.h
111113
DEPENDS
112114
.signal_utils
113-
libc.include.signal
114115
libc.src.errno.errno
116+
libc.src.signal.sigset_t
115117
)
116118

117119
add_entrypoint_object(
@@ -133,8 +135,8 @@ add_entrypoint_object(
133135
../sigfillset.h
134136
DEPENDS
135137
.signal_utils
136-
libc.include.signal
137138
libc.src.errno.errno
139+
libc.src.signal.sigset_t
138140
)
139141

140142
add_entrypoint_object(
@@ -145,6 +147,6 @@ add_entrypoint_object(
145147
../sigdelset.h
146148
DEPENDS
147149
.signal_utils
148-
libc.include.signal
149150
libc.src.errno.errno
151+
libc.src.signal.sigset_t
150152
)

‎libc/src/signal/linux/raise.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "src/signal/raise.h"
10-
#include "src/signal/linux/signal_utils.h"
1110

1211
#include "src/__support/common.h"
12+
#include "src/signal/linux/signal_utils.h"
13+
#include "src/signal/sigset_t.h"
1314

1415
namespace LIBC_NAMESPACE {
1516

1617
LLVM_LIBC_FUNCTION(int, raise, (int sig)) {
17-
::sigset_t sigset;
18+
sigset_t sigset;
1819
block_all_signals(sigset);
1920
long pid = LIBC_NAMESPACE::syscall_impl<long>(SYS_getpid);
2021
long tid = LIBC_NAMESPACE::syscall_impl<long>(SYS_gettid);

‎libc/src/signal/linux/sigaction.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "src/signal/sigaction.h"
10-
#include "src/errno/libc_errno.h"
11-
#include "src/signal/linux/signal_utils.h"
1210

1311
#include "src/__support/common.h"
14-
15-
#include <signal.h>
12+
#include "src/errno/libc_errno.h"
13+
#include "src/signal/linux/signal_utils.h"
14+
#include "src/signal/sigset_t.h"
1615

1716
namespace LIBC_NAMESPACE {
1817

‎libc/src/signal/linux/sigaddset.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "src/signal/sigaddset.h"
10+
1011
#include "src/__support/common.h"
1112
#include "src/errno/libc_errno.h"
1213
#include "src/signal/linux/signal_utils.h"
13-
14-
#include <signal.h>
14+
#include "src/signal/sigset_t.h"
1515

1616
namespace LIBC_NAMESPACE {
1717

‎libc/src/signal/linux/sigdelset.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "src/signal/sigdelset.h"
10+
1011
#include "src/__support/common.h"
1112
#include "src/errno/libc_errno.h"
1213
#include "src/signal/linux/signal_utils.h"
13-
14-
#include <signal.h>
14+
#include "src/signal/sigset_t.h"
1515

1616
namespace LIBC_NAMESPACE {
1717

‎libc/src/signal/linux/sigfillset.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "src/signal/sigfillset.h"
10+
1011
#include "src/__support/common.h"
1112
#include "src/errno/libc_errno.h"
1213
#include "src/signal/linux/signal_utils.h"
13-
14-
#include <signal.h>
14+
#include "src/signal/sigset_t.h"
1515

1616
namespace LIBC_NAMESPACE {
1717

‎libc/src/signal/linux/signal_utils.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111

1212
#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
1313
#include "src/__support/common.h"
14+
#include "src/signal/sigset_t.h"
1415

15-
#include <signal.h>
16+
#include <signal.h> // sigaction
1617
#include <stddef.h>
1718
#include <sys/syscall.h> // For syscall numbers.
1819

‎libc/src/signal/linux/sigprocmask.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "src/signal/sigprocmask.h"
10+
1011
#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
12+
#include "src/__support/common.h"
1113
#include "src/errno/libc_errno.h"
1214
#include "src/signal/linux/signal_utils.h"
15+
#include "src/signal/sigset_t.h"
1316

14-
#include "src/__support/common.h"
15-
16-
#include <signal.h>
1717
#include <sys/syscall.h> // For syscall numbers.
1818

1919
namespace LIBC_NAMESPACE {

‎libc/src/signal/sigaddset.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#ifndef LLVM_LIBC_SRC_SIGNAL_SIGADDSET_H
1010
#define LLVM_LIBC_SRC_SIGNAL_SIGADDSET_H
1111

12-
#include <signal.h>
12+
#include "src/signal/sigset_t.h"
1313

1414
namespace LIBC_NAMESPACE {
1515

‎libc/src/signal/sigdelset.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#ifndef LLVM_LIBC_SRC_SIGNAL_SIGDELSET_H
1010
#define LLVM_LIBC_SRC_SIGNAL_SIGDELSET_H
1111

12-
#include <signal.h>
12+
#include "src/signal/sigset_t.h"
1313

1414
namespace LIBC_NAMESPACE {
1515

‎libc/src/signal/sigemptyset.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#ifndef LLVM_LIBC_SRC_SIGNAL_SIGEMPTYSET_H
1010
#define LLVM_LIBC_SRC_SIGNAL_SIGEMPTYSET_H
1111

12-
#include <signal.h>
12+
#include "src/signal/sigset_t.h"
1313

1414
namespace LIBC_NAMESPACE {
1515

‎libc/src/signal/sigfillset.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#ifndef LLVM_LIBC_SRC_SIGNAL_SIGFILLSET_H
1010
#define LLVM_LIBC_SRC_SIGNAL_SIGFILLSET_H
1111

12-
#include <signal.h>
12+
#include "src/signal/sigset_t.h"
1313

1414
namespace LIBC_NAMESPACE {
1515

‎libc/src/signal/sigprocmask.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#ifndef LLVM_LIBC_SRC_SIGNAL_SIGPROCMASK_H
1010
#define LLVM_LIBC_SRC_SIGNAL_SIGPROCMASK_H
1111

12-
#include <signal.h>
12+
#include "src/signal/sigset_t.h"
1313

1414
namespace LIBC_NAMESPACE {
1515

‎libc/src/signal/sigset_t.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// TODO: license block
2+
#ifndef LLVM_LIBC_SRC_SIGNAL_SIGSET_T_H
3+
#define LLVM_LIBC_SRC_SIGNAL_SIGSET_T_H
4+
5+
#ifdef LIBC_FULL_BUILD
6+
7+
#include "include/llvm-libc-types/sigset_t.h"
8+
9+
#else
10+
11+
#include <signal.h>
12+
13+
#endif // LIBC_FULL_BUILD
14+
15+
#endif // LLVM_LIBC_SRC_SIGNAL_SIGSET_T_H

‎libc/src/sys/epoll/CMakeLists.txt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@ if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
22
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
33
endif()
44

5+
add_header_library(
6+
struct_epoll_event
7+
HDRS
8+
struct_epoll_event.h
9+
DEPENDS
10+
libc.include.llvm-libc-types.struct_epoll_event
11+
)
12+
513
add_entrypoint_object(
614
epoll_wait
715
ALIAS

‎libc/src/sys/epoll/epoll_pwait.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@
99
#ifndef LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT_H
1010
#define LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT_H
1111

12-
// TODO: Use this include once the include headers are also using quotes.
13-
// #include "include/llvm-libc-types/sigset_t.h"
14-
// #include "include/llvm-libc-types/struct_epoll_event.h"
15-
16-
#include <sys/epoll.h>
12+
#include "src/signal/sigset_t.h"
13+
#include "src/sys/epoll/struct_epoll_event.h"
1714

1815
namespace LIBC_NAMESPACE {
1916

‎libc/src/sys/epoll/epoll_pwait2.h

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,15 @@
99
#ifndef LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT2_H
1010
#define LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT2_H
1111

12-
// TODO: Use this include once the include headers are also using quotes.
13-
// #include "include/llvm-libc-types/sigset_t.h"
14-
// #include "include/llvm-libc-types/struct_epoll_event.h"
15-
// #include "include/llvm-libc-types/struct_timespec.h"
16-
17-
#include <sys/epoll.h>
12+
#include "src/signal/sigset_t.h"
13+
#include "src/sys/epoll/struct_epoll_event.h"
14+
#include "src/time/struct_timespec.h"
1815

1916
namespace LIBC_NAMESPACE {
2017

2118
// TODO: sigmask and timeout should be nullable
22-
int epoll_pwait2(int epfd, epoll_event *events, int maxevents,
23-
const timespec *timeout, const sigset_t *sigmask);
19+
int epoll_pwait2(int epfd, struct epoll_event *events, int maxevents,
20+
const struct timespec *timeout, const sigset_t *sigmask);
2421

2522
} // namespace LIBC_NAMESPACE
2623

0 commit comments

Comments
 (0)