'set -e' with '!' POSIX issue

'set -e' with '!' POSIX issue

[Martijn Dekker]

'set -e' ('set -o errexit') is not POSIX compliant on zsh because it
doesn't ignore a command beginning with "!".

The script:

    set -e
    ! true
    echo good

should output "good", and does, on every shell except zsh.

Ref.:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_25
| 2. The -e setting shall be ignored when executing the compound list
| following the while, until, if, or elif reserved word, a pipeline
| beginning with the ! reserved word, or any command of an AND-OR list
| other than the last.

(Note that in POSIX terms the definition of "pipeline" includes a simple
command.)

Thanks,

- Martijn

Re: 'set -e' with '!' POSIX issue

[Peter Stephenson]

On Sun, 2 Oct 2016 11:01:18 +0100
Martijn Dekker <martijn@inlv.org> wrote:
> 'set -e' ('set -o errexit') is not POSIX compliant on zsh because it
> doesn't ignore a command beginning with "!".

I don't really like the errexit logic as there are too many horrible
cases, since the place we need to test is different from where the code
is being executed (look at the particularly horrible oldnoeerrexit
logic).  This was not entirely trivial but we might get away with it.

The tests are for ERR_RETURN because that's easier to write; most of the
logic is shared with ERR_EXIT.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 349414c..a429428 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1226,6 +1226,7 @@ execlist(Estate state, int dont_change_job, int exiting)
     }
     while (wc_code(code) == WC_LIST && !breaks && !retflag && !errflag) {
 	int donedebug;
+	int this_noerrexit = 0;
 
 	ltype = WC_LIST_TYPE(code);
 	csp = cmdsp;
@@ -1309,9 +1310,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);
 	    next = state->pc + WC_SUBLIST_SKIP(code);
 	    if (!oldnoerrexit)
-		noerrexit = (WC_SUBLIST_TYPE(code) != WC_SUBLIST_END);
+		noerrexit = !isend;
+	    if ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT) && isend)
+		this_noerrexit = 1;
 	    switch (WC_SUBLIST_TYPE(code)) {
 	    case WC_SUBLIST_END:
 		/* End of sublist; just execute, ignoring status. */
@@ -1427,7 +1431,7 @@ sublist_done:
 	/* Check whether we are suppressing traps/errexit *
 	 * (typically in init scripts) and if we haven't  *
 	 * already performed them for this sublist.       */
-	if (!noerrexit && !donetrap) {
+	if (!noerrexit && !this_noerrexit && !donetrap) {
 	    if (sigtrapped[SIGZERR] && lastval) {
 		dotrap(SIGZERR);
 		donetrap = 1;
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index 83c05aa..3a65b28 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -519,6 +519,43 @@
 >Yes
 F:Must be tested with a top-level script rather than source or function
 
+  fn() {
+      emulate -L zsh
+      setopt errreturn
+      print before
+      false
+      print after
+  }
+  fn
+1:ERRRETURN, basic case
+>before
+
+  fn() {
+      emulate -L zsh
+      setopt errreturn
+      print before
+      ! true
+      ! false
+      print after
+  }
+  fn
+0:ERRETURN with "!"
+>before
+>after
+
+  fn() {
+      emulate -L zsh
+      setopt errreturn
+      print before
+      ! true
+      ! false
+      false
+      print after
+  }
+  fn
+1:ERRETURN with "!" and a following false
+>before
+
 %clean
 
   rm -f TRAPEXIT

Re: 'set -e' with '!' POSIX issue

[Vincent Lefevre]

On 2016-10-02 18:55:30 +0100, Peter Stephenson wrote:
> On Sun, 2 Oct 2016 11:01:18 +0100
> Martijn Dekker <martijn@inlv.org> wrote:
> > 'set -e' ('set -o errexit') is not POSIX compliant on zsh because it
> > doesn't ignore a command beginning with "!".
> 
> I don't really like the errexit logic as there are too many horrible
> cases, since the place we need to test is different from where the code
> is being executed (look at the particularly horrible oldnoeerrexit
> logic).  This was not entirely trivial but we might get away with it.
> 
> The tests are for ERR_RETURN because that's easier to write; most of the
> logic is shared with ERR_EXIT.

I thought that this was not supposed to change:

  http://www.zsh.org/mla/workers/2009/msg00572.html
  http://www.zsh.org/mla/workers/2009/msg00574.html

I haven't checked the current status for the other two cases.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: 'set -e' with '!' POSIX issue

[Peter Stephenson]

On Tue, 4 Oct 2016 09:45:38 +0200
Vincent Lefevre <vincent@vinc17.net> wrote:
> On 2016-10-02 18:55:30 +0100, Peter Stephenson wrote:
> > On Sun, 2 Oct 2016 11:01:18 +0100
> > Martijn Dekker <martijn@inlv.org> wrote:
> > > 'set -e' ('set -o errexit') is not POSIX compliant on zsh because it
> > > doesn't ignore a command beginning with "!".
> > 
> > I don't really like the errexit logic as there are too many horrible
> > cases, since the place we need to test is different from where the code
> > is being executed (look at the particularly horrible oldnoeerrexit
> > logic).  This was not entirely trivial but we might get away with it.
> > 
> > The tests are for ERR_RETURN because that's easier to write; most of the
> > logic is shared with ERR_EXIT.
> 
> I thought that this was not supposed to change:
> 
>   http://www.zsh.org/mla/workers/2009/msg00572.html
>   http://www.zsh.org/mla/workers/2009/msg00574.html
> 
> I haven't checked the current status for the other two cases.

The point I made there was I was waiting for things to clear up.  That
was 7 years ago.  I'm not going to wait any longer for things to clear
up...

pws

Re: 'set -e' with '!' POSIX issue

[Martijn Dekker]

Op 04-10-16 om 10:30 schreef Peter Stephenson:
> On Tue, 4 Oct 2016 09:45:38 +0200
> Vincent Lefevre <vincent@vinc17.net> wrote:
>> I thought that this was not supposed to change:
>>
>>   http://www.zsh.org/mla/workers/2009/msg00572.html
>>   http://www.zsh.org/mla/workers/2009/msg00574.html
>>
>> I haven't checked the current status for the other two cases.
> 
> The point I made there was I was waiting for things to clear up.  That
> was 7 years ago.  I'm not going to wait any longer for things to clear
> up...

FYI, your patch fixes the simple case ('! true'), but not the two more
complex cases mentioned back then:

On Fri, Mar 13, 2009 at 03:51:34PM +0100, Vincent Lefevre wrote:
> According to the new "set -e" proposal
>
>   http://www.opengroup.org/austin/mailarchives/ag/msg18258.html
>
> zsh -c 'set -e; ! if true; then false; fi; echo $?'
>
> should output 0, i.e. "false" should not make the shell exit, because
> it is under a "!" context (even though "!" doesn't apply on the "false"
> command directly).
>
> Note that every other shell (bash, ksh93, pdksh, dash, posh) output 0.

On Tue, Mar 17, 2009 at 12:46:20PM +0100, Vincent Lefevre wrote:
> POSIX shells (bash, dash, ksh93, pdksh, posh) return with no output
> and an exit status equal to 1 on:
>
>   sh -c 'set -e; foo() { false && false; }; foo; echo $?'
>
> but zsh doesn't, even with "emulate sh":
>
> $ zsh -fc 'emulate sh; set -e; foo() { false && false; }; foo; echo $?'
> 1
>
> zsh should match the existing practice (perhaps even without
> "emulate sh", unless this can break too many zsh scripts).

Thanks,

- M.

Re: 'set -e' with '!' POSIX issue

[Peter Stephenson]

On Wed, 5 Oct 2016 12:18:26 +0200
Martijn Dekker <martijn@inlv.org> wrote:
> On Fri, Mar 13, 2009 at 03:51:34PM +0100, Vincent Lefevre wrote:
> > According to the new "set -e" proposal
> >
> >   http://www.opengroup.org/austin/mailarchives/ag/msg18258.html
> >
> > zsh -c 'set -e; ! if true; then false; fi; echo $?'
> >
> > should output 0, i.e. "false" should not make the shell exit, because
> > it is under a "!" context (even though "!" doesn't apply on the "false"
> > command directly).

This one appears to be a straightforward extension.

diff --git a/Src/exec.c b/Src/exec.c
index f248ca2..741c80e 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1317,8 +1317,13 @@ execlist(Estate state, int dont_change_job, int exiting)
 	    next = state->pc + WC_SUBLIST_SKIP(code);
 	    if (!oldnoerrexit)
 		noerrexit = !isend;
-	    if ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT) && isend)
-		this_noerrexit = 1;
+	    if (WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT) {
+		/* suppress errexit for "! this_command" */
+		if (isend)
+		    this_noerrexit = 1;
+		/* suppress errexit for ! <list-of-shell-commands> */
+		noerrexit = 1;
+	    }
 	    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 3a65b28..0faec02 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -556,6 +556,33 @@ F:Must be tested with a top-level script rather than source or function
 1:ERRETURN with "!" and a following false
 >before
 
+  fn() {
+      emulate -L zsh
+      setopt errreturn
+      print before
+      ! if true; then
+        false
+      fi
+      print after
+  }
+  fn
+0:ERRETURN with "!" suppressed inside complex structure
+>before
+>after
+
+  fn() {
+      emulate -L zsh
+      setopt errreturn
+      print before
+      if true; then
+        false
+      fi
+      print after
+  }
+  fn
+1:ERRETURN with no "!" suppression (control case)
+>before
+
 %clean
 
   rm -f TRAPEXIT

Re: 'set -e' with '!' POSIX issue

[Peter Stephenson]

On Wed, 5 Oct 2016 12:18:26 +0200
Martijn Dekker <martijn@inlv.org> wrote:
> On Tue, Mar 17, 2009 at 12:46:20PM +0100, Vincent Lefevre wrote:
> > POSIX shells (bash, dash, ksh93, pdksh, posh) return with no output
> > and an exit status equal to 1 on:
> >
> >   sh -c 'set -e; foo() { false && false; }; foo; echo $?'
> >
> > but zsh doesn't, even with "emulate sh":
> >
> > $ zsh -fc 'emulate sh; set -e; foo() { false && false; }; foo; echo $?'
> > 1

I think this is a bug, regardless of the standard.  The "&&" within the
function shouldn't have anything to do with what happens outside, and it
works without that.

The fix appears to be similar to the one for "!".

Please reassure me that this is correct:

% ./zsh -fc 'set -e; foo() { false && false; echo OK; }; foo; echo $?'
OK
0

We hit the first "false" in the &&, which doesn't trigger ERR_EXIT so
the second "false" that would is never executed; therefore we proceed to
"echo OK"; therefore the status of the function is 0.  If there's no
controversy I may add this as a test case for future reference, together
with the "true & false" case that causes the function to be aborted.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 741c80e..c0ed2c4 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1229,7 +1229,7 @@ execlist(Estate state, int dont_change_job, int exiting)
     }
     while (wc_code(code) == WC_LIST && !breaks && !retflag && !errflag) {
 	int donedebug;
-	int this_noerrexit = 0;
+	int this_noerrexit = 0, this_donetrap = 0;
 
 	ltype = WC_LIST_TYPE(code);
 	csp = cmdsp;
@@ -1353,10 +1353,10 @@ execlist(Estate state, int dont_change_job, int exiting)
 			/* We've skipped to the end of the list, not executing *
 			 * the final pipeline, so don't perform error handling *
 			 * for this sublist.                                   */
-			donetrap = 1;
+			this_donetrap = 1;
 			goto sublist_done;
 		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END) {
-			donetrap = 1;
+			this_donetrap = 1;
 			/*
 			 * Treat this in the same way as if we reached
 			 * the end of the sublist normally.
@@ -1386,10 +1386,10 @@ execlist(Estate state, int dont_change_job, int exiting)
 			/* We've skipped to the end of the list, not executing *
 			 * the final pipeline, so don't perform error handling *
 			 * for this sublist.                                   */
-			donetrap = 1;
+			this_donetrap = 1;
 			goto sublist_done;
 		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END) {
-			donetrap = 1;
+			this_donetrap = 1;
 			/*
 			 * Treat this in the same way as if we reached
 			 * the end of the sublist normally.
@@ -1439,7 +1439,7 @@ sublist_done:
 	/* Check whether we are suppressing traps/errexit *
 	 * (typically in init scripts) and if we haven't  *
 	 * already performed them for this sublist.       */
-	if (!noerrexit && !this_noerrexit && !donetrap) {
+	if (!noerrexit && !this_noerrexit && !donetrap && !this_donetrap) {
 	    if (sigtrapped[SIGZERR] && lastval) {
 		dotrap(SIGZERR);
 		donetrap = 1;
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index 0faec02..5057dcf 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -476,7 +476,7 @@
     fi
   }
   fn
-0:ERRRETURN not triggered in if condition
+0:ERR_RETURN not triggered in if condition
 >Oh, yes
 
   fn() {
@@ -490,7 +490,7 @@
     fi
   }
   fn
-1:ERRRETURN in "if"
+1:ERR_RETURN in "if"
 
   fn() {
     emulate -L zsh
@@ -503,7 +503,7 @@
     fi
   }
   fn
-1:ERRRETURN in "else" branch (regression test)
+1:ERR_RETURN in "else" branch (regression test)
 
   $ZTST_testdir/../Src/zsh -f =(<<<"
   if false; then
@@ -515,7 +515,7 @@
     print Yes
   fi
   ")
-0:ERRRETURN when false "if" is the first statement in an "else" (regression)
+0:ERR_RETURN when false "if" is the first statement in an "else" (regression)
 >Yes
 F:Must be tested with a top-level script rather than source or function
 
@@ -527,7 +527,7 @@ F:Must be tested with a top-level script rather than source or function
       print after
   }
   fn
-1:ERRRETURN, basic case
+1:ERR_RETURN, basic case
 >before
 
   fn() {
@@ -539,7 +539,7 @@ F:Must be tested with a top-level script rather than source or function
       print after
   }
   fn
-0:ERRETURN with "!"
+0:ERR_RETURN with "!"
 >before
 >after
 
@@ -553,7 +553,7 @@ F:Must be tested with a top-level script rather than source or function
       print after
   }
   fn
-1:ERRETURN with "!" and a following false
+1:ERR_RETURN with "!" and a following false
 >before
 
   fn() {
@@ -566,7 +566,7 @@ F:Must be tested with a top-level script rather than source or function
       print after
   }
   fn
-0:ERRETURN with "!" suppressed inside complex structure
+0:ERR_RETURN with "!" suppressed inside complex structure
 >before
 >after
 
@@ -580,9 +580,22 @@ F:Must be tested with a top-level script rather than source or function
       print after
   }
   fn
-1:ERRETURN with no "!" suppression (control case)
+1:ERR_RETURN with no "!" suppression (control case)
 >before
 
+  (setopt err_return
+    fn() {
+      print before-in
+      false && false
+    }
+    print before-out
+    fn
+    print after-out
+  )
+1:ERR_RETURN with "&&" in function (regression test)
+>before-out
+>before-in
+
 %clean
 
   rm -f TRAPEXIT

Re: 'set -e' with '!' POSIX issue

[Peter Stephenson]

On Wed, 5 Oct 2016 14:47:03 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> Please reassure me that this is correct:
> 
> % ./zsh -fc 'set -e; foo() { false && false; echo OK; }; foo; echo $?'
> OK
> 0
> 
> We hit the first "false" in the &&, which doesn't trigger ERR_EXIT so
> the second "false" that would is never executed; therefore we proceed to
> "echo OK"; therefore the status of the function is 0.  If there's no
> controversy I may add this as a test case for future reference, together
> with the "true & false" case that causes the function to be aborted.

Here are corresponding tests.  Nothing fundamentally new compared with
previous tests, it's just a bit more complicated.

pws

diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index 5057dcf..74b83f3 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -596,6 +596,36 @@ F:Must be tested with a top-level script rather than source or function
 >before-out
 >before-in
 
+  (setopt err_return
+    fn() {
+      print before-in
+      false && false
+      print after-in
+    }
+    print before-out
+    fn
+    print after-out
+  )
+0:ERR_RETURN not triggered on LHS of "&&" in function
+>before-out
+>before-in
+>after-in
+>after-out
+
+  (setopt err_return
+    fn() {
+      print before-in
+      true && false
+      print after-in
+    }
+    print before-out
+    fn
+    print after-out
+  )
+1:ERR_RETURN triggered on RHS of "&&" in function
+>before-out
+>before-in
+
 %clean
 
   rm -f TRAPEXIT

Re: 'set -e' with '!' POSIX issue

[Martijn Dekker]

Op 05-10-16 om 15:47 schreef Peter Stephenson:
> Please reassure me that this is correct:
> 
> % ./zsh -fc 'set -e; foo() { false && false; echo OK; }; foo; echo $?'
> OK
> 0

Looks correct to me. It's consistent with the POSIX description and
bash, (d)ash, yash, ksh93, mksh/pdksh all behave the same.

- M.