fixes for SIGINT handling in asynchronous lists

CWRU/CWRU.chlog

@@ -5005,3 +5005,55 @@ lib/sh/random.c
 	- intrand32: use % operator instead of (mathematically equivalent)
 	  subtraction and multiplication, which can cause signed 32-bit
 	  overflow. Report from Sam James <sam@gentoo.org>
+
+lib/readline/input.c
+	- rl_getc: if the application hasn't defined a signal event handler,
+	  and readline is in callback mode, make sure SIGINT breaks out of
+	  operations like incremental and non-incremental searches and digit
+	  arguments by calling _rl_abort_internal, since the callback code
+	  expects the various contexts that rl_callback_sigcleanup() resets
+	  to be valid
+	- rl_getc: if there is a pending signal when we enter the while loop
+	  in callback mode, handle it in the same way we would if a signal
+	  arrived while we were waiting in select/pselect. That forces
+	  callback mode to call _rl_abort_internal in the right place.
+	  From a report from Andrew Burgess <aburgess@redhat.com>
+
+				   1/13
+				   ----
+trap.c
+	- SIG_ASYNCSIG: new value for sigmodes; means that the signal is
+	  ignored because an async command is being executed
+	- set_signal: if the original signal disposition is SIG_IGN only
+	  return if SIG_ASYNCSIG isn't set. This implements POSIX interp 751
+	  for setting a trap
+	- ignore_signal: if we're ignoring a signal that already has SIG_IGNORED
+	  set, make sure we set SIG_TRAPPED in case of SIG_ASYNCSIG (POSIX
+	  interp 751)
+	- reset_signal,restore_signal: if we want to reset or restore a signal
+	  that has SIG_ASYNCSIG set, make sure we reset to SIG_DFL if the
+	  signal is trapped to maintain POSIX semantics
+	- set_signal_async_ignored: new function to set original_signals to
+	  SIG_IGN and set the SIG_IGNORED and SIG_ASYNCSIG sigmode flags; used
+	  by setup_async_signals
+	- signal_is_async_ignored: return true if the SIG argument is being
+	  ignored because it's part of an async list
+
+trap.h
+	- set_signal_async_ignored,signal_is_async_ignored: extern declarations
+
+sig.c
+	- initialize_terminating_signals: if a signal is ignored because it's
+	  part of an async list, don't bother trying to initialize or set the
+	  hard ignored flag
+
+execute_cmd.c
+	- execute_simple_command: if we're executing a builtin or function
+	  asynchronously, call reset_terminating_signals like we do in
+	  execute_in_subshell()
+	- execute_subshell_builtin_or_function: only call set_sigint_handler
+	  if async is 0, so we don't undo the work done by setup_async_signals.
+	  If the old handler is SIG_IGN and the signal has SIG_ASYNCSIG set
+	  and isn't trapped, restore the old handler
+	- setup_async_signals: call set_signal_async_ignored to set the right
+	  flags

builtins/trap.def

