25415: Make DEBUG_BEFORE_CMD the default.

Reuse ERR_EXIT in DEBUG traps.
Clean up trapreturn code.

6 files changed, 101 insertions, 49 deletions

diff --git a/Src/builtin.c b/Src/builtin.c
index e1378d901..ee44b3776 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -4462,8 +4462,10 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func)
 	    retflag = 1;
 	    breaks = loops;
 	    lastval = num;
-	    if (trapreturn == -2)
-		trapreturn = lastval;
+	    if (trap_state == TRAP_STATE_PRIMED && trap_return == -2) {
+		trap_state = TRAP_STATE_FORCE_RETURN;
+		trap_return = lastval;
+	    }
 	    return lastval;
 	}
 	zexit(num, 0);	/* else treat return as logout/exit */
diff --git a/Src/exec.c b/Src/exec.c
index 5b360815f..6ca5b2f2b 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -65,16 +65,23 @@ int nohistsave;
 mod_export int errflag;
  
 /*
- * Status of return from a trap.
+ * State of trap return value.  Value is from enum trap_state.
+ */
+
+/**/
+int trap_state;
+
+/*
+ * Value associated with return from a trap.
  * This is only active if we are inside a trap, else its value
  * is irrelevant.  It is initialised to -1 for a function trap and
  * -2 for a non-function trap and if negative is decremented as
  * we go deeper into functions and incremented as we come back up.
  * The value is used to decide if an explicit "return" should cause
  * a return from the caller of the trap; it does this by setting
- * trapreturn to a status (i.e. a non-negative value).
+ * trap_return to a status (i.e. a non-negative value).
  *
- * In summary, trapreturn is
+ * In summary, trap_return is
  * - zero unless we are in a trap
  * - negative in a trap unless it has triggered.  Code uses this
  *   to detect an active trap.
@@ -83,7 +90,7 @@ mod_export int errflag;
  */
  
 /**/
-int trapreturn;
+int trap_return;
  
 /* != 0 if this is a subshell */
  
@@ -1061,6 +1068,10 @@ execlist(Estate state, int dont_change_job, int exiting)
 	}
 
 	if (sigtrapped[SIGDEBUG] && isset(DEBUGBEFORECMD)) {
+	    int oerrexit_opt = opts[ERREXIT];
+	    opts[ERREXIT] = 0;
+	    noerrexit = 1;
+
 	    exiting = donetrap;
 	    ret = lastval;
 	    dotrap(SIGDEBUG);
@@ -1071,7 +1082,8 @@ execlist(Estate state, int dont_change_job, int exiting)
 	     * Only execute the trap once per sublist, even
 	     * if the DEBUGBEFORECMD option changes.
 	     */
-	    donedebug = 1;
+	    donedebug = isset(ERREXIT) ? 2 : 1;
+	    opts[ERREXIT] = oerrexit_opt;
 	} else
 	    donedebug = 0;
 
@@ -1087,6 +1099,18 @@ execlist(Estate state, int dont_change_job, int exiting)
 
 	/* Loop through code followed by &&, ||, or end of sublist. */
 	code = *state->pc++;
+	if (donedebug == 2) {
+	    /* Skip sublist. */
+	    while (wc_code(code) == WC_SUBLIST) {
+		state->pc = state->pc + WC_SUBLIST_SKIP(code);
+		if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END)
+		    break;
+		code = *state->pc++;
+	    }
+	    donetrap = 1;
+	    /* yucky but consistent... */
+	    goto sublist_done;
+	}
 	while (wc_code(code) == WC_SUBLIST) {
 	    next = state->pc + WC_SUBLIST_SKIP(code);
 	    if (!oldnoerrexit)
@@ -1177,12 +1201,20 @@ sublist_done:
 	noerrexit = oldnoerrexit;
 
 	if (sigtrapped[SIGDEBUG] && !isset(DEBUGBEFORECMD) && !donedebug) {
+	    /*
+	     * Save and restore ERREXIT for consistency with
+	     * DEBUGBEFORECMD, even though it's not used.
+	     */
+	    int oerrexit_opt = opts[ERREXIT];
+	    opts[ERREXIT] = 0;
+	    noerrexit = 1;
 	    exiting = donetrap;
 	    ret = lastval;
 	    dotrap(SIGDEBUG);
 	    lastval = ret;
 	    donetrap = exiting;
 	    noerrexit = oldnoerrexit;
+	    opts[ERREXIT] = oerrexit_opt;
 	}
 
 	cmdsp = csp;
@@ -4145,8 +4177,8 @@ doshfunc(char *name, Eprog prog, LinkList doshargs, int flags, int noreturnval)
 
     oargv0 = NULL;
     obreaks = breaks;;
-    if (trapreturn < 0)
-	trapreturn--;
+    if (trap_state == TRAP_STATE_PRIMED)
+	trap_return--;
     oldlastval = lastval;
     oldnumpipestats = numpipestats;
     if (noreturnval) {
@@ -4264,8 +4296,8 @@ doshfunc(char *name, Eprog prog, LinkList doshargs, int flags, int noreturnval)
 
     endtrapscope();
 
-    if (trapreturn < -1)
-	trapreturn++;
+    if (trap_state == TRAP_STATE_PRIMED)
+	trap_return++;
     ret = lastval;
     if (noreturnval) {
 	lastval = oldlastval;
@@ -4528,7 +4560,9 @@ execsave(void)
     es->badcshglob = badcshglob;
     es->cmdoutpid = cmdoutpid;
     es->cmdoutval = cmdoutval;
-    es->trapreturn = trapreturn;
+    es->trap_return = trap_return;
+    es->trap_state = trap_state;
+    es->trapisfunc = trapisfunc;
     es->noerrs = noerrs;
     es->subsh_close = subsh_close;
     es->underscore = ztrdup(underscore);
@@ -4555,7 +4589,9 @@ execrestore(void)
     badcshglob = exstack->badcshglob;
     cmdoutpid = exstack->cmdoutpid;
     cmdoutval = exstack->cmdoutval;
-    trapreturn = exstack->trapreturn;
+    trap_return = exstack->trap_return;
+    trap_state = exstack->trap_state;
+    trapisfunc = exstack->trapisfunc;
     noerrs = exstack->noerrs;
     subsh_close = exstack->subsh_close;
     setunderscore(exstack->underscore);
diff --git a/Src/init.c b/Src/init.c
index f2dc99af7..d8a0dbc57 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -191,10 +191,6 @@ loop(int toplevel, int justonce)
 	    exit(lastval);
 	if (((!interact || sourcelevel) && errflag) || retflag)
 	    break;
-	if (intrap && trapreturn >= 0) {
-	    lastval = trapreturn;
-	    trapreturn = 0;
-	}
 	if (isset(SINGLECOMMAND) && toplevel) {
 	    if (sigtrapped[SIGEXIT])
 		dotrap(SIGEXIT);
@@ -880,7 +876,8 @@ setupvals(void)
     lastmailcheck = time(NULL);
     locallevel = sourcelevel = 0;
     sfcontext = SFC_NONE;
-    trapreturn = 0;
+    trap_return = 0;
+    trap_state = TRAP_STATE_INACTIVE;
     noerrexit = -1;
     nohistsave = 1;
     dirstack = znewlinklist();
@@ -1060,6 +1057,7 @@ source(char *s)
     char *old_scriptname = scriptname, *us;
     unsigned char *ocs;
     int ocsp;
+    int otrap_return = trap_return, otrap_state = trap_state;
 
     if (!s || 
 	(!(prog = try_source_file((us = unmeta(s)))) &&
@@ -1090,6 +1088,13 @@ source(char *s)
     dosetopt(SHINSTDIN, 0, 1);
     scriptname = s;
 
+    /*
+     * The special return behaviour of traps shouldn't
+     * trigger in files sourced from traps; the return
+     * is just a return from the file.
+     */
+    trap_state = TRAP_STATE_INACTIVE;
+
     sourcelevel++;
     if (prog) {
 	pushheap();
@@ -1100,6 +1105,9 @@ source(char *s)
 	loop(0, 0);		     /* loop through the file to be sourced  */
     sourcelevel--;
 
+    trap_state = otrap_state;
+    trap_return = otrap_return;
+
     /* restore the current shell state */
     if (prog)
 	freeeprog(prog);
diff --git a/Src/options.c b/Src/options.c
index bb99ba1e5..d6aa3c850 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -112,7 +112,7 @@ static struct optname optns[] = {
 {{NULL, "cshjunkiequotes",    OPT_EMULATE|OPT_CSH},	 CSHJUNKIEQUOTES},
 {{NULL, "cshnullcmd",	      OPT_EMULATE|OPT_CSH},	 CSHNULLCMD},
 {{NULL, "cshnullglob",	      OPT_EMULATE|OPT_CSH},	 CSHNULLGLOB},
-{{NULL, "debugbeforecmd",     OPT_EMULATE},		 DEBUGBEFORECMD},
+{{NULL, "debugbeforecmd",     OPT_ALL},			 DEBUGBEFORECMD},
 {{NULL, "emacs",	      0},			 EMACSMODE},
 {{NULL, "equals",	      OPT_EMULATE|OPT_ZSH},	 EQUALS},
 {{NULL, "errexit",	      OPT_EMULATE},		 ERREXIT},
diff --git a/Src/signals.c b/Src/signals.c
index 61fff64a2..d978c3dec 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -1082,12 +1082,10 @@ dotrapargs(int sig, int *sigtr, void *sigfn)
 {
     LinkList args;
     char *name, num[4];
-    int trapret = 0;
     int obreaks = breaks;
     int oretflag = retflag;
-    int otrapreturn = trapreturn;
     int isfunc;
-    int traperr;
+    int traperr, new_trap_state, new_trap_return;
 
     /* if signal is being ignored or the trap function		      *
      * is NULL, then return					      *
@@ -1123,6 +1121,7 @@ dotrapargs(int sig, int *sigtr, void *sigfn)
     *sigtr |= ZSIG_IGNORED;
 
     lexsave();
+    /* execsave will save the old trap_return and trap_state */
     execsave();
     breaks = retflag = 0;
     runhookdef(BEFORETRAPHOOK, NULL);
@@ -1148,7 +1147,8 @@ dotrapargs(int sig, int *sigtr, void *sigfn)
 	sprintf(num, "%d", sig);
 	zaddlinknode(args, num);
 
-	trapreturn = -1;	/* incremented by doshfunc */
+	trap_return = -1;	/* incremented by doshfunc */
+	trap_state = TRAP_STATE_PRIMED;
 	trapisfunc = isfunc = 1;
 
 	sfcontext = SFC_SIGNAL;
@@ -1156,46 +1156,32 @@ dotrapargs(int sig, int *sigtr, void *sigfn)
 	sfcontext = osc;
 	freelinklist(args, (FreeFunc) NULL);
 	zsfree(name);
-
     } else {
-	trapreturn = -2;	/* not incremented, used at current level */
+	trap_return = -2;	/* not incremented, used at current level */
+	trap_state = TRAP_STATE_PRIMED;
 	trapisfunc = isfunc = 0;
 
 	execode(sigfn, 1, 0);
     }
     runhookdef(AFTERTRAPHOOK, NULL);
 
-    if (trapreturn > 0 && isfunc) {
-	/*
-	 * Context was its own function.  We propagate the return
-	 * value specially.  Return value zero means don't do
-	 * anything special, so don't handle it.
-	 */
-	trapret = trapreturn;
-    } else if (trapreturn >= 0 && !isfunc) {
-	/*
-	 * Context was an inline trap.  If an explicit return value
-	 * was used, we need to set `lastval'.  Otherwise we use the
-	 * value restored by execrestore.  In this case, all return
-	 * values indicate an explicit return from the current function,
-	 * so always handle specially.  trapreturn is always restored by
-	 * execrestore.
-	 */
-	trapret = trapreturn + 1;
-    }
     traperr = errflag;
-    trapreturn = otrapreturn;
+
+    /* Grab values before they are restored */
+    new_trap_state = trap_state;
+    new_trap_return = trap_return;
+
     execrestore();
     lexrestore();
 
-    if (trapret > 0) {
+    if (new_trap_state == TRAP_STATE_FORCE_RETURN &&
+	/* zero return from function isn't special */
+	!(isfunc && new_trap_return == 0)) {
 	if (isfunc) {
 	    breaks = loops;
 	    errflag = 1;
-	    lastval = trapret;
-	} else {
-	    lastval = trapret-1;
 	}
+	lastval = new_trap_return;
 	/* return triggered */
 	retflag = 1;
     } else {
diff --git a/Src/zsh.h b/Src/zsh.h
index d245a416a..49c08c7ac 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -921,7 +921,9 @@ struct execstack {
     int badcshglob;
     pid_t cmdoutpid;
     int cmdoutval;
-    int trapreturn;
+    int trap_return;
+    int trap_state;
+    int trapisfunc;
     int noerrs;
     int subsh_close;
     char *underscore;
@@ -2225,6 +2227,24 @@ struct heap {
 #define ZSIG_ALIAS	(1<<3)  /* Trap is stored under an alias */
 #define ZSIG_SHIFT	4
 
+/*
+ * State of traps, stored in trap_state.
+ */
+enum trap_state {
+    /* Traps are not active; trap_return is not useful. */
+    TRAP_STATE_INACTIVE,
+    /*
+     * Traps are set but haven't triggered; trap_return gives
+     * minus function depth.
+     */
+    TRAP_STATE_PRIMED,
+    /*
+     * Trap has triggered to force a return; trap_return givens
+     * return value.
+     */
+    TRAP_STATE_FORCE_RETURN
+};
+
 /***********/
 /* Sorting */
 /***********/