Fix EXIT trap not called when shell is exiting under numerous circumstances

README

@@ -97,6 +97,29 @@ consistent and better aligned with the POSIX-2017 specification of
       f() { { false; echo "This is printed only since 5.10." } || true }
       if f; then true; fi
 
+EXIT traps are now correctly executed when the shell exits due to
+ERR_EXIT, `${var:?}', or NO_UNSET (set -u) triggering inside a
+function.  Previously, these code paths bypassed the function scope
+unwind, so EXIT traps saved by `starttrapscope()' on function entry
+were never restored or executed.  Examples:
+
+    setopt ERR_EXIT
+    trap 'echo exiting' EXIT
+    f() { false }
+    f
+    # "exiting" is printed only since 5.10.
+
+    trap 'echo exiting' EXIT
+    f() { : ${unset:?oops} }
+    f
+    # "exiting" is printed only since 5.10.
+
+    setopt NO_UNSET
+    trap 'echo exiting' EXIT
+    f() { echo $unset }
+    f
+    # "exiting" is printed only since 5.10.
+
 PCRE support is now PCRE2.
 
 Parameter names may begin with a "." and follow a relaxed implementation

Src/builtin.c

@@ -5823,6 +5823,25 @@ mod_export volatile int exit_pending;
 /**/
 mod_export volatile int exit_level;
 
+/*
+ * Set up exit_pending unwind: instead of exiting immediately,
+ * force all active functions to return so that endtrapscope()
+ * runs at each level and all saved EXIT traps are executed.
+ */
+
+/**/
+mod_export void
+set_exit_pending(int val)
+{
+    if (trap_state)
+	trap_state = TRAP_STATE_FORCE_RETURN;
+    retflag = 1;
+    breaks = loops;
+    exit_pending = 1;
+    exit_level = locallevel;
+    exit_val = val;
+}
+
 /* we have printed a 'you have stopped (running) jobs.' message */
 
 /**/
@@ -5905,13 +5924,7 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func)
 	     * a bad job.
 	     */
 	    if (stopmsg || (zexit(0, ZEXIT_DEFERRED), !stopmsg)) {
-		if (trap_state) 
-		    trap_state = TRAP_STATE_FORCE_RETURN;
-		retflag = 1;
-		breaks = loops;
-		exit_pending = 1;
-		exit_level = locallevel;
-		exit_val = num;
+		set_exit_pending(num);
 	    }
 	} else
 	    zexit(num, ZEXIT_NORMAL);

Src/exec.c

@@ -1616,12 +1616,22 @@ execlist(Estate state, int dont_change_job, int exiting)
 		    !(noerrexit & NOERREXIT_EXIT);
 		if (errexit) {
 		    errflag = 0;
-		    if (sigtrapped[SIGEXIT])
-			dotrap(SIGEXIT);
-		    if (mypid != getpid())
-			_realexit();
-		    else
-			realexit();
+		    if (locallevel > forklevel) {
+			/*
+			 * Inside a function: use the same unwind
+			 * mechanism as the exit builtin so that
+			 * endtrapscope() runs at each level and
+			 * all saved EXIT traps are executed.
+			 */
+			set_exit_pending(lastval);
+		    } else {
+			if (sigtrapped[SIGEXIT])
+			    dotrap(SIGEXIT);
+			if (mypid != getpid())
+			    _realexit();
+			else
+			    realexit();
+		    }
 		}
 		if (errreturn) {
 		    retflag = 1;

Src/signals.c

@@ -875,6 +875,36 @@ starttrapscope(void)
  * endparamscope() so that the locallevel has been decremented.
  */
 
+/*
+ * Restore a single saved trap entry: call settrap() to reinstall it,
+ * update sigtrapped[], and for function-style traps re-add the node
+ * to shfunctab.  Used by endtrapscope().
+ */
+
+static void
+restore_saved_trap(struct savetrap *st)
+{
+    int sig = st->sig;
+
+    dontsavetrap++;
+    if (st->flags & ZSIG_FUNC)
+	settrap(sig, NULL, ZSIG_FUNC);
+    else
+	settrap(sig, (Eprog) st->list, 0);
+    if (sig == SIGEXIT)
+	exit_trap_posix = st->posix;
+    dontsavetrap--;
+    /*
+     * counting of nsigtrapped should presumably be handled
+     * in settrap...
+     */
+    DPUTS((sigtrapped[sig] ^ st->flags) & ZSIG_TRAPPED,
+	  "BUG: settrap didn't restore correct ZSIG_TRAPPED");
+    if ((sigtrapped[sig] = st->flags) & ZSIG_FUNC)
+	shfunctab->addnode(shfunctab, ((Shfunc)st->list)->node.nam,
+			   (Shfunc) st->list);
+}
+
 /**/
 void
 endtrapscope(void)
@@ -911,24 +941,7 @@ endtrapscope(void)
 	    remnode(savetraps, ln);
 
 	    if (st->flags && (st->list != NULL)) {
-		/* prevent settrap from saving this */
-		dontsavetrap++;
-		if (st->flags & ZSIG_FUNC)
-		    settrap(sig, NULL, ZSIG_FUNC);
-		else
-			settrap(sig, (Eprog) st->list, 0);
-		if (sig == SIGEXIT)
-		    exit_trap_posix = st->posix;
-		dontsavetrap--;
-		/*
-		 * counting of nsigtrapped should presumably be handled
-		 * in settrap...
-		 */
-		DPUTS((sigtrapped[sig] ^ st->flags) & ZSIG_TRAPPED,
-		      "BUG: settrap didn't restore correct ZSIG_TRAPPED");
-		if ((sigtrapped[sig] = st->flags) & ZSIG_FUNC)
-		    shfunctab->addnode(shfunctab, ((Shfunc)st->list)->node.nam,
-				       (Shfunc) st->list);
+		restore_saved_trap(st);
 	    } else if (sigtrapped[sig]) {
 		/*
 		 * Don't restore the old state if someone has set a
@@ -947,7 +960,20 @@ endtrapscope(void)
 	 * We already made sure this wasn't set as a POSIX exit trap.
 	 * We respect the user's intention when the trap in question
 	 * was set.
+	 *
+	 * If we are exiting, clear errflag so the trap can run.
+	 * Matches what zexit() does before calling dotrap().
+	 *
+	 * If errflag is set in a non-interactive shell (which will
+	 * cause exit via the main loop), convert to exit_pending so
+	 * that all nested EXIT traps are properly executed bottom-up.
+	 * This handles NOUNSET errors and any other zerr()-based
+	 * errors that would cause exit.
 	 */
+	if (!exit_pending && !interact && (errflag & ERRFLAG_ERROR))
+	    set_exit_pending(lastval ? lastval : 1);
+	if (exit_pending)
+	    errflag = 0;
 	dotrapargs(SIGEXIT, &exittr, exitfn);
 	if (exittr & ZSIG_FUNC)
 	    shfunctab->freenode((HashNode)exitfn);
@@ -958,7 +984,6 @@ endtrapscope(void)
 	  "BUG: still saved traps outside all function scope");
 }
 
-
 /*
  * Decide whether a trap needs handling.
  * If so, see if the trap should be run now or queued.

Src/subst.c

@@ -3343,15 +3343,23 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
                      */
                     errflag |= ERRFLAG_HARD;
                     if (!interact) {
-                        if (mypid == getpid()) {
+                        if (locallevel > forklevel) {
                             /*
-                             * paranoia: don't check for jobs, but there
-                             * shouldn't be any if not interactive.
+                             * Inside a function: use the same unwind
+                             * mechanism as the exit builtin so that
+                             * endtrapscope() runs at each level and
+                             * all saved EXIT traps are executed.
+                             */
+                            set_exit_pending(1);
+                        } else {
+                            /*
+                             * Not inside a function: zexit() runs the
+                             * EXIT trap and handles both main process
+                             * and subshell (_exit) cases.
                              */
                             stopmsg = 1;
                             zexit(1, ZEXIT_NORMAL);
-                        } else
-                            _exit(1);
+                        }
                     }
                 }
 		return NULL;

Test/C03traps.ztst

@@ -1014,6 +1014,172 @@ F:Must be tested with a top-level script rather than source or function
 >one
 >one
 
+  (setopt nounset; trap 'echo EXIT' EXIT; echo $unset)
+1:EXIT trap called when NOUNSET triggers at top level
+*?*unset: parameter not set
+>EXIT
+
+  (trap 'echo EXIT' EXIT; echo "a${unsetvar:?oops}z")
+1:EXIT trap called when ${var:?} triggers at top level
+*?*unsetvar: oops
+>EXIT
+
+  (setopt err_exit; trap 'echo EXIT' EXIT; false)
+1:EXIT trap called when ERR_EXIT triggers at top level
+>EXIT
+
+  (setopt err_exit; trap 'echo EXIT' EXIT; f() { false; }; f)
+1:EXIT trap called when ERR_EXIT triggers inside function
+>EXIT
+
+  (setopt err_exit
+  trap 'echo A' EXIT
+  f1() {
+    trap 'echo B' EXIT
+    f2() {
+      trap 'echo C' EXIT
+      false
+    }
+    f2
+  }
+  f1)
+1:nested EXIT traps all called when ERR_EXIT triggers inside function
+>C
+>B
+>A
+
+  (trap 'echo EXIT' EXIT; f() { echo "a${unsetvar:?oops}z"; }; f)
+1:EXIT trap called when ${var:?} triggers inside function
+*?*unsetvar: oops
+>EXIT
+
+  (trap 'echo A' EXIT
+  f1() {
+    trap 'echo B' EXIT
+    f2() {
+      trap 'echo C' EXIT
+      echo "a${unsetvar:?oops}z"
+    }
+    f2
+  }
+  f1)
+1:nested EXIT traps all called when ${var:?} triggers inside function
+*?*unsetvar: oops
+>C
+>B
+>A
+
+  (setopt nounset; trap 'echo EXIT' EXIT; f() { echo $unset; }; f)
+1:EXIT trap called when NOUNSET triggers inside function
+*?*unset: parameter not set
+>EXIT
+
+  (setopt nounset
+  trap 'echo A' EXIT
+  f1() {
+    trap 'echo B' EXIT
+    f2() {
+      trap 'echo C' EXIT
+      echo $unset
+    }
+    f2
+  }
+  f1)
+1:nested EXIT traps all called when NOUNSET triggers inside function
+*?*unset: parameter not set
+>C
+>B
+>A
+
+  (setopt nounset; trap 'echo EXIT' EXIT; f() { echo $(( unset )); }; f)
+1:EXIT trap called when NOUNSET triggers in math context inside function
+*?*unset: parameter not set
+>EXIT
+
+  (setopt posix_traps err_exit
+  trap 'echo A' EXIT
+  f() { trap 'echo B' EXIT; false; }
+  f)
+1:POSIX_TRAPS with ERR_EXIT fires global (overridden) EXIT trap
+>B
+
+  (setopt posix_traps err_exit
+  trap 'echo A' EXIT
+  f() { trap 'echo B' EXIT; }
+  f
+  false)
+1:POSIX_TRAPS with ERR_EXIT fires global EXIT trap after function override
+>B
+
+  (setopt err_exit
+  trap 'echo EXIT' EXIT
+  f() { false; }
+  f || true
+  echo continued)
+0:ERR_EXIT suppressed by || does not exit, EXIT trap fires on normal exit
+>continued
+>EXIT
+
+  (setopt err_exit
+  f1() {
+    trap 'echo F1' EXIT
+    f2() {
+      f3() { trap 'echo F3' EXIT; false; }
+      f3
+    }
+    f2
+  }
+  f1)
+1:ERR_EXIT nested EXIT traps fire even when middle function has no trap
+>F3
+>F1
+
+  (setopt err_exit
+  trap 'echo A' EXIT
+  f() {
+    trap 'echo B; exit 42' EXIT
+    false
+  }
+  f)
+42:exit in function EXIT trap overrides exit status
+>B
+>A
+
+  (setopt err_exit
+  trap 'echo A; exit 42' EXIT
+  f() {
+    trap 'echo B; exit 7' EXIT
+    false
+  }
+  f)
+42:exit in nested EXIT traps uses outermost trap exit value
+>B
+>A
+
+  (trap '' EXIT
+  setopt err_exit
+  f() { false; }
+  f)
+1:ignored EXIT trap with ERR_EXIT does not interfere
+
+  (setopt err_exit
+  f() {
+    trap 'echo B' EXIT
+    (false)
+  }
+  f)
+1:ERR_EXIT from subshell failure inside function fires function EXIT trap
+>B
+
+  (setopt nounset
+  trap 'echo EXIT' EXIT
+  f() { echo $unset; }
+  f || true
+  echo continued)
+1:NOUNSET hard error not caught by ||, EXIT trap fires
+*?*unset: parameter not set
+>EXIT
+
   ( set -o ERR_RETURN; f() { false; echo NOT REACHED;  }; f || true; echo OK )
   ( set -o ERR_RETURN; f() { true && false; echo NOT REACHED;  }; f || true; echo OK )
   ( set -o ERR_RETURN; f() { true && { false }; echo NOT REACHED;  }; f || true; echo OK )