Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions libc/docs/dev/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ Navigate to the links below for information on the respective topics:
implementation_standard
undefined_behavior
printf_behavior
syscall_wrapper_refactor
68 changes: 68 additions & 0 deletions libc/docs/dev/syscall_wrapper_refactor.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
.. _syscall_wrapper_refactor:

==============================
Syscall Wrapper Refactoring
==============================

Purpose
=======

LLVM-libc is transitioning to a centralized system for Linux syscalls. The goal
is to move all direct ``syscall_impl`` calls into a dedicated directory:
``src/__support/OSUtil/linux/syscall_wrappers/``.

This refactor provides several benefits:

* **Type Safety**: Using ``ErrorOr<T>`` ensures that error conditions are
handled explicitly.
* **Consistency**: Standardizes the conversion of syscall return values into
errno-compatible objects.
* **Maintainability**: Centralizes platform-specific syscall logic, making it
easier to audit and update.
Comment thread
kaladron marked this conversation as resolved.

The Pattern
===========

Each syscall should have its own header-only library in the ``syscall_wrappers``
directory. The wrapper function should return an ``ErrorOr<T>``.

Example Wrapper (``src/__support/OSUtil/linux/syscall_wrappers/read.h``):
--------------------------------------------------------------------------

.. code-block:: c++

#include "src/__support/OSUtil/linux/syscall.h" // For syscall_impl
#include "src/__support/error_or.h"
#include "src/__support/common.h"
#include <sys/syscall.h> // For syscall numbers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I hope we can avoid including system header here if possible. E.g. today the only
way to get SYS_ numbers is to generate our own <sys/syscall.h> header:
https://github.com/llvm/llvm-project/blob/main/libc/include/sys/syscall.h.def

Probably these SYS_ numbers can be extracted out into our internal header under include/,
expose them via proxy header, and use here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Given how much this is used in the codebase already, I'd like to make this a subsequent CL to fix.


namespace LIBC_NAMESPACE_DECL {
namespace internal {
Comment thread
kaladron marked this conversation as resolved.
Outdated

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;
}

} // namespace internal
} // namespace LIBC_NAMESPACE_DECL

How to Migrate
==============

1. **Create the Wrapper**: Add a new header file in
``src/__support/OSUtil/linux/syscall_wrappers/``.
2. **Update CMake**: Add a ``add_header_library`` target for the new wrapper in
``src/__support/OSUtil/linux/syscall_wrappers/CMakeLists.txt``.
3. **Refactor Entrypoints**:

* Include the new wrapper header (e.g., ``read.h``).
* Replace direct ``syscall_impl`` calls with ``internal::<function_name>``.
* Update the entrypoint's ``DEPENDS`` in ``CMakeLists.txt`` to include the
new wrapper target.

4. **Cleanup OSUtil**: If the syscall was previously implemented manually in
Comment thread
kaladron marked this conversation as resolved.
Outdated
``OSUtil/linux/fcntl.cpp`` (or similar), remove it to avoid name collisions.
2 changes: 1 addition & 1 deletion libc/src/__support/HashTable/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ if (NOT ${getrandom_index} EQUAL -1)
message(STATUS "Using getrandom for hashtable randomness")
set(randomness_compile_flags -DLIBC_HASHTABLE_USE_GETRANDOM)
set(randomness_extra_depends
libc.src.__support.OSUtil.linux.getrandom
libc.src.__support.OSUtil.linux.syscall_wrappers.getrandom
libc.hdr.errno_macros)
endif()

Expand Down
2 changes: 1 addition & 1 deletion libc/src/__support/HashTable/randomness.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include "src/__support/macros/config.h"
#if defined(LIBC_HASHTABLE_USE_GETRANDOM)
#include "hdr/errno_macros.h"
#include "src/__support/OSUtil/linux/getrandom.h"
#include "src/__support/OSUtil/linux/syscall_wrappers/getrandom.h"
#endif

namespace LIBC_NAMESPACE_DECL {
Expand Down
1 change: 1 addition & 0 deletions libc/src/__support/OSUtil/linux/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ if(NOT EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_ARCHITECTURE})
endif()

add_subdirectory(${LIBC_TARGET_ARCHITECTURE})
add_subdirectory(syscall_wrappers)

