[libc-commits] [libc] e566313 - [libc][signal] clean up usage of sighandler_t (#125745)

via libc-commits libc-commits at lists.llvm.org
Fri Feb 7 10:44:00 PST 2025 

Author: Nick Desaulniers
Date: 2025-02-07T10:43:56-08:00
New Revision: e566313a1fac1b290c98454cc52b485ae4f644c5

URL: https://github.com/llvm/llvm-project/commit/e566313a1fac1b290c98454cc52b485ae4f644c5
DIFF: https://github.com/llvm/llvm-project/commit/e566313a1fac1b290c98454cc52b485ae4f644c5.diff

LOG: [libc][signal] clean up usage of sighandler_t (#125745)


`man 3 signal`'s declaration has a face _only a mother could love_.

sighandler_t and __sighandler_t are not defined in the C standard, or POSIX.

They are helpful typedefs provided by glibc and the Linux kernel UAPI headers
respectively since working with function pointers' syntax can be painful. But
we should not rely on them; in C++ we have `auto*` and `using` statements.

Remove the proxy header, and only include a typedef for sighandler_t when
targeting Linux, for compatibility with glibc.

Fixes: #125598

Added: 
    libc/include/llvm-libc-types/sighandler_t.h

Modified: 
    libc/hdr/types/CMakeLists.txt
    libc/include/CMakeLists.txt
    libc/include/llvm-libc-macros/gpu/signal-macros.h
    libc/include/llvm-libc-macros/linux/signal-macros.h
    libc/include/llvm-libc-types/CMakeLists.txt
    libc/include/llvm-libc-types/struct_sigaction.h
    libc/include/signal.yaml
    libc/src/signal/linux/CMakeLists.txt
    libc/src/signal/linux/signal.cpp
    libc/src/signal/signal.h
    libc/test/UnitTest/FPExceptMatcher.cpp
    libc/test/src/signal/CMakeLists.txt
    libc/test/src/signal/signal_test.cpp

Removed: 
    libc/hdr/types/sighandler_t.h
    libc/include/llvm-libc-types/__sighandler_t.h


################################################################################
diff  --git a/libc/hdr/types/CMakeLists.txt b/libc/hdr/types/CMakeLists.txt
index dfc90009ef54a81..84a2647ba664d5f 100644
--- a/libc/hdr/types/CMakeLists.txt
+++ b/libc/hdr/types/CMakeLists.txt
@@ -250,15 +250,6 @@ add_proxy_header_library(
     libc.include.locale
 )
 
-add_proxy_header_library(
-  sighandler_t
-  HDRS
-    sighandler_t.h
-  FULL_BUILD_DEPENDS
-    libc.include.llvm-libc-types.__sighandler_t
-    libc.include.signal
-)
-
 add_proxy_header_library(
   stack_t
   HDRS

diff  --git a/libc/hdr/types/sighandler_t.h b/libc/hdr/types/sighandler_t.h
deleted file mode 100644
index bc40dd8b4c8f4a9..000000000000000
--- a/libc/hdr/types/sighandler_t.h
+++ /dev/null
@@ -1,24 +0,0 @@
-//===-- Definition of macros from __sighandler_t.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_TYPES_SIGHANDLER_T_H
-#define LLVM_LIBC_HDR_TYPES_SIGHANDLER_T_H
-
-#ifdef LIBC_FULL_BUILD
-
-#include "include/llvm-libc-types/__sighandler_t.h"
-
-using sighandler_t = __sighandler_t;
-
-#else // overlay mode
-
-#include <signal.h>
-
-#endif // LLVM_LIBC_FULL_BUILD
-
-#endif // LLVM_LIBC_HDR_TYPES_SIGHANDLER_T_H

diff  --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 63745542662d593..867bd1e5ee20f3d 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -284,13 +284,14 @@ add_header_macro(
   signal.h
   DEPENDS
     .llvm-libc-macros.signal_macros
+    .llvm-libc-types.pid_t
     .llvm-libc-types.sig_atomic_t
+    .llvm-libc-types.sighandler_t
+    .llvm-libc-types.siginfo_t
     .llvm-libc-types.sigset_t
+    .llvm-libc-types.stack_t
     .llvm-libc-types.struct_sigaction
     .llvm-libc-types.union_sigval
-    .llvm-libc-types.siginfo_t
-    .llvm-libc-types.stack_t
-    .llvm-libc-types.pid_t
 )
 
 add_header_macro(

diff  --git a/libc/include/llvm-libc-macros/gpu/signal-macros.h b/libc/include/llvm-libc-macros/gpu/signal-macros.h
index 2d8159240de8bf3..f0d49ea34fe0efe 100644
--- a/libc/include/llvm-libc-macros/gpu/signal-macros.h
+++ b/libc/include/llvm-libc-macros/gpu/signal-macros.h
@@ -16,9 +16,9 @@
 #define SIGSEGV 11
 #define SIGTERM 15
 
-#define SIG_DFL ((__sighandler_t)(0))
-#define SIG_IGN ((__sighandler_t)(1))
-#define SIG_ERR ((__sighandler_t)(-1))
+#define SIG_DFL ((void (*)(int))(0))
+#define SIG_IGN ((void (*)(int))(1))
+#define SIG_ERR ((void (*)(int))(-1))
 
 // Max signal number
 #define NSIG 64

diff  --git a/libc/include/llvm-libc-macros/linux/signal-macros.h b/libc/include/llvm-libc-macros/linux/signal-macros.h
index 0b7317ebc9b80a1..d220241a382069a 100644
--- a/libc/include/llvm-libc-macros/linux/signal-macros.h
+++ b/libc/include/llvm-libc-macros/linux/signal-macros.h
@@ -86,9 +86,9 @@
 #error "Signal stack sizes not defined for your platform."
 #endif
 
-#define SIG_DFL ((__sighandler_t)0)
-#define SIG_IGN ((__sighandler_t)1)
-#define SIG_ERR ((__sighandler_t)-1)
+#define SIG_DFL ((void (*)(int))0)
+#define SIG_IGN ((void (*)(int))1)
+#define SIG_ERR ((void (*)(int))(-1))
 
 // SIGCHLD si_codes
 #define CLD_EXITED 1    // child has exited

diff  --git a/libc/include/llvm-libc-types/CMakeLists.txt b/libc/include/llvm-libc-types/CMakeLists.txt
index 9e8d2f818d4ed48..7ed69ab1af6d9c3 100644
--- a/libc/include/llvm-libc-types/CMakeLists.txt
+++ b/libc/include/llvm-libc-types/CMakeLists.txt
@@ -15,7 +15,6 @@ add_header(__pthread_start_t HDR __pthread_start_t.h)
 add_header(__pthread_tss_dtor_t HDR __pthread_tss_dtor_t.h)
 add_header(__qsortcompare_t HDR __qsortcompare_t.h)
 add_header(__qsortrcompare_t HDR __qsortrcompare_t.h)
-add_header(__sighandler_t HDR __sighandler_t.h)
 add_header(__thread_type HDR __thread_type.h)
 add_header(blkcnt_t HDR blkcnt_t.h)
 add_header(blksize_t HDR blksize_t.h)
@@ -66,6 +65,7 @@ if(LIBC_TYPES_TIME_T_IS_32_BIT)
 else()
   add_header(time_t HDR time_t_64.h DEST_HDR time_t.h)
 endif()
+add_header(sighandler_t HDR sighandler_t.h)
 add_header(stack_t HDR stack_t.h DEPENDS .size_t)
 add_header(suseconds_t HDR suseconds_t.h)
 add_header(struct_dirent HDR struct_dirent.h DEPENDS .ino_t .off_t)

diff  --git a/libc/include/llvm-libc-types/__sighandler_t.h b/libc/include/llvm-libc-types/sighandler_t.h
similarity index 52%
rename from libc/include/llvm-libc-types/__sighandler_t.h
rename to libc/include/llvm-libc-types/sighandler_t.h
index 9c1ac997fc4ee06..f39ab04685200bc 100644
--- a/libc/include/llvm-libc-types/__sighandler_t.h
+++ b/libc/include/llvm-libc-types/sighandler_t.h
@@ -1,4 +1,4 @@
-//===-- Definition of struct __sighandler_t -------------------------------===//
+//===-- Definition of sighandler_t ----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,9 +6,12 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_LIBC_TYPES___SIGHANDLER_T_H
-#define LLVM_LIBC_TYPES___SIGHANDLER_T_H
+#ifndef LLVM_LIBC_TYPES_SIGHANDLER_T_H
+#define LLVM_LIBC_TYPES_SIGHANDLER_T_H
 
-typedef void (*__sighandler_t)(int);
+#ifdef __linux__
+// For compatibility with glibc.
+typedef void (*sighandler_t)(int);
+#endif
 
-#endif // LLVM_LIBC_TYPES___SIGHANDLER_T_H
+#endif // LLVM_LIBC_TYPES_SIGHANDLER_T_H

diff  --git a/libc/include/llvm-libc-types/struct_sigaction.h b/libc/include/llvm-libc-types/struct_sigaction.h
index b4d0c965a4c6338..907418b5e0f9a05 100644
--- a/libc/include/llvm-libc-types/struct_sigaction.h
+++ b/libc/include/llvm-libc-types/struct_sigaction.h
@@ -25,6 +25,4 @@ struct sigaction {
 #endif
 };
 
-typedef void (*__sighandler_t)(int);
-
 #endif // LLVM_LIBC_TYPES_STRUCT_SIGACTION_H

diff  --git a/libc/include/signal.yaml b/libc/include/signal.yaml
index 576e77576ac740b..6fdd8c97ccbe2c6 100644
--- a/libc/include/signal.yaml
+++ b/libc/include/signal.yaml
@@ -3,12 +3,13 @@ header_template: signal.h.def
 macros: []
 types:
   - type_name: pid_t
-  - type_name: stack_t
+  - type_name: sig_atomic_t
+  - type_name: sighandler_t
   - type_name: siginfo_t
-  - type_name: struct_sigaction
   - type_name: sigset_t
+  - type_name: stack_t
+  - type_name: struct_sigaction
   - type_name: union_sigval
-  - type_name: sig_atomic_t
 enums: []
 objects: []
 functions:
@@ -69,10 +70,15 @@ functions:
   - name: signal
     standards:
       - stdc
-    return_type: __sighandler_t
+    # May the Geneva Convention have mercy on my soul... Why this insanity?
+    # Well: signal returns a function pointer to a function with no return
+    # value and which accepts an int. The parameter list appears on the far
+    # right of the declaration. i.e.
+    # void (*signal(int, void (*)(int)))(int);
+    return_type: void (*
     arguments:
       - type: int
-      - type: __sighandler_t
+      - type: void (*)(int)))(int
   - name: sigprocmask
     standards:
       - POSIX

diff  --git a/libc/src/signal/linux/CMakeLists.txt b/libc/src/signal/linux/CMakeLists.txt
index f7457d31cf4f859..c0dd61e473881b7 100644
--- a/libc/src/signal/linux/CMakeLists.txt
+++ b/libc/src/signal/linux/CMakeLists.txt
@@ -127,7 +127,6 @@ add_entrypoint_object(
   DEPENDS
     .sigaction
     libc.hdr.signal_macros
-    libc.hdr.types.sighandler_t
 )
 
 add_entrypoint_object(

diff  --git a/libc/src/signal/linux/signal.cpp b/libc/src/signal/linux/signal.cpp
index 1da0ef8c97a206d..7c8ea16c6cd2e24 100644
--- a/libc/src/signal/linux/signal.cpp
+++ b/libc/src/signal/linux/signal.cpp
@@ -8,14 +8,17 @@
 
 #include "src/signal/signal.h"
 #include "hdr/signal_macros.h"
-#include "hdr/types/sighandler_t.h"
 #include "src/__support/common.h"
 #include "src/__support/macros/config.h"
 #include "src/signal/sigaction.h"
 
 namespace LIBC_NAMESPACE_DECL {
 
-LLVM_LIBC_FUNCTION(sighandler_t, signal, (int signum, sighandler_t handler)) {
+// Our LLVM_LIBC_FUNCTION macro doesn't handle function pointer return types.
+using signal_handler = void (*)(int);
+
+LLVM_LIBC_FUNCTION(signal_handler, signal,
+                   (int signum, signal_handler handler)) {
   struct sigaction action, old;
   action.sa_handler = handler;
   action.sa_flags = SA_RESTART;

diff  --git a/libc/src/signal/signal.h b/libc/src/signal/signal.h
index 06e77e11bf0bd3c..e1f31a8e126c58b 100644
--- a/libc/src/signal/signal.h
+++ b/libc/src/signal/signal.h
@@ -9,12 +9,11 @@
 #ifndef LLVM_LIBC_SRC_SIGNAL_SIGNAL_H
 #define LLVM_LIBC_SRC_SIGNAL_SIGNAL_H
 
-#include "hdr/types/sighandler_t.h"
 #include "src/__support/macros/config.h"
 
 namespace LIBC_NAMESPACE_DECL {
 
-sighandler_t signal(int signum, sighandler_t handler);
+void (*signal(int signum, void (*handler)(int)))(int);
 
 } // namespace LIBC_NAMESPACE_DECL
 

diff  --git a/libc/test/UnitTest/FPExceptMatcher.cpp b/libc/test/UnitTest/FPExceptMatcher.cpp
index 119a06985b8f1cd..d66066023984e53 100644
--- a/libc/test/UnitTest/FPExceptMatcher.cpp
+++ b/libc/test/UnitTest/FPExceptMatcher.cpp
@@ -37,7 +37,7 @@ static void sigfpeHandler(int sig) {
 }
 
 FPExceptMatcher::FPExceptMatcher(FunctionCaller *func) {
-  sighandler_t oldSIGFPEHandler = signal(SIGFPE, &sigfpeHandler);
+  auto *oldSIGFPEHandler = signal(SIGFPE, &sigfpeHandler);
 
   caughtExcept = false;
   fenv_t oldEnv;

diff  --git a/libc/test/src/signal/CMakeLists.txt b/libc/test/src/signal/CMakeLists.txt
index a27f5b8f1000e9b..f86ce2ae96857c4 100644
--- a/libc/test/src/signal/CMakeLists.txt
+++ b/libc/test/src/signal/CMakeLists.txt
@@ -74,7 +74,6 @@ add_libc_unittest(
   SRCS
     signal_test.cpp
   DEPENDS
-    libc.hdr.types.sighandler_t
     libc.src.errno.errno
     libc.src.signal.raise
     libc.src.signal.signal

diff  --git a/libc/test/src/signal/signal_test.cpp b/libc/test/src/signal/signal_test.cpp
index 4b57311eee2d86f..bac9c3b8b68bb27 100644
--- a/libc/test/src/signal/signal_test.cpp
+++ b/libc/test/src/signal/signal_test.cpp
@@ -13,14 +13,12 @@
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
 
-#include "hdr/types/sighandler_t.h"
-
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
 
 TEST(LlvmLibcSignal, Invalid) {
   LIBC_NAMESPACE::libc_errno = 0;
-  sighandler_t valid = +[](int) {};
+  auto *valid = +[](int) {};
   EXPECT_THAT((void *)LIBC_NAMESPACE::signal(0, valid),
               Fails(EINVAL, (void *)SIG_ERR));
   EXPECT_THAT((void *)LIBC_NAMESPACE::signal(65, valid),

More information about the libc-commits mailing list