A repeating core, just sharing backtrace

A repeating core, just sharing backtrace

[Sebastian Gniazdowski]

Hello,
I've triggered loading a snippet with Zplugin, and core happened.
Tried again and it repeats. Zsh-5.5.1-dev-0, HEAD. Don't have energy
currently to track this, maybe someone has idea for a "economical"
debug print or other thing to reveal the cause?

frame #0: 0x00000001092f7e1f
zsh-5.5.1-dev-0`scanmatchtable(ht=<unavailable>,
pprog=0x0000000000000000, sorted=446721, flags1=0, flags2=0,
scanfunc=(zsh-5.5.1-dev-0`scanendscope at params.c:5570), scanflags=0)
at hashtable.c:424 [opt]
   421         for (i = 0; i < hsize; i++)
   422             for (st.u.u = nodes[i]; st.u.u; ) {
   423             HashNode hn = st.u.u;
-> 424             st.u.u = st.u.u->next;
   425             if ((!flags1 || (hn->flags & flags1)) &&
!(hn->flags & flags2)
   426                 && (!pprog || pattry(pprog, hn->nam))) {
   427                 match++;

-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin

Re: A repeating core, just sharing backtrace

[Sebastian Gniazdowski]

I bisected from 5.4.2 to HEAD. The core is fully repeatable. It's a
larger script (zplugin) operation that causes the core, so currently
this is kind of a black box.

f7519811e1bbe990ff1c3d499ffb70cfc2d034f8 is the first bad commit
commit f7519811e1bbe990ff1c3d499ffb70cfc2d034f8
Author: Ricardo Giorni <ricardo@giorni.co>
Date:   Sun Apr 29 12:05:39 2018 -0700

    47201: fix 42355 for multiple backslashes


I think this is a well pointed commit, because any backtrace I
occurred was going through zshlex:

    ...
    frame #5: 0x00007fff5766b256 libsystem_malloc.dylib`free_tiny + 628
    frame #6: 0x0000000100e076e8 zsh`zfree + 24
    frame #7: 0x0000000100dc4c3c zsh`gethere + 780
    frame #8: 0x0000000100dfafc1 zsh`zshlex + 369
    frame #9: 0x0000000100e26c6f zsh`par_redir + 655
    frame #10: 0x0000000100e29b5f zsh`par_simple + 2063
    ...


On 2 July 2018 at 12:09, Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> Hello,
> I've triggered loading a snippet with Zplugin, and core happened.
> Tried again and it repeats. Zsh-5.5.1-dev-0, HEAD. Don't have energy
> currently to track this, maybe someone has idea for a "economical"
> debug print or other thing to reveal the cause?
>
> frame #0: 0x00000001092f7e1f
> zsh-5.5.1-dev-0`scanmatchtable(ht=<unavailable>,
> pprog=0x0000000000000000, sorted=446721, flags1=0, flags2=0,
> scanfunc=(zsh-5.5.1-dev-0`scanendscope at params.c:5570), scanflags=0)
> at hashtable.c:424 [opt]
>    421         for (i = 0; i < hsize; i++)
>    422             for (st.u.u = nodes[i]; st.u.u; ) {
>    423             HashNode hn = st.u.u;
> -> 424             st.u.u = st.u.u->next;
>    425             if ((!flags1 || (hn->flags & flags1)) &&
> !(hn->flags & flags2)
>    426                 && (!pprog || pattry(pprog, hn->nam))) {
>    427                 match++;
>
> --
> Sebastian Gniazdowski
> News: https://twitter.com/ZdharmaI
> IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin



-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin

Re: A repeating core, just sharing backtrace

[Peter Stephenson]

On Thu, 5 Jul 2018 15:40:30 +0200
Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> I bisected from 5.4.2 to HEAD. The core is fully repeatable. It's a
> larger script (zplugin) operation that causes the core, so currently
> this is kind of a black box.
> 
> f7519811e1bbe990ff1c3d499ffb70cfc2d034f8 is the first bad commit
> commit f7519811e1bbe990ff1c3d499ffb70cfc2d034f8
> Author: Ricardo Giorni <ricardo@giorni.co>
> Date:   Sun Apr 29 12:05:39 2018 -0700
> 
>     47201: fix 42355 for multiple backslashes
> 
> 
> I think this is a well pointed commit, because any backtrace I
> occurred was going through zshlex:
> 
>     ...
>     frame #5: 0x00007fff5766b256 libsystem_malloc.dylib`free_tiny +
> 628 frame #6: 0x0000000100e076e8 zsh`zfree + 24
>     frame #7: 0x0000000100dc4c3c zsh`gethere + 780
>     frame #8: 0x0000000100dfafc1 zsh`zshlex + 369
>     frame #9: 0x0000000100e26c6f zsh`par_redir + 655
>     frame #10: 0x0000000100e29b5f zsh`par_simple + 2063
>     ...

The zfree() in that backtrace is on a locally allocated buffer, so this
looks like earlier memory corruption, which would fit.

The change in question is quite small but does cause bptr in that for
loop to be incremented possibly twice per loop.  So this change is
probably needed whether or not it fixes the bug.

I hope the pointer arithmetic here is also a bit more transparent[ly
correct].

pws

diff --git a/Src/exec.c b/Src/exec.c
index 5864020..47a4567 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4418,7 +4418,9 @@ gethere(char **strp, int typ)
 	while ((c = hgetc()) == '\t' && strip)
 	    ;
 	for (;;) {
-	    if (bptr == buf + bsiz) {
+	    if (bptr >= buf + bsiz - 1) {
+		ptrdiff_t toff = t - buf;
+		ptrdiff_t bptroff = bptr - buf;
 		char *newbuf = realloc(buf, 2 * bsiz);
 		if (!newbuf) {
 		    /* out of memory */
@@ -4426,8 +4428,8 @@ gethere(char **strp, int typ)
 		    return NULL;
 		}
 		buf = newbuf;
-		t = buf + bsiz - (bptr - t);
-		bptr = buf + bsiz;
+		t = buf + toff;
+		bptr = buf + bptroff;
 		bsiz *= 2;
 	    }
 	    if (lexstop || c == '\n')

Re: A repeating core, just sharing backtrace

[Peter Stephenson]

On Tue, 10 Jul 2018 14:33:19 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> diff --git a/Src/exec.c b/Src/exec.c
> index 5864020..47a4567 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -4418,7 +4418,9 @@ gethere(char **strp, int typ)
>  	while ((c = hgetc()) == '\t' && strip)
>  	    ;
>  	for (;;) {
> -	    if (bptr == buf + bsiz) {
> +	    if (bptr >= buf + bsiz - 1) {

... make that "-2", because we need to allow for two *more* updates.
The previous logic just allowed for where we'd already got to, because
it just needed to check one position.  That doesn't work any more.

pws

Re: A repeating core, just sharing backtrace

[Sebastian Gniazdowski]

On 10 July 2018 at 15:38, Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Tue, 10 Jul 2018 14:33:19 +0100
> Peter Stephenson <p.stephenson@samsung.com> wrote:
>> diff --git a/Src/exec.c b/Src/exec.c
>> index 5864020..47a4567 100644
>> --- a/Src/exec.c
>> +++ b/Src/exec.c
>> @@ -4418,7 +4418,9 @@ gethere(char **strp, int typ)
>>       while ((c = hgetc()) == '\t' && strip)
>>           ;
>>       for (;;) {
>> -         if (bptr == buf + bsiz) {
>> +         if (bptr >= buf + bsiz - 1) {
>
> ... make that "-2", because we need to allow for two *more* updates.
> The previous logic just allowed for where we'd already got to, because
> it just needed to check one position.  That doesn't work any more.

I've tested if core still repeats (who knows), then I've updated and
recompiled zsh, then tested 3 times the script invocation that caused
segfault, and there was no such problem anymore. I've also checked if
tests pass.

-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin