Re: segfault in completion for configure

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Mar 17, 11:15am, Bart Schaefer wrote:
}
} Yeah, nothing really new here, this is exactly what I would expect
} to see:  The hander returns and thereafter pattern matching is messed
} up and crashes when it tries to pick up where it left off.

Well, I think I've isolated this, but I don't know what to do about it.

The set of globals that are getting messed up are these:

/*
 * Run a pattern.
 */
static char *patinstart;	/* Start of input string */
static char *patinend;		/* End of input string */
static char *patinput;		/* String input pointer */
static char *patinpath;		/* Full path for use with ~ exclusions */
static int   patinlen;		/* Length of last successful match.
				 * Includes count of Meta characters.
				 */

static char *patbeginp[NSUBEXP];	/* Pointer to backref beginnings */
static char *patendp[NSUBEXP];		/* Pointer to backref ends */
static int parsfound;			/* parentheses (with backrefs) found */


This set is all referenced from pattryrefs() and other functions that it
calls.  The problem is that they also describe the state of backrefs, so
it's difficult to save/restore them such that $match et al. remain sane
across a signal handler (and when not sane, they point off into unknown
territory and crashing ensues).

The problem is that it's exactly during the time that these globals are
volatile that we want to allow signal handling, so that a deep recursive
glob or pathological pattern match can be interrupted [see patmatch(),
"while (scan && !errflag)"].

The patch below crams all those globals into a struct so they can be
saved/restored as one, and then tries pushing the signal queue management
down into patmatch() from pattryrefs().  A side-effect of this patch is
that the following is no longer guaranteed to work:

    typeset -i trap_handled;
    TRAPUSR2() {
      setopt localoptions extendedglob
      [[ $somestring = (#b)$~somepattern ]]
      trap_handled=1
    }
    TRAPUSR1() {
      [[ $TERM = screen ]] && print $something
    }
    while true; do
      if ((trap_handled)); then
       trap_handled=0
       print $match[0]	# This may not be the backref from TRAPUSR2
      fi
    done

Specifically, if the signals arrive in order USR1 USR2 such that USR2
is handled during the [[ ]] expression in TRAPUSR1, then the state from
TRAPUSR2 will be discarded.  I don't know if/how we ought to document.

Of course if this DOESN'T solve Vincent's crash, then all of the above
is moot because I've mis-diagnosed again.


diff --git a/Src/pattern.c b/Src/pattern.c
index 72c7d97..4e2f236 100644
--- a/Src/pattern.c
+++ b/Src/pattern.c
@@ -1838,7 +1838,8 @@ pattail(long p, long val)
 /* do pattail, but on operand of first argument; nop if operandless */
 
 /**/
-static void patoptail(long p, long val)
+static void
+patoptail(long p, long val)
 {
     Upat ptr = (Upat)patout + p;
     int op = P_OP(ptr);
@@ -1854,19 +1855,34 @@ static void patoptail(long p, long val)
 /*
  * Run a pattern.
  */
-static char *patinstart;	/* Start of input string */
-static char *patinend;		/* End of input string */
-static char *patinput;		/* String input pointer */
-static char *patinpath;		/* Full path for use with ~ exclusions */
-static int   patinlen;		/* Length of last successful match.
+struct rpat {
+    char *patinstart;		/* Start of input string */
+    char *patinend;		/* End of input string */
+    char *patinput;		/* String input pointer */
+    char *patinpath;		/* Full path for use with ~ exclusions */
+    int   patinlen;		/* Length of last successful match.
 				 * Includes count of Meta characters.
 				 */
 
-static char *patbeginp[NSUBEXP];	/* Pointer to backref beginnings */
-static char *patendp[NSUBEXP];		/* Pointer to backref ends */
-static int parsfound;			/* parentheses (with backrefs) found */
+    char *patbeginp[NSUBEXP];	/* Pointer to backref beginnings */
+    char *patendp[NSUBEXP];	/* Pointer to backref ends */
+    int parsfound;		/* parentheses (with backrefs) found */
+
+    int globdots;		/* Glob initial dots? */
+};
+
+static struct rpat pattrystate;
+
+#define patinstart	(pattrystate.patinstart)
+#define patinend	(pattrystate.patinend)
+#define patinput	(pattrystate.patinput)
+#define patinpath	(pattrystate.patinpath)
+#define patinlen	(pattrystate.patinlen)
+#define patbeginp	(pattrystate.patbeginp)
+#define patendp		(pattrystate.patendp)
+#define parsfound	(pattrystate.parsfound)
+#define globdots	(pattrystate.globdots)
 
-static int globdots;			/* Glob initial dots? */
 
 /*
  * Character functions operating on unmetafied strings.
@@ -2302,7 +2318,7 @@ pattryrefs(Patprog prog, char *string, int stringlen, int unmetalenin,
 	 * Either we are testing against a pure string,
 	 * or we can match anything at all.
 	 */
-	int ret, pstrlen;
+	int pstrlen;
 	char *pstr;
 	if (patstralloc->alloced)
 	{
@@ -2404,11 +2420,7 @@ pattryrefs(Patprog prog, char *string, int stringlen, int unmetalenin,
 		}
 	    }
 	}
-
-	return ret;
     } else {
-	int q = queue_signal_level();
-
 	/*
 	 * Test for a `must match' string, unless we're scanning for a match
 	 * in which case we don't need to do this each time.
@@ -2440,9 +2452,8 @@ pattryrefs(Patprog prog, char *string, int stringlen, int unmetalenin,
 		    ret = 0;
 	    }
 	}
-	if (!ret) {
+	if (!ret)
 	    return 0;
-	}
 
 	patglobflags = prog->globflags;
 	if (!(patflags & PAT_FILE)) {
@@ -2454,8 +2465,6 @@ pattryrefs(Patprog prog, char *string, int stringlen, int unmetalenin,
 
 	patinput = patinstart;
 
-	dont_queue_signals();
-
 	if (patmatch((Upat)progstr)) {
 	    /*
 	     * we were lazy and didn't save the globflags if an exclusion
@@ -2594,11 +2603,9 @@ pattryrefs(Patprog prog, char *string, int stringlen, int unmetalenin,
 	    ret = 1;
 	} else
 	    ret = 0;
-
-	restore_queue_signals(q);
-
-	return ret;
     }
+
+    return ret;
 }
 
 /*
@@ -2674,6 +2681,26 @@ patmatch(Upat prog)
     int savglobflags, op, no, min, fail = 0, saverrsfound;
     zrange_t from, to, comp;
     patint_t nextch;
+    int q = queue_signal_level();
+
+    /*
+     * To avoid overhead of saving state if there are no queued signals
+     * waiting, we pierce the signals.h veil and examine queue state.
+     */
+#define check_for_signals() do if (queue_front != queue_rear) { \
+	    int savpatflags = patflags, savpatglobflags = patglobflags; \
+            char *savexactpos = exactpos, *savexactend = exactend; \
+	    struct rpat savpattrystate = pattrystate; \
+	    dont_queue_signals(); \
+	    restore_queue_signals(q); \
+	    exactpos = savexactpos; \
+	    exactend = savexactend; \
+	    patflags = savpatflags; \
+	    patglobflags = savpatglobflags; \
+	    pattrystate = savpattrystate; \
+	} while (0)
+
+    check_for_signals();
 
     while  (scan && !errflag) {
 	next = PATNEXT(scan);
@@ -3508,6 +3535,9 @@ patmatch(Upat prog)
 	}
 
 	scan = next;
+
+	/* Allow handlers to run once per loop */
+	check_for_signals();
     }
 
     return 0;