[libc] Codify header inclusion policy (llvm#87017)

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 and the epoll implemenations over to this convention as an example.
Ericsson · Apr 11, 2024 · f626a35 · f626a35
1 parent 5122a2c
commit f626a35
Show file tree

Hide file tree

Showing 29 changed files with 200 additions and 73 deletions.
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
@@ -186,3 +186,36 @@ We expect contributions to be free of warnings from the `minimum supported
 compiler versions`__ (and newer).
 
 .. __: https://libc.llvm.org/compiler_support.html#minimum-supported-versions
+
+Header Inclusion Policy
+=======================
+
+Because llvm-libc supports
+`Overlay Mode <https://libc.llvm.org/overlay_mode.html>`__ and
+`Fullbuild Mode <https://libc.llvm.org/fullbuild_mode.html>`__ care must be
+taken when ``#include``'ing certain headers.
+
+The ``include/`` directory contains public facing headers that users must
+consume for fullbuild mode. As such, types defined here will have ABI
+implications as these definitions may differ from the underlying system for
+overlay mode and are NEVER appropriate to include in ``libc/src/`` without
+preprocessor guards for ``LLVM_LIBC_FULL_BUILD``.
+
+Consider the case where an implementation in ``libc/src/`` may wish to refer to
+a ``sigset_t``, what header should be included? ``<signal.h>``, ``<spawn.h>``,
+``<sys/select.h>``?
+
+None of the above. Instead, code under ``src/`` should ``#include
+"hdr/types/sigset_t.h"`` which contains preprocessor guards on
+``LLVM_LIBC_FULL_BUILD`` to either include the public type (fullbuild mode) or
+the underlying system header (overlay mode).
+
+Implementations in ``libc/src/`` should NOT be ``#include``'ing using ``<>`` or
+``"include/*``, except for these "proxy" headers that first check for
+``LLVM_LIBC_FULL_BUILD``.
+
+These "proxy" headers are similarly used when referring to preprocessor
+defines. Code under ``libc/src/`` should ``#include`` a proxy header from
+``hdr/``, which contains a guard on ``LLVM_LIBC_FULL_BUILD`` to either include
+our header from ``libc/include/`` (fullbuild) or the corresponding underlying
+system header (overlay).
diff --git a/libc/docs/usage_modes.rst b/libc/docs/usage_modes.rst
@@ -6,6 +6,10 @@ The libc can used in two different modes:
 
 #. The **overlay** mode: In this mode, the link order semantics are exploited
    to overlay implementations from LLVM's libc over the system libc. See
-   :ref:`overlay_mode` for more information about this mode.
+   :ref:`overlay_mode` for more information about this mode. In this mode, libc
+   uses the ABI of the system it's being overlayed onto. Headers are NOT
+   generated. libllvmlibc.a is the only build artifact.
 #. The **fullbuild** mode: In this mode, LLVM's libc is used as the only libc
    for the binary. See :ref:`fullbuild_mode` for information about this mode.
+   In this mode, libc uses its own ABI. Headers are generated along with a
+   libc.a.
diff --git a/libc/hdr/CMakeLists.txt b/libc/hdr/CMakeLists.txt
@@ -40,3 +40,5 @@ add_proxy_header_library(
     libc.include.llvm-libc-macros.fenv_macros
     libc.include.fenv
 )
+
+add_subdirectory(types)
diff --git a/libc/hdr/types/CMakeLists.txt b/libc/hdr/types/CMakeLists.txt
@@ -0,0 +1,23 @@
+add_proxy_header_library(
+  sigset_t
+  HDRS
+    sigset_t.h
+  FULL_BUILD_DEPENDS
+    libc.include.llvm-libc-types.sigset_t
+)
+
+add_proxy_header_library(
+  struct_epoll_event
+  HDRS
+    struct_epoll_event.h
+  FULL_BUILD_DEPENDS
+    libc.include.llvm-libc-types.struct_epoll_event
+)
+
+add_proxy_header_library(
+  struct_timespec
+  HDRS
+    struct_timespec.h
+  FULL_BUILD_DEPENDS
+    libc.include.llvm-libc-types.struct_timespec
+)
diff --git a/libc/hdr/types/sigset_t.h b/libc/hdr/types/sigset_t.h
@@ -0,0 +1,21 @@
+//===-- Proxy for sigset_t ------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_LIBC_HDR_TYPES_SIGSET_T_H
+#define LLVM_LIBC_HDR_TYPES_SIGSET_T_H
+
+#ifdef LIBC_FULL_BUILD
+
+#include "include/llvm-libc-types/sigset_t.h"
+
+#else
+
+#include <signal.h>
+
+#endif // LIBC_FULL_BUILD
+
+#endif // LLVM_LIBC_HDR_TYPES_SIGSET_T_H
diff --git a/libc/hdr/types/struct_epoll_event.h b/libc/hdr/types/struct_epoll_event.h
@@ -0,0 +1,21 @@
+//===-- Proxy for struct epoll_event --------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_LIBC_HDR_TYPES_STRUCT_EPOLL_EVENT_H
+#define LLVM_LIBC_HDR_TYPES_STRUCT_EPOLL_EVENT_H
+
+#ifdef LIBC_FULL_BUILD
+
+#include "include/llvm-libc-types/struct_epoll_event.h"
+
+#else
+
+#include <sys/epoll.h>
+
+#endif // LIBC_FULL_BUILD
+
+#endif // LLVM_LIBC_HDR_TYPES_STRUCT_EPOLL_EVENT_H
diff --git a/libc/hdr/types/struct_timespec.h b/libc/hdr/types/struct_timespec.h
@@ -0,0 +1,21 @@
+//===-- Proxy for struct timespec  ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_LIBC_HDR_TYPES_STRUCT_TIMESPEC_H
+#define LLVM_LIBC_HDR_TYPES_STRUCT_TIMESPEC_H
+
+#ifdef LIBC_FULL_BUILD
+
+#include "include/llvm-libc-types/struct_timespec.h"
+
+#else
+
+#include <time.h>
+
+#endif // LIBC_FULL_BUILD
+
+#endif // LLVM_LIBC_HDR_TYPES_STRUCT_TIMESPEC_H
diff --git a/libc/src/signal/linux/CMakeLists.txt b/libc/src/signal/linux/CMakeLists.txt
@@ -3,6 +3,8 @@ add_header_library(
   HDRS
     signal_utils.h
   DEPENDS
+    libc.hdr.types.sigset_t
+    libc.include.signal
     libc.include.sys_syscall
     libc.src.__support.OSUtil.osutil
 )
@@ -28,7 +30,7 @@ add_entrypoint_object(
     ../raise.h
   DEPENDS
     .signal_utils
-    libc.include.signal
+    libc.hdr.types.sigset_t
     libc.include.sys_syscall
     libc.src.__support.OSUtil.osutil
 )
@@ -57,7 +59,7 @@ add_entrypoint_object(
     ../sigaction.h
   DEPENDS
     .__restore
-    libc.include.signal
+    libc.hdr.types.sigset_t
     libc.include.sys_syscall
     libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
@@ -84,7 +86,7 @@ add_entrypoint_object(
     ../sigprocmask.h
   DEPENDS
     .signal_utils
-    libc.include.signal
+    libc.hdr.types.sigset_t
     libc.include.sys_syscall
     libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
@@ -98,7 +100,7 @@ add_entrypoint_object(
     ../sigemptyset.h
   DEPENDS
     .signal_utils
-    libc.include.signal
+    libc.hdr.types.sigset_t
     libc.src.errno.errno
 )
 
@@ -110,7 +112,7 @@ add_entrypoint_object(
     ../sigaddset.h
   DEPENDS
     .signal_utils
-    libc.include.signal
+    libc.hdr.types.sigset_t
     libc.src.errno.errno
 )
 
@@ -133,7 +135,7 @@ add_entrypoint_object(
     ../sigfillset.h
   DEPENDS
     .signal_utils
-    libc.include.signal
+    libc.hdr.types.sigset_t
     libc.src.errno.errno
 )
 
@@ -145,6 +147,6 @@ add_entrypoint_object(
     ../sigdelset.h
   DEPENDS
     .signal_utils
-    libc.include.signal
+    libc.hdr.types.sigset_t
     libc.src.errno.errno
 )
diff --git a/libc/src/signal/linux/raise.cpp b/libc/src/signal/linux/raise.cpp
@@ -7,14 +7,15 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/signal/raise.h"
-#include "src/signal/linux/signal_utils.h"
 
+#include "hdr/types/sigset_t.h"
 #include "src/__support/common.h"
+#include "src/signal/linux/signal_utils.h"
 
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(int, raise, (int sig)) {
-  ::sigset_t sigset;
+  sigset_t sigset;
   block_all_signals(sigset);
   long pid = LIBC_NAMESPACE::syscall_impl<long>(SYS_getpid);
   long tid = LIBC_NAMESPACE::syscall_impl<long>(SYS_gettid);

diff --git a/libc/src/signal/linux/sigaction.cpp b/libc/src/signal/linux/sigaction.cpp
@@ -7,12 +7,11 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/signal/sigaction.h"
-#include "src/errno/libc_errno.h"
-#include "src/signal/linux/signal_utils.h"
 
+#include "hdr/types/sigset_t.h"
 #include "src/__support/common.h"
-
-#include <signal.h>
+#include "src/errno/libc_errno.h"
+#include "src/signal/linux/signal_utils.h"
 
 namespace LIBC_NAMESPACE {
 

diff --git a/libc/src/signal/linux/sigaddset.cpp b/libc/src/signal/linux/sigaddset.cpp
@@ -7,12 +7,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/signal/sigaddset.h"
+
+#include "hdr/types/sigset_t.h"
 #include "src/__support/common.h"
 #include "src/errno/libc_errno.h"
 #include "src/signal/linux/signal_utils.h"
 
-#include <signal.h>
-
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(int, sigaddset, (sigset_t * set, int signum)) {

diff --git a/libc/src/signal/linux/sigdelset.cpp b/libc/src/signal/linux/sigdelset.cpp
@@ -7,12 +7,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/signal/sigdelset.h"
+
+#include "hdr/types/sigset_t.h"
 #include "src/__support/common.h"
 #include "src/errno/libc_errno.h"
 #include "src/signal/linux/signal_utils.h"
 
-#include <signal.h>
-
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(int, sigdelset, (sigset_t * set, int signum)) {

diff --git a/libc/src/signal/linux/sigfillset.cpp b/libc/src/signal/linux/sigfillset.cpp
@@ -7,12 +7,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/signal/sigfillset.h"
+
+#include "hdr/types/sigset_t.h"
 #include "src/__support/common.h"
 #include "src/errno/libc_errno.h"
 #include "src/signal/linux/signal_utils.h"
 
-#include <signal.h>
-
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(int, sigfillset, (sigset_t * set)) {

diff --git a/libc/src/signal/linux/signal_utils.h b/libc/src/signal/linux/signal_utils.h
@@ -9,10 +9,11 @@
 #ifndef LLVM_LIBC_SRC_SIGNAL_LINUX_SIGNAL_UTILS_H
 #define LLVM_LIBC_SRC_SIGNAL_LINUX_SIGNAL_UTILS_H
 
+#include "hdr/types/sigset_t.h"
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
 
-#include <signal.h>
+#include <signal.h> // sigaction
 #include <stddef.h>
 #include <sys/syscall.h>          // For syscall numbers.
 

diff --git a/libc/src/signal/linux/sigprocmask.cpp b/libc/src/signal/linux/sigprocmask.cpp
@@ -7,13 +7,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/signal/sigprocmask.h"
+
+#include "hdr/types/sigset_t.h"
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/common.h"
 #include "src/errno/libc_errno.h"
 #include "src/signal/linux/signal_utils.h"
 
-#include "src/__support/common.h"
-
-#include <signal.h>
 #include <sys/syscall.h> // For syscall numbers.
 
 namespace LIBC_NAMESPACE {

diff --git a/libc/src/signal/sigaddset.h b/libc/src/signal/sigaddset.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_SIGNAL_SIGADDSET_H
 #define LLVM_LIBC_SRC_SIGNAL_SIGADDSET_H
 
-#include <signal.h>
+#include "hdr/types/sigset_t.h"
 
 namespace LIBC_NAMESPACE {
 

diff --git a/libc/src/signal/sigdelset.h b/libc/src/signal/sigdelset.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_SIGNAL_SIGDELSET_H
 #define LLVM_LIBC_SRC_SIGNAL_SIGDELSET_H
 
-#include <signal.h>
+#include "hdr/types/sigset_t.h"
 
 namespace LIBC_NAMESPACE {
 

diff --git a/libc/src/signal/sigemptyset.h b/libc/src/signal/sigemptyset.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_SIGNAL_SIGEMPTYSET_H
 #define LLVM_LIBC_SRC_SIGNAL_SIGEMPTYSET_H
 
-#include <signal.h>
+#include "hdr/types/sigset_t.h"
 
 namespace LIBC_NAMESPACE {
 

diff --git a/libc/src/signal/sigfillset.h b/libc/src/signal/sigfillset.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_SIGNAL_SIGFILLSET_H
 #define LLVM_LIBC_SRC_SIGNAL_SIGFILLSET_H
 
-#include <signal.h>
+#include "hdr/types/sigset_t.h"
 
 namespace LIBC_NAMESPACE {
 

diff --git a/libc/src/signal/sigprocmask.h b/libc/src/signal/sigprocmask.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_LIBC_SRC_SIGNAL_SIGPROCMASK_H
 #define LLVM_LIBC_SRC_SIGNAL_SIGPROCMASK_H
 
-#include <signal.h>
+#include "hdr/types/sigset_t.h"
 
 namespace LIBC_NAMESPACE {
 

diff --git a/libc/src/sys/epoll/epoll_pwait.h b/libc/src/sys/epoll/epoll_pwait.h
@@ -9,11 +9,8 @@
 #ifndef LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT_H
 #define LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT_H
 
-// TODO: Use this include once the include headers are also using quotes.
-// #include "include/llvm-libc-types/sigset_t.h"
-// #include "include/llvm-libc-types/struct_epoll_event.h"
-
-#include <sys/epoll.h>
+#include "hdr/types/sigset_t.h"
+#include "hdr/types/struct_epoll_event.h"
 
 namespace LIBC_NAMESPACE {
 

diff --git a/libc/src/sys/epoll/epoll_pwait2.h b/libc/src/sys/epoll/epoll_pwait2.h
@@ -9,12 +9,9 @@
 #ifndef LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT2_H
 #define LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT2_H
 
-// TODO: Use this include once the include headers are also using quotes.
-// #include "include/llvm-libc-types/sigset_t.h"
-// #include "include/llvm-libc-types/struct_epoll_event.h"
-// #include "include/llvm-libc-types/struct_timespec.h"
-
-#include <sys/epoll.h>
+#include "hdr/types/sigset_t.h"
+#include "hdr/types/struct_epoll_event.h"
+#include "hdr/types/struct_timespec.h"
 
 namespace LIBC_NAMESPACE {