Skip to content

[libc] Implement fcntl when only SYS_fcntl64 is available #97740

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.

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

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

mikhailramalho
Copy link
Member

This patch tries to implement fcntl with either SYS_fcntl or SYS_fcntl64, and also changes a test case to call the fcntl function instead of using the syscall

@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2024

@llvm/pr-subscribers-libc

Author: Mikhail R. Gadelha (mikhailramalho)

Changes

This patch tries to implement fcntl with either SYS_fcntl or SYS_fcntl64, and also changes a test case to call the fcntl function instead of using the syscall


Full diff: https://github.com/llvm/llvm-project/pull/97740.diff

3 Files Affected:

  • (modified) libc/src/__support/OSUtil/linux/fcntl.cpp (+12-4)
  • (modified) libc/test/src/sys/mman/linux/CMakeLists.txt (+1)
  • (modified) libc/test/src/sys/mman/linux/shm_test.cpp (+2-3)
diff --git a/libc/src/__support/OSUtil/linux/fcntl.cpp b/libc/src/__support/OSUtil/linux/fcntl.cpp
index 083deb8c336813..2336d6993bd2a3 100644
--- a/libc/src/__support/OSUtil/linux/fcntl.cpp
+++ b/libc/src/__support/OSUtil/linux/fcntl.cpp
@@ -19,6 +19,14 @@
 #include <stdarg.h>
 #include <sys/syscall.h> // For syscall numbers.
 
+#if SYS_fcntl
+constexpr auto FCNTL_SYSCALL_ID = SYS_fcntl;
+#elif defined(SYS_fcntl64)
+constexpr auto FCNTL_SYSCALL_ID = SYS_fcntl64;
+#else
+#error "fcntl and fcntl64 syscalls not available."
+#endif
+
 namespace LIBC_NAMESPACE::internal {
 
 int fcntl(int fd, int cmd, void *arg) {
@@ -33,7 +41,7 @@ int fcntl(int fd, int cmd, void *arg) {
     flk64.l_len = flk->l_len;
     flk64.l_pid = flk->l_pid;
     // create a syscall
-    return LIBC_NAMESPACE::syscall_impl<int>(SYS_fcntl, fd, cmd, &flk64);
+    return LIBC_NAMESPACE::syscall_impl<int>(FCNTL_SYSCALL_ID, fd, cmd, &flk64);
   }
   case F_OFD_GETLK:
   case F_OFD_SETLK: {
@@ -46,7 +54,7 @@ int fcntl(int fd, int cmd, void *arg) {
     flk64.l_len = flk->l_len;
     flk64.l_pid = flk->l_pid;
     // create a syscall
-    int retVal = LIBC_NAMESPACE::syscall_impl<int>(SYS_fcntl, fd, cmd, &flk64);
+    int retVal = LIBC_NAMESPACE::syscall_impl<int>(FCNTL_SYSCALL_ID, fd, cmd, &flk64);
     // On failure, return
     if (retVal == -1)
       return -1;
@@ -68,7 +76,7 @@ int fcntl(int fd, int cmd, void *arg) {
   case F_GETOWN: {
     struct f_owner_ex fex;
     int ret =
-        LIBC_NAMESPACE::syscall_impl<int>(SYS_fcntl, fd, F_GETOWN_EX, &fex);
+        LIBC_NAMESPACE::syscall_impl<int>(FCNTL_SYSCALL_ID, fd, F_GETOWN_EX, &fex);
     if (ret >= 0)
       return fex.type == F_OWNER_PGRP ? -fex.pid : fex.pid;
     libc_errno = -ret;
@@ -77,7 +85,7 @@ int fcntl(int fd, int cmd, void *arg) {
   // The general case
   default: {
     int retVal = LIBC_NAMESPACE::syscall_impl<int>(
-        SYS_fcntl, fd, cmd, reinterpret_cast<void *>(arg));
+        FCNTL_SYSCALL_ID, fd, cmd, reinterpret_cast<void *>(arg));
     if (retVal >= 0) {
       return retVal;
     }
diff --git a/libc/test/src/sys/mman/linux/CMakeLists.txt b/libc/test/src/sys/mman/linux/CMakeLists.txt
index 0762a2d846a8e0..b63c76f4306fac 100644
--- a/libc/test/src/sys/mman/linux/CMakeLists.txt
+++ b/libc/test/src/sys/mman/linux/CMakeLists.txt
@@ -138,6 +138,7 @@ add_libc_unittest(
     libc.include.sys_mman
     libc.include.sys_syscall
     libc.src.errno.errno
+    libc.src.fcntl.fcntl
     libc.src.sys.mman.shm_open
     libc.src.sys.mman.shm_unlink
     libc.src.sys.mman.mmap
diff --git a/libc/test/src/sys/mman/linux/shm_test.cpp b/libc/test/src/sys/mman/linux/shm_test.cpp
index 3b1a2aa33b56ad..4b8971f670581c 100644
--- a/libc/test/src/sys/mman/linux/shm_test.cpp
+++ b/libc/test/src/sys/mman/linux/shm_test.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/__support/OSUtil/syscall.h"
+#include "src/fcntl/fcntl.h"
 #include "src/sys/mman/mmap.h"
 #include "src/sys/mman/munmap.h"
 #include "src/sys/mman/shm_open.h"
@@ -29,9 +30,7 @@ TEST(LlvmLibcShmTest, Basic) {
               returns(GE(0)).with_errno(EQ(0)));
 
   // check that FD_CLOEXEC is set by default.
-  // TODO: use fcntl when implemented.
-  // https://github.com/llvm/llvm-project/issues/84968
-  long flag = LIBC_NAMESPACE::syscall_impl(SYS_fcntl, fd, F_GETFD);
+  long flag = LIBC_NAMESPACE::fcntl(fd, F_GETFD);
   ASSERT_GE(static_cast<int>(flag), 0);
   EXPECT_NE(static_cast<int>(flag) & FD_CLOEXEC, 0);
 

Copy link

github-actions bot commented Jul 4, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

This patch tries to implement fcntl with either SYS_fcntl or
SYS_fcntl64, and also changes a test case to call the fcntl function
instead of using the syscall
@mikhailramalho mikhailramalho merged commit 4f77677 into llvm:main Jul 4, 2024
4 of 5 checks passed
@mikhailramalho mikhailramalho deleted the fix-fcntl branch July 4, 2024 16:55
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
This patch tries to implement fcntl with either SYS_fcntl or SYS_fcntl64, and also changes a test case to call the fcntl function instead of using the syscall
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants