Re: errexit and (Z)ERR trap regression

[Philippe Altherr <philippe.altherr@xxxxxxxxx>]

Note that just adding braces is enough to trigger the bug:

     $ zsh -e -c 'true && false; echo NOT REACHED'     # no output; correct
     $ zsh -e -c 'true && {false; echo NOT REACHED}'  # incorrect output
     NOT REACHED

and

     $ zsh -c 'trap "echo Trapped!" ERR; true && false'     # correct output
     Trapped!
     $ zsh -c 'trap "echo Trapped!" ERR; true && {false}'  # no output; bug

I think that the correct fix is the following:

    if (isandor || isnot)

        noerrexit = oldnoerrexit | NOERREXIT_EXIT | NOERREXIT_RETURN;

    else

        noerrexit = oldnoerrexit;

For reminder, here is the current code:

    if (isandor || isnot)

        noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;

In other words, every sub-command in a list should only depend on the noerrexit that was in effect at the start of the evaluation of the list. It should in no way depend on the noerrexit that results from the evaluation of proceeding sub-commands as is currently the case. The current code only works for lists where the last sub-command is a simple command, thanks to the "noerrexit = oldnoerrexit;" performed after the last sub-command just before checking whether an ERREXIT should be triggered.

Here are two other examples fixed by this patch:

    zsh -c 'trap "echo Trapped!" ERR; true && if true; then false; fi' 

    zsh -c 'trap "echo Trapped!" ERR; true && {false} always {true}' 

Philippe

On Tue, Jun 25, 2024 at 1:03 AM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:

On Fri, Jun 21, 2024 at 10:50 PM Lawrence Velázquez <larryv@xxxxxxx> wrote:
>
> >    set -e; true && { false; echo one; } || echo two
> >
> > Is "false" there considered to be "any command of an AND-OR list" ?
>
> This ought to work the same as the subshell version: early exit is
> suppressed for the commands "true" and "{ false; echo one; }", so
> the overall output is still "one".

The following seems to fix all six of the mentioned cases from this
thread, then.

diff --git a/Src/exec.c b/Src/exec.c
index e955e85df..916a3567d 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1485,7 +1485,9 @@ execlist(Estate state, int dont_change_job, int exiting)
 	    next = state->pc + WC_SUBLIST_SKIP(code);
 	    /* suppress errexit for commands before && and || and after ! */
 	    if (isandor || isnot)
-		noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;
+		noerrexit = oldnoerrexit | NOERREXIT_EXIT | NOERREXIT_RETURN;
+	    else
+		noerrexit = oldnoerrexit;
 	    switch (WC_SUBLIST_TYPE(code)) {
 	    case WC_SUBLIST_END:
 		/* End of sublist; just execute, ignoring status. */
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index de57765a0..9fefdf713 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -995,6 +995,25 @@ F:Must be tested with a top-level script rather than source or function
 ?loop 0
 ?loop 1
 
+  ( set -e; true && {false; echo NOT REACHED} )
+  ( trap "print Trapped!" ERR; true && {false} )
+  ( trap "print Trapped!" ERR; true && if true; then false; fi )
+  ( trap "print Trapped!" ERR; true && {false} always {true} )
+  ( true && (set -e; false; echo NOT REACHED) )
+  ( true && (trap "print Trapped!" ERR; false) )
+  ( true && { set -e; false; echo NOT REACHED } )
+  ( true && { trap "print Trapped!" ERR; false } )
+  ( set -e; true && (false; echo one) || echo two )
+  ( set -e; true && { false; echo one; } || echo two )
+0:ERR_EXIT is triggered by last command in an AND-OR list
+>Trapped!
+>Trapped!
+>Trapped!
+>Trapped!
+>Trapped!
+>one
+>one
+
   if zmodload zsh/system 2>/dev/null; then
   (
     trap 'echo TERM; exit 2' TERM