From cf8abf91bee57994da2b891077e6c7d4fb113445 Mon Sep 17 00:00:00 2001 From: Bruno Haible Date: Tue, 19 Mar 2019 23:32:42 +0100 Subject: [PATCH] Help making signal handlers more reliable. * m4/gnulib-common.m4 (gl_COMMON_BODY): Emit definition of _GL_ASYNC_SAFE into config.h. * lib/nanosleep.c (sighandler): Mark as _GL_ASYNC_SAFE. * lib/fatal-signal.h (at_fatal_signal): Add _GL_ASYNC_SAFE marker to argument. * lib/fatal-signal.c (action_t, uninstall_handlers, fatal_signal_handler): Mark as _GL_ASYNC_SAFE. * lib/clean-temp.c (cleanup_action): Mark as _GL_ASYNC_SAFE. * lib/wait-process.c (cleanup_slaves, cleanup_slaves_action): Mark as _GL_ASYNC_SAFE. * lib/c-stack.h (c_stack_action): Add _GL_ASYNC_SAFE marker to argument. * lib/c-stack.c: Add _GL_ASYNC_SAFE markers. --- ChangeLog | 16 ++++++++++++++++ lib/c-stack.c | 28 +++++++++++++--------------- lib/c-stack.h | 2 +- lib/clean-temp.c | 2 +- lib/fatal-signal.c | 6 +++--- lib/fatal-signal.h | 5 +++-- lib/nanosleep.c | 2 +- lib/wait-process.c | 4 ++-- m4/gnulib-common.m4 | 29 ++++++++++++++++++++++++++++- 9 files changed, 68 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index 02bed463ab..9313fdc488 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2019-03-19 Bruno Haible + + Help making signal handlers more reliable. + * m4/gnulib-common.m4 (gl_COMMON_BODY): Emit definition of + _GL_ASYNC_SAFE into config.h. + * lib/nanosleep.c (sighandler): Mark as _GL_ASYNC_SAFE. + * lib/fatal-signal.h (at_fatal_signal): Add _GL_ASYNC_SAFE marker to + argument. + * lib/fatal-signal.c (action_t, uninstall_handlers, + fatal_signal_handler): Mark as _GL_ASYNC_SAFE. + * lib/clean-temp.c (cleanup_action): Mark as _GL_ASYNC_SAFE. + * lib/wait-process.c (cleanup_slaves, cleanup_slaves_action): Mark as + _GL_ASYNC_SAFE. + * lib/c-stack.h (c_stack_action): Add _GL_ASYNC_SAFE marker to argument. + * lib/c-stack.c: Add _GL_ASYNC_SAFE markers. + 2019-03-18 Bruno Haible _Noreturn: clang and MSVC do support [[noreturn]] in C++11 mode. diff --git a/lib/c-stack.c b/lib/c-stack.c index c62726b45d..929bb3a315 100644 --- a/lib/c-stack.c +++ b/lib/c-stack.c @@ -91,7 +91,7 @@ typedef struct sigaltstack stack_t; /* The user-specified action to take when a SEGV-related program error or stack overflow occurs. */ -static void (* volatile segv_action) (int); +static _GL_ASYNC_SAFE void (* volatile segv_action) (int); /* Translated messages for program errors and stack overflow. Do not translate them in the signal handler, since gettext is not @@ -107,7 +107,7 @@ static char const * volatile stack_overflow_message; appears to have been a stack overflow, or with a core dump otherwise. This function is async-signal-safe. */ -static _Noreturn void +static _GL_ASYNC_SAFE _Noreturn void die (int signo) { char const *message; @@ -146,8 +146,8 @@ static union void *p; } alternate_signal_stack; -static void -null_action (int signo __attribute__ ((unused))) +static _GL_ASYNC_SAFE void +null_action (int signo _GL_UNUSED) { } @@ -164,8 +164,8 @@ static volatile int segv_handler_missing; /* Handle a segmentation violation and exit if it cannot be stack overflow. This function is async-signal-safe. */ -static int segv_handler (void *address __attribute__ ((unused)), - int serious) +static _GL_ASYNC_SAFE int +segv_handler (void *address _GL_UNUSED, int serious) { # if DEBUG { @@ -185,9 +185,8 @@ static int segv_handler (void *address __attribute__ ((unused)), /* Handle a segmentation violation that is likely to be a stack overflow and exit. This function is async-signal-safe. */ -static _Noreturn void -overflow_handler (int emergency, - stackoverflow_context_t context __attribute__ ((unused))) +static _GL_ASYNC_SAFE _Noreturn void +overflow_handler (int emergency, stackoverflow_context_t context _GL_UNUSED) { # if DEBUG { @@ -202,7 +201,7 @@ overflow_handler (int emergency, } int -c_stack_action (void (*action) (int)) +c_stack_action (_GL_ASYNC_SAFE void (*action) (int)) { segv_action = action ? action : null_action; program_error_message = _("program error"); @@ -229,9 +228,8 @@ c_stack_action (void (*action) (int)) /* Handle a segmentation violation and exit. This function is async-signal-safe. */ -static _Noreturn void -segv_handler (int signo, siginfo_t *info, - void *context __attribute__ ((unused))) +static _GL_ASYNC_SAFE _Noreturn void +segv_handler (int signo, siginfo_t *info, void *context _GL_UNUSED) { /* Clear SIGNO if it seems to have been a stack overflow. */ # if ! HAVE_XSI_STACK_OVERFLOW_HEURISTIC @@ -278,7 +276,7 @@ segv_handler (int signo, siginfo_t *info, # endif int -c_stack_action (void (*action) (int)) +c_stack_action (_GL_ASYNC_SAFE void (*action) (int)) { int r; stack_t st; @@ -325,7 +323,7 @@ c_stack_action (void (*action) (int)) && HAVE_STACK_OVERFLOW_HANDLING) || HAVE_LIBSIGSEGV) */ int -c_stack_action (void (*action) (int) __attribute__ ((unused))) +c_stack_action (_GL_ASYNC_SAFE void (*action) (int) _GL_UNUSED) { errno = ENOTSUP; return -1; diff --git a/lib/c-stack.h b/lib/c-stack.h index fcfe66f05e..36dd4ec3ec 100644 --- a/lib/c-stack.h +++ b/lib/c-stack.h @@ -41,4 +41,4 @@ This function may install a handler for the SIGSEGV signal or for the SIGBUS signal or exercise other system dependent exception handling APIs. */ -extern int c_stack_action (void (* /*action*/) (int)); +extern int c_stack_action (_GL_ASYNC_SAFE void (* /*action*/) (int)); diff --git a/lib/clean-temp.c b/lib/clean-temp.c index 0cb4a7e406..d333f56c09 100644 --- a/lib/clean-temp.c +++ b/lib/clean-temp.c @@ -180,7 +180,7 @@ string_hash (const void *x) /* The signal handler. It gets called asynchronously. */ -static void +static _GL_ASYNC_SAFE void cleanup_action (int sig _GL_UNUSED) { size_t i; diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c index 7f12f12c16..c9153f1e77 100644 --- a/lib/fatal-signal.c +++ b/lib/fatal-signal.c @@ -107,7 +107,7 @@ init_fatal_signals (void) /* ========================================================================= */ -typedef void (*action_t) (int sig); +typedef _GL_ASYNC_SAFE void (*action_t) (int sig); /* Type of an entry in the actions array. The 'action' field is accessed from within the fatal_signal_handler(), @@ -131,7 +131,7 @@ static struct sigaction saved_sigactions[64]; /* Uninstall the handlers. */ -static void +static _GL_ASYNC_SAFE void uninstall_handlers (void) { size_t i; @@ -148,7 +148,7 @@ uninstall_handlers (void) /* The signal handler. It gets called asynchronously. */ -static void +static _GL_ASYNC_SAFE void fatal_signal_handler (int sig) { for (;;) diff --git a/lib/fatal-signal.h b/lib/fatal-signal.h index c7d8e35d62..8ac57cf552 100644 --- a/lib/fatal-signal.h +++ b/lib/fatal-signal.h @@ -36,7 +36,8 @@ extern "C" { occurs. Restrictions for the cleanup function: - - The cleanup function can do all kinds of system calls. + - The cleanup function can do all kinds of system calls. It may also + modify (clobber) errno. - It can also access application dependent memory locations and data structures provided they are in a consistent state. One way to ensure this is through block_fatal_signals()/unblock_fatal_signals(), see @@ -51,7 +52,7 @@ extern "C" { The cleanup function is executed asynchronously. It is unspecified whether during its execution the catchable fatal signals are blocked or not. */ -extern void at_fatal_signal (void (*function) (int sig)); +extern void at_fatal_signal (_GL_ASYNC_SAFE void (*function) (int sig)); /* Sometimes it is necessary to block the usually fatal signals while the diff --git a/lib/nanosleep.c b/lib/nanosleep.c index 6f6cc89ea7..3150ff9b9d 100644 --- a/lib/nanosleep.c +++ b/lib/nanosleep.c @@ -194,7 +194,7 @@ static sig_atomic_t volatile suspended; /* Handle SIGCONT. */ -static void +static _GL_ASYNC_SAFE void sighandler (int sig) { suspended = 1; diff --git a/lib/wait-process.c b/lib/wait-process.c index b51e2447a0..7c6993317e 100644 --- a/lib/wait-process.c +++ b/lib/wait-process.c @@ -80,7 +80,7 @@ static size_t slaves_allocated = SIZEOF (static_slaves); #endif /* The cleanup action. It gets called asynchronously. */ -static void +static _GL_ASYNC_SAFE void cleanup_slaves (void) { for (;;) @@ -104,7 +104,7 @@ cleanup_slaves (void) /* The cleanup action, taking a signal argument. It gets called asynchronously. */ -static void +static _GL_ASYNC_SAFE void cleanup_slaves_action (int sig _GL_UNUSED) { cleanup_slaves (); diff --git a/m4/gnulib-common.m4 b/m4/gnulib-common.m4 index 20666a552a..8ad963e355 100644 --- a/m4/gnulib-common.m4 +++ b/m4/gnulib-common.m4 @@ -1,4 +1,4 @@ -# gnulib-common.m4 serial 43 +# gnulib-common.m4 serial 44 dnl Copyright (C) 2007-2019 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -86,6 +86,33 @@ AC_DEFUN([gl_COMMON_BODY], [ #else # define _GL_ATTRIBUTE_MALLOC /* empty */ #endif +]) + AH_VERBATIM([async_safe], +[/* The _GL_ASYNC_SAFE marker should be attached to functions that are + signal handlers (for signals other than SIGABRT, SIGPIPE) or can be + invoked from such signal handlers. Such functions have some restrictions: + * All functions that it calls should be marked _GL_ASYNC_SAFE as well, + or should be listed as async-signal-safe in POSIX + + section 2.4.3. Note that malloc(), sprintf(), and fwrite(), in + particular, are NOT async-signal-safe. + * All memory locations (variables and struct fields) that these functions + access must be marked 'volatile'. This holds for both read and write + accesses. Otherwise the compiler might optimize away stores to and + reads from such locations that occur in the program, depending on its + data flow analysis. For example, when the program contains a loop + that is intended to inspect a variable set from within a signal handler + while (!signal_occurred) + ; + the compiler is allowed to transform this into an endless loop if the + variable 'signal_occurred' is not declared 'volatile'. + Additionally, recall that: + * A signal handler should not modify errno (except if it is a handler + for a fatal signal and ends by raising the same signal again, thus + provoking the termination of the process). If it invokes a function + that may clobber errno, it needs to save and restore the value of + errno. */ +#define _GL_ASYNC_SAFE ]) dnl Preparation for running test programs: dnl Tell glibc to write diagnostics from -D_FORTIFY_SOURCE=2 to stderr, not -- 2.39.5