[libc-commits] [libc] [libc] Introduce a typed syscall wrapper and use it in mmap (PR #197459)
via libc-commits
libc-commits at lists.llvm.org
Wed May 13 07:42:11 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libc
Author: Pavel Labath (labath)
<details>
<summary>Changes</summary>
Linux reserves a range of values (everything above -4096u, aka MAX_ERRNO) as an error value, so the check can be performed without knowing the details of the specific syscall. libc functions where these values would be a valid result (e.g. PTRACE_PEEKDATA) are implemented differently at the kernel level (e.g. returning the result through a pointer argument). The only exception are a handful of syscalls (getpid, getuid, ...) which can never fail, and where this could be an actual user/group ID (particularly on 32-bit systems).
Specifically, for mmap, this lets us remove the is_valid_mmap helper and SYS_mmap2 ifdefs in various places.
More generally, this can simplify many syscall wrappers as often the only thing they are doing is converting the return value into an ErrorOr.
---
Full diff: https://github.com/llvm/llvm-project/pull/197459.diff
9 Files Affected:
- (modified) libc/docs/dev/syscall_wrapper_refactor.rst (+2-6)
- (modified) libc/src/__support/OSUtil/linux/CMakeLists.txt (+3)
- (modified) libc/src/__support/OSUtil/linux/auxv.h (+7-11)
- (modified) libc/src/__support/OSUtil/linux/syscall.h (+18-10)
- (modified) libc/src/__support/OSUtil/linux/syscall_wrappers/mmap.h (+3-12)
- (modified) libc/src/__support/threads/linux/CMakeLists.txt (+2)
- (modified) libc/src/__support/threads/linux/thread.cpp (+14-23)
- (modified) libc/startup/linux/x86_64/CMakeLists.txt (+2)
- (modified) libc/startup/linux/x86_64/tls.cpp (+7-20)
``````````diff
diff --git a/libc/docs/dev/syscall_wrapper_refactor.rst b/libc/docs/dev/syscall_wrapper_refactor.rst
index 40535b04d90eb..329d0f32ec0d0 100644
--- a/libc/docs/dev/syscall_wrapper_refactor.rst
+++ b/libc/docs/dev/syscall_wrapper_refactor.rst
@@ -36,7 +36,7 @@ Example Wrapper (``src/__support/OSUtil/linux/syscall_wrappers/read.h``):
.. code-block:: c++
#include "hdr/types/ssize_t.h"
- #include "src/__support/OSUtil/linux/syscall.h" // For syscall_impl
+ #include "src/__support/OSUtil/linux/syscall.h" // For __syscall
#include "src/__support/common.h"
#include "src/__support/error_or.h"
#include "src/__support/macros/config.h"
@@ -46,11 +46,7 @@ Example Wrapper (``src/__support/OSUtil/linux/syscall_wrappers/read.h``):
namespace linux_syscalls {
LIBC_INLINE ErrorOr<ssize_t> read(int fd, void *buf, size_t count) {
- ssize_t ret = syscall_impl<ssize_t>(SYS_read, fd, buf, count);
- if (ret < 0) {
- return Error(-static_cast<int>(ret));
- }
- return ret;
+ return __syscall<ssize_t>(SYS_read, fd, buf, count);
}
} // namespace linux_syscalls
diff --git a/libc/src/__support/OSUtil/linux/CMakeLists.txt b/libc/src/__support/OSUtil/linux/CMakeLists.txt
index dc1865967a348..5d42bc67912fb 100644
--- a/libc/src/__support/OSUtil/linux/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/linux/CMakeLists.txt
@@ -17,6 +17,7 @@ add_object_library(
.${LIBC_TARGET_ARCHITECTURE}.linux_${LIBC_TARGET_ARCHITECTURE}_util
libc.src.__support.common
libc.src.__support.CPP.string_view
+ libc.src.__support.error_or
libc.hdr.fcntl_macros
libc.hdr.types.struct_flock
libc.hdr.types.struct_flock64
@@ -32,6 +33,8 @@ add_header_library(
DEPENDS
libc.hdr.fcntl_macros
libc.hdr.sys_auxv_macros
+ libc.hdr.sys_mman_macros
+ libc.src.__support.OSUtil.linux.syscall_wrappers.mmap
libc.src.__support.OSUtil.osutil
libc.src.__support.common
libc.src.__support.CPP.optional
diff --git a/libc/src/__support/OSUtil/linux/auxv.h b/libc/src/__support/OSUtil/linux/auxv.h
index 1dc9bf0930b7c..a14f0ad731b05 100644
--- a/libc/src/__support/OSUtil/linux/auxv.h
+++ b/libc/src/__support/OSUtil/linux/auxv.h
@@ -11,11 +11,12 @@
#include "hdr/fcntl_macros.h" // For open flags
#include "hdr/sys_auxv_macros.h" // For AT_ macros
+#include "hdr/sys_mman_macros.h" // For mmap flags
+#include "src/__support/OSUtil/linux/syscall_wrappers/mmap.h"
#include "src/__support/OSUtil/syscall.h"
#include "src/__support/common.h"
#include "src/__support/threads/callonce.h"
-#include <linux/mman.h> // For mmap flags
#include <linux/param.h> // For EXEC_PAGESIZE
#include <linux/prctl.h> // For prctl
#include <sys/syscall.h> // For syscall numbers
@@ -82,20 +83,15 @@ LIBC_INLINE void Vector::initialize_unsafe(const Entry *auxv) {
[[gnu::cold]]
LIBC_INLINE void Vector::fallback_initialize_unsync() {
constexpr size_t AUXV_MMAP_SIZE = FALLBACK_AUXV_ENTRIES * sizeof(Entry);
-#ifdef SYS_mmap2
- constexpr int MMAP_SYSNO = SYS_mmap2;
-#else
- constexpr int MMAP_SYSNO = SYS_mmap;
-#endif
- long mmap_ret = syscall_impl<long>(MMAP_SYSNO, nullptr, AUXV_MMAP_SIZE,
- PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ ErrorOr<void *> mmap_ret =
+ linux_syscalls::mmap(nullptr, AUXV_MMAP_SIZE, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
// We do not proceed if mmap fails.
- if (!linux_utils::is_valid_mmap(mmap_ret))
+ if (!mmap_ret.has_value())
return;
// Initialize the auxv array with AT_NULL entries.
- Entry *vector = reinterpret_cast<Entry *>(mmap_ret);
+ Entry *vector = static_cast<Entry *>(mmap_ret.value());
for (size_t i = 0; i < FALLBACK_AUXV_ENTRIES; ++i) {
vector[i].type = AT_NULL;
vector[i].val = AT_NULL;
diff --git a/libc/src/__support/OSUtil/linux/syscall.h b/libc/src/__support/OSUtil/linux/syscall.h
index 0e25618e81942..73aa3c250738c 100644
--- a/libc/src/__support/OSUtil/linux/syscall.h
+++ b/libc/src/__support/OSUtil/linux/syscall.h
@@ -11,6 +11,7 @@
#include "src/__support/CPP/bit.h"
#include "src/__support/common.h"
+#include "src/__support/error_or.h"
#include "src/__support/macros/config.h"
#include "src/__support/macros/optimization.h"
#include "src/__support/macros/properties/architectures.h"
@@ -29,24 +30,31 @@
namespace LIBC_NAMESPACE_DECL {
+// This function performs no error checking. For most syscalls, it's better to
+// use __syscall below.
template <typename R, typename... Ts>
LIBC_INLINE R syscall_impl(long __number, Ts... ts) {
static_assert(sizeof...(Ts) <= 6, "Too many arguments for syscall");
return cpp::bit_or_static_cast<R>(syscall_impl(__number, (long)ts...));
}
-// Linux-specific function for checking
-namespace linux_utils {
+namespace linux_syscalls {
LIBC_INLINE_VAR constexpr unsigned long MAX_ERRNO = 4095;
-// Ideally, this should be defined using PAGE_OFFSET
-// However, that is a configurable parameter. We mimic kernel's behavior
-// by checking against MAX_ERRNO.
-template <typename PointerLike>
-LIBC_INLINE constexpr bool is_valid_mmap(PointerLike ptr) {
- long addr = cpp::bit_cast<long>(ptr);
- return LIBC_LIKELY(addr > 0 || addr < -static_cast<long>(MAX_ERRNO));
+
+// Helper function to perform a system call, check the result and cast the
+// result to the expected type. This function is safe to use on most syscalls,
+// with the exception of a handful of syscalls (getpid, getuid, ...) that never
+// fail.
+template <typename R, typename... Ts>
+LIBC_INLINE ErrorOr<R> __syscall(long __number, Ts... ts) {
+ static_assert(sizeof...(Ts) <= 6, "Too many arguments");
+ unsigned long ret =
+ static_cast<unsigned long>(syscall_impl(__number, (long)ts...));
+ if (ret >= -MAX_ERRNO)
+ return Error(static_cast<int>(-ret));
+ return cpp::bit_or_static_cast<R>(ret);
}
-} // namespace linux_utils
+} // namespace linux_syscalls
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/__support/OSUtil/linux/syscall_wrappers/mmap.h b/libc/src/__support/OSUtil/linux/syscall_wrappers/mmap.h
index 983973e70e205..aa1f2a90ec532 100644
--- a/libc/src/__support/OSUtil/linux/syscall_wrappers/mmap.h
+++ b/libc/src/__support/OSUtil/linux/syscall_wrappers/mmap.h
@@ -10,7 +10,7 @@
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_MMAP_H
#include "hdr/types/off_t.h"
-#include "src/__support/OSUtil/linux/syscall.h" // syscall_impl, linux_utils::is_valid_mmap
+#include "src/__support/OSUtil/linux/syscall.h" // For __syscall
#include "src/__support/common.h"
#include "src/__support/error_or.h"
#include "src/__support/macros/config.h"
@@ -40,17 +40,8 @@ LIBC_INLINE ErrorOr<void *> mmap(void *addr, size_t size, int prot, int flags,
// Explicit cast to silence "implicit conversion loses integer precision"
// warnings when compiling for 32-bit systems.
- long ret =
- syscall_impl<long>(syscall_number, reinterpret_cast<long>(addr), size,
- prot, flags, fd, static_cast<long>(offset));
-
- // A negative return value from the syscall can also be an error-free
- // value returned by the syscall. However, since a valid return address
- // cannot be within the last page, a return value corresponding to a
- // location in the last page is an error value.
- if (!linux_utils::is_valid_mmap(ret))
- return Error(-static_cast<int>(ret));
- return reinterpret_cast<void *>(ret);
+ return __syscall<void *>(syscall_number, reinterpret_cast<long>(addr), size,
+ prot, flags, fd, static_cast<long>(offset));
}
} // namespace linux_syscalls
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index b9273b42bb5f1..8ce19634c41b1 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -34,12 +34,14 @@ add_object_library(
libc.include.sys_syscall
libc.hdr.fcntl_macros
libc.hdr.errno_macros
+ libc.hdr.sys_mman_macros
libc.src.errno.errno
libc.src.__support.CPP.atomic
libc.src.__support.CPP.stringstream
libc.src.__support.CPP.string_view
libc.src.__support.common
libc.src.__support.error_or
+ libc.src.__support.OSUtil.linux.syscall_wrappers.mmap
libc.src.__support.threads.thread_common
COMPILE_OPTIONS
${libc_opt_high_flag}
diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp
index bdbb8aefeef0c..2c9744ba66338 100644
--- a/libc/src/__support/threads/linux/thread.cpp
+++ b/libc/src/__support/threads/linux/thread.cpp
@@ -11,6 +11,7 @@
#include "src/__support/CPP/atomic.h"
#include "src/__support/CPP/string_view.h"
#include "src/__support/CPP/stringstream.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/mmap.h"
#include "src/__support/OSUtil/syscall.h" // For syscall functions.
#include "src/__support/common.h"
#include "src/__support/error_or.h"
@@ -25,22 +26,14 @@
#include "hdr/errno_macros.h"
#include "hdr/fcntl_macros.h"
#include "hdr/stdint_proxy.h"
+#include "hdr/sys_mman_macros.h" // For PROT_* and MAP_* definitions.
#include <linux/param.h> // For EXEC_PAGESIZE.
#include <linux/prctl.h> // For PR_SET_NAME
#include <linux/sched.h> // For CLONE_* flags.
-#include <sys/mman.h> // For PROT_* and MAP_* definitions.
#include <sys/syscall.h> // For syscall numbers.
namespace LIBC_NAMESPACE_DECL {
-#ifdef SYS_mmap2
-static constexpr long MMAP_SYSCALL_NUMBER = SYS_mmap2;
-#elif defined(SYS_mmap)
-static constexpr long MMAP_SYSCALL_NUMBER = SYS_mmap;
-#else
-#error "mmap or mmap2 syscalls not available."
-#endif
-
static constexpr size_t NAME_SIZE_MAX = 16; // Includes the null terminator
static constexpr uint32_t CLEAR_TID_VALUE = 0xABCD1234;
static constexpr unsigned CLONE_SYSCALL_FLAGS =
@@ -93,29 +86,27 @@ LIBC_INLINE ErrorOr<void *> alloc_stack(size_t stacksize, size_t guardsize) {
// TODO: Maybe add MAP_STACK? Currently unimplemented on linux but helps
// future-proof.
- long mmap_result = LIBC_NAMESPACE::syscall_impl<long>(
- MMAP_SYSCALL_NUMBER,
- 0, // No special address
- size, prot,
- MAP_ANONYMOUS | MAP_PRIVATE, // Process private.
- -1, // Not backed by any file
- 0 // No offset
- );
- if (!linux_utils::is_valid_mmap(mmap_result))
- return Error{int(-mmap_result)};
+ ErrorOr<void *> mmap_result =
+ linux_syscalls::mmap(nullptr, size, prot,
+ MAP_ANONYMOUS | MAP_PRIVATE, // Process private.
+ -1, // Not backed by any file
+ 0 // No offset
+ );
+ if (!mmap_result.has_value())
+ return mmap_result;
+
+ char *stack = static_cast<char *>(mmap_result.value()) + guardsize;
if (guardsize) {
// Give read/write permissions to actual stack.
// TODO: We are assuming stack growsdown here.
long result = LIBC_NAMESPACE::syscall_impl<long>(
- SYS_mprotect, mmap_result + guardsize, stacksize,
- PROT_READ | PROT_WRITE);
+ SYS_mprotect, stack, stacksize, PROT_READ | PROT_WRITE);
if (result != 0)
return Error{int(-result)};
}
- mmap_result += guardsize;
- return reinterpret_cast<void *>(mmap_result);
+ return stack;
}
// This must always be inlined as we may be freeing the calling threads stack in
diff --git a/libc/startup/linux/x86_64/CMakeLists.txt b/libc/startup/linux/x86_64/CMakeLists.txt
index 44259944078d6..522bbc85ad663 100644
--- a/libc/startup/linux/x86_64/CMakeLists.txt
+++ b/libc/startup/linux/x86_64/CMakeLists.txt
@@ -4,9 +4,11 @@ add_startup_object(
tls.cpp
DEPENDS
libc.config.app_h
+ libc.hdr.sys_mman_macros
libc.include.sys_mman
libc.include.sys_syscall
libc.src.__support.OSUtil.osutil
+ libc.src.__support.OSUtil.linux.syscall_wrappers.mmap
libc.src.string.memory_utils.inline_memcpy
COMPILE_OPTIONS
-fno-stack-protector
diff --git a/libc/startup/linux/x86_64/tls.cpp b/libc/startup/linux/x86_64/tls.cpp
index 497f744e89e28..6d2f318e6f699 100644
--- a/libc/startup/linux/x86_64/tls.cpp
+++ b/libc/startup/linux/x86_64/tls.cpp
@@ -6,25 +6,17 @@
//
//===----------------------------------------------------------------------===//
-#include "src/__support/OSUtil/syscall.h"
+#include "hdr/sys_mman_macros.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/mmap.h"
#include "src/__support/macros/config.h"
#include "src/string/memory_utils/inline_memcpy.h"
#include "startup/linux/do_start.h"
#include <asm/prctl.h>
-#include <sys/mman.h>
#include <sys/syscall.h>
namespace LIBC_NAMESPACE_DECL {
-#ifdef SYS_mmap2
-static constexpr long MMAP_SYSCALL_NUMBER = SYS_mmap2;
-#elif SYS_mmap
-static constexpr long MMAP_SYSCALL_NUMBER = SYS_mmap;
-#else
-#error "mmap and mmap2 syscalls not available."
-#endif
-
// TODO: Also generalize this routine and handle dynamic loading properly.
void init_tls(TLSDescriptor &tls_descriptor) {
if (app.tls.size == 0) {
@@ -45,17 +37,12 @@ void init_tls(TLSDescriptor &tls_descriptor) {
// offset 0x28 (40) and is of size uintptr_t.
uintptr_t tls_size_with_addr = tls_size + sizeof(uintptr_t) + 40;
- // We cannot call the mmap function here as the functions set errno on
- // failure. Since errno is implemented via a thread local variable, we cannot
- // use errno before TLS is setup.
- long mmap_retval = syscall_impl<long>(
- MMAP_SYSCALL_NUMBER, nullptr, tls_size_with_addr, PROT_READ | PROT_WRITE,
- MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- // We cannot check the return value with MAP_FAILED as that is the return
- // of the mmap function and not the mmap syscall.
- if (!linux_utils::is_valid_mmap(mmap_retval))
+ ErrorOr<void *> mmap_ret =
+ linux_syscalls::mmap(nullptr, tls_size_with_addr, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+ if (!mmap_ret.has_value())
syscall_impl<long>(SYS_exit, 1);
- uintptr_t *tls_addr = reinterpret_cast<uintptr_t *>(mmap_retval);
+ uintptr_t *tls_addr = static_cast<uintptr_t *>(mmap_ret.value());
// x86_64 TLS faces down from the thread pointer with the first entry
// pointing to the address of the first real TLS byte.
``````````
</details>
https://github.com/llvm/llvm-project/pull/197459
More information about the libc-commits
mailing list