* [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD signal semantics @ 2021-06-17 18:55 Pedro Alves 2021-06-17 21:05 ` John Baldwin 2021-06-22 13:49 ` [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD " Tom Tromey 0 siblings, 2 replies; 5+ messages in thread From: Pedro Alves @ 2021-06-17 18:55 UTC (permalink / raw) To: gdb-patches The new "maint selftest scoped_ignore_sigpipe" unit test currently fails on Solaris, and probably on all BSDs. The problem is that the test registers a SIGPIPE signal handler, and doesn't account for BSD semantics where we need to rearm the signal disposition every time the signal handler is called. The result is that GDB dies from SIGPIPE because the test triggers a SIGPIPE more than once: $ gdb -q -ex "maint selftest scoped_ignore_sigpipe" Running selftest scoped_ignore_sigpipe. $ (gdb exited due to SIGPIPE) Fix this by using sigaction. I'm not adding the usual #ifdef HAVE_SIGACTION goo, because I really want to believe that all systems that support SIGPIPE support sigaction nowadays. GNU/Linux, Hurd, BSDs, macOS, Cygwin, DJGPP, AIX, etc., anything resembling a modern Unix does support it AFAIK, only mingw does not support it, but OTOH, it also doesn't define SIGPIPE. Confirmed by cross building GDB for mingw-w64. We could probably remove the HAVE_SIGPROCMASK check too, actually. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <pedro@palves.net> * unittests/scoped_ignore_signal-selftests.c (test_sigpipe): Use sigaction instead of signal to avoid BSD signal semantics. Change-Id: I08aa6be097e1140efdb4cc63e95a845595ce4069 --- gdb/unittests/scoped_ignore_signal-selftests.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/gdb/unittests/scoped_ignore_signal-selftests.c b/gdb/unittests/scoped_ignore_signal-selftests.c index 29826e325dc..5927d566552 100644 --- a/gdb/unittests/scoped_ignore_signal-selftests.c +++ b/gdb/unittests/scoped_ignore_signal-selftests.c @@ -45,8 +45,14 @@ handle_sigpipe (int) static void test_sigpipe () { - auto *osig = signal (SIGPIPE, handle_sigpipe); - SCOPE_EXIT { signal (SIGPIPE, osig); }; + struct sigaction new_action, old_action; + + new_action.sa_handler = handle_sigpipe; + sigemptyset (&new_action.sa_mask); + new_action.sa_flags = 0; + + sigaction (SIGPIPE, &new_action, &old_action); + SCOPE_EXIT { sigaction (SIGPIPE, &old_action, nullptr); }; #ifdef HAVE_SIGPROCMASK /* Make sure SIGPIPE isn't blocked. */ base-commit: 336b30e58ae98fe66862ab8480d3f7bb885fef23 -- 2.26.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD signal semantics 2021-06-17 18:55 [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD signal semantics Pedro Alves @ 2021-06-17 21:05 ` John Baldwin 2021-06-17 21:23 ` [pushed] Fix scoped_ignore_sigpipe selftest on systems with SysV " Pedro Alves 2021-06-22 13:49 ` [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD " Tom Tromey 1 sibling, 1 reply; 5+ messages in thread From: John Baldwin @ 2021-06-17 21:05 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 6/17/21 11:55 AM, Pedro Alves wrote: > The new "maint selftest scoped_ignore_sigpipe" unit test currently > fails on Solaris, and probably on all BSDs. The problem is that the > test registers a SIGPIPE signal handler, and doesn't account for BSD > semantics where we need to rearm the signal disposition every time the > signal handler is called. The result is that GDB dies from SIGPIPE > because the test triggers a SIGPIPE more than once: > > $ gdb -q -ex "maint selftest scoped_ignore_sigpipe" > Running selftest scoped_ignore_sigpipe. > $ > (gdb exited due to SIGPIPE) > > Fix this by using sigaction. I'm not adding the usual #ifdef > HAVE_SIGACTION goo, because I really want to believe that all systems > that support SIGPIPE support sigaction nowadays. GNU/Linux, Hurd, > BSDs, macOS, Cygwin, DJGPP, AIX, etc., anything resembling a modern > Unix does support it AFAIK, only mingw does not support it, but OTOH, > it also doesn't define SIGPIPE. Confirmed by cross building GDB for > mingw-w64. FWIW, BSD's dropped this behavior long ago. FreeBSD passes the existing test currently with gdb master. There's a comment about this in the signal(3) manpage on FreeBSD (and likely others) that goes back to 4.2BSD: The handled signal is unblocked when the function returns and the process continues from where it left off when the signal occurred. Unlike previous signal facilities, the handler func() remains installed after a signal has been delivered. The comment was added in this commit in 1985: https://svnweb.freebsd.org/csrg?view=revision&revision=20133 That said, using sigaction directly seems fine to me. -- John Baldwin ^ permalink raw reply [flat|nested] 5+ messages in thread
* [pushed] Fix scoped_ignore_sigpipe selftest on systems with SysV signal semantics 2021-06-17 21:05 ` John Baldwin @ 2021-06-17 21:23 ` Pedro Alves 0 siblings, 0 replies; 5+ messages in thread From: Pedro Alves @ 2021-06-17 21:23 UTC (permalink / raw) To: John Baldwin, gdb-patches On 2021-06-17 10:05 p.m., John Baldwin wrote: > On 6/17/21 11:55 AM, Pedro Alves wrote: >> The new "maint selftest scoped_ignore_sigpipe" unit test currently >> fails on Solaris, and probably on all BSDs. The problem is that the >> test registers a SIGPIPE signal handler, and doesn't account for BSD >> semantics where we need to rearm the signal disposition every time the >> signal handler is called. The result is that GDB dies from SIGPIPE >> because the test triggers a SIGPIPE more than once: >> >> $ gdb -q -ex "maint selftest scoped_ignore_sigpipe" >> Running selftest scoped_ignore_sigpipe. >> $ >> (gdb exited due to SIGPIPE) >> >> Fix this by using sigaction. I'm not adding the usual #ifdef >> HAVE_SIGACTION goo, because I really want to believe that all systems >> that support SIGPIPE support sigaction nowadays. GNU/Linux, Hurd, >> BSDs, macOS, Cygwin, DJGPP, AIX, etc., anything resembling a modern >> Unix does support it AFAIK, only mingw does not support it, but OTOH, >> it also doesn't define SIGPIPE. Confirmed by cross building GDB for >> mingw-w64. > > FWIW, BSD's dropped this behavior long ago. FreeBSD passes the existing > test currently with gdb master. There's a comment about this in the > signal(3) manpage on FreeBSD (and likely others) that goes back to 4.2BSD: > > The handled signal is unblocked when the function returns and the process > continues from where it left off when the signal occurred. Unlike > previous signal facilities, the handler func() remains installed after a > signal has been delivered. > > The comment was added in this commit in 1985: > > https://svnweb.freebsd.org/csrg?view=revision&revision=20133 > Ah, thanks for digging that! Turns out I got the history backwards :-/ -- the old broken semantics were in old Unix and then SysV, and it was BSD that fixed it. Solaris is of SVR4 heritage, which I guess explains its behavior. Sorry for the confusion. > That said, using sigaction directly seems fine to me. > I've installed the patch with a fixed subject/log, as below. Thanks! From b2b3c0c07ae5f63ac4b6869fa602bb1bce86d587 Mon Sep 17 00:00:00 2001 From: Pedro Alves <pedro@palves.net> Date: Thu, 17 Jun 2021 18:30:51 +0100 Subject: [PATCH] Fix scoped_ignore_sigpipe selftest on systems with SysV signal semantics The new "maint selftest scoped_ignore_sigpipe" unit test currently fails on Solaris. The problem is that the test registers a SIGPIPE signal handler, and doesn't account for old SysV semantics where we need to rearm the signal disposition every time the signal handler is called. The result is that GDB dies from SIGPIPE because the test triggers a SIGPIPE more than once: $ gdb -q -ex "maint selftest scoped_ignore_sigpipe" Running selftest scoped_ignore_sigpipe. $ (gdb exited due to SIGPIPE) Fix this by using sigaction. I'm not adding the usual #ifdef HAVE_SIGACTION goo, because I really want to believe that all systems that support SIGPIPE support sigaction nowadays. GNU/Linux, Hurd, BSDs, macOS, Cygwin, DJGPP, AIX, etc., anything resembling a modern Unix does support it AFAIK, only mingw does not support it, but OTOH, it also doesn't define SIGPIPE. Confirmed by cross building GDB for mingw-w64. We could probably remove the HAVE_SIGPROCMASK check too, actually. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <pedro@palves.net> * unittests/scoped_ignore_signal-selftests.c (test_sigpipe): Use sigaction instead of signal to avoid SysV signal semantics. Change-Id: I08aa6be097e1140efdb4cc63e95a845595ce4069 --- gdb/ChangeLog | 6 ++++++ gdb/unittests/scoped_ignore_signal-selftests.c | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index c70f6ef5329..8333a17f841 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2021-06-17 Pedro Alves <pedro@palves.net> + + * unittests/scoped_ignore_signal-selftests.c (test_sigpipe): + Use sigaction instead of signal to avoid SysV signal + semantics. + 2021-06-17 Pedro Alves <pedro@palves.net> * scoped_ignore_signal.h (scoped_ignore_signal): Add diff --git a/gdb/unittests/scoped_ignore_signal-selftests.c b/gdb/unittests/scoped_ignore_signal-selftests.c index 29826e325dc..5927d566552 100644 --- a/gdb/unittests/scoped_ignore_signal-selftests.c +++ b/gdb/unittests/scoped_ignore_signal-selftests.c @@ -45,8 +45,14 @@ handle_sigpipe (int) static void test_sigpipe () { - auto *osig = signal (SIGPIPE, handle_sigpipe); - SCOPE_EXIT { signal (SIGPIPE, osig); }; + struct sigaction new_action, old_action; + + new_action.sa_handler = handle_sigpipe; + sigemptyset (&new_action.sa_mask); + new_action.sa_flags = 0; + + sigaction (SIGPIPE, &new_action, &old_action); + SCOPE_EXIT { sigaction (SIGPIPE, &old_action, nullptr); }; #ifdef HAVE_SIGPROCMASK /* Make sure SIGPIPE isn't blocked. */ base-commit: 336b30e58ae98fe66862ab8480d3f7bb885fef23 -- 2.26.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD signal semantics 2021-06-17 18:55 [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD signal semantics Pedro Alves 2021-06-17 21:05 ` John Baldwin @ 2021-06-22 13:49 ` Tom Tromey 2021-06-22 19:00 ` Pedro Alves 1 sibling, 1 reply; 5+ messages in thread From: Tom Tromey @ 2021-06-22 13:49 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Pedro> Fix this by using sigaction. I'm not adding the usual #ifdef Pedro> HAVE_SIGACTION goo, because I really want to believe that all systems Pedro> that support SIGPIPE support sigaction nowadays. GNU/Linux, Hurd, Pedro> BSDs, macOS, Cygwin, DJGPP, AIX, etc., anything resembling a modern Pedro> Unix does support it AFAIK, only mingw does not support it, but OTOH, Pedro> it also doesn't define SIGPIPE. Confirmed by cross building GDB for Pedro> mingw-w64. gnulib seems to agree: https://www.gnu.org/software/gnulib/manual/html_node/sigprocmask.html Pedro> We could probably remove the HAVE_SIGPROCMASK check too, actually. Sounds good. Tom ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD signal semantics 2021-06-22 13:49 ` [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD " Tom Tromey @ 2021-06-22 19:00 ` Pedro Alves 0 siblings, 0 replies; 5+ messages in thread From: Pedro Alves @ 2021-06-22 19:00 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 2021-06-22 2:49 p.m., Tom Tromey wrote: > Pedro> Fix this by using sigaction. I'm not adding the usual #ifdef > Pedro> HAVE_SIGACTION goo, because I really want to believe that all systems > Pedro> that support SIGPIPE support sigaction nowadays. GNU/Linux, Hurd, > Pedro> BSDs, macOS, Cygwin, DJGPP, AIX, etc., anything resembling a modern > Pedro> Unix does support it AFAIK, only mingw does not support it, but OTOH, > Pedro> it also doesn't define SIGPIPE. Confirmed by cross building GDB for > Pedro> mingw-w64. > > gnulib seems to agree: > > https://www.gnu.org/software/gnulib/manual/html_node/sigprocmask.html > Thanks, I didn't remember to check that. That's the sigprocmask page, but the sigaction one (which no doubt you meant to paste) is similar. > Pedro> We could probably remove the HAVE_SIGPROCMASK check too, actually. > > Sounds good. I'll try to remember to do that soon. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-22 19:00 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-17 18:55 [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD signal semantics Pedro Alves 2021-06-17 21:05 ` John Baldwin 2021-06-17 21:23 ` [pushed] Fix scoped_ignore_sigpipe selftest on systems with SysV " Pedro Alves 2021-06-22 13:49 ` [PATCH] Fix scoped_ignore_sigpipe selftest on systems with BSD " Tom Tromey 2021-06-22 19:00 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).