[libc] update abort implementation and lift it for internal usage (#189756)

libc/src/signal/linux/CMakeLists.txt

@@ -11,6 +11,7 @@ add_header_library(
     libc.src.__support.OSUtil.linux.vdso
     libc.src.__support.OSUtil.osutil
     libc.src.__support.error_or
+    libc.src.__support.threads.raw_rwlock
 )
 
 add_entrypoint_object(

libc/src/signal/linux/sigaction.cpp

@@ -18,7 +18,7 @@ namespace LIBC_NAMESPACE_DECL {
 LLVM_LIBC_FUNCTION(int, sigaction,
                    (int signal, const struct sigaction *__restrict libc_new,
                     struct sigaction *__restrict libc_old)) {
-  ErrorOr<int> ret = do_sigaction(signal, libc_new, libc_old);
+  ErrorOr<int> ret = checked_sigaction(signal, libc_new, libc_old);
   if (ret)
     return ret.value();
 

libc/src/signal/linux/signal_utils.h

@@ -19,6 +19,7 @@
 #include "src/__support/common.h"
 #include "src/__support/error_or.h"
 #include "src/__support/macros/config.h"
+#include "src/__support/threads/raw_rwlock.h"
 
 #include <sys/syscall.h> // For syscall numbers.
 
@@ -113,9 +114,50 @@ LIBC_INLINE int restore_signals(const sigset_t &set) {
                                            &set, nullptr, sizeof(sigset_t));
 }
 
+LIBC_INLINE int unblock_signal(int signal) {
+  sigset_t set = empty_set();
+  if (!add_signal(set, signal))
+    return -EINVAL;
+  return LIBC_NAMESPACE::syscall_impl<int>(SYS_rt_sigprocmask, SIG_UNBLOCK,
+                                           &set, nullptr, sizeof(sigset_t));
+}
+
+// This guard is used to:
+// 1. temporarily block the all signal, avoid post fork invalid state to be
+//    exposed to async signal handlers.
+// 2. ensure the ordering between sigaction and fork/spawn, so that forked
+//    processes can see modification from a just returned concurrent call.
+class SigAbortGuard {
+private:
+  sigset_t old_mask;
+  LIBC_INLINE_VAR static RawRwLock abort_lock;
+
+public:
+  LIBC_INLINE constexpr SigAbortGuard(bool exclusive) : old_mask{} {
+    RawRwLock::LockResult result = RawRwLock::LockResult::Success;
+    do {
+      if (exclusive)
+        result = abort_lock.write_lock(cpp::nullopt);
+      else
+        result = abort_lock.read_lock(cpp::nullopt);
+    } while (result == RawRwLock::LockResult::Overflow);
+
+    // This uses a valid sigset_t size and internal storage. A failure here
+    // would indicate a kernel ABI mismatch, which is not actionable here.
+    block_all_signals(old_mask);
+  }
+
+  LIBC_INLINE ~SigAbortGuard() {
+    // This restores a previously saved mask from internal storage. A failure
+    // here would likewise be a non-recoverable kernel ABI issue.
+    restore_signals(old_mask);
+    (void)abort_lock.unlock();
+  }
+};
+
 LIBC_INLINE ErrorOr<int>
-do_sigaction(int signal, const struct sigaction *__restrict libc_new,
-             struct sigaction *__restrict libc_old) {
+unchecked_sigaction(int signal, const struct sigaction *__restrict libc_new,
+                    struct sigaction *__restrict libc_old) {
   vdso::TypedSymbol<vdso::VDSOSym::RTSigReturn> rt_sigreturn;
   KernelSigaction kernel_new;
   if (libc_new) {
@@ -138,6 +180,18 @@ do_sigaction(int signal, const struct sigaction *__restrict libc_new,
   return 0;
 }
 
+LIBC_INLINE ErrorOr<int>
+checked_sigaction(int signal, const struct sigaction *__restrict libc_new,
+                  struct sigaction *__restrict libc_old) {
+  if (signal <= 0 || signal >= NSIG)
+    return Error(EINVAL);
+  if (signal == SIGABRT) {
+    SigAbortGuard guard(true);
+    return unchecked_sigaction(signal, libc_new, libc_old);
+  }
+  return unchecked_sigaction(signal, libc_new, libc_old);
+}
+
 } // namespace LIBC_NAMESPACE_DECL
 
 #endif // LLVM_LIBC_SRC_SIGNAL_LINUX_SIGNAL_UTILS_H

libc/src/spawn/linux/CMakeLists.txt

@@ -13,4 +13,5 @@ add_entrypoint_object(
     libc.src.__support.CPP.optional
     libc.src.__support.OSUtil.osutil
     libc.src.spawn.file_actions
+    libc.src.signal.linux.signal_utils
 )

libc/src/spawn/linux/posix_spawn.cpp

@@ -16,6 +16,7 @@
 
 #include "hdr/fcntl_macros.h"
 #include "hdr/types/mode_t.h"
+#include "src/signal/linux/signal_utils.h"
 #include <signal.h> // For SIGCHLD
 #include <spawn.h>
 #include <sys/syscall.h> // For syscall numbers.
@@ -25,6 +26,8 @@ namespace LIBC_NAMESPACE_DECL {
 namespace {
 
 pid_t fork() {
+  // Block signal and stop abort sigaction modification.
+  SigAbortGuard guard(/*exclusive=*/false);
   // TODO: Use only the clone syscall and use a sperate small stack in the child
   // to avoid duplicating the complete stack from the parent. A new stack will
   // be created on exec anyway so duplicating the full stack is unnecessary.

libc/src/stdlib/linux/CMakeLists.txt

@@ -1,3 +1,14 @@
+add_header_library(
+  abort_utils
+  HDRS
+    abort_utils.h
+  DEPENDS
+    libc.src.__support.OSUtil.linux.syscall_wrappers.raise
+    libc.src.__support.OSUtil.osutil
+    libc.src.signal.linux.__restore
+    libc.src.signal.linux.signal_utils
+)
+
 add_entrypoint_object(
   abort
   SRCS
@@ -5,7 +16,7 @@ add_entrypoint_object(
   HDRS
     ../abort.h
   DEPENDS
+    .abort_utils
+    libc.src.signal.linux.__restore
     libc.include.stdlib
-    libc.src.signal.raise
-    libc.src.stdlib._Exit
 )

libc/src/stdlib/linux/abort.cpp

@@ -6,26 +6,11 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "src/__support/common.h"
-#include "src/__support/macros/config.h"
-#include "src/signal/raise.h"
-#include "src/stdlib/_Exit.h"
-
 #include "src/stdlib/abort.h"
+#include "src/stdlib/linux/abort_utils.h"
 
 namespace LIBC_NAMESPACE_DECL {
 
-LLVM_LIBC_FUNCTION(void, abort, ()) {
-  // TODO: When sigprocmask and sigaction land:
-  // Unblock SIGABRT, raise it, if it was ignored or the handler returned,
-  // change its action to SIG_DFL, raise it again.
-  // TODO: When C11 mutexes land:
-  // Acquire recursive mutex (in case the current signal handler for SIGABRT
-  // itself calls abort we don't want to deadlock on the same thread trying
-  // to acquire it's own mutex.)
-  LIBC_NAMESPACE::raise(SIGABRT);
-  LIBC_NAMESPACE::raise(SIGKILL);
-  LIBC_NAMESPACE::_Exit(127);
-}
+LLVM_LIBC_FUNCTION(void, abort, ()) { abort_utils::abort(); }
 
 } // namespace LIBC_NAMESPACE_DECL

libc/src/stdlib/linux/abort_utils.h

@@ -0,0 +1,54 @@
+//===-- Internal header for Linux abort -------------------------*- 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_STDLIB_LINUX_ABORT_UTILS_H
+#define LLVM_LIBC_SRC_STDLIB_LINUX_ABORT_UTILS_H
+
+#include "hdr/types/sigset_t.h"
+#include "hdr/types/struct_sigaction.h"
+#include "src/__support/OSUtil/exit.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/raise.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/config.h"
+#include "src/signal/linux/signal_utils.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+namespace abort_utils {
+[[noreturn]] LIBC_INLINE void abort() {
+  // Try to raise SIGABRT.
+  // If this fails, or if a handler returns, keep going with the hard-abort
+  // sequence below.
+  linux_syscalls::raise(SIGABRT);
+
+  // We get back from abort, potentially from a abort handler.
+  // We recover the handler to default and raise it again. Since this is the
+  // real abort routine, we demand exclusive access to the abort lock.
+  // We have already returned from the first raise, so it is okay to grab
+  // exclusive access.
+  SigAbortGuard guard(true);
+  struct sigaction sa{};
+  sa.sa_handler = SIG_DFL;
+  sa.sa_flags = 0;
+  // There is no recovery path from sigaction failure while aborting.
+  unchecked_sigaction(SIGABRT, &sa, nullptr);
+  // If this still returns, fall through to the final termination path.
+  linux_syscalls::raise(SIGABRT);
+
+  // Now unblock the signal. The pending abort signal is now unblocked and
+  // should be delivered to its default handler.
+  // If this fails, there is still no meaningful recovery path while aborting.
+  unblock_signal(SIGABRT);
+
+  internal::exit(127);
+}
+} // namespace abort_utils
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDLIB_LINUX_ABORT_UTILS_H

libc/test/integration/src/stdlib/CMakeLists.txt

@@ -16,3 +16,26 @@ add_integration_test(
     FRANCE=Paris
     GERMANY=Berlin
 )
+
+if(${LIBC_TARGET_OS} STREQUAL "linux")
+  add_integration_test(
+    abort_test
+    SUITE
+      stdlib-integration-tests
+    SRCS
+      abort_test.cpp
+    DEPENDS
+      libc.include.signal
+      libc.include.sys_wait
+      libc.include.unistd
+      libc.src.signal.signal
+      libc.src.stdlib._Exit
+      libc.src.stdlib.abort
+      libc.src.sys.wait.waitpid
+      libc.src.unistd.close
+      libc.src.unistd.fork
+      libc.src.unistd.pipe
+      libc.src.unistd.read
+      libc.src.unistd.write
+  )
+endif()

libc/test/integration/src/stdlib/abort_test.cpp

@@ -0,0 +1,89 @@
+//===-- Integration tests for abort --------------------------------------===//
+//
+// 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/signal/signal.h"
+#include "src/stdlib/_Exit.h"
+#include "src/stdlib/abort.h"
+#include "src/stdlib/linux/abort_utils.h"
+#include "src/sys/wait/waitpid.h"
+#include "src/unistd/close.h"
+#include "src/unistd/fork.h"
+#include "src/unistd/pipe.h"
+#include "src/unistd/read.h"
+#include "src/unistd/write.h"
+
+#include "test/IntegrationTest/test.h"
+
+#include <signal.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+namespace {
+
+constexpr char HANDLER_MARKER = 'A';
+int handler_pipe_fd = -1;
+
+void expect_child_died_with_signal(pid_t pid, int signal) {
+  int status = 0;
+  ASSERT_EQ(LIBC_NAMESPACE::waitpid(pid, &status, 0), pid);
+  ASSERT_TRUE(WIFSIGNALED(status));
+  ASSERT_EQ(WTERMSIG(status), signal);
+}
+
+void child_abort() { LIBC_NAMESPACE::abort(); }
+
+void returning_sigabrt_handler(int) {
+  if (handler_pipe_fd >= 0)
+    LIBC_NAMESPACE::write(handler_pipe_fd, &HANDLER_MARKER, 1);
+}
+
+void child_abort_with_returning_handler() {
+  auto previous = LIBC_NAMESPACE::signal(SIGABRT, returning_sigabrt_handler);
+  ASSERT_NE(previous, SIG_ERR);
+  LIBC_NAMESPACE::abort();
+}
+
+void abort_kills_child_with_sigabrt() {
+  pid_t pid = LIBC_NAMESPACE::fork();
+  if (pid == 0)
+    child_abort();
+
+  ASSERT_TRUE(pid > 0);
+  expect_child_died_with_signal(pid, SIGABRT);
+}
+
+void abort_reraises_sigabrt_after_returning_handler() {
+  int pipefd[2];
+  ASSERT_EQ(LIBC_NAMESPACE::pipe(pipefd), 0);
+
+  pid_t pid = LIBC_NAMESPACE::fork();
+  if (pid == 0) {
+    LIBC_NAMESPACE::close(pipefd[0]);
+    handler_pipe_fd = pipefd[1];
+    child_abort_with_returning_handler();
+  }
+
+  ASSERT_TRUE(pid > 0);
+  ASSERT_EQ(LIBC_NAMESPACE::close(pipefd[1]), 0);
+
+  expect_child_died_with_signal(pid, SIGABRT);
+
+  char marker = 0;
+  ASSERT_EQ(LIBC_NAMESPACE::read(pipefd[0], &marker, 1), ssize_t(1));
+  ASSERT_EQ(marker, HANDLER_MARKER);
+  ASSERT_EQ(LIBC_NAMESPACE::close(pipefd[0]), 0);
+}
+
+} // namespace
+
+TEST_MAIN([[maybe_unused]] int argc, [[maybe_unused]] char **argv,
+          [[maybe_unused]] char **envp) {
+  abort_kills_child_with_sigabrt();
+  abort_reraises_sigabrt_after_returning_handler();
+  return 0;
+}