usr.bin/make 2nd try: fix signal stuff

[Max Okumoto <okumoto@xxxxxxxx>]

Another patch missed from mega patch.

			Max

Fix a (very) long standing bug in make (this has been there probably
from the beginning). Make used to handle all its interrupt-time stuff
directly from the signal handler, including calls to printf, accessing
global data and so on. This is of course wrong and could provoke a core
dump when interrupting make. Just set a flag in the signal handler and
do everything else from the main thread.

PatchSet 370
Date: 2004/11/12 07:57:17
Author: harti
Log:
Fix a (very) long standing bug in make (this has been there probably
from the beginning). Make used to handle all its interrupt-time stuff
directly from the signal handler, including calls to printf, accessing
global data and so on. This is of course wrong and could provoke a core
dump when interrupting make. Just set a flag in the signal handler and
do everything else from the main thread.

PR:		bin/29103

Members: 
	compat.c:1.38->1.39 
	job.c:1.55->1.56 

Index: compat.c
===================================================================
RCS file: /usr/home/okumoto/Work/make/fbsd-cvs/src/usr.bin/make/compat.c,v
retrieving revision 1.38
retrieving revision 1.39
diff -u -r1.38 -r1.39
--- compat.c	23 Oct 2004 21:34:41 -0000	1.38
+++ compat.c	12 Nov 2004 07:57:17 -0000	1.39
@@ -79,6 +79,8 @@
 
 static GNode	    *curTarg = NULL;
 static GNode	    *ENDNode;
+static sig_atomic_t interrupted;
+
 static void CompatInterrupt(int);
 static int CompatMake(void *, void *);
 static int shellneed(char *);
@@ -101,6 +103,16 @@
     meta[0] = 1;
 }
 
+/*
+ * Interrupt handler - set flag and defer handling to the main code
+ */
+static void
+CompatCatchSig(int signo)
+{
+
+	interrupted = signo;
+}
+
 /*-
  *-----------------------------------------------------------------------
  * CompatInterrupt --
@@ -120,6 +132,17 @@
 CompatInterrupt (int signo)
 {
     GNode   *gn;
+    sigset_t nmask, omask;
+
+    sigemptyset(&nmask);
+    sigaddset(&nmask, SIGINT);
+    sigaddset(&nmask, SIGTERM);
+    sigaddset(&nmask, SIGHUP);
+    sigaddset(&nmask, SIGQUIT);
+    sigprocmask(SIG_SETMASK, &nmask, &omask);
+
+    /* prevent recursion in evaluation of .INTERRUPT */
+    interrupted = 0;
 
     if ((curTarg != NULL) && !Targ_Precious (curTarg)) {
 	char	  *p1;
@@ -129,18 +152,20 @@
 	    printf ("*** %s removed\n", file);
 	}
 	free(p1);
+    }
 
-	/*
-	 * Run .INTERRUPT only if hit with interrupt signal
-	 */
-	if (signo == SIGINT) {
-	    gn = Targ_FindNode(".INTERRUPT", TARG_NOCREATE);
-	    if (gn != NULL) {
-		Lst_ForEach(gn->commands, Compat_RunCommand, (void *)gn);
-	    }
+    /*
+     * Run .INTERRUPT only if hit with interrupt signal
+     */
+    if (signo == SIGINT) {
+	gn = Targ_FindNode(".INTERRUPT", TARG_NOCREATE);
+	if (gn != NULL) {
+	    Lst_ForEach(gn->commands, Compat_RunCommand, (void *)gn);
 	}
-
     }
+
+    sigprocmask(SIG_SETMASK, &omask, NULL);
+
     if (signo == SIGQUIT)
 	exit(signo);
     (void) signal(signo, SIG_DFL);
@@ -373,10 +398,12 @@
     while (1) {
 
 	while ((rstat = wait(&reason)) != cpid) {
-	    if (rstat == -1 && errno != EINTR) {
-		break;
+	    if (interrupted || (rstat == -1 && errno != EINTR)) {
+		    break;
 	    }
 	}
+	if (interrupted)
+	    CompatInterrupt(interrupted);
 
 	if (rstat > -1) {
 	    if (WIFSTOPPED(reason)) {
@@ -660,16 +687,16 @@
     Shell_Init();		/* Set up shell. */
 
     if (signal(SIGINT, SIG_IGN) != SIG_IGN) {
-	signal(SIGINT, CompatInterrupt);
+	signal(SIGINT, CompatCatchSig);
     }
     if (signal(SIGTERM, SIG_IGN) != SIG_IGN) {
-	signal(SIGTERM, CompatInterrupt);
+	signal(SIGTERM, CompatCatchSig);
     }
     if (signal(SIGHUP, SIG_IGN) != SIG_IGN) {
-	signal(SIGHUP, CompatInterrupt);
+	signal(SIGHUP, CompatCatchSig);
     }
     if (signal(SIGQUIT, SIG_IGN) != SIG_IGN) {
-	signal(SIGQUIT, CompatInterrupt);
+	signal(SIGQUIT, CompatCatchSig);
     }
 
     ENDNode = Targ_FindNode(".END", TARG_CREATE);
Index: job.c
===================================================================
RCS file: /usr/home/okumoto/Work/make/fbsd-cvs/src/usr.bin/make/job.c,v
retrieving revision 1.55
retrieving revision 1.56
diff -u -r1.55 -r1.56
--- job.c	11 Nov 2004 12:52:15 -0000	1.55
+++ job.c	12 Nov 2004 07:57:17 -0000	1.56
@@ -259,6 +259,9 @@
 				 * limits or externally */
 
 
+static sig_atomic_t interrupted;
+
+
 #if defined(USE_PGRP) && defined(SYSV)
 # define KILL(pid, sig)		killpg(-(pid), (sig))
 #else
@@ -306,6 +309,19 @@
 static void JobInterrupt(int, int);
 static void JobRestartJobs(void);
 
+/*
+ * JobCatchSignal
+ *
+ * Got a signal. Set global variables and hope that someone will
+ * handle it.
+ */
+static void
+JobCatchSig(int signo)
+{
+
+	interrupted = signo;
+}
+
 /*-
  *-----------------------------------------------------------------------
  * JobCondPassSig --
@@ -351,6 +367,10 @@
     sigset_t nmask, omask;
     struct sigaction act;
 
+    sigemptyset(&nmask);
+    sigaddset(&nmask, signo);
+    sigprocmask(SIG_SETMASK, &nmask, &omask);
+
     DEBUGF(JOB, ("JobPassSig(%d) called.\n", signo));
     Lst_ForEach(jobs, JobCondPassSig, (void *) &signo);
 
@@ -377,10 +397,8 @@
      * Note we block everything else possible while we're getting the signal.
      * This ensures that all our jobs get continued when we wake up before
      * we take any other signal.
+     * XXX this comment seems wrong.
      */
-    sigemptyset(&nmask);
-    sigaddset(&nmask, signo);
-    sigprocmask(SIG_SETMASK, &nmask, &omask);
     act.sa_handler = SIG_DFL;
     sigemptyset(&act.sa_mask);
     act.sa_flags = 0;
@@ -1366,6 +1384,10 @@
     Boolean 	  noExec;     /* Set true if we decide not to run the job */
     int		  tfd;	      /* File descriptor for temp file */
 
+    if (interrupted) {
+        JobPassSig(interrupted);
+        return (JOB_ERROR);
+    }
     if (previous != NULL) {
 	previous->flags &= ~(JOB_FIRST|JOB_IGNERR|JOB_SILENT);
 	job = previous;
@@ -1709,6 +1731,14 @@
 
 	nRead = read(job->inPipe, &job->outBuf[job->curPos],
 			 JOB_BUFSIZE - job->curPos);
+	/*
+	 * Check for interrupt here too, because the above read may block
+	 * when the child process is stopped. In this case the interrupt
+	 * will unblock it (we don't use SA_RESTART).
+	 */
+	if (interrupted)
+	    JobPassSig(interrupted);
+
 	if (nRead < 0) {
 	    DEBUGF(JOB, ("JobDoOutput(piperead)"));
 	    nr = 0;
@@ -1921,6 +1951,8 @@
 
 	JobFinish(job, &status);
     }
+    if (interrupted)
+        JobPassSig(interrupted);
 }
 
 /*-
@@ -1961,6 +1993,8 @@
 	if ((nfds = kevent(kqfd, NULL, 0, kev, KEV_SIZE, NULL)) == -1) {
 	    if (errno != EINTR)
 		Punt("kevent: %s", strerror(errno));
+	    if (interrupted)
+		JobPassSig(interrupted);
 	} else {
 	    for (i = 0; i < nfds; i++) {
 		if (kev[i].flags & EV_ERROR) {
@@ -1984,9 +2018,11 @@
 	timeout.tv_usec = SEL_USEC;
 
 	if ((nfds = select(FD_SETSIZE, &readfds, (fd_set *) 0,
-			   (fd_set *) 0, &timeout)) <= 0)
+			   (fd_set *) 0, &timeout)) <= 0) {
+	    if (interrupted)
+		JobPassSig(interrupted);
 	    return;
-	else {
+	} else {
 	    if (Lst_Open(jobs) == FAILURE) {
 		Punt("Cannot open job table");
 	    }
@@ -2055,6 +2091,7 @@
 Job_Init(int maxproc)
 {
     GNode         *begin;     /* node for commands to do at the very start */
+    struct sigaction sa;
 
     jobs =  	  Lst_Init(FALSE);
     stoppedJobs = Lst_Init(FALSE);
@@ -2087,19 +2124,24 @@
 
     /*
      * Catch the four signals that POSIX specifies if they aren't ignored.
-     * JobPassSig will take care of calling JobInterrupt if appropriate.
+     * JobCatchSignal will just set global variables and hope someone
+     * else is going to handle the interrupt.
      */
+    sa.sa_handler = JobCatchSig;
+    sigemptyset(&sa.sa_mask);
+    sa.sa_flags = 0;
+
     if (signal(SIGINT, SIG_IGN) != SIG_IGN) {
-	(void) signal(SIGINT, JobPassSig);
+	(void) sigaction(SIGINT, &sa, NULL);
     }
     if (signal(SIGHUP, SIG_IGN) != SIG_IGN) {
-	(void) signal(SIGHUP, JobPassSig);
+	(void) sigaction(SIGHUP, &sa, NULL);
     }
     if (signal(SIGQUIT, SIG_IGN) != SIG_IGN) {
-	(void) signal(SIGQUIT, JobPassSig);
+	(void) sigaction(SIGQUIT, &sa, NULL);
     }
     if (signal(SIGTERM, SIG_IGN) != SIG_IGN) {
-	(void) signal(SIGTERM, JobPassSig);
+	(void) sigaction(SIGTERM, &sa, NULL);
     }
     /*
      * There are additional signals that need to be caught and passed if
@@ -2109,16 +2151,16 @@
      */
 #if defined(USE_PGRP)
     if (signal(SIGTSTP, SIG_IGN) != SIG_IGN) {
-	(void) signal(SIGTSTP, JobPassSig);
+	(void) sigaction(SIGTSTP, &sa, NULL);
     }
     if (signal(SIGTTOU, SIG_IGN) != SIG_IGN) {
-	(void) signal(SIGTTOU, JobPassSig);
+	(void) sigaction(SIGTTOU, &sa, NULL);
     }
     if (signal(SIGTTIN, SIG_IGN) != SIG_IGN) {
-	(void) signal(SIGTTIN, JobPassSig);
+	(void) sigaction(SIGTTIN, &sa, NULL);
     }
     if (signal(SIGWINCH, SIG_IGN) != SIG_IGN) {
-	(void) signal(SIGWINCH, JobPassSig);
+	(void) sigaction(SIGWINCH, &sa, NULL);
     }
 #endif
 
@@ -2447,6 +2489,10 @@
     }
 
     if (runINTERRUPT && !touchFlag) {
+	/* clear the interrupted flag because we would get an
+	 * infinite loop otherwise */
+	interrupted = 0;
+
 	interrupt = Targ_FindNode(".INTERRUPT", TARG_NOCREATE);
 	if (interrupt != NULL) {
 	    ignoreErrors = FALSE;