Re: Crash Re: Trial for 5.0.7

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Thu, 02 Oct 2014 21:58:56 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Oct 2,  8:33pm, Bart Schaefer wrote:
> }
> } git-bisect says this is the bad revision:
> } 
> } commit 8189e12312ede991827efc6683b7ce8463deb0bf
> } 
> }     32552 (updated by 32560): fix segfault when using process substitution in
> } anonymous function argument list
> }     
> }     Also disallow process substitution in function name position.
> 
> OK, I've narrowed it down to the parse.c hunks of the patch.  Remove
> just that change, and the crash on piping to an anonymous function is
> fixed, but this test fails:
> 
> ./C04funcdef.ztst: starting.
> Test ./C04funcdef.ztst failed: bad status 1, expected 0 from:
>   () (cat $1 $2) <(print process expanded) =(print expanded to file)
> Error output:
> (eval):1: process substitution <(print process expanded) cannot be used here
> Was testing: Process substitution with anonymous functions
> ./C04funcdef.ztst: test failed.
> 
> Here's the patch to revert workers/32552 (parse.c).  I haven't figured
> out why fiddling with *complex here mangles the wordcode, some help is
> appreciated.  Obviously the above test needs to pass.

(I presume that *is* an anonymous function, I'd forgotten that
functions could be defined with parentheses for the bodies.  Presumably
using braces makes no difference, though?)

The only effect of setting *complex should be instead of going through
execsimple(), which is used for ultra trivial cases, the execution of
the parsed shell code goes through the main path starting with
execlist() and ending up in execcmd() where the expansions are done.

What's simple enough not to need the complex flag isn't written down
anywhere; it's just controlled by whatever execsimple() can actually
cope with.  I believe anything involving a pipeline has to be complex,
so that may be what you're running into in the crash you originally
had.  So restoring a complex flag for that case is probably the right
thing to do (but I'm saying all this without having actually looked).

The error message you're now getting ("process substitution cannot be
used here") comes when you go down the execsimple() path, which doesn't
have all the infrastructure for jobs.  Prima facie your change actually
makes it more likely things are treated as complex.  Somehow that
failing code is being marked as simple with the change (reversion of
parse.c), though.  I think that's the thing to focus on.

Part of the problem here may be that functions (including anonymous
functions) make things more complicated because we have two sets of
commands that may or may not be simple: the command line, and the code
from the function.  The arguments to an anoymous function are a
particularly unusual case, and it's probably here the issues with
complex are arriving, i.e. the command line rather than the function
contents.  In other words, the issue above is to do with whether
complexities in the arguments of the anonymous function correctly cause
the complex flag to be set.

I suspect I could have put that more simply at half the length.

pws