[pushed] Fix scoped_ignore_sigpipe selftest on systems with SysV signal semantics

Message ID 1d54490c-9d58-c6f8-b02d-0c8bb34ad16f@palves.net
State New
Headers show
Series
  • [pushed] Fix scoped_ignore_sigpipe selftest on systems with SysV signal semantics
Related show

Commit Message

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


base-commit: 336b30e58ae98fe66862ab8480d3f7bb885fef23
-- 
2.26.2

Patch

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