From 463f6cb576fc90bd4ffdf56fe9b19e42ea02604d Mon Sep 17 00:00:00 2001 From: Schrodinger ZHU Yifan Date: Fri, 3 Apr 2026 18:40:52 -0400 Subject: [PATCH] [libc] update abort implementation and lift it for internal usage (#189756) --- libc/src/signal/linux/CMakeLists.txt | 1 + libc/src/signal/linux/sigaction.cpp | 2 +- libc/src/signal/linux/signal_utils.h | 58 +++++++++++- libc/src/spawn/linux/CMakeLists.txt | 1 + libc/src/spawn/linux/posix_spawn.cpp | 3 + libc/src/stdlib/linux/CMakeLists.txt | 15 +++- libc/src/stdlib/linux/abort.cpp | 19 +--- libc/src/stdlib/linux/abort_utils.h | 54 +++++++++++ .../integration/src/stdlib/CMakeLists.txt | 23 +++++ .../integration/src/stdlib/abort_test.cpp | 89 +++++++++++++++++++ 10 files changed, 243 insertions(+), 22 deletions(-) create mode 100644 libc/src/stdlib/linux/abort_utils.h create mode 100644 libc/test/integration/src/stdlib/abort_test.cpp diff --git a/libc/src/signal/linux/CMakeLists.txt b/libc/src/signal/linux/CMakeLists.txt index 44c41737b89b..45ffb6994573 100644 --- a/libc/src/signal/linux/CMakeLists.txt +++ b/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( diff --git a/libc/src/signal/linux/sigaction.cpp b/libc/src/signal/linux/sigaction.cpp index 9e76c188ed44..c76371827846 100644 --- a/libc/src/signal/linux/sigaction.cpp +++ b/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 ret = do_sigaction(signal, libc_new, libc_old); + ErrorOr ret = checked_sigaction(signal, libc_new, libc_old); if (ret) return ret.value(); diff --git a/libc/src/signal/linux/signal_utils.h b/libc/src/signal/linux/signal_utils.h index be4501c27dba..20504b1b5fce 100644 --- a/libc/src/signal/linux/signal_utils.h +++ b/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 // 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(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 -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 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 +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 diff --git a/libc/src/spawn/linux/CMakeLists.txt b/libc/src/spawn/linux/CMakeLists.txt index 26148fe1c76d..a01125a4ae44 100644 --- a/libc/src/spawn/linux/CMakeLists.txt +++ b/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 ) diff --git a/libc/src/spawn/linux/posix_spawn.cpp b/libc/src/spawn/linux/posix_spawn.cpp index f05815805dc0..1a4ec72ce0d0 100644 --- a/libc/src/spawn/linux/posix_spawn.cpp +++ b/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 // For SIGCHLD #include #include // 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. diff --git a/libc/src/stdlib/linux/CMakeLists.txt b/libc/src/stdlib/linux/CMakeLists.txt index 1d3c00a5e0dd..6f52acdc856a 100644 --- a/libc/src/stdlib/linux/CMakeLists.txt +++ b/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 ) diff --git a/libc/src/stdlib/linux/abort.cpp b/libc/src/stdlib/linux/abort.cpp index d78ea675593a..0374e9315cba 100644 --- a/libc/src/stdlib/linux/abort.cpp +++ b/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 diff --git a/libc/src/stdlib/linux/abort_utils.h b/libc/src/stdlib/linux/abort_utils.h new file mode 100644 index 000000000000..09be8a814a54 --- /dev/null +++ b/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 diff --git a/libc/test/integration/src/stdlib/CMakeLists.txt b/libc/test/integration/src/stdlib/CMakeLists.txt index 1773d9fc9f0f..cf39b7ceb2f8 100644 --- a/libc/test/integration/src/stdlib/CMakeLists.txt +++ b/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() diff --git a/libc/test/integration/src/stdlib/abort_test.cpp b/libc/test/integration/src/stdlib/abort_test.cpp new file mode 100644 index 000000000000..5b8a38655ef8 --- /dev/null +++ b/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 +#include +#include + +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; +}