Skip to content
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

[libc] Implement process_mrelease #117851

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

moar55
Copy link
Contributor

@moar55 moar55 commented Nov 27, 2024

This PR implements process_mrelease.
A previous PR was merged #117503, but failed on merge due to an issue in the tests. Namely the failing tests were comparing against return type as opposed to errno. This is fixed in this PR.

@llvmbot llvmbot added the libc label Nov 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 27, 2024

@llvm/pr-subscribers-libc

Author: Omar Hossam (moar55)

Changes

This PR implements process_mrelease.
A previous PR was merged #117503, but failed on merge due to an issue in the tests. Namely the failing tests were comparing against return type as opposed to errno. This is fixed in this PR.


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

12 Files Affected:

  • (modified) libc/config/linux/aarch64/entrypoints.txt (+1)
  • (modified) libc/config/linux/riscv/entrypoints.txt (+1)
  • (modified) libc/config/linux/x86_64/entrypoints.txt (+1)
  • (modified) libc/include/sys/syscall.h.def (+7)
  • (modified) libc/newhdrgen/yaml/sys/mman.yaml (+7)
  • (modified) libc/spec/linux.td (+6)
  • (modified) libc/src/sys/mman/CMakeLists.txt (+6)
  • (modified) libc/src/sys/mman/linux/CMakeLists.txt (+11-1)
  • (added) libc/src/sys/mman/linux/process_mrelease.cpp (+33)
  • (added) libc/src/sys/mman/process_mrelease.h (+21)
  • (modified) libc/test/src/sys/mman/linux/CMakeLists.txt (+20)
  • (added) libc/test/src/sys/mman/linux/process_mrelease_test.cpp (+74)
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 74ca3742977a5f..08b5072499da69 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -252,6 +252,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.sys.mman.munlockall
     libc.src.sys.mman.munmap
     libc.src.sys.mman.remap_file_pages
+    libc.src.sys.mman.process_mrelease
     libc.src.sys.mman.posix_madvise
     libc.src.sys.mman.shm_open
     libc.src.sys.mman.shm_unlink
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 5419462d4f5b3b..4ea65f76d7948d 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -251,6 +251,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.sys.mman.munmap
     libc.src.sys.mman.remap_file_pages
     libc.src.sys.mman.posix_madvise
+    libc.src.sys.mman.process_mrelease
     libc.src.sys.mman.shm_open
     libc.src.sys.mman.shm_unlink
 
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 957e28bd66cc4c..d0651c06b930ad 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -252,6 +252,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.sys.mman.munmap
     libc.src.sys.mman.remap_file_pages
     libc.src.sys.mman.posix_madvise
+    libc.src.sys.mman.process_mrelease
     libc.src.sys.mman.shm_open
     libc.src.sys.mman.shm_unlink
 
diff --git a/libc/include/sys/syscall.h.def b/libc/include/sys/syscall.h.def
index 03c19eb0885ed6..11758ea8336ddf 100644
--- a/libc/include/sys/syscall.h.def
+++ b/libc/include/sys/syscall.h.def
@@ -2349,5 +2349,12 @@
 #define SYS_writev __NR_writev
 #endif
 
+#ifdef __NR_process_mrelease
+#define SYS_process_mrelease __NR_process_mrelease
+#endif
+
+#ifdef __NR_pidfd_open
+#define SYS_pidfd_open __NR_pidfd_open
+#endif
 
 #endif // LLVM_LIBC_SYS_SYSCALL_H
diff --git a/libc/newhdrgen/yaml/sys/mman.yaml b/libc/newhdrgen/yaml/sys/mman.yaml
index 962ca3591917f7..dd53eb60d1ec57 100644
--- a/libc/newhdrgen/yaml/sys/mman.yaml
+++ b/libc/newhdrgen/yaml/sys/mman.yaml
@@ -132,3 +132,10 @@ functions:
     return_type: int
     arguments:
       - type: const char *
+  - name: process_mrelease
+    standards:
+      - Linux
+    return_type: int
+    arguments:
+      - type: int
+      - type: unsigned int
diff --git a/libc/spec/linux.td b/libc/spec/linux.td
index 9b5dc8e30c95e4..99e0949a592dfa 100644
--- a/libc/spec/linux.td
+++ b/libc/spec/linux.td
@@ -112,6 +112,12 @@ def Linux : StandardSpec<"Linux"> {
             ArgSpec<IntType>,
             ArgSpec<SizeTType>,
             ArgSpec<IntType>,
+        FunctionSpec<
+          "process_mrelease",
+          RetValSpec<IntType>,
+          [
+            ArgSpec<IntType>,
+            ArgSpec<UnsignedIntType>
           ]
         >,
         FunctionSpec<
diff --git a/libc/src/sys/mman/CMakeLists.txt b/libc/src/sys/mman/CMakeLists.txt
index 4d4c2ad376050e..281efc0ffcdf20 100644
--- a/libc/src/sys/mman/CMakeLists.txt
+++ b/libc/src/sys/mman/CMakeLists.txt
@@ -113,3 +113,9 @@ add_entrypoint_object(
   DEPENDS
     .${LIBC_TARGET_OS}.mremap
 )
+
+add_entrypoint_object(
+  process_mrelease 
+  ALIAS 
+  DEPENDS
+    .${LIBC_TARGET_OS}.process_mrelease)
diff --git a/libc/src/sys/mman/linux/CMakeLists.txt b/libc/src/sys/mman/linux/CMakeLists.txt
index 89a0ad1527a065..aa2ca4b160181a 100644
--- a/libc/src/sys/mman/linux/CMakeLists.txt
+++ b/libc/src/sys/mman/linux/CMakeLists.txt
@@ -36,7 +36,6 @@ add_entrypoint_object(
     libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
 )
-
 add_entrypoint_object(
   munmap
   SRCS
@@ -214,3 +213,14 @@ add_entrypoint_object(
     libc.src.unistd.unlink
     .shm_common
 )
+
+add_entrypoint_object(
+  process_mrelease
+  SRCS
+    process_mrelease.cpp
+  HDRS
+    ../process_mrelease.h
+  DEPENDS
+    libc.include.sys_syscall
+    libc.src.__support.OSUtil.osutil
+    libc.src.errno.errno)
diff --git a/libc/src/sys/mman/linux/process_mrelease.cpp b/libc/src/sys/mman/linux/process_mrelease.cpp
new file mode 100644
index 00000000000000..e86bbec1b1b661
--- /dev/null
+++ b/libc/src/sys/mman/linux/process_mrelease.cpp
@@ -0,0 +1,33 @@
+//===---------- Linux implementation of the mrelease function -----------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/sys/mman/process_mrelease.h"
+
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/common.h"
+
+#include "src/__support/macros/config.h"
+#include "src/errno/libc_errno.h"
+#include <linux/param.h> // For EXEC_PAGESIZE.
+#include <sys/syscall.h> // For syscall numbers.
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, process_mrelease, (int pidfd, unsigned int flags)) {
+  long ret =
+      LIBC_NAMESPACE::syscall_impl<int>(SYS_process_mrelease, pidfd, flags);
+
+  if (ret < 0) {
+    libc_errno = static_cast<int>(-ret);
+    return -1;
+  }
+
+  return 0;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/sys/mman/process_mrelease.h b/libc/src/sys/mman/process_mrelease.h
new file mode 100644
index 00000000000000..ec4a6e4768bcac
--- /dev/null
+++ b/libc/src/sys/mman/process_mrelease.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for process_mrelease function --------*- 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_SYS_MMAN_PROCESS_MRELEASE_H
+#define LLVM_LIBC_SRC_SYS_MMAN_PROCESS_MRELEASE_H
+
+#include "src/__support/macros/config.h"
+#include <sys/mman.h> // For size_t and off_t
+
+namespace LIBC_NAMESPACE_DECL {
+
+int process_mrelease(int pidfd, unsigned int flags);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_SYS_MMAN_PROCESS_MRELEASE_H
diff --git a/libc/test/src/sys/mman/linux/CMakeLists.txt b/libc/test/src/sys/mman/linux/CMakeLists.txt
index 44ed11aadfe8b7..6021ccd8c89a82 100644
--- a/libc/test/src/sys/mman/linux/CMakeLists.txt
+++ b/libc/test/src/sys/mman/linux/CMakeLists.txt
@@ -181,3 +181,23 @@ add_libc_unittest(
     libc.hdr.fcntl_macros
     libc.test.UnitTest.ErrnoSetterMatcher
 )
+
+add_libc_unittest(
+  process_mrelease_test
+  SUITE
+    libc_sys_mman_unittests
+  SRCS
+    process_mrelease_test.cpp
+  DEPENDS
+    libc.include.sys_mman
+    libc.include.sys_syscall
+    libc.src.errno.errno
+    libc.src.sys.mman.process_mrelease
+    libc.src.unistd.close
+    libc.src.signal.kill
+    libc.include.signal
+    libc.src.stdlib.exit
+    libc.src.__support.OSUtil.osutil
+    libc.src.__support.threads.sleep
+    libc.test.UnitTest.ErrnoSetterMatcher
+    )
diff --git a/libc/test/src/sys/mman/linux/process_mrelease_test.cpp b/libc/test/src/sys/mman/linux/process_mrelease_test.cpp
new file mode 100644
index 00000000000000..1c3c549aa9bebd
--- /dev/null
+++ b/libc/test/src/sys/mman/linux/process_mrelease_test.cpp
@@ -0,0 +1,74 @@
+//===-- Unittests for process_mrelease ------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/threads/sleep.h"
+#include "src/errno/libc_errno.h"
+#include "src/signal/kill.h"
+#include "src/stdlib/exit.h"
+#include "src/sys/mman/process_mrelease.h"
+#include "src/unistd/close.h"
+#include "src/unistd/fork.h"
+#include "test/UnitTest/ErrnoSetterMatcher.h"
+#include "test/UnitTest/LibcTest.h"
+
+#include <sys/syscall.h>
+
+using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;
+
+int pidfd_open(pid_t pid, unsigned int flags) {
+  return LIBC_NAMESPACE::syscall_impl(SYS_pidfd_open, pid, flags);
+}
+
+TEST(LlvmLibcMProcessMReleaseTest, NoError) {
+  pid_t child_pid = fork();
+  EXPECT_GE(child_pid, 0);
+
+  if (child_pid == 0) {
+    // Child process: wait a bit then exit gracefully.
+    LIBC_NAMESPACE::sleep_briefly();
+    LIBC_NAMESPACE::exit(0);
+  } else {
+    // Parent process: wait a bit and then kill the child.
+    // Give child process some time to start.
+    LIBC_NAMESPACE::sleep_briefly();
+    int pidfd = pidfd_open(child_pid, 0);
+    EXPECT_GE(pidfd, 0);
+
+    // Send SIGKILL to child process
+    LIBC_NAMESPACE::kill(child_pid, SIGKILL);
+
+    EXPECT_THAT(LIBC_NAMESPACE::process_mrelease(pidfd, 0), Succeeds());
+
+    LIBC_NAMESPACE::close(pidfd);
+  }
+}
+
+TEST(LlvmLibcMProcessMReleaseTest, ErrorNotKilled) {
+  pid_t child_pid = fork();
+  EXPECT_GE(child_pid, 0);
+
+  if (child_pid == 0) {
+    // Child process: wait a bit then exit gracefully.
+    LIBC_NAMESPACE::sleep_briefly();
+    LIBC_NAMESPACE::exit(0);
+  } else {
+    // Give child process some time to start.
+    LIBC_NAMESPACE::sleep_briefly();
+    int pidfd = pidfd_open(child_pid, 0);
+    EXPECT_GE(pidfd, 0);
+
+    EXPECT_THAT(LIBC_NAMESPACE::process_mrelease(pidfd, 0), Fails(EINVAL));
+
+    LIBC_NAMESPACE::close(pidfd);
+  }
+}
+
+TEST(LlvmLibcMProcessMReleaseTest, ErrorNonExistingPidfd) {
+  EXPECT_THAT(LIBC_NAMESPACE::process_mrelease(-1, 0), Fails(EBADF));
+}

@moar55
Copy link
Contributor Author

moar55 commented Nov 27, 2024

@SchrodingerZhu hey can you re-review this :)

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff b0bdbf4288a664179024eead871a5e7b7fae6dda f3a87b4246bf6578794b95253efee96932a5f447 --extensions h,cpp -- libc/src/sys/mman/linux/process_mrelease.cpp libc/src/sys/mman/process_mrelease.h libc/test/src/sys/mman/linux/process_mrelease_test.cpp
View the diff from clang-format here.
diff --git a/libc/src/sys/mman/process_mrelease.h b/libc/src/sys/mman/process_mrelease.h
index ec4a6e4768..45c896eb13 100644
--- a/libc/src/sys/mman/process_mrelease.h
+++ b/libc/src/sys/mman/process_mrelease.h
@@ -1,4 +1,5 @@
-//===-- Implementation header for process_mrelease function --------*- C++ -*-===//
+//===-- Implementation header for process_mrelease function --------*- C++
+//-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

@moar55
Copy link
Contributor Author

moar55 commented Nov 27, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff b0bdbf4288a664179024eead871a5e7b7fae6dda f3a87b4246bf6578794b95253efee96932a5f447 --extensions h,cpp -- libc/src/sys/mman/linux/process_mrelease.cpp libc/src/sys/mman/process_mrelease.h libc/test/src/sys/mman/linux/process_mrelease_test.cpp

View the diff from clang-format here.

diff --git a/libc/src/sys/mman/process_mrelease.h b/libc/src/sys/mman/process_mrelease.h
index ec4a6e4768..45c896eb13 100644
--- a/libc/src/sys/mman/process_mrelease.h
+++ b/libc/src/sys/mman/process_mrelease.h
@@ -1,4 +1,5 @@
-//===-- Implementation header for process_mrelease function --------*- C++ -*-===//
+//===-- Implementation header for process_mrelease function --------*- C++
+//-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

That doesn't look like the right format 🤔

@SchrodingerZhu
Copy link
Contributor

@moar55 it should be the case that the header is too long. You can shrink the license header by removing some -

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