[libc-commits] [libc] [libc] start porting process unit tests to hermetic mode (PR #135124)

Schrodinger ZHU Yifan via libc-commits libc-commits at lists.llvm.org
Thu Apr 10 08:24:46 PDT 2025


https://github.com/SchrodingerZhu updated https://github.com/llvm/llvm-project/pull/135124

>From 68de66be0f2df49084601b661ba9e8623fb1db58 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i at zhuyi.fan>
Date: Wed, 9 Apr 2025 23:29:32 -0400
Subject: [PATCH 1/4] [libc] start porting process unit tests to hermetic mode

---
 libc/hdr/CMakeLists.txt                       |   9 ++
 libc/hdr/sys_wait_macros.h                    |  22 +++
 libc/test/UnitTest/CMakeLists.txt             |  17 +++
 libc/test/UnitTest/ExecuteFunctionUnix.cpp    | 128 ++++++++++++------
 libc/test/UnitTest/LibcDeathTestExecutors.cpp |   5 +-
 5 files changed, 135 insertions(+), 46 deletions(-)
 create mode 100644 libc/hdr/sys_wait_macros.h

diff --git a/libc/hdr/CMakeLists.txt b/libc/hdr/CMakeLists.txt
index db2dac9ff2822..d59d379305198 100644
--- a/libc/hdr/CMakeLists.txt
+++ b/libc/hdr/CMakeLists.txt
@@ -210,6 +210,15 @@ add_proxy_header_library(
     libc.include.sys_auxv
 )
 
+add_proxy_header_library(
+  sys_wait_macros
+  HDRS
+    sys_wait_macros.h
+  FULL_BUILD_DEPENDS
+    libc.include.llvm-libc-macros.sys_wait_macros
+    libc.include.sys_wait
+)
+
 add_header_library(wchar_overlay HDRS wchar_overlay.h)
 
 add_proxy_header_library(
diff --git a/libc/hdr/sys_wait_macros.h b/libc/hdr/sys_wait_macros.h
new file mode 100644
index 0000000000000..132702a81f797
--- /dev/null
+++ b/libc/hdr/sys_wait_macros.h
@@ -0,0 +1,22 @@
+//===-- Definition of macros from sys/wait.h ------------------------------===//
+//
+// 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_SYS_WAIT_MACROS_H
+#define LLVM_LIBC_HDR_SYS_WAIT_MACROS_H
+
+#ifdef LIBC_FULL_BUILD
+
+#include "include/llvm-libc-macros/sys-wait-macros.h"
+
+#else // Overlay mode
+
+#include <sys/wait.h>
+
+#endif // LLVM_LIBC_FULL_BUILD
+
+#endif // LLVM_LIBC_HDR_SYS_WAIT_MACROS_H
diff --git a/libc/test/UnitTest/CMakeLists.txt b/libc/test/UnitTest/CMakeLists.txt
index b0a3a7431c222..bb86db65e11bc 100644
--- a/libc/test/UnitTest/CMakeLists.txt
+++ b/libc/test/UnitTest/CMakeLists.txt
@@ -91,6 +91,23 @@ add_unittest_framework_library(
     ${libc_death_test_srcs}
   HDRS
     ExecuteFunction.h
+  DEPENDS
+    libc.hdr.sys_epoll_macros
+    libc.hdr.sys_wait_macros
+    libc.src.__support.CPP.new
+    libc.src.__support.common
+    libc.src.__support.libc_assert
+    libc.src.signal.kill
+    libc.src.stdio.fflush
+    libc.src.stdlib.exit
+    libc.src.string.strsignal
+    libc.src.sys.epoll.epoll_create1
+    libc.src.sys.epoll.epoll_ctl
+    libc.src.sys.epoll.epoll_wait
+    libc.src.sys.wait.waitpid
+    libc.src.unistd.close
+    libc.src.unistd.fork
+    libc.src.unistd.pipe
 )
 
 add_unittest_framework_library(
diff --git a/libc/test/UnitTest/ExecuteFunctionUnix.cpp b/libc/test/UnitTest/ExecuteFunctionUnix.cpp
index c0e85c2144005..39d4263ef6468 100644
--- a/libc/test/UnitTest/ExecuteFunctionUnix.cpp
+++ b/libc/test/UnitTest/ExecuteFunctionUnix.cpp
@@ -6,17 +6,26 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "src/__support/CPP/new.h"
+
 #include "ExecuteFunction.h"
+// TODO: for now, we use epoll directly. Use poll for unix compatibility when it
+// is available.
+#include "hdr/sys_epoll_macros.h"
+#include "hdr/sys_wait_macros.h"
+#include "src/__support/libc_assert.h"
 #include "src/__support/macros/config.h"
-#include "test/UnitTest/ExecuteFunction.h" // FunctionCaller
-#include <assert.h>
-#include <poll.h>
-#include <signal.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <sys/wait.h>
-#include <unistd.h>
+#include "src/signal/kill.h"
+#include "src/stdio/fflush.h"
+#include "src/stdlib/exit.h"
+#include "src/string/strsignal.h"
+#include "src/sys/epoll/epoll_create1.h"
+#include "src/sys/epoll/epoll_ctl.h"
+#include "src/sys/epoll/epoll_wait.h"
+#include "src/sys/wait/waitpid.h"
+#include "src/unistd/close.h"
+#include "src/unistd/fork.h"
+#include "src/unistd/pipe.h"
 
 namespace LIBC_NAMESPACE_DECL {
 namespace testutils {
@@ -24,7 +33,7 @@ namespace testutils {
 bool ProcessStatus::exited_normally() { return WIFEXITED(platform_defined); }
 
 int ProcessStatus::get_exit_code() {
-  assert(exited_normally() && "Abnormal termination, no exit code");
+  LIBC_ASSERT(exited_normally() && "Abnormal termination, no exit code");
   return WEXITSTATUS(platform_defined);
 }
 
@@ -34,59 +43,92 @@ int ProcessStatus::get_fatal_signal() {
   return WTERMSIG(platform_defined);
 }
 
-ProcessStatus invoke_in_subprocess(FunctionCaller *func, int timeout_ms) {
+template <typename T> struct DeleteGuard {
+  T *ptr;
+  DeleteGuard(T *p) : ptr(p) {}
+  ~DeleteGuard() {
+    if (ptr)
+      delete ptr;
+  }
+};
+
+ProcessStatus invoke_in_subprocess(FunctionCaller *func, unsigned timeout_ms) {
+  DeleteGuard<FunctionCaller> guard(func);
+
   int pipe_fds[2];
-  if (::pipe(pipe_fds) == -1) {
-    delete func;
+  if (LIBC_NAMESPACE::pipe(pipe_fds) == -1)
     return ProcessStatus::error("pipe(2) failed");
-  }
 
   // Don't copy the buffers into the child process and print twice.
-  ::fflush(stderr);
-  ::fflush(stdout);
-  pid_t pid = ::fork();
-  if (pid == -1) {
-    delete func;
+  LIBC_NAMESPACE::fflush(stdout);
+  LIBC_NAMESPACE::fflush(stderr);
+  pid_t pid = fork();
+  if (pid == -1)
     return ProcessStatus::error("fork(2) failed");
-  }
 
   if (!pid) {
     (*func)();
-    delete func;
-    ::exit(0);
+    LIBC_NAMESPACE::exit(0);
+  }
+
+  LIBC_NAMESPACE::close(pipe_fds[1]);
+
+  // Create an epoll instance.
+  int epfd = LIBC_NAMESPACE::epoll_create1(0);
+  if (epfd == -1) {
+    return ProcessStatus::error("epoll_create1(2) failed");
+  }
+
+  // Register the pipe FD with epoll. We monitor for reads (EPOLLIN)
+  // plus any half-close/hangup events (EPOLLRDHUP). epoll will set
+  // EPOLLHUP automatically if the peer closes, but typically you also
+  // include EPOLLIN or EPOLLRDHUP in the event mask.
+  epoll_event ev = {};
+  ev.events = EPOLLIN | EPOLLRDHUP;
+  ev.data.fd = pipe_fds[0];
+
+  if (LIBC_NAMESPACE::epoll_ctl(epfd, EPOLL_CTL_ADD, pipe_fds[0], &ev) == -1) {
+    LIBC_NAMESPACE::close(epfd);
+    return ProcessStatus::error("epoll_ctl(2) failed");
+  }
+
+  // Block until epoll signals an event or times out.
+  epoll_event result = {};
+  int nfds = LIBC_NAMESPACE::epoll_wait(epfd, &result, 1, timeout_ms);
+  LIBC_NAMESPACE::close(
+      epfd); // We’re done with the epoll FD regardless of outcome.
+
+  if (nfds == -1) {
+    return ProcessStatus::error("epoll_wait(2) failed");
   }
-  ::close(pipe_fds[1]);
-
-  struct pollfd poll_fd {
-    pipe_fds[0], 0, 0
-  };
-  // No events requested so this call will only return after the timeout or if
-  // the pipes peer was closed, signaling the process exited.
-  if (::poll(&poll_fd, 1, timeout_ms) == -1) {
-    delete func;
-    return ProcessStatus::error("poll(2) failed");
+
+  // If no FDs became “ready,” the child didn’t close the pipe before the
+  // timeout.
+  if (nfds == 0) {
+    LIBC_NAMESPACE::kill(pid, SIGKILL);
+    return ProcessStatus::timed_out_ps();
   }
-  // If the pipe wasn't closed by the child yet then timeout has expired.
-  if (!(poll_fd.revents & POLLHUP)) {
-    ::kill(pid, SIGKILL);
-    delete func;
+
+  // If we did get an event, check for EPOLLHUP (or EPOLLRDHUP).
+  // If those are not set, the pipe wasn't closed in the manner we expected.
+  if (!(result.events & (EPOLLHUP | EPOLLRDHUP))) {
+    LIBC_NAMESPACE::kill(pid, SIGKILL);
     return ProcessStatus::timed_out_ps();
   }
 
   int wstatus = 0;
   // Wait on the pid of the subprocess here so it gets collected by the system
   // and doesn't turn into a zombie.
-  pid_t status = ::waitpid(pid, &wstatus, 0);
-  if (status == -1) {
-    delete func;
+  pid_t status = LIBC_NAMESPACE::waitpid(pid, &wstatus, 0);
+  if (status == -1)
     return ProcessStatus::error("waitpid(2) failed");
-  }
-  assert(status == pid);
-  delete func;
+  LIBC_ASSERT(status == pid);
   return {wstatus};
 }
 
-const char *signal_as_string(int signum) { return ::strsignal(signum); }
+const char *signal_as_string(int signum) {
+  return LIBC_NAMESPACE::strsignal(signum);
+}
 
 } // namespace testutils
 } // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/test/UnitTest/LibcDeathTestExecutors.cpp b/libc/test/UnitTest/LibcDeathTestExecutors.cpp
index 943e2c23c5fde..fdb9aa796ecf1 100644
--- a/libc/test/UnitTest/LibcDeathTestExecutors.cpp
+++ b/libc/test/UnitTest/LibcDeathTestExecutors.cpp
@@ -8,12 +8,11 @@
 
 #include "LibcTest.h"
 
+#include "src/__support/libc_assert.h"
 #include "src/__support/macros/config.h"
 #include "test/UnitTest/ExecuteFunction.h"
 #include "test/UnitTest/TestLogger.h"
 
-#include <assert.h>
-
 namespace {
 constexpr unsigned TIMEOUT_MS = 10000;
 } // Anonymous namespace
@@ -50,7 +49,7 @@ bool Test::testProcessKilled(testutils::FunctionCaller *Func, int Signal,
   }
 
   int KilledBy = Result.get_fatal_signal();
-  assert(KilledBy != 0 && "Not killed by any signal");
+  LIBC_ASSERT(KilledBy != 0 && "Not killed by any signal");
   if (Signal == -1 || KilledBy == Signal)
     return true;
 

>From db135c611c8d841a5e199d863a4434faf5036200 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Thu, 10 Apr 2025 11:05:32 -0400
Subject: [PATCH 2/4] adjust

---
 libc/test/UnitTest/ExecuteFunctionUnix.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libc/test/UnitTest/ExecuteFunctionUnix.cpp b/libc/test/UnitTest/ExecuteFunctionUnix.cpp
index 39d4263ef6468..5f737c6a10959 100644
--- a/libc/test/UnitTest/ExecuteFunctionUnix.cpp
+++ b/libc/test/UnitTest/ExecuteFunctionUnix.cpp
@@ -47,13 +47,12 @@ template <typename T> struct DeleteGuard {
   T *ptr;
   DeleteGuard(T *p) : ptr(p) {}
   ~DeleteGuard() {
-    if (ptr)
       delete ptr;
   }
 };
 
 ProcessStatus invoke_in_subprocess(FunctionCaller *func, unsigned timeout_ms) {
-  DeleteGuard<FunctionCaller> guard(func);
+  DeleteGuard guard(func);
 
   int pipe_fds[2];
   if (LIBC_NAMESPACE::pipe(pipe_fds) == -1)

>From 6016154c52a16a3c473601b2c152dd90da0d2401 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Thu, 10 Apr 2025 11:10:31 -0400
Subject: [PATCH 3/4] adjust

---
 libc/test/UnitTest/ExecuteFunctionUnix.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libc/test/UnitTest/ExecuteFunctionUnix.cpp b/libc/test/UnitTest/ExecuteFunctionUnix.cpp
index 5f737c6a10959..4f4f63eabec95 100644
--- a/libc/test/UnitTest/ExecuteFunctionUnix.cpp
+++ b/libc/test/UnitTest/ExecuteFunctionUnix.cpp
@@ -104,14 +104,14 @@ ProcessStatus invoke_in_subprocess(FunctionCaller *func, unsigned timeout_ms) {
   // If no FDs became “ready,” the child didn’t close the pipe before the
   // timeout.
   if (nfds == 0) {
-    LIBC_NAMESPACE::kill(pid, SIGKILL);
+    while (LIBC_NAMESPACE::kill(pid, SIGKILL) == 0);
     return ProcessStatus::timed_out_ps();
   }
 
   // If we did get an event, check for EPOLLHUP (or EPOLLRDHUP).
   // If those are not set, the pipe wasn't closed in the manner we expected.
   if (!(result.events & (EPOLLHUP | EPOLLRDHUP))) {
-    LIBC_NAMESPACE::kill(pid, SIGKILL);
+    while (LIBC_NAMESPACE::kill(pid, SIGKILL) == 0);
     return ProcessStatus::timed_out_ps();
   }
 

>From 7e11c005bb0e86f0e74cb77a1cce226783cb6aef Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Thu, 10 Apr 2025 11:24:22 -0400
Subject: [PATCH 4/4] make unit test pass

---
 libc/test/UnitTest/CMakeLists.txt          |  3 ++
 libc/test/UnitTest/ExecuteFunction.h       |  2 +-
 libc/test/UnitTest/ExecuteFunctionUnix.cpp | 41 ++++++++++++++++++----
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/libc/test/UnitTest/CMakeLists.txt b/libc/test/UnitTest/CMakeLists.txt
index bb86db65e11bc..dcd3b71765dec 100644
--- a/libc/test/UnitTest/CMakeLists.txt
+++ b/libc/test/UnitTest/CMakeLists.txt
@@ -25,6 +25,9 @@ function(add_unittest_framework_library name)
     endif()
   endforeach()
 
+  target_compile_definitions(${name}.unit PRIVATE LIBC_UNIT_TEST_FRAMEWORK)
+  target_compile_definitions(${name}.hermetic PRIVATE LIBC_HERMETIC_TEST_FRAMEWORK)
+
   if(LLVM_LIBC_FULL_BUILD)
     # TODO: Build test framework with LIBC_FULL_BUILD in full build mode after
     # making LibcFPExceptionHelpers and LibcDeathTestExecutors hermetic.
diff --git a/libc/test/UnitTest/ExecuteFunction.h b/libc/test/UnitTest/ExecuteFunction.h
index 93ab6e9733804..878f621e9dee0 100644
--- a/libc/test/UnitTest/ExecuteFunction.h
+++ b/libc/test/UnitTest/ExecuteFunction.h
@@ -43,7 +43,7 @@ struct ProcessStatus {
 };
 
 ProcessStatus invoke_in_subprocess(FunctionCaller *func,
-                                   int timeout_ms = ProcessStatus::TIMEOUT);
+                                   unsigned timeout_ms = ProcessStatus::TIMEOUT);
 
 const char *signal_as_string(int signum);
 
diff --git a/libc/test/UnitTest/ExecuteFunctionUnix.cpp b/libc/test/UnitTest/ExecuteFunctionUnix.cpp
index 4f4f63eabec95..a7e4e2e4f4737 100644
--- a/libc/test/UnitTest/ExecuteFunctionUnix.cpp
+++ b/libc/test/UnitTest/ExecuteFunctionUnix.cpp
@@ -6,9 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#ifdef LIBC_HERMETIC_TEST_FRAMEWORK
 #include "src/__support/CPP/new.h"
-
-#include "ExecuteFunction.h"
 // TODO: for now, we use epoll directly. Use poll for unix compatibility when it
 // is available.
 #include "hdr/sys_epoll_macros.h"
@@ -26,6 +25,32 @@
 #include "src/unistd/close.h"
 #include "src/unistd/fork.h"
 #include "src/unistd/pipe.h"
+#else
+#include "ExecuteFunction.h"
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/epoll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+namespace LIBC_NAMESPACE_DECL {
+using ::close;
+using ::exit;
+using ::fflush;
+using ::fork;
+using ::kill;
+using ::pipe;
+using ::waitpid;
+using ::epoll_create1;
+using ::epoll_ctl;
+using ::epoll_wait;
+using ::strsignal;
+#define LIBC_ASSERT(...) assert(__VA_ARGS__)
+} // namespace LIBC_NAMESPACE_DECL
+#endif
 
 namespace LIBC_NAMESPACE_DECL {
 namespace testutils {
@@ -46,10 +71,10 @@ int ProcessStatus::get_fatal_signal() {
 template <typename T> struct DeleteGuard {
   T *ptr;
   DeleteGuard(T *p) : ptr(p) {}
-  ~DeleteGuard() {
-      delete ptr;
-  }
+  ~DeleteGuard() { delete ptr; }
 };
+// deduction guide
+template <typename T> DeleteGuard(T *) -> DeleteGuard<T>;
 
 ProcessStatus invoke_in_subprocess(FunctionCaller *func, unsigned timeout_ms) {
   DeleteGuard guard(func);
@@ -104,14 +129,16 @@ ProcessStatus invoke_in_subprocess(FunctionCaller *func, unsigned timeout_ms) {
   // If no FDs became “ready,” the child didn’t close the pipe before the
   // timeout.
   if (nfds == 0) {
-    while (LIBC_NAMESPACE::kill(pid, SIGKILL) == 0);
+    while (LIBC_NAMESPACE::kill(pid, SIGKILL) == 0)
+      ;
     return ProcessStatus::timed_out_ps();
   }
 
   // If we did get an event, check for EPOLLHUP (or EPOLLRDHUP).
   // If those are not set, the pipe wasn't closed in the manner we expected.
   if (!(result.events & (EPOLLHUP | EPOLLRDHUP))) {
-    while (LIBC_NAMESPACE::kill(pid, SIGKILL) == 0);
+    while (LIBC_NAMESPACE::kill(pid, SIGKILL) == 0)
+      ;
     return ProcessStatus::timed_out_ps();
   }
 



More information about the libc-commits mailing list