On 8/28/21 8:40 AM, Thiago Jung Bauermann via bug-diffutils via All
diffutils discussion. wrote:
I believe this is the same problem reported in bug 34519.
The Debian build also fails with "diff: standard output: Broken pipe".
Thanks for tracking that down. Frédéric's analysis in
<https://bugs.debian.org/922552#19> was helpful.
I found some time to look into this bug, and installed into
Savannah.gnu.org diffutils the attached patch, which I hope fixes the
bug although I don't have the relevant platform to test it. Please give
it a try.
Once this patch is part of a release, Debian shouldn't need any patches
for diffutils.
For now I am closing the diffutils bug report
<https://bugs.gnu.org/36488>; if I was too optimistic and the patch
doesn't fix things we can always reopen it.
From 9b20182d48481c7ca647ff8926feeb8e1da4f7b0 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 28 Aug 2021 23:49:32 -0700
Subject: [PATCH] diff: cleanup signal handling just before exit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This should fix an unlikely signal handling bug with colored
output, and should also fix a Debian FTBFS (Fails To Build From
Source) on powerpc64le-linux. See Bug#34519 and Frédéric
Bonnard’s report in:
https://bugs.debian.org/922552#19
* bootstrap.conf (gnulib_modules): Add raise, sigprocmask.
* src/diff.c (main): Call cleanup_signal_handlers before exiting.
Don’t bother calling ‘exit’; no longer needed nowadays.
* src/util.c (sigprocmask, siginterrupt) [!SA_NOCLDSTOP]:
Define to 0 instead of empty, since the results are now used.
(sigset_t) [!SA_NOCLDSTOP]: Remove; we now rely on Gnulib.
(xsigaction) [SA_NOCLDSTOP]: New function.
(xsigaddset, xsigismember, xsignal, xsigprocmask): New functions.
(some_signals_caught): New static var.
(process_signals): Omit a conditional branch.
Don’t bother loading interrupt_signal if stop_signal_count is nonzero.
(process_signals, install_signal_handlers):
Check for failures from sigprocmask etc.
(sig, nsig): Now at top level, since multiple functions need them.
(install_signal_handlers): No need for caught_sig array;
just use caught_signals. However, set some_signals_caught.
(cleanup_signal_handlers): New function.
---
bootstrap.conf | 2 +
src/diff.c | 2 +-
src/diff.h | 1 +
src/util.c | 187 ++++++++++++++++++++++++++++++++-----------------
4 files changed, 127 insertions(+), 65 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 6560e9a..e51b2d8 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -65,11 +65,13 @@ mktime
nstrftime
progname
propername
+raise
rawmemchr
readme-release
regex
sh-quote
signal
+sigprocmask
stat
stat-macros
stat-time
diff --git a/src/diff.c b/src/diff.c
index a4e5538..3b901aa 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -853,7 +853,7 @@ main (int argc, char **argv)
print_message_queue ();
check_stdout ();
- exit (exit_status);
+ cleanup_signal_handlers ();
return exit_status;
}
diff --git a/src/diff.h b/src/diff.h
index 03f00a6..f346b43 100644
--- a/src/diff.h
+++ b/src/diff.h
@@ -388,6 +388,7 @@ extern struct change *find_change (struct change *);
extern struct change *find_reverse_change (struct change *);
extern enum changes analyze_hunk (struct change *, lin *, lin *, lin *, lin *);
extern void begin_output (void);
+extern void cleanup_signal_handlers (void);
extern void debug_script (struct change *);
extern void fatal (char const *) __attribute__((noreturn));
extern void finish_output (void);
diff --git a/src/util.c b/src/util.c
index dd6d3bf..8e676c8 100644
--- a/src/util.c
+++ b/src/util.c
@@ -31,10 +31,9 @@
present. */
#ifndef SA_NOCLDSTOP
# define SA_NOCLDSTOP 0
-# define sigprocmask(How, Set, Oset) /* empty */
-# define sigset_t int
+# define sigprocmask(How, Set, Oset) 0
# if ! HAVE_SIGINTERRUPT
-# define siginterrupt(sig, flag) /* empty */
+# define siginterrupt(sig, flag) 0
# endif
#endif
@@ -160,16 +159,63 @@ print_message_queue (void)
}
}
-/* The set of signals that are caught. */
+#if SA_NOCLDSTOP
+static void
+xsigaction (int sig, struct sigaction const *restrict act,
+ struct sigaction *restrict oact)
+{
+ if (sigaction (sig, act, oact) != 0)
+ pfatal_with_name ("sigaction");
+}
+#endif
+
+static void
+xsigaddset (sigset_t *set, int sig)
+{
+ if (sigaddset (set, sig) != 0)
+ pfatal_with_name ("sigaddset");
+}
+
+static bool
+xsigismember (sigset_t const *set, int sig)
+{
+ int mem = sigismember (set, sig);
+ if (mem < 0)
+ pfatal_with_name ("sigismember");
+ assume (mem == 1);
+ return mem;
+}
+
+typedef void (*signal_handler) (int);
+static signal_handler
+xsignal (int sig, signal_handler func)
+{
+ signal_handler h = signal (sig, func);
+ if (h == SIG_ERR)
+ pfatal_with_name ("signal");
+ return h;
+}
+
+static void
+xsigprocmask (int how, sigset_t const *restrict set, sigset_t *restrict oset)
+{
+ if (sigprocmask (how, set, oset) != 0)
+ pfatal_with_name ("sigprocmask");
+}
+
+/* If true, some signals are caught. This is separate from
+ 'caught_signals' because POSIX doesn't require an all-zero sigset_t
+ to be valid. */
+static bool some_signals_caught;
+
+/* The set of signals that are caught. */
static sigset_t caught_signals;
/* If nonzero, the value of the pending fatal signal. */
-
static sig_atomic_t volatile interrupt_signal;
/* A count of the number of pending stop signals that have been received. */
-
static sig_atomic_t volatile stop_signal_count;
/* An ordinary signal was received; arrange for the program to exit. */
@@ -202,21 +248,17 @@ stophandler (int sig)
static void
process_signals (void)
{
- while (interrupt_signal || stop_signal_count)
+ while (interrupt_signal | stop_signal_count)
{
- int sig;
- int stops;
- sigset_t oldset;
-
set_color_context (RESET_CONTEXT);
fflush (stdout);
- sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
+ sigset_t oldset;
+ xsigprocmask (SIG_BLOCK, &caught_signals, &oldset);
- /* Reload interrupt_signal and stop_signal_count, in case a new
- signal was handled before sigprocmask took effect. */
- sig = interrupt_signal;
- stops = stop_signal_count;
+ /* Reload stop_signal_count and (if needed) interrupt_signal, in
+ case a new signal was handled before sigprocmask took effect. */
+ int stops = stop_signal_count, sig;
/* SIGTSTP is special, since the application can receive that signal
more than once. In this case, don't set the signal handler to the
@@ -227,82 +269,99 @@ process_signals (void)
sig = SIGSTOP;
}
else
- signal (sig, SIG_DFL);
+ {
+ sig = interrupt_signal;
+ xsignal (sig, SIG_DFL);
+ }
/* Exit or suspend the program. */
- raise (sig);
- sigprocmask (SIG_SETMASK, &oldset, NULL);
+ if (raise (sig) != 0)
+ pfatal_with_name ("raise");
+ xsigprocmask (SIG_SETMASK, &oldset, NULL);
/* If execution reaches here, then the program has been
continued (after being suspended). */
}
}
-static void
-install_signal_handlers (void)
-{
- /* The signals that are trapped, and the number of such signals. */
- static int const sig[] =
- {
- /* This one is handled specially. */
- SIGTSTP,
+/* The signals that can be caught, the number of such signals,
+ and which of them are actually caught. */
+static int const sig[] =
+ {
+ /* This one is handled specially. */
+ SIGTSTP,
- /* The usual suspects. */
- SIGALRM, SIGHUP, SIGINT, SIGPIPE, SIGQUIT, SIGTERM,
+ /* The usual suspects. */
+ SIGALRM, SIGHUP, SIGINT, SIGPIPE, SIGQUIT, SIGTERM,
#ifdef SIGPOLL
- SIGPOLL,
+ SIGPOLL,
#endif
#ifdef SIGPROF
- SIGPROF,
+ SIGPROF,
#endif
#ifdef SIGVTALRM
- SIGVTALRM,
+ SIGVTALRM,
#endif
#ifdef SIGXCPU
- SIGXCPU,
+ SIGXCPU,
#endif
#ifdef SIGXFSZ
- SIGXFSZ,
+ SIGXFSZ,
#endif
- };
- enum { nsigs = sizeof (sig) / sizeof *(sig) };
+ };
+enum { nsigs = sizeof (sig) / sizeof *(sig) };
+
+static void
+install_signal_handlers (void)
+{
+ if (sigemptyset (&caught_signals) != 0)
+ pfatal_with_name ("sigemptyset");
-#if ! SA_NOCLDSTOP
- bool caught_sig[nsigs];
-#endif
- {
- int j;
#if SA_NOCLDSTOP
- struct sigaction act;
+ for (int j = 0; j < nsigs; j++)
+ {
+ struct sigaction actj;
+ xsigaction (sig[j], NULL, &actj);
+ if (actj.sa_handler != SIG_IGN)
+ xsigaddset (&caught_signals, sig[j]);
+ }
- sigemptyset (&caught_signals);
- for (j = 0; j < nsigs; j++)
+ struct sigaction act;
+ act.sa_mask = caught_signals;
+ act.sa_flags = SA_RESTART;
+
+ for (int j = 0; j < nsigs; j++)
+ if (xsigismember (&caught_signals, sig[j]))
{
- sigaction (sig[j], NULL, &act);
- if (act.sa_handler != SIG_IGN)
- sigaddset (&caught_signals, sig[j]);
+ act.sa_handler = sig[j] == SIGTSTP ? stophandler : sighandler;
+ xsigaction (sig[j], &act, NULL);
+ some_signals_caught = true;
}
-
- act.sa_mask = caught_signals;
- act.sa_flags = SA_RESTART;
-
- for (j = 0; j < nsigs; j++)
- if (sigismember (&caught_signals, sig[j]))
- {
- act.sa_handler = sig[j] == SIGTSTP ? stophandler : sighandler;
- sigaction (sig[j], &act, NULL);
- }
#else
- for (j = 0; j < nsigs; j++)
+ for (int j = 0; j < nsigs; j++)
+ if (xsignal (sig[j], SIG_IGN) != SIG_IGN)
{
- caught_sig[j] = (signal (sig[j], SIG_IGN) != SIG_IGN);
- if (caught_sig[j])
- {
- signal (sig[j], sig[j] == SIGTSTP ? stophandler : sighandler);
- siginterrupt (sig[j], 0);
- }
+ xsigaddset (&caught_signals, sig[j]);
+ xsignal (sig[j], sig[j] == SIGTSTP ? stophandler : sighandler);
+ some_signals_caught = true;
+ if (siginterrupt (sig[j], 0) != 0)
+ pfatal_with_name ("siginterrupt");
}
#endif
+}
+
+/* Clean up signal handlers just before exiting the program. Do this
+ by resetting signal actions back to default, and then processing
+ any signals that arrived before resetting. */
+void
+cleanup_signal_handlers (void)
+{
+ if (some_signals_caught)
+ {
+ for (int j = 0; j < nsigs; j++)
+ if (xsigismember (&caught_signals, sig[j]))
+ xsignal (sig[j], SIG_DFL);
+ process_signals ();
}
}
--
2.30.2
