Insidious exit status bugs

Insidious exit status bugs

[Bart Schaefer]

The following really messed me up, trying to write a script that will work
whether sh is bash or zsh.  Here's zsh 3.0.5 (3.1.4 is the same):

zagzig% echo yyy | fgrep -q xxx && echo ok
zagzig% echo yyy | fgrep -q `echo xxx` && echo ok
ok

It appears that the exit status of `echo xxx` is masking the exit status of
`fgrep -q`, but I'm not certain.  I had to do this workaround to get both
shells to behave the same:

xxx=`echo xxx`
echo yyy | fgrep -q $xxx && echo ok

Bash, of course, has an equally annoying bug:

( echo xxx | while read; do exit 0; done; exit 1; )
echo $?

Execute the above lines in bash and you'll see that $? = 1 whereas in zsh
the $? = 0.  Since I'd like to have the script exit nonzero only if the
read reaches end-of-file, I was forced into this sort of foolishness:

( echo xxx | while read; do exit 0; done; exit 1; )
ok=$?

compute some things |
while read some things
do
    [ "$some" != "$things" ] && continue
    exit $ok
done || exit 0
exit 1

In zsh, the "exit $ok" terminates the whole script with $? = 0.  In bash,
it only terminates the loop, triggering the || exit 0 clause.  In either
case, if the read gets EOF, the loop finishes sucessfully and the exit 1
then follows it.

Oof.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: Insidious exit status bugs

[Peter Stephenson]

"Bart Schaefer" wrote:
> The following really messed me up, trying to write a script that will work
> whether sh is bash or zsh.  Here's zsh 3.0.5 (3.1.4 is the same):
> 
> zagzig% echo yyy | fgrep -q xxx && echo ok
> zagzig% echo yyy | fgrep -q `echo xxx` && echo ok
> ok
> 
> It appears that the exit status of `echo xxx` is masking the exit status of
> `fgrep -q`, but I'm not certain.

Insidious is definitely right.  This is serious harrow-up-thy-soul
territory.  There appear to be two things happening.  The first I
think I understand, though not why it's there.  The second currently
has me stumped.

First: the exit status from the `...` or $(...) is being explicitly
passed back as lastval, i.e. $?.  Why???  There's a variable cmdoutval
whose only function in life is to remember that status to be assigned
to lastval, so someone thought it was a good idea.  Here's the
offending chunk from getoutput() in exec.c which is for the parent
side of the forked $(...).

    } else if (pid) {
	LinkList retval;

	zclose(pipes[1]);
	retval = readoutput(pipes[0], qt);
	fdtable[pipes[0]] = 0;
	child_suspend(0);		/* unblocks */
	lastval = cmdoutval;
	return retval;
    }

`lastval = cmdoutval' is deliberately messing up the return status for
reasons I can't even guess at.  Perhaps if the second bug wasn't
there, this wouldn't be seen, but it had me confused anyway.


Second:  I commented the cmdoutval stuff out, and now things start to
go really haywire.  It looks like you have to have a builtin in the
first part of the pipe, and an external command in the second part to
see this.  Bearing this in mind, I commented out the chunk in
execpline2() where `builtin as first command in pipe' is specially
handled, so as to simplify things, and added some messages; it still
goes wrong:

+11:58% echo yyy | fgrep -q $(echo xxx)
Got status 0 from process 36923               OK, that's the $(echo xxx)
...was procsub process.
Got status 0 from process 36410               This must be the echo yyy...
Setting status of 36410 to 0 (1)
Exit status 0 from 36410                      ...and this means that
                                                 it's being handled
                                                 as if it were last in
                                                 the pipeline.
setting lastval to 0 (1)
Got status 0 from process -1
...error: No child processes                  self explanatory, but...
+11:58% Got status 256 from process 36412     now we're back to the
                                              prompt, and 36412 (fgrep)
                                              has only just been handled.
Got status 256 from process -1
...error: No child processes


So it looks like the fgrep is being run, or at least waited for,
asynchronously instead of synchronously, and all because of the $(echo
xxx).  I've wasted too much time on this already, so that's it for
now.

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Gruppo Teorico, Dipartimento di Fisica
Piazza Torricelli 2, 56100 Pisa, Italy

Re: Insidious exit status bugs

[Zefram]

