[Patch] Fix ERR_RETURN behavior in and/or statements

[Philippe Altherr <philippe.altherr@xxxxxxxxx>]

ATTENTION: This patch depends on the following patches described here:

- patch-01-revert-mistaken-errexit-patches.txt

- patch-03-fix-negation-statement.txt

I have found another issue in execlist, which affects the behavior of ERR_RETURN in convoluted cases that involve at least two &&/|| commands and a function call. The following example exhibits the issue.

set -o err_return
fn() { { false; echo fn1 } && echo fn2 }
fn; echo done
fn && echo done

The execution of "fn; echo done" prints "fn1", "fn2", and "done". The execution of "fn && echo done" should print the same but currently prints nothing.

The problem is that this test in "execlist" is too conservative. Inside "fn", "oldnoerrexit" is equal to "NOERREXIT_EXIT". Indeed the "&&" in "fn && echo done"  first sets "noerrexit" to  "NOERREXIT_EXIT | NOERREXIT_RETURN". The call to "fn" then removes "NOERREXIT_RETURN". Thus, at the beginning of "execlist" in the call to "fn", "noerrexit" is equal to "NOERREXIT_EXIT" and "oldnoerrexit" is initialized to the same value. Since "oldnoerrexit" is non-zero, the test fails to readd "NOERREXIT_RETURN" for the evaluation of "{ false; echo fn1 }". Therefore the evaluation of "false" triggers an early return of "fn" wîth exit status 1, which in turn triggers an early return of the toplevel and thus nothing is printed.

The solution is to unconditionally set "noerrexit" to "NOERREXIT_EXIT | NOERREXIT_RETURN" for any command on the left of "&&" and "||" (and for any command on the right of "!"). The patch does exactly that (and adds a regression test).

Philippe

diff --git a/Src/exec.c b/Src/exec.c
index 43df8211a..711d8f374 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1427,14 +1427,12 @@ execlist(Estate state, int dont_change_job, int exiting)
 	    goto sublist_done;
 	}
 	while (wc_code(code) == WC_SUBLIST) {
-	    int isend = (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END);
+	    int isandor = WC_SUBLIST_TYPE(code) != WC_SUBLIST_END;
+	    int isnot = WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT;
 	    next = state->pc + WC_SUBLIST_SKIP(code);
-	    if (!oldnoerrexit)
-		noerrexit = isend ? 0 : NOERREXIT_EXIT | NOERREXIT_RETURN;
-	    if (WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT) {
-		/* suppress errexit for the commands in ! <list-of-commands> */
+	    /* suppress errexit for commands before && and || and after ! */
+	    if (isandor || isnot)
 		noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
-	    }
 	    switch (WC_SUBLIST_TYPE(code)) {
 	    case WC_SUBLIST_END:
 		/* End of sublist; just execute, ignoring status. */
@@ -1444,7 +1442,7 @@ execlist(Estate state, int dont_change_job, int exiting)
 		    execpline(state, code, ltype, (ltype & Z_END) && exiting);
 		state->pc = next;
 		/* suppress errexit for the command "! ..." */
-		if (WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT)
+		if (isnot)
 		  this_noerrexit = 1;
 		goto sublist_done;
 		break;
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index a8880673f..b7132da81 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -670,6 +670,22 @@ F:Must be tested with a top-level script rather than source or function
 >before-out
 >before-in
 
+  (set -o err_return
+    fn() {
+      print before-in
+      { false; true } && true
+      print after-in
+    }
+    print before-out
+    fn && true
+    print after-out
+  )
+0:ERR_RETURN not triggered on LHS of "&&" in function on LHS of "&&" (regression test)
+>before-out
+>before-in
+>after-in
+>after-out
+
   mkdir -p zdotdir
   print >zdotdir/.zshenv '
   setopt norcs errreturn