Regression / err_return / arithmetic 5.4.x ?

Regression / err_return / arithmetic 5.4.x ?

[Phil Pennock]

Folks,

A function I use has stopped working on the box with zsh 5.4.2 installed
but still works on 5.3.1.  It looks as though the impact of a false
arithmetic return in the control for a while loop is causing early
function return instead of being considered "handled" by being part of
the control structure.

The function is called "arin" and is one of a bunch of
autoloads-via-symlink which reference a common "xwhois" function.

--------------------------8< whence -f arin >8--------------------------
arin () {
	setopt local_options warn_create_global err_return no_unset
#...
	local -i loop=1
	while ((loop--))
	do
		case $cmd in
#...
			(arin|arinwhois) registry=whois.arin.net  ;;
		esac
	done
	$whois -h $registry -- "${override_args:-}$*" | less -FXM
}
--------------------------8< whence -f arin >8--------------------------

---------------------------8< fail: 5.4.2 >8----------------------------
% whence -v ztrace{
ztrace{ is an alias for () { setopt localoptions xtrace;
% ztrace{ arin 192.0.2.1 }
+(anon):0> arin 192.0.2.1
+arin:4> setopt local_options warn_create_global err_return no_unset
+arin:6> local cmd=arin
+arin:7> local whois=''
+arin:8> local registry
+arin:9> local override_args
+arin:10> local f
+arin:11> f=whois
+arin:13> [[ -x /usr/bin/whois ]]
+arin:14> whois=/usr/bin/whois
+arin:15> break
+arin:19> [[ -z /usr/bin/whois ]]
+arin:24> local -i loop=1
+arin:25> (( loop-- ))
+arin:27> case arin (xwhois)
+arin:27> case arin (ripe|ripewhois)
+arin:27> case arin (testripe|testripewhois)
+arin:27> case arin (org|orgwhois)
+arin:27> case arin (core|corewhois)
+arin:27> case arin (arin|arinwhois)
+arin:46> registry=whois.arin.net
+arin:25> (( loop-- ))
---------------------------8< fail: 5.4.2 >8----------------------------

Return status from that function is 1.

On 5.3.1, the same invocation adds:

-----------------------8< success extra: 5.3.1 >8-----------------------
+arin:64> /usr/bin/whois -h whois.arin.net -- 121.42.217.44
+arin:64> less -FXM
-----------------------8< success extra: 5.3.1 >8-----------------------

I can remove the err_return safety check, but this looks like a
regression, so I'm reporting it.

5.3.1% zsh -fc 'setopt err_return;(){ local -i l=3; while ((l--)) { echo ": $l" }; echo done }; echo $?'
: 2
: 1
: 0
done
0
5.3.1%


5.4.2% % zsh -fc 'setopt err_return;(){ local -i l=3; while ((l--)) { echo ": $l" }; echo done }; echo $?'
: 2
: 1
: 0
1
5.4.2%

Re: Regression / err_return / arithmetic 5.4.x ?

[Bart Schaefer]

On Sep 15,  4:13pm, Phil Pennock wrote:
}
} It looks as though the impact of a false arithmetic return in the
} control for a while loop is causing early function return instead of
} being considered "handled" by being part of the control structure.

Almost has to be one of commit ebcea98e or 97d4bdb.

Re: Regression / err_return / arithmetic 5.4.x ?

[Bart Schaefer]

On Sep 15,  2:55pm, Bart Schaefer wrote:
} Subject: Re: Regression / err_return / arithmetic 5.4.x ?
}
} On Sep 15,  4:13pm, Phil Pennock wrote:
} }
} } It looks as though the impact of a false arithmetic return in the
} } control for a while loop is causing early function return
} 
} Almost has to be one of commit ebcea98e or 97d4bdb.

I reverted 97d4bdb and the problem disappears.  Haven't gotten any
further than that, yet.

Re: Regression / err_return / arithmetic 5.4.x ?

[Bart Schaefer]

On Sep 15,  7:38pm, Bart Schaefer wrote:
}
} I reverted 97d4bdb and the problem disappears.  Haven't gotten any
} further than that, yet.

Looks like just one case where use of the new bitflags was missed:

diff --git a/Src/loop.c b/Src/loop.c
index 40e3bcb..1013aeb 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -428,7 +428,7 @@ execwhile(Estate state, UNUSED(int do_exec))
     } else
         for (;;) {
             state->pc = loop;
-            noerrexit = 1;
+            noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
 
 	    /* In case the test condition is a functional no-op,
 	     * make sure signal handlers recognize ^C to end the loop. */

Re: Regression / err_return / arithmetic 5.4.x ?

[Roman Neuhauser]

# schaefer@brasslantern.com / 2017-09-17 17:50:28 -0700:
> On Sep 15,  7:38pm, Bart Schaefer wrote:
> }
> } I reverted 97d4bdb and the problem disappears.  Haven't gotten any
> } further than that, yet.
> 
> Looks like just one case where use of the new bitflags was missed:
> 
> diff --git a/Src/loop.c b/Src/loop.c
> index 40e3bcb..1013aeb 100644
> --- a/Src/loop.c
> +++ b/Src/loop.c
> @@ -428,7 +428,7 @@ execwhile(Estate state, UNUSED(int do_exec))
>      } else
>          for (;;) {
>              state->pc = loop;
> -            noerrexit = 1;
> +            noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
>  
>  	    /* In case the test condition is a functional no-op,
>  	     * make sure signal handlers recognize ^C to end the loop. */

which zsh release has this fix?  and does it cover just arithmetic
expansion or does it fix while conditions in general?  are there any
tests for errreturn?  i'm asking because i see this with 5.4.2:

  $ cat > errreturn.zsh <<\EOF
  > function
  > {
  >   while false; do
  >     :
  >   done
  >   echo you should see this
  > }
  > echo this should come second
  > EOF

  $ zsh -fo noerrreturn errreturn.zsh
  > you should see this
  > this should come second

  $ zsh -fo errreturn errreturn.zsh
  > this should come second

-- 
roman

Re: Regression / err_return / arithmetic 5.4.x ?

[Peter Stephenson]

On Wed, 6 Dec 2017 18:10:58 +0100
Roman Neuhauser <neuhauser@sigpipe.cz> wrote:
> which zsh release has this fix?  and does it cover just arithmetic
> expansion or does it fix while conditions in general?  are there any
> tests for errreturn?

There's no release with it in but you can check the current archive for
remaining problems --- several aspects did get fixed.

We should make a release soon (Daniel has volunteered).

pws

Re: Regression / err_return / arithmetic 5.4.x ?

[Bart Schaefer]

I've posted at least one other diff that hasn't been pushed to git yet
because my usual "zsh work machine" has been down for a couple of
weeks.


On Wed, Dec 6, 2017 at 9:35 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Wed, 6 Dec 2017 18:10:58 +0100
> Roman Neuhauser <neuhauser@sigpipe.cz> wrote:
>> which zsh release has this fix?  and does it cover just arithmetic
>> expansion or does it fix while conditions in general?  are there any
>> tests for errreturn?
>
> There's no release with it in but you can check the current archive for
> remaining problems --- several aspects did get fixed.
>
> We should make a release soon (Daniel has volunteered).
>
> pws

Re: Regression / err_return / arithmetic 5.4.x ?

[Daniel Shahaf]

Bart Schaefer wrote on Wed, Dec 06, 2017 at 11:39:02 -0800:
> On Wed, Dec 6, 2017 at 9:35 AM, Peter Stephenson
> <p.stephenson@samsung.com> wrote:
> > On Wed, 6 Dec 2017 18:10:58 +0100
> > Roman Neuhauser <neuhauser@sigpipe.cz> wrote:
> >> which zsh release has this fix?  and does it cover just arithmetic
> >> expansion or does it fix while conditions in general?  are there any
> >> tests for errreturn?
> >
> > There's no release with it in but you can check the current archive for
> > remaining problems --- several aspects did get fixed.
> >
> > We should make a release soon (Daniel has volunteered).
>
> I've posted at least one other diff that hasn't been pushed to git yet
> because my usual "zsh work machine" has been down for a couple of
> weeks.

Likewise, I've been swamped and haven't found time to finish the 'typeset -m
-p1' patch (workers/41942).  In fact, I meant to write to -workers@ asking if
someone wanted to pick it up and finish it, since I didn't know when I'd have
time to do that.