Peter Stephenson wrote:
>First: the exit status from the `...` or $(...) is being explicitly
>passed back as lastval, i.e. $?.  Why???  There's a variable cmdoutval
>whose only function in life is to remember that status to be assigned
>to lastval, so someone thought it was a good idea.

Assignments such as

	foo=$(grep -l foo *)

are supposed to set $? to the exit status of the $() command.  POSIX.2
clause 3.9.1 requires this.

-zefram

PATCH: 3.1.4: Insidious exit status bug

[Peter Stephenson]

Bart wrote:
> zagzig% echo yyy | fgrep -q `echo xxx` && echo ok
> ok

First:  getoutput():  OK, I'll stop worrying about a $(...) setting
lastval.  It's not necessary in the middle of an ordinary command, but
when the rest of the code is working it's harmless.

Second:  Phew.  Here's what was happening.

1) The `echo yyy' was added to the process list for the current job;
that job finished straight away since it was short.  (It had to fork
even for the builtin because of the pipeline.)

2) The shell handled the arguments for fgrep, called the `echo xxx',
and waited for it.  As `echo yyy' was already finished, it was
harvested, too.  As it was (at that point) the last job in the
pipeline, the job had the STAT_DONE flag set.

3) The fgrep was added to the job table, making a second process in
the pipeline, but the STAT_DONE flag was not unset.

4) The shell now waited for all jobs to finish.  However, in waitjobs(),
the current status is updated (via printjob()) to check whether
everything has already finished.  As the STAT_DONE flag was set on the
job, it was wiped.

5) The fgrep process was harvested anyway, but it didn't have a job
table entry corresponding to it any more, so the shell didn't know it
was supposed to use the return status for lastval/$?.

Benissimo.  Here's the simplest fix: turn off the STAT_DONE flag
explicitly when adding a new process.  As noted in the comment, it's
important that the shell doesn't try to check job statuses between the
wait() which harvested the first process, and the time the second
process is added, else the bug will reappear.  At the moment things
look OK.  (Maybe specific processes like $(...) should have a
waitpid() for their own process, rather than wreaking havoc on the job
table?)

For a more sophisticated fix:  add a flags field to struct process,
record there by an extra argument to addproc() whether the process is
the last in the pipeline, don't set STAT_DONE if that process isn't
yet in the procs list.  I'll do that if there's a preference.

(Quite likely applies to 3.0.5 too, could certainly be done by hand
without much effort.)

*** Src/jobs.c.cout	Thu Jul  9 12:04:42 1998
--- Src/jobs.c	Fri Sep 25 16:40:34 1998
***************
*** 658,663 ****
--- 658,670 ----
  	/* first process for this job */
  	jobtab[thisjob].procs = pn;
      }
+     /* If the first process in the job finished before any others were *
+      * added, maybe STAT_DONE got set incorrectly.  This can happen if *
+      * a $(...) was waited for and the last existing job in the        *
+      * pipeline was already finished.  We need to be very careful that *
+      * there was no call to printjob() between then and now, else      *
+      * the job will already have been deleted from the table.          */
+     jobtab[thisjob].stat &= ~STAT_DONE;
  }
  
  /* Check if we have files to delete.  We need to check this to see *

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Gruppo Teorico, Dipartimento di Fisica
Piazza Torricelli 2, 56100 Pisa, Italy

Re: PATCH: 3.1.4: Insidious exit status bug

[Bart Schaefer]

On Sep 25,  5:01pm, Peter Stephenson wrote:
} Subject: PATCH: 3.1.4: Insidious exit status bug
}
} Bart wrote:
} > zagzig% echo yyy | fgrep -q `echo xxx` && echo ok
} > ok
} 
} 2) The shell handled the arguments for fgrep, called the `echo xxx',
} and waited for it.  As `echo yyy' was already finished, it was
} harvested, too.  As it was (at that point) the last job in the
} pipeline, the job had the STAT_DONE flag set.

I'll bet this is exactly the same bug I described back in April:
    http://www.zsh.org/mla/workers-1998/msg00178.html
    http://www.zsh.org/mla/workers-1998/msg00180.html

The context (a builtin piped to an external with a $(...) argument) is
certainly exactly the same.

And indeed, with your patch applied to 3.0.5 (it applies fine with fuzz)
I'm unable to repeat the bug describe in msg00178.html.  In a version of
3.0.5 that does NOT have your patch, I can repeat the bug within five or
six tries.

Thanks for the patch!

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com