add_object_library(
linux_util
Expand Down
22 changes: 0 additions & 22 deletions libc/src/__support/OSUtil/linux/fcntl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,27 +116,5 @@ ErrorOr<int> fcntl(int fd, int cmd, void *arg) {
return ret;
}

ErrorOr<int> open(const char *path, int flags, mode_t mode_flags) {
#ifdef SYS_open
int fd = LIBC_NAMESPACE::syscall_impl<int>(SYS_open, path, flags, mode_flags);
#else
int fd = LIBC_NAMESPACE::syscall_impl<int>(SYS_openat, AT_FDCWD, path, flags,
mode_flags);
#endif
if (fd < 0)
return Error(-fd);

return fd;
}

ErrorOr<int> close(int fd) {
int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_close, fd);

if (ret < 0)
return Error(-ret);

return ret;
}

} // namespace internal
} // namespace LIBC_NAMESPACE_DECL
64 changes: 64 additions & 0 deletions libc/src/__support/OSUtil/linux/syscall_wrappers/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
add_header_library(
getrandom
HDRS
getrandom.h
DEPENDS
libc.src.__support.OSUtil.osutil
libc.src.__support.common
libc.src.__support.error_or
libc.src.__support.macros.config
libc.hdr.types.ssize_t
libc.include.sys_syscall
)

add_header_library(
close
HDRS
close.h
DEPENDS
libc.src.__support.OSUtil.osutil
libc.src.__support.common
libc.src.__support.error_or
libc.src.__support.macros.config
libc.include.sys_syscall
)

add_header_library(
read
HDRS
read.h
DEPENDS
libc.src.__support.OSUtil.osutil
libc.src.__support.common
libc.src.__support.error_or
libc.src.__support.macros.config
libc.hdr.types.ssize_t
libc.include.sys_syscall
)

add_header_library(
write
HDRS
write.h
DEPENDS
libc.src.__support.OSUtil.osutil
libc.src.__support.common
libc.src.__support.error_or
libc.src.__support.macros.config
libc.hdr.types.ssize_t
libc.include.sys_syscall
)

add_header_library(
open
HDRS
open.h
DEPENDS
libc.src.__support.OSUtil.osutil
libc.src.__support.common
libc.src.__support.error_or
libc.src.__support.macros.config
libc.hdr.fcntl_macros
libc.hdr.types.mode_t
libc.include.sys_syscall
)
31 changes: 31 additions & 0 deletions libc/src/__support/OSUtil/linux/syscall_wrappers/close.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//===-- Implementation header for close -------------------------*- C++ -*-===//
//
// 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_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_CLOSE_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_CLOSE_H

#include "src/__support/OSUtil/linux/syscall.h" // syscall_impl
#include "src/__support/common.h"
#include "src/__support/error_or.h"
#include "src/__support/macros/config.h"
#include <sys/syscall.h> // For syscall numbers
Comment thread
vonosmas marked this conversation as resolved.

namespace LIBC_NAMESPACE_DECL {
namespace internal {

LIBC_INLINE ErrorOr<int> close(int fd) {
int ret = syscall_impl<int>(SYS_close, fd);
if (ret < 0)
return Error(-static_cast<int>(ret));
return ret;
}

} // namespace internal
} // namespace LIBC_NAMESPACE_DECL

#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_CLOSE_H
33 changes: 33 additions & 0 deletions libc/src/__support/OSUtil/linux/syscall_wrappers/getrandom.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//===-- Implementation header for getrandom ---------------------*- C++ -*-===//
//
// 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_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_GETRANDOM_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_GETRANDOM_H

#include "hdr/types/ssize_t.h"
#include "src/__support/OSUtil/linux/syscall.h" // syscall_impl
#include "src/__support/common.h"
#include "src/__support/error_or.h"
#include "src/__support/macros/config.h"
#include <sys/syscall.h> // For syscall numbers

namespace LIBC_NAMESPACE_DECL {
namespace internal {

LIBC_INLINE ErrorOr<ssize_t> getrandom(void *buf, size_t buflen,
unsigned int flags) {
ssize_t ret = syscall_impl<ssize_t>(SYS_getrandom, buf, buflen, flags);
if (ret < 0)
return Error(-static_cast<int>(ret));
return ret;
}

} // namespace internal
} // namespace LIBC_NAMESPACE_DECL

