Crash Re: Trial for 5.0.7

Crash Re: Trial for 5.0.7

[Bart Schaefer]

On Oct 2,  8:40pm, Peter Stephenson wrote:
} Subject: Trial for 5.0.7
}
} I'm going to release zsh 5.0.7 shortly and I've uploaded a trial
} version, 5.0.6-dev-1. to http://www.zsh.org/pub/development/ if anyone
} happens to get a chance to give it a quick once over.

I've got a core dump:

    print foo | () { cat }

#0  0x00000000 in ?? ()
#1  0x0806283d in execsimple (state=0xbfe41060)
    at ../../zsh-5.0/Src/exec.c:1125
#2  0x08062b3c in execlist (state=0xbfe41060, dont_change_job=0, exiting=0)
    at ../../zsh-5.0/Src/exec.c:1238
#3  0x0806269e in execode (p=0xb7d227a8, dont_change_job=0, exiting=0, 
    context=0x813e613 "toplevel") at ../../zsh-5.0/Src/exec.c:1073
#4  0x0807da72 in loop (toplevel=1, justonce=0) at ../../zsh-5.0/Src/init.c:185
#5  0x08080dc7 in zsh_main (argc=2, argv=0xbfe411b4)
    at ../../zsh-5.0/Src/init.c:1638
#6  0x0804c0a6 in main (argc=2, argv=0xbfe411b4) at ../../zsh-5.0/Src/main.c:93


Happens in zsh-5.0.6-126-g0738a7c so it pre-dates the redirect patches
for function definitions.

Re: Crash Re: Trial for 5.0.7

[Bart Schaefer]

git-bisect says this is the bad revision:

commit 8189e12312ede991827efc6683b7ce8463deb0bf
Author: Andrew Waldron <a.a.w@gmx.us>
Date:   Fri Apr 18 07:30:36 2014 -0700

    32552 (updated by 32560): fix segfault when using process substitution in
anonymous function argument list
    
    Also disallow process substitution in function name position.

Re: Crash Re: Trial for 5.0.7

[Bart Schaefer]

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.

diff --git a/Src/parse.c b/Src/parse.c
index 6cba050..a3fa83a 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1486,6 +1486,7 @@ par_funcdef(int *complex)
 	if (num == 0) {
 	    /* Anonymous function, possibly with arguments */
 	    incmdpos = 0;
+	    *complex = 1;
 	}
 	zshlex();
     } else if (unset(SHORTLOOPS)) {
@@ -1517,7 +1518,6 @@ par_funcdef(int *complex)
 	    num++;
 	    zshlex();
 	}
-	*complex = (num > 0);
 	ecbuf[parg] = ecused - parg; /*?*/
 	ecbuf[parg+1] = num;
     }
@@ -1751,6 +1751,7 @@ par_simple(int *complex, int nr)
 		if (argc == 0) {
 		    /* Anonymous function, possibly with arguments */
 		    incmdpos = 0;
+		    *complex = 1;
 		}
 		zshlex();
 	    } else {
@@ -1790,7 +1791,6 @@ par_simple(int *complex, int nr)
 		    argc++;
 		    zshlex();
 		}
-		*complex = (argc > 0);
 		ecbuf[parg] = ecused - parg; /*?*/
 		ecbuf[parg+1] = argc;
 	    }

Re: Crash Re: Trial for 5.0.7

[Peter Stephenson]

On Thu, 02 Oct 2014 21:58:56 -0700
Bart Schaefer <schaefer@brasslantern.com> 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