Re: sigaction, SA_SIGINFO, and SIG_IGN
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 6/22/2008 1:58 PM: | I don't understand the Due to autoconf conventions. We are not using | HAVE_SIGACTION as a #define in config.h. We are using it as a shell variable. | This shell variable is not set by either AC_REPLACE_FUNCS([sigaction]) | nor by AC_CHECK_TYPE(...). So there is no conflict. I see your point. AC_REPLACE_FUNCS([sigaction]) indeed sets the config.h preprocessor variable HAVE_SIGACTION, and I omitted AC_CHECK_TYPE([sigaction]) because it would also produce HAVE_SIGACTION; but in config.h.in, we used only @HAVE_SIGACTION@, which is set independently. | | Proposed comment change: | | *** lib/signal.in.h.orig 2008-06-22 21:55:31.0 +0200 | --- 118,125 | typedef struct siginfo_t siginfo_t; | # endif /* [EMAIL PROTECTED]@ */ | | ! /* We assume that platforms which lack the sigaction() function also lack | !the 'struct sigaction' type, and vice versa. */ Fine. - -- Don't work too hard, make some time for fun as well! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkhfj38ACgkQ84KuGfSFAYC83wCdGnKSl3ah1Hzwlqt1Iwl9ASl2 fCQAoMCGrOvZbSeQ1beWwn3nwLIEq7FS =tEj2 -END PGP SIGNATURE-
Re: sigaction, SA_SIGINFO, and SIG_IGN
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 6/22/2008 2:06 PM: | In the context of lib/fatal-signal.c, I find that SA_RESETHAND is not | appropriate: If a fatal signal handler cleanup is interrupted by another | fatal signal, it is better to start the cleanup a second time rather than | terminating the program with incomplete cleanup. | | OK to apply? Sounds okay to me. - -- Don't work too hard, make some time for fun as well! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkhfkBUACgkQ84KuGfSFAYB5EgCeK8+V8xe6zo78H4TTnSR5nBM1 yssAoLlfZR9IoCtDpI3HzZh5m4UzBYRd =xwtT -END PGP SIGNATURE-
Re: sigaction, SA_SIGINFO, and SIG_IGN
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 6/21/2008 4:31 PM: | Done as follows, as part of committing sigaction. It turns out that since | mingw lacks struct sigaction, this only makes sense when using | sigaction(). I named the helper file sig-handler.h. I was a little overeager. sigprocmask should not use sig-handler.h, since it does not rely on sigaction (and can't, or that would be a recursive module dependency), so it was failing to compile on mingw. Checking in this: - -- Don't work too hard, make some time for fun as well! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkhfkXcACgkQ84KuGfSFAYAC/wCfVaucD71JscZMnR9ZpTNvOJWB QRgAoJCJaWzHA8EnNNwY2GjTLeuJmlEO =0U1C -END PGP SIGNATURE- From 748f9f2821b9c61a6a523564fce059ae8bec884f Mon Sep 17 00:00:00 2001 From: Eric Blake [EMAIL PROTECTED] Date: Mon, 23 Jun 2008 06:02:40 -0600 Subject: [PATCH] Revert use of sig-handler.h in sigprocmask.c. * modules/sigprocmask (Files): Don't rely on sig-handler.h, since it requires the existence of struct sigaction. * lib/sigprocmask.c (handler_t): Restore typedef. (rpl_signal, old_handlers): Use local type. Signed-off-by: Eric Blake [EMAIL PROTECTED] --- ChangeLog |8 lib/sigprocmask.c | 12 ++-- modules/sigprocmask |1 - 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index a3bdbfe..fe2dade 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2008-06-23 Eric Blake [EMAIL PROTECTED] + + Revert use of sig-handler.h in sigprocmask.c. + * modules/sigprocmask (Files): Don't rely on sig-handler.h, since + it requires the existence of struct sigaction. + * lib/sigprocmask.c (handler_t): Restore typedef. + (rpl_signal, old_handlers): Use local type. + 2008-06-22 Bruno Haible [EMAIL PROTECTED] * tests/test-stdint.c: Disable the INTMAX_MAX preprocessor test diff --git a/lib/sigprocmask.c b/lib/sigprocmask.c index 54e9c68..d6daca6 100644 --- a/lib/sigprocmask.c +++ b/lib/sigprocmask.c @@ -24,8 +24,6 @@ #include stdint.h #include stdlib.h -#include sig-handler.h - /* We assume that a platform without POSIX signal blocking functions also does not have the POSIX sigaction() function, only the signal() function. We also assume signal() has SysV semantics, @@ -45,6 +43,8 @@ # define SIGSTOP (-1) #endif +typedef void (*handler_t) (int); + int sigismember (const sigset_t *set, int sig) { @@ -133,7 +133,7 @@ sigpending (sigset_t *set) /* The previous signal handlers. Only the array elements corresponding to blocked signals are relevant. */ -static volatile sa_handler_t old_handlers[NSIG]; +static volatile handler_t old_handlers[NSIG]; int sigprocmask (int operation, const sigset_t *set, sigset_t *old_set) @@ -208,8 +208,8 @@ sigprocmask (int operation, const sigset_t *set, sigset_t *old_set) /* Install the handler FUNC for signal SIG, and return the previous handler. */ -sa_handler_t -rpl_signal (int sig, sa_handler_t handler) +handler_t +rpl_signal (int sig, handler_t handler) { /* We must provide a wrapper, so that a user can query what handler they installed even if that signal is currently blocked. */ @@ -227,7 +227,7 @@ rpl_signal (int sig, sa_handler_t handler) stale information if it calls signal(B). Oh well - signal handlers really shouldn't try to manipulate the installed handlers of unrelated signals. */ - sa_handler_t result = old_handlers[sig]; + handler_t result = old_handlers[sig]; old_handlers[sig] = handler; return result; } diff --git a/modules/sigprocmask b/modules/sigprocmask index 37adc63..e093112 100644 --- a/modules/sigprocmask +++ b/modules/sigprocmask @@ -3,7 +3,6 @@ POSIX compatible signal blocking. Files: lib/sigprocmask.c -lib/sig-handler.h m4/signalblocking.m4 Depends-on: -- 1.5.6
Re: sigaction, SA_SIGINFO, and SIG_IGN
Eric Blake wrote: it was failing to compile on mingw. Checking in this: To keep the *.m4 macros in sync with the code, I'm committing this: 2008-06-23 Bruno Haible [EMAIL PROTECTED] * m4/signalblocking.m4 (gl_PREREQ_SIG_HANDLER_H): Remove macro. (gl_PREREQ_SIGPROCMASK): Don't invoke it. * m4/sigaction.m4 (gl_PREREQ_SIG_HANDLER_H): New macro, moved here from m4/signalblocking.m4. (gl_PREREQ_SIGACTION): Don't invoke it. * m4/nanosleep.m4 (gl_PREREQ_NANOSLEEP): Invoke gl_PREREQ_SIG_HANDLER_H. * m4/fatal-signal.m4 (gl_FATAL_SIGNAL): Likewise. Don't check for sigaction here. --- m4/fatal-signal.m4.orig 2008-06-23 22:40:02.0 +0200 +++ m4/fatal-signal.m4 2008-06-23 22:39:10.0 +0200 @@ -1,4 +1,4 @@ -# fatal-signal.m4 serial 5 +# fatal-signal.m4 serial 6 dnl Copyright (C) 2003-2004, 2006, 2008 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -8,5 +8,5 @@ [ AC_REQUIRE([gt_TYPE_SIG_ATOMIC_T]) AC_CHECK_HEADERS_ONCE(unistd.h) - AC_CHECK_FUNCS(sigaction) + gl_PREREQ_SIG_HANDLER_H ]) --- m4/nanosleep.m4.orig2008-06-23 22:40:02.0 +0200 +++ m4/nanosleep.m4 2008-06-23 22:38:50.0 +0200 @@ -1,4 +1,4 @@ -#serial 24 +#serial 25 dnl From Jim Meyering. dnl Check for the nanosleep function. @@ -112,4 +112,5 @@ AC_DEFUN([gl_PREREQ_NANOSLEEP], [ AC_CHECK_HEADERS_ONCE(sys/select.h) + gl_PREREQ_SIG_HANDLER_H ]) --- m4/sigaction.m4.orig2008-06-23 22:40:02.0 +0200 +++ m4/sigaction.m4 2008-06-23 22:37:42.0 +0200 @@ -1,4 +1,4 @@ -# sigaction.m4 serial 2 +# sigaction.m4 serial 3 dnl Copyright (C) 2008 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -30,5 +30,11 @@ HAVE_SIGINFO_T=0 AC_SUBST([HAVE_SIGINFO_T]) fi - gl_PREREQ_SIG_HANDLER_H +]) + +# Prerequisites of lib/sig-handler.h. +AC_DEFUN([gl_PREREQ_SIG_HANDLER_H], +[ + AC_REQUIRE([AC_C_INLINE]) + : ]) --- m4/signalblocking.m4.orig 2008-06-23 22:40:02.0 +0200 +++ m4/signalblocking.m42008-06-23 22:37:20.0 +0200 @@ -1,4 +1,4 @@ -# signalblocking.m4 serial 8 +# signalblocking.m4 serial 9 dnl Copyright (C) 2001-2002, 2006-2008 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -39,12 +39,4 @@ dnl HAVE_SIGSET_T is 1 if the system lacks the sigprocmask function but has dnl the sigset_t type. AC_SUBST([HAVE_SIGSET_T]) - gl_PREREQ_SIG_HANDLER_H -]) - -# Prerequisites of lib/sig-handler.h. -AC_DEFUN([gl_PREREQ_SIG_HANDLER_H], -[ - AC_REQUIRE([AC_C_INLINE]) - : ])
Re: sigaction, SA_SIGINFO, and SIG_IGN
Eric Blake wrote: I named the helper file sig-handler.h. Since it uses 'inline', it needs AC_C_INLINE. 2008-06-22 Bruno Haible [EMAIL PROTECTED] * m4/signalblocking.m4 (gl_PREREQ_SIG_HANDLER_H): New macro. (gl_PREREQ_SIGPROCMASK): Invoke it. * m4/sigaction.m4 (gl_PREREQ_SIGACTION): Likewise. *** m4/sigaction.m4.orig2008-06-22 21:40:53.0 +0200 --- m4/sigaction.m4 2008-06-22 21:40:46.0 +0200 *** *** 1,4 ! # sigaction.m4 serial 1 dnl Copyright (C) 2008 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, --- 1,4 ! # sigaction.m4 serial 2 dnl Copyright (C) 2008 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, *** *** 18,29 fi ]) ! # Prerequisites of the part of lib/signal.in.h and of lib/sigprocmask.c. AC_DEFUN([gl_PREREQ_SIGACTION], [ AC_REQUIRE([AC_C_RESTRICT]) AC_REQUIRE([AC_TYPE_UID_T]) - AC_REQUIRE([gl_SIGNAL_H_DEFAULTS]) AC_CHECK_FUNCS_ONCE([sigaltstack siginterrupt]) AC_CHECK_TYPES([siginfo_t], [], [], [[ #include signal.h --- 18,29 fi ]) ! # Prerequisites of the part of lib/signal.in.h and of lib/sigaction.c. AC_DEFUN([gl_PREREQ_SIGACTION], [ + AC_REQUIRE([gl_SIGNAL_H_DEFAULTS]) AC_REQUIRE([AC_C_RESTRICT]) AC_REQUIRE([AC_TYPE_UID_T]) AC_CHECK_FUNCS_ONCE([sigaltstack siginterrupt]) AC_CHECK_TYPES([siginfo_t], [], [], [[ #include signal.h *** *** 32,35 --- 32,36 HAVE_SIGINFO_T=0 AC_SUBST([HAVE_SIGINFO_T]) fi + gl_PREREQ_SIG_HANDLER_H ]) *** m4/signalblocking.m4.orig 2008-06-22 21:40:53.0 +0200 --- m4/signalblocking.m42008-06-22 21:40:46.0 +0200 *** *** 1,4 ! # signalblocking.m4 serial 7 dnl Copyright (C) 2001-2002, 2006-2008 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, --- 1,4 ! # signalblocking.m4 serial 8 dnl Copyright (C) 2001-2002, 2006-2008 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, *** *** 39,42 --- 39,50 dnl HAVE_SIGSET_T is 1 if the system lacks the sigprocmask function but has dnl the sigset_t type. AC_SUBST([HAVE_SIGSET_T]) + gl_PREREQ_SIG_HANDLER_H + ]) + + # Prerequisites of lib/sig-handler.h. + AC_DEFUN([gl_PREREQ_SIG_HANDLER_H], + [ + AC_REQUIRE([AC_C_INLINE]) + : ])
Re: sigaction, SA_SIGINFO, and SIG_IGN
Since gl_SIGACTION may set HAVE_SIGACTION to its non-default value, the default value assignment must occur before it. 2008-06-22 Bruno Haible [EMAIL PROTECTED] * m4/sigaction.m4 (gl_SIGACTION): Invoke gl_SIGNAL_H_DEFAULTS. *** m4/sigaction.m4.orig2008-06-22 21:44:50.0 +0200 --- m4/sigaction.m4 2008-06-22 21:44:43.0 +0200 *** *** 7,12 --- 7,13 # Determine if sigaction interface is present. AC_DEFUN([gl_SIGACTION], [ + AC_REQUIRE([gl_SIGNAL_H_DEFAULTS]) dnl Due to autoconf conventions, we can't tell if HAVE_SIGACTION dnl means we have the type or means we have the function. We assume dnl that all implementations either have both or neither.
Re: sigaction, SA_SIGINFO, and SIG_IGN
In a comment you say that the mingw replacement for sigaction does not implement SA_RESTART. We can do better, since the msvcrt library never sets errno = EINTR anyway: Define SA_RESTART and ignore it. 2008-06-22 Bruno Haible [EMAIL PROTECTED] * lib/signal.in.h (SA_RESTART): New macro. * lib/sigaction.c: Update comment. *** lib/signal.in.h.orig2008-06-22 21:50:37.0 +0200 --- lib/signal.in.h 2008-06-22 21:50:26.0 +0200 *** *** 142,147 --- 142,148 /* Unsupported flags are not present. */ # define SA_RESETHAND 1 # define SA_NODEFER 2 + # define SA_RESTART 4 extern int sigaction (int, const struct sigaction *restrict, struct sigaction *restrict); *** lib/sigaction.c.orig2008-06-22 21:50:37.0 +0200 --- lib/sigaction.c 2008-06-22 21:50:26.0 +0200 *** *** 35,45 the situation by reading static storage in a signal handler, which POSIX warns is not generically async-signal-safe. Oh well. !Additionally, SIGCHLD is not defined, so we don't implement !SA_NOCLDSTOP or SA_NOCLDWAIT; sigaltstack() is not present, so we !don't implement SA_ONSTACK; and siginterrupt() is not present, so !we don't implement SA_RESTART. Supporting SA_SIGINFO is impossible !to do portably. POSIX states that an application should not mix signal() and sigaction(). We support the use of signal() within the gnulib --- 35,49 the situation by reading static storage in a signal handler, which POSIX warns is not generically async-signal-safe. Oh well. !Additionally: ! - We don't implement SA_NOCLDSTOP or SA_NOCLDWAIT, because SIGCHLD !is not defined. ! - We don't implement SA_ONSTACK, because sigaltstack() is not present. ! - We ignore SA_RESTART, because blocking Win32 calls are not interrupted !anyway when an asynchronous signal occurs, and the MSVCRT runtime !never sets errno to EINTR. ! - We don't implement SA_SIGINFO because it is impossible to do so !portably. POSIX states that an application should not mix signal() and sigaction(). We support the use of signal() within the gnulib
Re: sigaction, SA_SIGINFO, and SIG_IGN
I don't understand the Due to autoconf conventions. We are not using HAVE_SIGACTION as a #define in config.h. We are using it as a shell variable. This shell variable is not set by either AC_REPLACE_FUNCS([sigaction]) nor by AC_CHECK_TYPE(...). So there is no conflict. Proposed comment change: *** lib/signal.in.h.orig2008-06-22 21:55:31.0 +0200 --- lib/signal.in.h 2008-06-22 21:55:10.0 +0200 *** *** 118,126 typedef struct siginfo_t siginfo_t; # endif /* [EMAIL PROTECTED]@ */ ! /* Due to autoconf conventions, we can't tell if HAVE_SIGACTION ! means we have the type or means we have the function. We assume ! that all implementations either have both or neither. */ struct sigaction { --- 118,125 typedef struct siginfo_t siginfo_t; # endif /* [EMAIL PROTECTED]@ */ ! /* We assume that platforms which lack the sigaction() function also lack !the 'struct sigaction' type, and vice versa. */ struct sigaction { *** m4/sigaction.m4.orig2008-06-22 21:55:31.0 +0200 --- m4/sigaction.m4 2008-06-22 21:55:10.0 +0200 *** *** 8,16 AC_DEFUN([gl_SIGACTION], [ AC_REQUIRE([gl_SIGNAL_H_DEFAULTS]) - dnl Due to autoconf conventions, we can't tell if HAVE_SIGACTION - dnl means we have the type or means we have the function. We assume - dnl that all implementations either have both or neither. AC_REPLACE_FUNCS([sigaction]) if test $ac_cv_func_sigaction = no ; then HAVE_SIGACTION=0 --- 8,13
Re: sigaction, SA_SIGINFO, and SIG_IGN
The fact that the next POSIX calls siginterrupt 'obsolescent' is something that cannot be fixed by gnulib; therefore IMO it belongs outside the list of things that gnulib does not fix. Also, siginterrupt is related to SA_RESTART, not SA_NODEFER. 2008-06-22 Bruno Haible [EMAIL PROTECTED] * doc/posix-functions/siginterrupt.texi: Move note. --- doc/posix-functions/siginterrupt.texi.orig 2008-06-22 22:00:39.0 +0200 +++ doc/posix-functions/siginterrupt.texi 2008-06-22 21:58:59.0 +0200 @@ -15,7 +15,7 @@ @item This function is missing on some platforms: Solaris 2.5.1, mingw, Interix 3.5, BeOS. - [EMAIL PROTECTED] -POSIX recommends using @code{sigaction} with SA_NODEFER instead. @end itemize + +Note: POSIX recommends using @code{sigaction} with SA_RESTART instead of [EMAIL PROTECTED] (sig, 0)}.
Re: sigaction, SA_SIGINFO, and SIG_IGN
In the context of lib/fatal-signal.c, I find that SA_RESETHAND is not appropriate: If a fatal signal handler cleanup is interrupted by another fatal signal, it is better to start the cleanup a second time rather than terminating the program with incomplete cleanup. OK to apply? 2008-06-22 Bruno Haible [EMAIL PROTECTED] * lib/fatal-signal.c (fatal_signal_handler): Update comment. (install_handlers): Don't set the SA_RESETHAND flag. --- lib/fatal-signal.c.orig 2008-06-22 22:04:56.0 +0200 +++ lib/fatal-signal.c 2008-06-22 21:55:10.0 +0200 @@ -160,9 +160,10 @@ } /* Now execute the signal's default action. - If any cleanup action blocks the signal that triggered the cleanup, the - re-raised signal is delivered when this handler returns; otherwise it - is delivered already during raise(). */ + If the signal being delivered was blocked, the re-raised signal would be + delivered when this handler returns. But the way we install this handler, + no signal is blocked, and the re-raised signal is delivered already + during raise(). */ uninstall_handlers (); raise (sig); } @@ -176,9 +177,10 @@ struct sigaction action; action.sa_handler = fatal_signal_handler; - /* One-shot handling - if we fault while handling a fault, the - cleanup actions are intentionally cut short. */ - action.sa_flags = SA_NODEFER | SA_RESETHAND; + /* If we get a fatal signal while executing fatal_signal_handler, enter + fatal_signal_handler recursively, since it is reentrant. Hence no + SA_RESETHAND. */ + action.sa_flags = SA_NODEFER; sigemptyset (action.sa_mask); for (i = 0; i num_fatal_signals; i++) if (fatal_signals[i] = 0)
Re: sigaction, SA_SIGINFO, and SIG_IGN
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 6/20/2008 6:12 PM: | if (sigaction (fatal_signals[i], NULL, action) == 0 |get_handler (action) == SIG_IGN) | fatal_signals[i] = -1; | | This is acceptable to me. You are free to add this to lib/fatal-signal.c | (marking the function as static inline, please). | | Erik Blake wrote: | Perhaps it would be nicer if the prelude were part of | signal.in.h, so that both modules could use it? | | Better use a private include file for it. signal.in.h is supposed to be | only the replacement for the POSIX/glibc signal.h. The fact that it's | used unconditionally by modules/signal is only a missed optimization. Done as follows, as part of committing sigaction. It turns out that since mingw lacks struct sigaction, this only makes sense when using sigaction(). I named the helper file sig-handler.h. - -- Don't work too hard, make some time for fun as well! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkhdgU0ACgkQ84KuGfSFAYBOyACfSEp6R9fuG9nrP4A3KmboRjQ9 c/oAoIXF/R82OqfcwGaDjmfbJvztUCtj =hXJU -END PGP SIGNATURE- From 6dbadedb84d0fe02292a39f22942b8c172487ebf Mon Sep 17 00:00:00 2001 From: Eric Blake [EMAIL PROTECTED] Date: Sat, 21 Jun 2008 07:06:28 -0600 Subject: [PATCH] Improve robustness of sigprocmask by overriding signal. * lib/signal.in.h (rpl_signal): Override signal when sigprocmask is in use. * lib/sigprocmask.c (blocked_handler): Reinstall block handler. (SIGKILL, SIGSTOP): Provide fallbacks. (rpl_signal): Implement. (old_handlers, blocked_set): Mark volatile, since sigprocmask and signal can be called inside handlers. Signed-off-by: Eric Blake [EMAIL PROTECTED] --- ChangeLog |9 +++ lib/signal.in.h | 14 --- lib/sigprocmask.c | 65 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9f4d999..7d08ae6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ 2008-06-21 Eric Blake [EMAIL PROTECTED] + Improve robustness of sigprocmask by overriding signal. + * lib/signal.in.h (rpl_signal): Override signal when sigprocmask + is in use. + * lib/sigprocmask.c (blocked_handler): Reinstall block handler. + (SIGKILL, SIGSTOP): Provide fallbacks. + (rpl_signal): Implement. + (old_handlers, blocked_set): Mark volatile, since sigprocmask and + signal can be called inside handlers. + Fix nanosleep module on mingw. * modules/nanosleep (Depends-on): Add sys_select. * lib/nanosleep.c (HAVE_SYS_SELECT_H): Rely on gnulib module. diff --git a/lib/signal.in.h b/lib/signal.in.h index 9f82ba7..eb16afb 100644 --- a/lib/signal.in.h +++ b/lib/signal.in.h @@ -1,6 +1,6 @@ /* A GNU-like signal.h. - Copyright (C) 2006-2007 Free Software Foundation, Inc. + Copyright (C) 2006-2008 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -33,6 +33,10 @@ /* The definition of GL_LINK_WARNING is copied here. */ +/* Mingw defines sigset_t not in signal.h, but in sys/types.h. */ +#if [EMAIL PROTECTED]@ +# include sys/types.h +#endif #ifdef __cplusplus extern C { @@ -41,9 +45,6 @@ extern C { #if [EMAIL PROTECTED]@ -/* Mingw defines sigset_t not in signal.h, but in sys/types.h. */ -# include sys/types.h - /* Maximum signal number + 1. */ # ifndef NSIG # define NSIG 32 @@ -85,6 +86,11 @@ extern int sigpending (sigset_t *set); # define SIG_UNBLOCK 2 /* blocked_set = blocked_set ~*set; */ extern int sigprocmask (int operation, const sigset_t *set, sigset_t *old_set); +# define signal rpl_signal +/* Install the handler FUNC for signal SIG, and return the previous + handler. */ +extern void (*signal (int sig, void (*func) (int))) (int); + #endif diff --git a/lib/sigprocmask.c b/lib/sigprocmask.c index 456545a..87d585c 100644 --- a/lib/sigprocmask.c +++ b/lib/sigprocmask.c @@ -24,9 +24,24 @@ #include stdint.h #include stdlib.h -/* We assume that a platform without POSIX signal blocking functions also - does not have the POSIX sigaction() function, only the signal() function. - This is true for Woe32 platforms. */ +/* We assume that a platform without POSIX signal blocking functions + also does not have the POSIX sigaction() function, only the + signal() function. We also assume signal() has SysV semantics, + where any handler is uninstalled prior to being invoked. This is + true for Woe32 platforms. */ + +/* We use raw signal(), but also provide a wrapper rpl_signal() so + that applications can query or change a blocked signal. */ +#undef signal + +/* Provide invalid signal
Re: sigaction, SA_SIGINFO, and SIG_IGN
Bruno Haible [EMAIL PROTECTED] writes: This will not compile on Interix 3.5, whereas the current code does. True; how about working around that problem by inserting the following prelude into fatal-signal.c: typedef void (*sa_handler_t) (int); /* Return the handler of a signal, as a sa_handler_t value regardless of its true type. The resulting function can be compared to special values like SIG_IGN but it is not portable to call it. */ static sa_handler_t get_handler (struct sigaction const *a) { #ifdef SA_SIGINFO /* POSIX says that special values like SIG_IGN can only occur when action.sa_flags does not contain SA_SIGINFO. But in Linux 2.4, for example, sa_sigaction and sa_handler are aliases and a signal is ignored if sa_sigaction (after casting) equals SIG_IGN. So use (and cast) sa_sigaction in that case. */ if (a-sa_flags SA_SIGINFO) return (sa_handler_t) a-sa_sigaction; #endif return a-sa_handler; } and then using the following code in init_fatal_signals? if (sigaction (fatal_signals[i], NULL, action) == 0 get_handler (action) == SIG_IGN) fatal_signals[i] = -1;
Re: sigaction, SA_SIGINFO, and SIG_IGN
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Paul Eggert on 6/20/2008 12:41 PM: | Bruno Haible [EMAIL PROTECTED] writes: | | This will not compile on Interix 3.5, whereas the current code does. | | True; how about working around that problem by inserting the following | prelude into fatal-signal.c: I like the idea, but will wait for Bruno's response. But nanosleep.c has the same problem. Perhaps it would be nicer if the prelude were part of signal.in.h, so that both modules could use it? - -- Don't work too hard, make some time for fun as well! Eric Blake [EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkhb/dgACgkQ84KuGfSFAYDHnACgmWx2Ljfy5q9rlG50C+n1lhaC 4XcAn3IMhX3eXIj6lPuDFZFLbDgWcTrm =rzNB -END PGP SIGNATURE-
Re: sigaction, SA_SIGINFO, and SIG_IGN
Hi Paul, how about working around that problem by inserting the following prelude into fatal-signal.c: typedef void (*sa_handler_t) (int); /* Return the handler of a signal, as a sa_handler_t value regardless of its true type. The resulting function can be compared to special values like SIG_IGN but it is not portable to call it. */ static sa_handler_t get_handler (struct sigaction const *a) { #ifdef SA_SIGINFO /* POSIX says that special values like SIG_IGN can only occur when action.sa_flags does not contain SA_SIGINFO. But in Linux 2.4, for example, sa_sigaction and sa_handler are aliases and a signal is ignored if sa_sigaction (after casting) equals SIG_IGN. So use (and cast) sa_sigaction in that case. */ if (a-sa_flags SA_SIGINFO) return (sa_handler_t) a-sa_sigaction; #endif return a-sa_handler; } and then using the following code in init_fatal_signals? if (sigaction (fatal_signals[i], NULL, action) == 0 get_handler (action) == SIG_IGN) fatal_signals[i] = -1; This is acceptable to me. You are free to add this to lib/fatal-signal.c (marking the function as static inline, please). Erik Blake wrote: Perhaps it would be nicer if the prelude were part of signal.in.h, so that both modules could use it? Better use a private include file for it. signal.in.h is supposed to be only the replacement for the POSIX/glibc signal.h. The fact that it's used unconditionally by modules/signal is only a missed optimization. Bruno
Re: sigaction, SA_SIGINFO, and SIG_IGN
Bruno Haible [EMAIL PROTECTED] writes: if (sigaction (fatal_signals[i], NULL, action) = 0 + /* POSIX says that SIG_IGN can only occur when action.sa_flags + does not contain SA_SIGINFO. But in Linux 2.4, for example, + SA_SIGINFO can actually be set and is ignored when sa_handler + is SIG_IGN. So don't bother testing for SA_SIGINFO. */ action.sa_handler == SIG_IGN) fatal_signals[i] = -1; I'd feel a bit safer if we wrote the code to conform to POSIX rather than assume the typical implementation where sa_handler and sa_sigaction overlap. How about something like this instead? if (sigaction (fatal_signals[i], NULL, action) == 0 ((action.sa_flags SA_SIGINFO ? (void (*) (int)) action.sa_sigaction : action.sa_handler) == SIG_IGN)) fatal_signals[i] = -1; This avoids undefined behavior from the point of view of C, while remaining portable to implementations where sa_handler and sa_sigaction don't overlap. On typical implementations with a good optimizing compiler, this code should run just as fast as what's in there now.
Re: sigaction, SA_SIGINFO, and SIG_IGN
Paul Eggert wrote: I'd feel a bit safer if we wrote the code to conform to POSIX rather than assume the typical implementation where sa_handler and sa_sigaction overlap. Sorry, I can't follow the argumentation. The set of systems where sa_handler and sa_sigaction are disjoint is empty, and you make assumptions how these systems will likely behave. Since such systems don't exist, it is speculation. You base your speculation on POSIX, but the case we are discussing is when the application does not obey POSIX and the system does not detect it. How do you want to use POSIX as an argument here? How about something like this instead? if (sigaction (fatal_signals[i], NULL, action) == 0 ((action.sa_flags SA_SIGINFO ? (void (*) (int)) action.sa_sigaction : action.sa_handler) == SIG_IGN)) fatal_signals[i] = -1; This will not compile on Interix 3.5, whereas the current code does. And while Interix is not POSIX conformant, it is an existing system. Bruno
Re: sigaction, SA_SIGINFO, and SIG_IGN
Jason Zions wrote about Interix 3.5: Interix and the Vista/WS08 Subsystem for UNIX Applications do not include an sa_sigaction member, as you point out, but they don't claim to conform to the C Extensions or to IEEE Std 1003.1-2001. Then, if the system does not claim to conform to POSIX, you don't need to CC the Austin Group. Interix 3.5 does not even conform to ISO C 99: it lacks elementary header files such as stdint.h, inttypes.h, complex.h, etc. Bruno
Re: sigaction, SA_SIGINFO, and SIG_IGN
Eric Blake wrote: | POSIX is not correct about the type of SIG_DFL and SIG_IGN: | In http://www.opengroup.org/susv3/basedefs/signal.h.html it says | ... constants, each of which expands to a distinct constant expression of |the type void (*)(int) ...: SIG_DFL SIG_IGN | but also | void (*)(int, siginfo_t *, void *) sa_sigaction |Pointer to signal handler function or one of the macros SIG_IGN or SIG_DFL. | But sa_sigaction cannot be SIG_IGN or SIG_DFL because they are of different | types! I think you misquoted that. In both 2001 (you gave the URL) and draft 5.1 of 200x, the description of struct sigaction only mention SIG_IGN and SIG_DFL in relation to sa_handler, while the test for sa_sigaction states only Pointer to a signal-catching function. You're right here. I was looking at an older copy of the same URL. They now fixed it. So, SIG_IGN and SIG_DFL can *only* be used in sa_handler. | What if someone actually installs a handler with SA_SIGINFO and SIG_IGN? In theory, they can't, since SIG_IGN is typed differently than sa_sigaction, but sa_sigaction is the only field that can be used when SA_SIGINFO is set. There is no statement which fields of the struct a programmer may use or not. (Actually when a programmer copies a 'struct sigaction' he copies all fields.) The paragraph that applies is (from the sigaction description): If the SA_SIGINFO flag (see below) is cleared in the sa_flags field of the sigaction structure, the sa_handler field identifies the action to be associated with the specified signal. If the SA_SIGINFO flag is set in the sa_flags field, the sa_sigaction field specifies a signal-catching function. So yes, SIG_IGN with SA_SIGINFO is not allowed by POSIX. In theory. Because in practice, on Linux, the program -- #include signal.h #include stdio.h int main () { struct sigaction s; s.sa_handler = SIG_IGN; s.sa_flags = SA_SIGINFO; int ret = sigaction (SIGINT, s, NULL); printf (ret = %d\n, ret); for (;;) ; } --- prints ret = 0 and then is not interruptible by Ctrl-C. So, Linux has ignored the SA_SIGINFO flag here. And this is why in fatal-signal.c I test for SIG_IGN *regardless* of the SA_SIGINFO flag. gcc's abort() is acceptable if the user attempted to invoke the function via the wrong type, but NOT on just casting it. Yes. gcc's abort() comes in situations like this where a known external function is casted and then immediately called: extern double sin (double x); long double x = ((long double (*) (long double)) sin) (0L); Does anyone know of a system where sa_handler and sa_sigaction do not overlap, or where the kernel sets a signal handler to SIG_DFL or SIG_IGN but accidentally sets the SA_SIGINFO flag in the return parameter of sigaction? The include files of all systems that I have access to contain sa_handler and sa_sigaction as elements of the same union, i.e. they overlap. Only Interix 3.5 has a struct sigaction that lacks the 'sa_sigaction' member. But don't waste your time on that system, it's not worth porting to. Bruno 2008-06-18 Bruno Haible [EMAIL PROTECTED] * lib/fatal-signal.c (init_fatal_signals): Add comment. Reported by Eric Blake. *** lib/fatal-signal.c.orig 2008-06-19 03:33:07.0 +0200 --- lib/fatal-signal.c 2008-06-19 03:32:28.0 +0200 *** *** 96,101 --- 96,105 struct sigaction action; if (sigaction (fatal_signals[i], NULL, action) = 0 + /* POSIX says that SIG_IGN can only occur when action.sa_flags +does not contain SA_SIGINFO. But in Linux 2.4, for example, +SA_SIGINFO can actually be set and is ignored when sa_handler +is SIG_IGN. So don't bother testing for SA_SIGINFO. */ action.sa_handler == SIG_IGN) fatal_signals[i] = -1; }
RE: sigaction, SA_SIGINFO, and SIG_IGN
The include files of all systems that I have access to contain sa_handler and sa_sigaction as elements of the same union, i.e. they overlap. Only Interix 3.5 has a struct sigaction that lacks the 'sa_sigaction' member. But don't waste your time on that system, it's not worth porting to. Sez you. A few hundred thousand users of applications ported to that environment might disagree. Interix and the Vista/WS08 Subsystem for UNIX Applications do not include an sa_sigaction member, as you point out, but they don't claim to conform to the C Extensions or to IEEE Std 1003.1-2001. Dozens of other useful systems fall under those same exclusions for various reasons. So let's get back to factual matters, m'kay? It really doesn't matter if any implementations of 1003.1-2001 or later have sa_handler and sa_sigaction as separate fields. A conforming application is required to behave as if the fields overlap. Which means to say, a conforming implementation isn't permitted to force an application to behave as if those fields were in simultaneous use. The signal.h.html page at opengroup.org is quite clear about this: The storage occupied by sa_handler and sa_sigaction may overlap, and a conforming application shall not use both simultaneously. -Original Message- From: Bruno Haible [mailto:[EMAIL PROTECTED] Sent: Wednesday, June 18, 2008 6:35 PM To: Eric Blake Cc: bug-gnulib@gnu.org Subject: Re: sigaction, SA_SIGINFO, and SIG_IGN Eric Blake wrote: | POSIX is not correct about the type of SIG_DFL and SIG_IGN: | In http://www.opengroup.org/susv3/basedefs/signal.h.html it says | ... constants, each of which expands to a distinct constant expression of |the type void (*)(int) ...: SIG_DFL SIG_IGN | but also | void (*)(int, siginfo_t *, void *) sa_sigaction |Pointer to signal handler function or one of the macros SIG_IGN or SIG_DFL. | But sa_sigaction cannot be SIG_IGN or SIG_DFL because they are of different | types! I think you misquoted that. In both 2001 (you gave the URL) and draft 5.1 of 200x, the description of struct sigaction only mention SIG_IGN and SIG_DFL in relation to sa_handler, while the test for sa_sigaction states only Pointer to a signal-catching function. You're right here. I was looking at an older copy of the same URL. They now fixed it. So, SIG_IGN and SIG_DFL can *only* be used in sa_handler. | What if someone actually installs a handler with SA_SIGINFO and SIG_IGN? In theory, they can't, since SIG_IGN is typed differently than sa_sigaction, but sa_sigaction is the only field that can be used when SA_SIGINFO is set. There is no statement which fields of the struct a programmer may use or not. (Actually when a programmer copies a 'struct sigaction' he copies all fields.) The paragraph that applies is (from the sigaction description): If the SA_SIGINFO flag (see below) is cleared in the sa_flags field of the sigaction structure, the sa_handler field identifies the action to be associated with the specified signal. If the SA_SIGINFO flag is set in the sa_flags field, the sa_sigaction field specifies a signal-catching function. So yes, SIG_IGN with SA_SIGINFO is not allowed by POSIX. In theory. Because in practice, on Linux, the program -- #include signal.h #include stdio.h int main () { struct sigaction s; s.sa_handler = SIG_IGN; s.sa_flags = SA_SIGINFO; int ret = sigaction (SIGINT, s, NULL); printf (ret = %d\n, ret); for (;;) ; } --- prints ret = 0 and then is not interruptible by Ctrl-C. So, Linux has ignored the SA_SIGINFO flag here. And this is why in fatal-signal.c I test for SIG_IGN *regardless* of the SA_SIGINFO flag. gcc's abort() is acceptable if the user attempted to invoke the function via the wrong type, but NOT on just casting it. Yes. gcc's abort() comes in situations like this where a known external function is casted and then immediately called: extern double sin (double x); long double x = ((long double (*) (long double)) sin) (0L); Does anyone know of a system where sa_handler and sa_sigaction do not overlap, or where the kernel sets a signal handler to SIG_DFL or SIG_IGN but accidentally sets the SA_SIGINFO flag in the return parameter of sigaction? The include files of all systems that I have access to contain sa_handler and sa_sigaction as elements of the same union, i.e. they overlap. Only Interix 3.5 has a struct sigaction that lacks the 'sa_sigaction' member. But don't waste your time on that system, it's not worth porting to. Bruno 2008-06-18 Bruno Haible [EMAIL PROTECTED] * lib/fatal-signal.c (init_fatal_signals): Add comment. Reported by Eric Blake. *** lib/fatal-signal.c.orig 2008-06-19 03:33:07.0 +0200 --- lib/fatal-signal.c 2008-06-19 03:32:28.0 +0200 *** *** 96,101
Re: sigaction, SA_SIGINFO, and SIG_IGN
Eric Blake wrote: | struct sigaction action; | | if (sigaction (fatal_signals[i], NULL, action) = 0 | + (action.sa_flags SA_SIGINFO) == 0 | action.sa_handler == SIG_IGN) |fatal_signals[i] = -1; |} Pre-existing bug. POSIX states that sa_handler and sa_siginfo may, but not must, occupy overlapping space, and that sa_handler must not be accessed if sa_flags includes SA_SIGINFO. Therefore, it is conceivable to have an implementation where action.sa_handler is garbage and happens to be SIG_IGN, because only action.sa_siginfo was valid to access. (In reality, all implementations I am aware of, including my replacement, use a union and let the two function definitions overlap, so this is more of a theoretical bug ... Sometimes you can also kill working programs by attempting too much standards compliance: What if someone actually installs a handler with SA_SIGINFO and SIG_IGN? The kernel will not deliver such signals. I.e. the right way to fix the code would be if (sigaction (fatal_signals[i], NULL, action) = 0 (action.sa_flags SA_SIGINFO ? (void *) action.sa_sigaction == (void *) SIG_IGN : action.sa_handler == SIG_IGN)) [Note that you cannot cast a function pointer into a different function pointer. gcc generates code equivalent to an abort() sometimes if you do that.] sa_handler must not be accessed if sa_flags includes SA_SIGINFO. The wording is not as clear as you might think. POSIX is not correct about the type of SIG_DFL and SIG_IGN: In http://www.opengroup.org/susv3/basedefs/signal.h.html it says ... constants, each of which expands to a distinct constant expression of the type void (*)(int) ...: SIG_DFL SIG_IGN but also void (*)(int, siginfo_t *, void *) sa_sigaction Pointer to signal handler function or one of the macros SIG_IGN or SIG_DFL. But sa_sigaction cannot be SIG_IGN or SIG_DFL because they are of different types! Also in file:/other/www/www.opengroup.org/susv3/functions/sigaction.html the sentence If the SA_SIGINFO flag is set in the sa_flags field, and the implementation supports the Realtime Signals Extension option or the XSI Extension option, the sa_sigaction field specifies a signal-catching function. indicates that the sa_sigaction field can never be SIG_DFL or SIG_IGN. Hence you always need to test sa_handler for that. According to this reading of POSIX, one is not allowed to use SA_SIGINFO with SIG_DFL or SIG_IGN; in this case, the existing code in lib/fatal-signal.c is plain correct. In summary, this is too much theoretical head-ache for something which works in practice - since sa_handler and sa_sigaction share the same memory location. You can put in a comment if you like. But no need to change code that works everywhere. Bruno
Re: sigaction, SA_SIGINFO, and SIG_IGN
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 6/17/2008 8:12 PM: | | Eric Blake wrote: | | | if (sigaction (fatal_signals[i], NULL, action) = 0 | | + (action.sa_flags SA_SIGINFO) == 0 | |action.sa_handler == SIG_IGN) | | fatal_signals[i] = -1; | | } | | Pre-existing bug. POSIX states that sa_handler and sa_siginfo may, but | not must, occupy overlapping space, and that sa_handler must not be | accessed if sa_flags includes SA_SIGINFO. Therefore, it is conceivable to | have an implementation where action.sa_handler is garbage and happens to | be SIG_IGN, because only action.sa_siginfo was valid to access. (In | reality, all implementations I am aware of, including my replacement, use | a union and let the two function definitions overlap, so this is more of a | theoretical bug ... | | What if someone actually installs a handler with SA_SIGINFO and SIG_IGN? In theory, they can't, since SIG_IGN is typed differently than sa_sigaction, but sa_sigaction is the only field that can be used when SA_SIGINFO is set. I'm adding the Austin Group, in case anyone else wants to chime in. | The kernel will not deliver such signals. I.e. the right way to fix the code | would be | | if (sigaction (fatal_signals[i], NULL, action) = 0 | (action.sa_flags SA_SIGINFO | ? (void *) action.sa_sigaction == (void *) SIG_IGN | : action.sa_handler == SIG_IGN)) Not portable, and by your own arguments, not necessary. C89 and C99 requires this to trigger a compilation warning, since SIG_IGN is typed as a function pointer, which is not necessarily compatible with void * (although POSIX 200x adds the restriction that void* must be compatible with all function pointers). | | [Note that you cannot cast a function pointer into a different function | pointer. gcc generates code equivalent to an abort() sometimes if you do that.] Casting between function pointer types is different than casting between function pointer and void*. C99 states this: A pointer to a function of one type may be converted to a pointer to a function of another type and back again; the result shall compare equal to the original pointer. If a converted pointer is used to call a function whose type is not compatible with the pointed-to type, the behavior is undefined. gcc's abort() is acceptable if the user attempted to invoke the function via the wrong type, but NOT on just casting it. | | sa_handler must not be accessed if sa_flags includes SA_SIGINFO. | | The wording is not as clear as you might think. | | POSIX is not correct about the type of SIG_DFL and SIG_IGN: | In http://www.opengroup.org/susv3/basedefs/signal.h.html it says | ... constants, each of which expands to a distinct constant expression of |the type void (*)(int) ...: SIG_DFL SIG_IGN | but also | void (*)(int, siginfo_t *, void *) sa_sigaction |Pointer to signal handler function or one of the macros SIG_IGN or SIG_DFL. | But sa_sigaction cannot be SIG_IGN or SIG_DFL because they are of different | types! I think you misquoted that. In both 2001 (you gave the URL) and draft 5.1 of 200x, the description of struct sigaction only mention SIG_IGN and SIG_DFL in relation to sa_handler, while the test for sa_sigaction states only Pointer to a signal-catching function. But in researching this, I noticed that sigaction now has a rather big bug introduced in the 200x draft. In the process of moving real-time-extension and some XSI functionality into base, line 60982 of draft 5.1 now states: The sigaction( ) function shall fail if: [ENOTSUP] The SA_SIGINFO bit flag is set in the sa_flags field of the sigaction structure. In POSIX 2001, this was meant for the case where sigaction was supported but SA_SIGINFO was not; but now that 200x requires SA_SIGINFO in Base, this sentence taken literally means that you can't use SA_SIGINFO without causing sigaction to fail with ENOTSUP! What was intended was probably removing the ENOTSUP clause altogether. Furthermore, it adds line 60986: In addition, the sigaction( ) function may fail if the SA_SIGINFO flag is set in the sa_flags field of the sigaction structure for a signal not in the range SIGRTMIN to SIGRTMAX but without designation of which errno to use, and which seems to be in conflict with sigqueue which has no similar restriction on SA_SIGINFO only being used on SIGRT* signals. | | Also in file:/other/www/www.opengroup.org/susv3/functions/sigaction.html | the sentence | If the SA_SIGINFO flag is set in the sa_flags field, and the implementation |supports the Realtime Signals Extension option or the XSI Extension option, |the sa_sigaction field specifies a signal-catching function. | indicates that the sa_sigaction field can never be SIG_DFL or SIG_IGN. Hence | you always need to test sa_handler for that. | | According to this reading of POSIX, one is not allowed to use SA_SIGINFO with | SIG_DFL or SIG_IGN; in