@@ -210,7 +210,13 @@ trap_builtin (WORD_LIST *list)
 		      {
 		      case SIGINT:
 			/* XXX - should we do this if original disposition
-			   was SIG_IGN? */
+			   was SIG_IGN? Yes, because setup_async_signals can
+			   set the original disposition to SIG_IGN to keep
+			   restore_signal from restoring the SIGINT
+			   disposition from when the shell was invoked, and
+			   we want to allow an asynchronous subshell to
+			   use `trap' to restore the default disposition or
+			   set a trap command. */
 			if (interactive)
 			  set_signal_handler (SIGINT, sigint_sighandler);
 			/* special cases for interactive == 0 */

execute_cmd.c

@@ -1480,8 +1480,7 @@ execute_in_subshell (COMMAND *command, int asynchronous, int pipe_in, int pipe_o
 
   /* If a command is asynchronous in a subshell (like ( foo ) & or
      the special case of an asynchronous GROUP command where the
-
-     the subshell bit is turned on down in case cm_group: below),
+     subshell bit is turned on down in case cm_group: below),
      turn off `asynchronous', so that two subshells aren't spawned.
      XXX - asynchronous used to be set to 0 in this block, but that
      means that setup_async_signals was never run.  Now it's set to
@@ -1576,6 +1575,8 @@ execute_in_subshell (COMMAND *command, int asynchronous, int pipe_in, int pipe_o
       asynchronous = 0;
     }
   else
+    /* XXX - restore if old handler is SIG_IGN like we do in
+       execute_subshell_builtin_or_function? */
     set_sigint_handler ();
 
 #if defined (JOB_CONTROL)
@@ -4628,7 +4629,7 @@ run_builtin:
         }
       if (already_forked)
 	{
-	  /* reset_terminating_signals (); */	/* XXX */
+	  reset_terminating_signals ();	/* XXX */
 	  /* Reset the signal handlers in the child, but don't free the
 	     trap strings.  Set a flag noting that we have to free the
 	     trap strings if we run trap to change a signal disposition. */
@@ -5266,7 +5267,18 @@ execute_subshell_builtin_or_function (WORD_LIST *words, REDIRECT *redirects,
   without_job_control ();
 #endif /* JOB_CONTROL */
 
-  set_sigint_handler ();
+  /* We don't call set_sigint_handler if async is 1 so we don't undo the work
+     done by setup_async_signals() in the caller. This is similar to what
+     execute_in_subshell() does. */
+  if (async == 0)
+    {
+      SigHandler *handler;
+
+      handler = set_sigint_handler ();
+      if (handler == SIG_IGN && signal_is_async_ignored (SIGINT) &&
+	  signal_is_trapped (SIGINT) == 0 && signal_is_hard_ignored (SIGINT) == 0)
+	set_signal_handler (SIGINT, handler);
+    }
 
   if (fds_to_close)
     close_fd_bitmap (fds_to_close);
@@ -5442,19 +5454,26 @@ setup_async_signals (void)
   set_signal_handler (SIGHUP, SIG_IGN);	/* they want csh-like behavior */
 #endif
 
-#if defined (JOB_CONTROL)
   if (job_control == 0)
-#endif
     {
       /* Make sure we get the original signal dispositions now so we don't
 	 confuse the trap builtin later if the subshell tries to use it to
-	 reset SIGINT/SIGQUIT.  Don't call set_signal_ignored; that sets
-	 the value of original_signals to SIG_IGN. Posix interpretation 751. */
+	 reset SIGINT/SIGQUIT. Don't call set_signal_ignored; we use
+	 set_signal_async_ignored to set the value of original_signals to
+	 SIG_IGN and set additional flags to satisfy Posix interpretation 751. */
       get_original_signal (SIGINT);
       set_signal_handler (SIGINT, SIG_IGN);
+      /* We use set_signal_async_ignored here to make sure that restore_signal
+	 doesn't undo this work. We want processes that are created by this
+	 asynchronous subshell to ignore SIGINT as well. We can't set the
+	 hard ignored flag because that would prevent the trap builtin from
+	 setting a trap. We also have to special-case set_signal so we can
+	 set a trap for one of these signals. */
+      set_signal_async_ignored (SIGINT);
 
       get_original_signal (SIGQUIT);
       set_signal_handler (SIGQUIT, SIG_IGN);
+      set_signal_async_ignored (SIGQUIT);
     }
 }
 

lib/readline/input.c

@@ -811,7 +811,7 @@ rl_read_key (void)
 int
 rl_getc (FILE *stream)
 {
-  int result;
+  int result, ostate, osig;
   unsigned char c;
   int fd;
 #if defined (HAVE_PSELECT) || defined (HAVE_SELECT)
@@ -822,8 +822,22 @@ rl_getc (FILE *stream)
   fd = fileno (stream);
   while (1)
     {
+      osig = _rl_caught_signal;
+      ostate = rl_readline_state;
+
       RL_CHECK_SIGNALS ();
 
+#if defined (READLINE_CALLBACKS)
+      /* Do signal handling post-processing here, but just in callback mode
+	 for right now because the signal cleanup can change some of the
+	 callback state, and we need to either let the application have a
+	 chance to react or abort some current operation that gets cleaned
+	 up by rl_callback_sigcleanup(). If not, we'll just run through the
+	 loop again. */
+      if (osig != 0 && (ostate & RL_STATE_CALLBACK))
+	goto postproc_signal;
+#endif
+
       /* We know at this point that _rl_caught_signal == 0 */
 
 #if defined (__MINGW32__)
@@ -887,6 +901,9 @@ rl_getc (FILE *stream)
 /* fprintf(stderr, "rl_getc: result = %d errno = %d\n", result, errno); */
 
 handle_error:
+      osig = _rl_caught_signal;
+      ostate = rl_readline_state;
+
       /* If the error that we received was EINTR, then try again,
 	 this is simply an interrupted system call to read ().  We allow
 	 the read to be interrupted if we caught SIGHUP, SIGTERM, or any
@@ -927,8 +944,17 @@ handle_error:
         RL_CHECK_SIGNALS ();
 #endif  /* SIGALRM */
 
+postproc_signal:
+      /* POSIX says read(2)/pselect(2)/select(2) don't return EINTR for any
+	 reason other than being interrupted by a signal, so we can safely
+	 call the application's signal event hook. */
       if (rl_signal_event_hook)
 	(*rl_signal_event_hook) ();
+#if defined (READLINE_CALLBACKS)
+      else if (osig == SIGINT && (ostate & RL_STATE_CALLBACK) && (ostate & (RL_STATE_ISEARCH|RL_STATE_NSEARCH|RL_STATE_NUMERICARG)))
+        /* just these cases for now */
+        _rl_abort_internal ();
+#endif
     }
 }
 

lib/sh/random.c

@@ -59,11 +59,13 @@ intrand32 (u_bits32_t last)
      Park and Miller, Communications of the ACM, vol. 31, no. 10,
      October 1988, p. 1195. Filtered through FreeBSD.
 
-     x(n+1) = 16807 * x(n) mod (m).
+     x(n+1) = 16807 * x(n) mod (m)
+
+     where 16807 == 7^5.
 
      We split up the calculations to avoid overflow.
 
-     h = last / q; l = x - h * q; t = a * l - h * r
+     h = last / q; l = last % q; t = a * l - h * r
      m = 2147483647, a = 16807, q = 127773, r = 2836
 
      There are lots of other combinations of constants to use; look at

parse.y

@@ -37,6 +37,7 @@
 #include <stdio.h>
 #include "chartypes.h"
 #include <signal.h>
+#include <errno.h>
 
 #include "memalloc.h"
 
@@ -2691,8 +2692,15 @@ pop_alias:
   if (uc == 0 && shell_input_line_terminator == EOF)
     return ((shell_input_line_index != 0) ? '\n' : EOF);
   else if (uc == 0 && shell_input_line_terminator == READERR)
-    /* Treat read errors like EOF here. */
-    return ((shell_input_line_index != 0) ? '\n' : EOF);
+    {
+#if defined (FATAL_READERROR)
+      report_error (_("script file read error: %s"), strerror (errno));
+      exit_shell (2);
+#else
+      /* Treat read errors like EOF here. */
+      return ((shell_input_line_index != 0) ? '\n' : EOF);
+#endif
+    }
 
 #if defined (ALIAS) || defined (DPAREN_ARITHMETIC)
   /* We already know that we are not parsing an alias expansion because of the

sig.c

@@ -252,6 +252,8 @@ initialize_terminating_signals (void)
       /* If we've already trapped it, don't do anything. */
       if (signal_is_trapped (XSIG (i)))
 	continue;
+      if (signal_is_async_ignored (XSIG (i)))
+	continue;
 
       sigaction (XSIG (i), &act, &oact);
       XHANDLER(i) = oact.sa_handler;

trap.c

@@ -66,6 +66,7 @@ extern int errno;
 #define SIG_INPROGRESS	0x10	/* Signal handler currently executing. */
 #define SIG_CHANGED	0x20	/* Trap value changed in trap handler. */
 #define SIG_IGNORED	0x40	/* The signal is currently being ignored. */
+#define SIG_ASYNCSIG	0x80	/* The signal is ignored because it's in an asynchronous command. */
 
 #define SPECIAL_TRAP(s)	((s) == EXIT_TRAP || (s) == DEBUG_TRAP || (s) == ERROR_TRAP || (s) == RETURN_TRAP)
 
@@ -804,8 +805,8 @@ set_signal (int sig, const char *string)
       /* If we aren't sure of the original value, check it. */
       if (original_signals[sig] == IMPOSSIBLE_TRAP_HANDLER)
         GETORIGSIG (sig);
-      if (original_signals[sig] == SIG_IGN)
-	return;
+      if (original_signals[sig] == SIG_IGN && (sigmodes[sig] & SIG_ASYNCSIG) == 0)
+	return;			/* XXX */
     }
 
   /* Only change the system signal handler if SIG_NO_TRAP is not set.
@@ -939,7 +940,11 @@ ignore_signal (int sig)
 
   /* If already trapped and ignored, no change necessary. */
   if (sigmodes[sig] & SIG_IGNORED)
