[libc][signal] clean up usage of sighandler_t #125745

nickdesaulniers · 2025-02-04T19:04:36Z

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

llvmbot · 2025-02-04T19:05:13Z

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

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 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 typedefs for these two typedefs when
targeting Linux, for compatibility with glibc.

Also, adds a typedef for non-double-underscore-prefixed sighandler_t that we
were missing.

Fixes: #125598

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

14 Files Affected:

(modified) libc/hdr/types/CMakeLists.txt (-9)
(removed) libc/hdr/types/sighandler_t.h (-24)
(modified) libc/include/llvm-libc-macros/gpu/signal-macros.h (+3-3)
(modified) libc/include/llvm-libc-macros/linux/signal-macros.h (+3-3)
(modified) libc/include/llvm-libc-types/__sighandler_t.h (+5)
(modified) libc/include/llvm-libc-types/struct_sigaction.h (-2)
(modified) libc/include/signal.h.def (+4)
(modified) libc/include/signal.yaml (+10-5)
(modified) libc/src/signal/linux/CMakeLists.txt (-1)
(modified) libc/src/signal/linux/signal.cpp (+4-2)
(modified) libc/src/signal/signal.h (+1-2)
(modified) libc/test/UnitTest/FPExceptMatcher.cpp (+1-1)
(modified) libc/test/src/signal/CMakeLists.txt (-1)
(modified) libc/test/src/signal/signal_test.cpp (+1-3)

diff --git a/libc/hdr/types/CMakeLists.txt b/libc/hdr/types/CMakeLists.txt
index 3dfa38a020fad02..83c01fd6aeafefb 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/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..6afa7facd8091f6 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/__sighandler_t.h b/libc/include/llvm-libc-types/__sighandler_t.h
index 9c1ac997fc4ee06..0fa412568a6912b 100644
--- a/libc/include/llvm-libc-types/__sighandler_t.h
+++ b/libc/include/llvm-libc-types/__sighandler_t.h
@@ -9,6 +9,11 @@
 #ifndef LLVM_LIBC_TYPES___SIGHANDLER_T_H
 #define LLVM_LIBC_TYPES___SIGHANDLER_T_H
 
+#ifndef __linux__
+#error "sighandler_t only available on linux"
+#endif
+
 typedef void (*__sighandler_t)(int);
+typedef __sighandler_t sighandler_t;
 
 #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.h.def b/libc/include/signal.h.def
index 50a5f44c7337aa6..11e56138879d59c 100644
--- a/libc/include/signal.h.def
+++ b/libc/include/signal.h.def
@@ -16,6 +16,10 @@
 
 #include "llvm-libc-macros/signal-macros.h"
 
+#ifdef __linux__
+#include "llvm-libc-types/__sighandler_t.h"
+#endif
+
 %%public_api()
 
 #endif // LLVM_LIBC_SIGNAL_H
diff --git a/libc/include/signal.yaml b/libc/include/signal.yaml
index 576e77576ac740b..5530d63c4bff979 100644
--- a/libc/include/signal.yaml
+++ b/libc/include/signal.yaml
@@ -3,12 +3,12 @@ header_template: signal.h.def
 macros: []
 types:
   - type_name: pid_t
-  - type_name: stack_t
+  - type_name: sig_atomic_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 +69,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..f196aa52634e47b 100644
--- a/libc/src/signal/linux/signal.cpp
+++ b/libc/src/signal/linux/signal.cpp
@@ -8,14 +8,16 @@
 
 #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),

nickdesaulniers · 2025-02-04T19:07:06Z

libc/src/signal/linux/signal.cpp

 #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);


perhaps decltype would be better here?

hmm...no nevermind. We'd only be able to do decltype(signal), which would not be the type of the parameter or return value.

Perhaps auto would work...

ah, no, I get an error related to overload resolution. Ok will leave for now.
error: functions that differ only in their return type cannot be overloaded

github-actions · 2025-02-04T19:09:36Z

✅ With the latest revision this PR passed the C/C++ code formatter.

nickdesaulniers · 2025-02-04T19:40:10Z

failure in presubmit:

llvm-project/build/libc/include/signal.h:20:10: fatal error: 'llvm-libc-types/__sighandler_t.h' file not found
   20 | #include "llvm-libc-types/__sighandler_t.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

the libc.include.signal.__generated_hdr__ target doesn't have a dependency expressed on libc.include.llvm-libc-types.__sighandler_t. Need to add that.

