Skip to content

Commit 8730ca0

Browse files
nickdesaulniersIcohedron
authored andcommitted
[libc][signal] clean up usage of sighandler_t (llvm#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: llvm#125598
1 parent 725f8f9 commit 8730ca0

File tree

15 files changed

+38
-65
lines changed

15 files changed

+38
-65
lines changed

libc/hdr/types/CMakeLists.txt

-9
Original file line numberDiff line numberDiff line change
@@ -250,15 +250,6 @@ add_proxy_header_library(
250250
libc.include.locale
251251
)
252252

253-
add_proxy_header_library(
254-
sighandler_t
255-
HDRS
256-
sighandler_t.h
257-
FULL_BUILD_DEPENDS
258-
libc.include.llvm-libc-types.__sighandler_t
259-
libc.include.signal
260-
)
261-
262253
add_proxy_header_library(
263254
stack_t
264255
HDRS

libc/hdr/types/sighandler_t.h

-24
This file was deleted.

libc/include/CMakeLists.txt

+4-3
Original file line numberDiff line numberDiff line change
@@ -284,13 +284,14 @@ add_header_macro(
284284
signal.h
285285
DEPENDS
286286
.llvm-libc-macros.signal_macros
287+
.llvm-libc-types.pid_t
287288
.llvm-libc-types.sig_atomic_t
289+
.llvm-libc-types.sighandler_t
290+
.llvm-libc-types.siginfo_t
288291
.llvm-libc-types.sigset_t
292+
.llvm-libc-types.stack_t
289293
.llvm-libc-types.struct_sigaction
290294
.llvm-libc-types.union_sigval
291-
.llvm-libc-types.siginfo_t
292-
.llvm-libc-types.stack_t
293-
.llvm-libc-types.pid_t
294295
)
295296

296297
add_header_macro(

libc/include/llvm-libc-macros/gpu/signal-macros.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
#define SIGSEGV 11
1717
#define SIGTERM 15
1818

19-
#define SIG_DFL ((__sighandler_t)(0))
20-
#define SIG_IGN ((__sighandler_t)(1))
21-
#define SIG_ERR ((__sighandler_t)(-1))
19+
#define SIG_DFL ((void (*)(int))(0))
20+
#define SIG_IGN ((void (*)(int))(1))
21+
#define SIG_ERR ((void (*)(int))(-1))
2222

2323
// Max signal number
2424
#define NSIG 64

libc/include/llvm-libc-macros/linux/signal-macros.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,9 @@
8686
#error "Signal stack sizes not defined for your platform."
8787
#endif
8888

89-
#define SIG_DFL ((__sighandler_t)0)
90-
#define SIG_IGN ((__sighandler_t)1)
91-
#define SIG_ERR ((__sighandler_t)-1)
89+
#define SIG_DFL ((void (*)(int))0)
90+
#define SIG_IGN ((void (*)(int))1)
91+
#define SIG_ERR ((void (*)(int))(-1))
9292

9393
// SIGCHLD si_codes
9494
#define CLD_EXITED 1 // child has exited

libc/include/llvm-libc-types/CMakeLists.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ add_header(__pthread_start_t HDR __pthread_start_t.h)
1515
add_header(__pthread_tss_dtor_t HDR __pthread_tss_dtor_t.h)
1616
add_header(__qsortcompare_t HDR __qsortcompare_t.h)
1717
add_header(__qsortrcompare_t HDR __qsortrcompare_t.h)
18-
add_header(__sighandler_t HDR __sighandler_t.h)
1918
add_header(__thread_type HDR __thread_type.h)
2019
add_header(blkcnt_t HDR blkcnt_t.h)
2120
add_header(blksize_t HDR blksize_t.h)
@@ -66,6 +65,7 @@ if(LIBC_TYPES_TIME_T_IS_32_BIT)
6665
else()
6766
add_header(time_t HDR time_t_64.h DEST_HDR time_t.h)
6867
endif()
68+
add_header(sighandler_t HDR sighandler_t.h)
6969
add_header(stack_t HDR stack_t.h DEPENDS .size_t)
7070
add_header(suseconds_t HDR suseconds_t.h)
7171
add_header(struct_dirent HDR struct_dirent.h DEPENDS .ino_t .off_t)
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
1-
//===-- Definition of struct __sighandler_t -------------------------------===//
1+
//===-- Definition of sighandler_t ----------------------------------------===//
22
//
33
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
44
// See https://llvm.org/LICENSE.txt for license information.
55
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#ifndef LLVM_LIBC_TYPES___SIGHANDLER_T_H
10-
#define LLVM_LIBC_TYPES___SIGHANDLER_T_H
9+
#ifndef LLVM_LIBC_TYPES_SIGHANDLER_T_H
10+
#define LLVM_LIBC_TYPES_SIGHANDLER_T_H
1111

12-
typedef void (*__sighandler_t)(int);
12+
#ifdef __linux__
13+
// For compatibility with glibc.
14+
typedef void (*sighandler_t)(int);
15+
#endif
1316

14-
#endif // LLVM_LIBC_TYPES___SIGHANDLER_T_H
17+
#endif // LLVM_LIBC_TYPES_SIGHANDLER_T_H

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

-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,4 @@ struct sigaction {
2525
#endif
2626
};
2727

28-
typedef void (*__sighandler_t)(int);
29-
3028
#endif // LLVM_LIBC_TYPES_STRUCT_SIGACTION_H

libc/include/signal.yaml

+11-5
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@ header_template: signal.h.def
33
macros: []
44
types:
55
- type_name: pid_t
6-
- type_name: stack_t
6+
- type_name: sig_atomic_t
7+
- type_name: sighandler_t
78
- type_name: siginfo_t
8-
- type_name: struct_sigaction
99
- type_name: sigset_t
10+
- type_name: stack_t
11+
- type_name: struct_sigaction
1012
- type_name: union_sigval
11-
- type_name: sig_atomic_t
1213
enums: []
1314
objects: []
1415
functions:
@@ -69,10 +70,15 @@ functions:
6970
- name: signal
7071
standards:
7172
- stdc
72-
return_type: __sighandler_t
73+
# May the Geneva Convention have mercy on my soul... Why this insanity?
74+
# Well: signal returns a function pointer to a function with no return
75+
# value and which accepts an int. The parameter list appears on the far
76+
# right of the declaration. i.e.
77+
# void (*signal(int, void (*)(int)))(int);
78+
return_type: void (*
7379
arguments:
7480
- type: int
75-
- type: __sighandler_t
81+
- type: void (*)(int)))(int
7682
- name: sigprocmask
7783
standards:
7884
- POSIX

libc/src/signal/linux/CMakeLists.txt

-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,6 @@ add_entrypoint_object(
127127
DEPENDS
128128
.sigaction
129129
libc.hdr.signal_macros
130-
libc.hdr.types.sighandler_t
131130
)
132131

133132
add_entrypoint_object(

libc/src/signal/linux/signal.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,17 @@
88

99
#include "src/signal/signal.h"
1010
#include "hdr/signal_macros.h"
11-
#include "hdr/types/sighandler_t.h"
1211
#include "src/__support/common.h"
1312
#include "src/__support/macros/config.h"
1413
#include "src/signal/sigaction.h"
1514

1615
namespace LIBC_NAMESPACE_DECL {
1716

18-
LLVM_LIBC_FUNCTION(sighandler_t, signal, (int signum, sighandler_t handler)) {
17+
// Our LLVM_LIBC_FUNCTION macro doesn't handle function pointer return types.
18+
using signal_handler = void (*)(int);
19+
20+
LLVM_LIBC_FUNCTION(signal_handler, signal,
21+
(int signum, signal_handler handler)) {
1922
struct sigaction action, old;
2023
action.sa_handler = handler;
2124
action.sa_flags = SA_RESTART;

libc/src/signal/signal.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,11 @@
99
#ifndef LLVM_LIBC_SRC_SIGNAL_SIGNAL_H
1010
#define LLVM_LIBC_SRC_SIGNAL_SIGNAL_H
1111

12-
#include "hdr/types/sighandler_t.h"
1312
#include "src/__support/macros/config.h"
1413

1514
namespace LIBC_NAMESPACE_DECL {
1615

17-
sighandler_t signal(int signum, sighandler_t handler);
16+
void (*signal(int signum, void (*handler)(int)))(int);
1817

1918
} // namespace LIBC_NAMESPACE_DECL
2019

libc/test/UnitTest/FPExceptMatcher.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ static void sigfpeHandler(int sig) {
3737
}
3838

3939
FPExceptMatcher::FPExceptMatcher(FunctionCaller *func) {
40-
sighandler_t oldSIGFPEHandler = signal(SIGFPE, &sigfpeHandler);
40+
auto *oldSIGFPEHandler = signal(SIGFPE, &sigfpeHandler);
4141

4242
caughtExcept = false;
4343
fenv_t oldEnv;

libc/test/src/signal/CMakeLists.txt

-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ add_libc_unittest(
7474
SRCS
7575
signal_test.cpp
7676
DEPENDS
77-
libc.hdr.types.sighandler_t
7877
libc.src.errno.errno
7978
libc.src.signal.raise
8079
libc.src.signal.signal

libc/test/src/signal/signal_test.cpp

+1-3
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,12 @@
1313
#include "test/UnitTest/ErrnoSetterMatcher.h"
1414
#include "test/UnitTest/Test.h"
1515

16-
#include "hdr/types/sighandler_t.h"
17-
1816
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
1917
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
2018

2119
TEST(LlvmLibcSignal, Invalid) {
2220
LIBC_NAMESPACE::libc_errno = 0;
23-
sighandler_t valid = +[](int) {};
21+
auto *valid = +[](int) {};
2422
EXPECT_THAT((void *)LIBC_NAMESPACE::signal(0, valid),
2523
Fails(EINVAL, (void *)SIG_ERR));
2624
EXPECT_THAT((void *)LIBC_NAMESPACE::signal(65, valid),

0 commit comments

Comments
 (0)