public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [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).