enh-google · 2025-02-04T19:43:19Z

shouldn't __sighandler_t come from the uapi header, and only if you're using the uapi headers? (so nothing except the uapi headers should ever refer to that type, and the uapi headers themselves should be fine because they'll take care of their own transitive includes.)

nickdesaulniers · 2025-02-04T22:38:00Z

shouldn't __sighandler_t come from the uapi header,

Did you mean __signalfn_t? It looks like asm-generic/signal-defs.h depends on __sighandler_t, but they themselves do not define that typedef. (unless something is really wrong with my grep-fu).

and only if you're using the uapi headers?

We're inconsistent about our use of uapi headers. I would like to try to use them more, but I suspect most libcs have run into issues with the uapi headers. Probably a longer conversation to have there...

enh-google · 2025-02-04T22:43:10Z

shouldn't __sighandler_t come from the uapi header,

Did you mean __signalfn_t? It looks like asm-generic/signal-defs.h depends on __sighandler_t, but they themselves do not define that typedef. (unless something is really
wrong with my grep-fu).

not in AOSP's clean copy (external/kernel-headers/). it's just split in two for readability (though i feel like the __user destroys that), but they're on adjacent lines in the file you mentioned:

typedef void __signalfn_t(int);
typedef __signalfn_t __user *__sighandler_t;

and only if you're using the uapi headers?

We're inconsistent about our use of uapi headers. I would like to try to use them more, but I suspect most libcs have run into issues with the uapi headers. Probably a longer conversation to have there...

yeah, they're kind of a mess, with some hacks for glibc compatibility (which is weird because glibc copy & pastes instead; bionic and musl compatibility would make more sense!). but, yes, if a side-effect of llvm-libc was making the uapi headers a bit less direct-libc-use-hostile, that would be good.

nickdesaulniers · 2025-02-04T22:50:10Z

Regardless, looking at the usage of __sighandler_t in asm-generic/signal-defs.h, I suspect that the libc is expected to provide the typedef to the kernel uapi header (which seems ass-backwards to me, but perhaps there's a good reason I'm unfamiliar with). (Also, perhaps I should just send a kernel patch to just use void (*member)(int); then in 100 years we can depend on it).

So I worry about a (hypothetical, perhaps contrived) case where: a user of llvm-libc somehow manages to #include <asm-generic/signal.h> before include llvm-libc's <signal.h> such that __sighandler_t is not defined. Because the kernel headers themselves do not define __sighandler_t, that results in a compilation failure since there is no definition of __sighandler_t. Is that possible? Or something else I'm misssing?

enh-google · 2025-02-05T16:45:52Z

Regardless, looking at the usage of __sighandler_t in asm-generic/signal-defs.h, I suspect that the libc is expected to provide the typedef to the kernel uapi header (which seems ass-backwards to me, but perhaps there's a good reason I'm unfamiliar with). (Also, perhaps I should just send a kernel patch to just use void (*member)(int); then in 100 years we can depend on it).

i wasn't involved in the upstream uapi stuff, but as a user of the uapi headers i've always assumed the opposite: that they deliberately use "not quite the names" to avoid conflicts with userspace. (it seems obvious that they try somewhat, even if they're not particularly successful/systematic.)

So I worry about a (hypothetical, perhaps contrived) case where: a user of llvm-libc somehow manages to #include <asm-generic/signal.h> before include llvm-libc's <signal.h> such that __sighandler_t is not defined. Because the kernel headers themselves do not define __sighandler_t, that results in a compilation failure since there is no definition of __sighandler_t. Is that possible? Or something else I'm misssing?

i still claim you're wrong there --- my unadulterated copy of the uapi headers does define __sighandler_t, as i copy & pasted above and will repeat here:

typedef void __signalfn_t(int);
typedef __signalfn_t __user *__sighandler_t;

and i think -- without ever having talked to anyone responsible -- this is the whole point of all these double-underscore "almost right" names; they want things to "just work" wherever possible, without accidentally getting in the way.

(it's perhaps worth mentioning that it's very spotty. one of the main things we do when transforming the uapi headers from their upstream form to the form included in bionic is that we do a lot of "replace struct/type with a #include of our corresponding <bits/...> file" so we can have consistent behavior in all the cases the uapi headers didn't think about. as i think i already mentioned, glibc mainly gets by with copy & paste, and musl seems to be on its usual "don't do that then" side of the fence.)

enh-google · 2025-02-05T16:48:13Z

asm-generic/signal.h

(it's possible you missed that the relevant #include is not at the top of the file ... it's just before the struct that needs the __sighandler_t type.)

nickdesaulniers · 2025-02-05T17:00:01Z

not in AOSP's clean copy (external/kernel-headers/).

Oh, shoot, I am blind. For some reason both ag and rg don't search these kernel headers, but grep -rn does...wtf.

nickdesaulniers · 2025-02-05T17:02:41Z

Looks like ag and rg default ignore searching .h files unless -uuu is passed...good to know...

nickdesaulniers · 2025-02-05T17:08:23Z

Ok, given my misunderstanding due to my poor grep-fu, I think it's best to eliminate any reference to the double underscore prefix symbol (both in sources, and in file names). I'll clean that up.

enh-google · 2025-02-05T21:59:22Z

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

+#ifndef LLVM_LIBC_TYPES_SIGHANDLER_T_H
+#define LLVM_LIBC_TYPES_SIGHANDLER_T_H
+
+#ifdef __linux__


sure you don't want a _GNU_SOURCE or whatever your internal equivalent is? (both glibc and bionic have __USE_GNU as the post-<features.h>-inclusion internal equivalent.)

No, we don't currently hold folk's code hostage based on them defining _GNU_SOURCE or not, and I don't plan to add it.

i flirted with that myself for bionic, but it's a bit more complicated in practice --- "what when the GNU variant conflicts with the POSIX one?", say, or "what when people are trying to build code that's incompatible with GNU extensions [because they use some of the names themselves, say]"...

(the weird one for Android is _BSD_SOURCE where by historical accident that's always on. but _GNU_SOURCE is something developers are used to turning on/off either via -std=gnu23 vs -std=c23 or -D/#define.)

I was thinking more about this last night, and qsort_r came to mind.

So I guess I should change my stance slightly from "never" to "perhaps when there is a conflict."

Is there a conflict here? Well, sig_t and sighandler_t don't conflict with each other, and the issue isn't as severe as qsort_r. But Bionic provides both without requiring users to pledge allegiance; if bionic doesn't llvm-libc should not either (IMO).

I learned it from you, Dad! /runs away crying

But I will keep that in mind that _GNU_SOURCE and _BSD_SOURCE do exist more for than just pledges of allegiance and holding user code hostage, and that we may need to deploy those in llvm-libc in the future. I still don't think we need to here (yet; famous last words).

but _GNU_SOURCE is something developers are used to turning on/off either via -std=gnu23 vs -std=c23

Wait...what?!

$ clang -std=gnu++23 --target=x86_64-linux-gnu -dM -E -x c++ /dev/null | grep GNU_SOURCE #define _GNU_SOURCE 1 $ clang -std=gnu23 --target=x86_64-linux-gnu -dM -E -x c /dev/null | grep GNU_SOURCE $ g++ -dM -E -x c++ /dev/null | grep GNU_SOURCE #define _GNU_SOURCE 1 $ gcc -dM -E -x c /dev/null | grep GNU_SOURCE $

uh...so perhaps not a bug in clang as the first two statements made me think...but WTF!?

I was thinking more about this last night, and qsort_r came to mind.

So I guess I should change my stance slightly from "never" to "perhaps when there is a conflict."

Is there a conflict here? Well, sig_t and sighandler_t don't conflict with each other, and the issue isn't as severe as qsort_r. But Bionic provides both without requiring users to pledge allegiance; if bionic doesn't llvm-libc should not either (IMO).

I learned it from you, Dad! /runs away crying

But I will keep that in mind that _GNU_SOURCE and _BSD_SOURCE do exist more for than just pledges of allegiance and holding user code hostage, and that we may need to deploy those in llvm-libc in the future. I still don't think we need to here (yet; famous last words).

yeah, as you can probably tell, i do think "provide all the things until/unless proven problematic" is a developer-friendly option.

the only real counterargument i've heard is "it's bad for source portability in the other direction". but i've always cared more that people's stuff "just works" when ported to the libc i have control over rather than what might happen when ported away (in part because that's probably less common anyway; any library that's first developed on Android seems likely to be Android-specific in some way, or you'd probably make it cross-platform from the beginning).

but _GNU_SOURCE is something developers are used to turning on/off either via -std=gnu23 vs -std=c23

Wait...what?!

$ clang -std=gnu++23 --target=x86_64-linux-gnu -dM -E -x c++ /dev/null | grep GNU_SOURCE #define _GNU_SOURCE 1 $ clang -std=gnu23 --target=x86_64-linux-gnu -dM -E -x c /dev/null | grep GNU_SOURCE $ g++ -dM -E -x c++ /dev/null | grep GNU_SOURCE #define _GNU_SOURCE 1 $ gcc -dM -E -x c /dev/null | grep GNU_SOURCE $

uh...so perhaps not a bug in clang as the first two statements made me think...but WTF!?

oh, yeah, that's it ... i never do this myself, so i can never remember which combinations imply _GNU_SOURCE, just that there are dragons in that area.

madness indeed. (especially because gnu vs c does let you opt in/out of language extensions. if you were going to do anything here, consistency would have been nice. i assume there was a historical accident somewhere -- like bionic's always-on _BSD_SOURCE and they just couldn't fix it by the time they realized.)

libc/include/CMakeLists.txt

libc/include/signal.yaml

libc/include/llvm-libc-types/__sighandler_t.h

llvm-ci · 2025-02-08T04:08:34Z

LLVM Buildbot has detected a new failure on builder premerge-monolithic-linux running on premerge-linux-1 while building libc at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/22293

Here is the relevant piece of the build log for the reference

Step 7 (test-build-unified-tree-check-all) failure: test (failure)
...
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Integer/negate-overflow.cpp (94187 of 98221)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Integer/div-zero.cpp (94188 of 98221)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Integer/incdec-overflow.cpp (94189 of 98221)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Integer/suppressions-builtin.cpp (94190 of 98221)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Integer/sub-overflow.cpp (94191 of 98221)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Integer/no-recover.cpp (94192 of 98221)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Misc/Posix/sigaction.cpp (94193 of 98221)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Integer/unsigned-shift.cpp (94194 of 98221)
PASS: UBSan-Standalone-lld-x86_64 :: TestCases/Misc/Posix/ubsan_options.cpp (94195 of 98221)
TIMEOUT: MLIR :: Examples/standalone/test.toy (94196 of 98221)
******************** TEST 'MLIR :: Examples/standalone/test.toy' FAILED ********************
Exit Code: -9
Timeout: Reached timeout of 60 seconds

Command Output (stdout):
--
# RUN: at line 1
"/etc/cmake/bin/cmake" "/build/buildbot/premerge-monolithic-linux/llvm-project/mlir/examples/standalone" -G "Ninja"  -DCMAKE_CXX_COMPILER=/usr/bin/clang++  -DCMAKE_C_COMPILER=/usr/bin/clang   -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir  -DLLVM_USE_LINKER=lld  -DPython3_EXECUTABLE="/usr/bin/python3.10"
# executed command: /etc/cmake/bin/cmake /build/buildbot/premerge-monolithic-linux/llvm-project/mlir/examples/standalone -G Ninja -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir -DLLVM_USE_LINKER=lld -DPython3_EXECUTABLE=/usr/bin/python3.10
# .---command stdout------------
# | -- The CXX compiler identification is Clang 16.0.6
# | -- The C compiler identification is Clang 16.0.6
# | -- Detecting CXX compiler ABI info
# | -- Detecting CXX compiler ABI info - done
# | -- Check for working CXX compiler: /usr/bin/clang++ - skipped
# | -- Detecting CXX compile features
# | -- Detecting CXX compile features - done
# | -- Detecting C compiler ABI info
# | -- Detecting C compiler ABI info - done
# | -- Check for working C compiler: /usr/bin/clang - skipped
# | -- Detecting C compile features
# | -- Detecting C compile features - done
# | -- Looking for histedit.h
# | -- Looking for histedit.h - found
# | -- Found LibEdit: /usr/include (found version "2.11") 
# | -- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.2.11") 
# | -- Found LibXml2: /usr/lib/x86_64-linux-gnu/libxml2.so (found version "2.9.13") 
# | -- Using MLIRConfig.cmake in: /build/buildbot/premerge-monolithic-linux/build/lib/cmake/mlir
# | -- Using LLVMConfig.cmake in: /build/buildbot/premerge-monolithic-linux/build/lib/cmake/llvm
# | -- Linker detection: unknown
# | -- Performing Test LLVM_LIBSTDCXX_MIN
# | -- Performing Test LLVM_LIBSTDCXX_MIN - Success
# | -- Performing Test LLVM_LIBSTDCXX_SOFT_ERROR
# | -- Performing Test LLVM_LIBSTDCXX_SOFT_ERROR - Success
# | -- Performing Test CXX_SUPPORTS_CUSTOM_LINKER
# | -- Performing Test CXX_SUPPORTS_CUSTOM_LINKER - Success
# | -- Performing Test C_SUPPORTS_FPIC
# | -- Performing Test C_SUPPORTS_FPIC - Success
# | -- Performing Test CXX_SUPPORTS_FPIC

`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: llvm#125598

nickdesaulniers added 4 commits February 4, 2025 10:27

[libc][signal] clean up sighandler_t declarations

62e8a9c

clean up our usage of sighandler_t

c84b8f2

only include __sighandler_t.h on linux

669bb96

update signal.h hdrgen

19e518a

llvmbot added the libc label Feb 4, 2025

nickdesaulniers requested review from jhuber6, lntue, enh-google and michaelrj-google February 4, 2025 19:05

nickdesaulniers commented Feb 4, 2025

View reviewed changes

reformat

d486708

nickdesaulniers marked this pull request as draft February 4, 2025 19:40

add dependency so include/llvm-libc-types/__sighandler_t.h gets copied

5864bf7

nickdesaulniers marked this pull request as ready for review February 4, 2025 22:45

lntue approved these changes Feb 5, 2025

View reviewed changes

nickdesaulniers marked this pull request as draft February 5, 2025 17:08

remove any typedef for __sighandler_t, rename files

3730708

nickdesaulniers marked this pull request as ready for review February 5, 2025 17:17

nickdesaulniers requested a review from lntue February 5, 2025 17:17

enh-google reviewed Feb 5, 2025

View reviewed changes

libc/include/CMakeLists.txt Outdated Show resolved Hide resolved

enh-google reviewed Feb 5, 2025

View reviewed changes

libc/include/signal.yaml Show resolved Hide resolved

enh-google reviewed Feb 5, 2025

View reviewed changes

libc/include/llvm-libc-types/__sighandler_t.h Outdated Show resolved Hide resolved

nickdesaulniers requested a review from enh-google February 6, 2025 21:11

enh-google approved these changes Feb 7, 2025

View reviewed changes

nickdesaulniers merged commit e566313 into llvm:main Feb 7, 2025
16 checks passed

nickdesaulniers deleted the sighandler_t branch February 7, 2025 18:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc][signal] clean up usage of sighandler_t #125745

[libc][signal] clean up usage of sighandler_t #125745

nickdesaulniers commented Feb 4, 2025 •

edited

Loading

llvmbot commented Feb 4, 2025

nickdesaulniers Feb 4, 2025

nickdesaulniers Feb 4, 2025

nickdesaulniers Feb 4, 2025

nickdesaulniers Feb 4, 2025 •

edited

Loading

github-actions bot commented Feb 4, 2025 •

edited

Loading

nickdesaulniers commented Feb 4, 2025

enh-google commented Feb 4, 2025

nickdesaulniers commented Feb 4, 2025

enh-google commented Feb 4, 2025

nickdesaulniers commented Feb 4, 2025

enh-google commented Feb 5, 2025

enh-google commented Feb 5, 2025

nickdesaulniers commented Feb 5, 2025

nickdesaulniers commented Feb 5, 2025

nickdesaulniers commented Feb 5, 2025

enh-google Feb 5, 2025

nickdesaulniers Feb 5, 2025

enh-google Feb 6, 2025

nickdesaulniers Feb 6, 2025

enh-google Feb 6, 2025

llvm-ci commented Feb 8, 2025

[libc][signal] clean up usage of sighandler_t #125745

[libc][signal] clean up usage of sighandler_t #125745

Conversation

nickdesaulniers commented Feb 4, 2025 • edited Loading

llvmbot commented Feb 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickdesaulniers Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Feb 4, 2025 • edited Loading

nickdesaulniers commented Feb 4, 2025

enh-google commented Feb 4, 2025

nickdesaulniers commented Feb 4, 2025

enh-google commented Feb 4, 2025

nickdesaulniers commented Feb 4, 2025

enh-google commented Feb 5, 2025

enh-google commented Feb 5, 2025

nickdesaulniers commented Feb 5, 2025

nickdesaulniers commented Feb 5, 2025

nickdesaulniers commented Feb 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

llvm-ci commented Feb 8, 2025

nickdesaulniers commented Feb 4, 2025 •

edited

Loading

nickdesaulniers Feb 4, 2025 •

edited

Loading

github-actions bot commented Feb 4, 2025 •

edited

Loading