-    return;
+    {
+      sigmodes[sig] |= SIG_TRAPPED;	/* just make sure */
+      /* XXX - turn off SIG_ASYNCSIG here? */
+      return;
+    }
 
   /* Only change the signal handler for SIG if it allows it. */
   if ((sigmodes[sig] & SIG_NO_TRAP) == 0)
@@ -1301,6 +1306,15 @@ free_trap_string (int sig)
 static void
 reset_signal (int sig)
 {
+#if 1
+  /* If we have a trapped signal that was previously ignored because it was
+     SIGINT or SIGQUIT in an asynchronous list, then we need to restore the
+     default signal and not the (artificial) SIG_IGN. We know that the signal
+     was not ignored at shell invocation because you can't trap it if
+     SIG_HARD_IGNORE is set. */
+  if ((sigmodes[sig] & SIG_ASYNCSIG) && signal_is_trapped (sig) && original_signals[sig] == SIG_IGN)
+    original_signals[sig] = SIG_DFL;
+#endif
   set_signal_handler (sig, original_signals[sig]);
   sigmodes[sig] &= ~SIG_TRAPPED;		/* XXX - SIG_INPROGRESS? */
 }
@@ -1310,6 +1324,15 @@ reset_signal (int sig)
 static void
 restore_signal (int sig)
 {
+#if 1
+  /* If we have a trapped signal that was previously ignored because it was
+     SIGINT or SIGQUIT in an asynchronous list, then we need to restore the
+     default signal and not the (artificial) SIG_IGN. We know that the signal
+     was not ignored at shell invocation because you can't trap it if
+     SIG_HARD_IGNORE is set. */
+  if ((sigmodes[sig] & SIG_ASYNCSIG) && signal_is_trapped (sig) && original_signals[sig] == SIG_IGN)
+    original_signals[sig] = SIG_DFL;
+#endif
   set_signal_handler (sig, original_signals[sig]);
   change_signal (sig, (char *)DEFAULT_SIG);
   sigmodes[sig] &= ~SIG_TRAPPED;
@@ -1522,6 +1545,19 @@ set_signal_ignored (int sig)
   original_signals[sig] = SIG_IGN;
 }
 
+void
+set_signal_async_ignored (int sig)
+{
+  original_signals[sig] = SIG_IGN;
+  sigmodes[sig] |= SIG_ASYNCSIG|SIG_IGNORED;
+}
+
+int
+signal_is_async_ignored (int sig)
+{
+  return (sigmodes[sig] & SIG_ASYNCSIG);
+}
+
 int
 signal_in_progress (int sig)
 {

trap.h

@@ -116,6 +116,8 @@ extern int signal_is_ignored (int);
 extern int signal_is_hard_ignored (int);
 extern void set_signal_hard_ignored (int);
 extern void set_signal_ignored (int);
+extern void set_signal_async_ignored (int);
+extern int signal_is_async_ignored (int);
 extern int signal_in_progress (int);
 
 extern void set_trap_state (int);