Fix scoped_ignore_sigpipe selftest on systems with BSD signal semantics

Message ID 20210617185545.3826154-1-pedro@palves.net
State New
Headers show
Series
  • Fix scoped_ignore_sigpipe selftest on systems with BSD signal semantics
Related show

Commit Message

Pedro Alves June 17, 2021, 6:55 p.m.
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(-)


base-commit: 336b30e58ae98fe66862ab8480d3f7bb885fef23
-- 
2.26.2

Comments

John Baldwin June 17, 2021, 9:05 p.m. | #1
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
Tom Tromey June 22, 2021, 1:49 p.m. | #2
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
Pedro Alves June 22, 2021, 7 p.m. | #3
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.

Patch

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.  */