#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_GETRANDOM_H
37 changes: 37 additions & 0 deletions libc/src/__support/OSUtil/linux/syscall_wrappers/open.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//===-- Implementation header for open --------------------------*- C++ -*-===//
//
// 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_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_OPEN_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_OPEN_H

#include "hdr/fcntl_macros.h"
#include "hdr/types/mode_t.h"
#include "src/__support/OSUtil/linux/syscall.h" // syscall_impl
#include "src/__support/common.h"
#include "src/__support/error_or.h"
#include "src/__support/macros/config.h"
#include <sys/syscall.h> // For syscall numbers

namespace LIBC_NAMESPACE_DECL {
namespace internal {

LIBC_INLINE ErrorOr<int> open(const char *path, int flags, mode_t mode_flags) {
#ifdef SYS_open
int fd = syscall_impl<int>(SYS_open, path, flags, mode_flags);
#else
int fd = syscall_impl<int>(SYS_openat, AT_FDCWD, path, flags, mode_flags);
#endif
if (fd < 0)
return Error(-fd);
return fd;
}

} // namespace internal
} // namespace LIBC_NAMESPACE_DECL

#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_OPEN_H
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
//===------------ Implementation of getrandom function ----------*- C++ -*-===//
//===-- Implementation header for read --------------------------*- C++ -*-===//
//
// 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_SRC___SUPPORT_OSUTIL_GETRANDOM_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_GETRANDOM_H
#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_READ_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_READ_H

#include "hdr/types/ssize_t.h"
#include "src/__support/OSUtil/linux/syscall.h" // syscall_impl
Expand All @@ -19,17 +19,14 @@
namespace LIBC_NAMESPACE_DECL {
namespace internal {

LIBC_INLINE static ErrorOr<ssize_t> getrandom(void *buf, size_t buflen,
unsigned int flags) {
ssize_t ret =
LIBC_NAMESPACE::syscall_impl<ssize_t>(SYS_getrandom, buf, buflen, flags);
if (ret < 0) {
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;
}

} // namespace internal
} // namespace LIBC_NAMESPACE_DECL

#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_GETRANDOM_H
#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_READ_H
32 changes: 32 additions & 0 deletions libc/src/__support/OSUtil/linux/syscall_wrappers/write.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//===-- Implementation header for write -------------------------*- C++ -*-===//
//
// 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_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_WRITE_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_WRITE_H

#include "hdr/types/ssize_t.h"
#include "src/__support/OSUtil/linux/syscall.h" // syscall_impl
#include "src/__support/common.h"
#include "src/__support/error_or.h"
#include "src/__support/macros/config.h"
#include <sys/syscall.h> // For syscall numbers

namespace LIBC_NAMESPACE_DECL {
namespace internal {

LIBC_INLINE ErrorOr<ssize_t> write(int fd, const void *buf, size_t count) {
ssize_t ret = syscall_impl<ssize_t>(SYS_write, fd, buf, count);
if (ret < 0)
return Error(-static_cast<int>(ret));
return ret;
}

} // namespace internal
} // namespace LIBC_NAMESPACE_DECL

#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_WRITE_H
2 changes: 1 addition & 1 deletion libc/src/fcntl/linux/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ add_entrypoint_object(
DEPENDS
libc.hdr.types.mode_t
libc.hdr.fcntl_macros
libc.src.__support.OSUtil.osutil
libc.src.__support.OSUtil.linux.syscall_wrappers.open
libc.src.errno.errno
)

Expand Down
2 changes: 1 addition & 1 deletion libc/src/fcntl/linux/open.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

#include "hdr/fcntl_macros.h"
#include "hdr/types/mode_t.h"
#include "src/__support/OSUtil/fcntl.h"
#include "src/__support/OSUtil/linux/syscall_wrappers/open.h"
#include "src/__support/common.h"
#include "src/__support/libc_errno.h"
#include "src/__support/macros/config.h"
Expand Down
2 changes: 2 additions & 0 deletions libc/src/sys/mman/linux/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,10 @@ add_entrypoint_object(
HDRS
../shm_open.h
DEPENDS
libc.hdr.fcntl_macros
libc.hdr.types.mode_t
libc.src.errno.errno
libc.src.__support.OSUtil.linux.syscall_wrappers.open
.shm_common
)

Expand Down
Loading
Loading