Segfault with zsh 5.2

Segfault with zsh 5.2

[Christian Neukirchen]

zsh 5.2 (x86_64-unknown-linux-gnu)
zsh-5.2-0-gc86c20a

I use the following lines in my .zshrc:

if [[ $1 == eval ]]; then
  shift
  ICMD="$@"
  set --
  zle-line-init() {
    BUFFER="$ICMD"
    zle accept-line
    zle -D zle-line-init
  }
  zle -N zle-line-init
fi

Then I can run `zsh -is eval sleep 0 0 0 0` and it will run the
command and keep the shell open.  I run this code fine since 2013-01-30.
Now, this results sometimes in the following segfault with 5.2:

Core was generated by `zsh -is eval sleep 0 0 0 0'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  malloc_consolidate (av=av@entry=0x7fd1fe56ac40 <main_arena>)
    at malloc.c:4161
4161	malloc.c: No such file or directory.
(gdb) bt
#0  malloc_consolidate (av=av@entry=0x7fd1fe56ac40 <main_arena>)
    at malloc.c:4161
#1  0x00007fd1fe2446ab in _int_malloc (
    av=av@entry=0x7fd1fe56ac40 <main_arena>, bytes=bytes@entry=1024)
    at malloc.c:3444
#2  0x00007fd1fe2462c8 in __GI___libc_malloc (bytes=1024) at malloc.c:2907
#3  0x0000557079a38c7c in zalloc (size=<optimized out>, size@entry=1024)
    at mem.c:956
#4  0x0000557079a4f730 in init_parse () at parse.c:463
#5  0x0000557079a4fb74 in parse_event (endtok=endtok@entry=37) at parse.c:562
#6  0x0000557079a22c19 in loop (toplevel=toplevel@entry=1, 
    justonce=justonce@entry=0) at init.c:146
#7  0x0000557079a26754 in zsh_main (argc=<optimized out>, argv=<optimized out>)
    at init.c:1680
#8  0x00007fd1fe1eb680 in __libc_start_main (main=0x5570799f0790 <main>, 
    argc=8, argv=0x7ffd47aa62d8, init=<optimized out>, fini=<optimized out>, 
    rtld_fini=<optimized out>, stack_end=0x7ffd47aa62c8) at libc-start.c:289
#9  0x00005570799f07c9 in _start () at ../sysdeps/x86_64/start.S:108

(Several arguments need to be passed to trigger it more often.)

Thanks,
-- 
Christian Neukirchen  <chneukirchen@gmail.com>  http://chneukirchen.org

Re: Segfault with zsh 5.2

[Peter Stephenson]

On Mon, 7 Dec 2015 14:01:57 +0100
Christian Neukirchen <chneukirchen@gmail.com> wrote:
>   zle-line-init() {
>     BUFFER="$ICMD"
>     zle accept-line
>     zle -D zle-line-init
>   }

I couldn't get your crash to happen easily, and the crash
actually happened in a normal alloc high up in the execution tree so
doesn't give us much direct help apart from pointing at memory
management.  (The call was protected by signal queueing, by the way.)

However, there's definitely something very dodgy in memory management
for the code above.  It's always been this way, so I think the fact it's
just shown up is an accident.  I couldn't get valgrind to show it up,
for some reason, but the evidence from gdb is incontrovertible.

Does the following help?  Unless I've screwed up it needs committing
anyway.

(I hope the widget flags aren't so heavily overloaded now as to make
this unsafe.)

pws

diff --git a/Src/Zle/zle.h b/Src/Zle/zle.h
index 2d672de..e9b1428 100644
--- a/Src/Zle/zle.h
+++ b/Src/Zle/zle.h
@@ -213,6 +213,8 @@ struct widget {
 #define ZLE_KEEPSUFFIX	(1<<9)	/* DON'T remove added suffix */
 #define ZLE_NOTCOMMAND  (1<<10)	/* widget should not alter lastcmd */
 #define ZLE_ISCOMP      (1<<11)	/* usable for new style completion */
+#define WIDGET_INUSE    (1<<12) /* widget is in use */
+#define WIDGET_FREE     (1<<13) /* request to free when no longer in use */
 
 /* thingies */
 
diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index 38427e8..1f0c07d 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -1344,6 +1344,8 @@ execzlefunc(Thingy func, char **args, int set_bindk)
 	    eofsent = 1;
 	    ret = 1;
 	} else {
+	    int inuse = wflags & WIDGET_INUSE;
+	    w->flags |= WIDGET_INUSE;
 	    if(!(wflags & ZLE_KEEPSUFFIX))
 		removesuffix();
 	    if(!(wflags & ZLE_MENUCMP)) {
@@ -1367,6 +1369,12 @@ execzlefunc(Thingy func, char **args, int set_bindk)
 		ret = w->u.fn(args);
 		unqueue_signals();
 	    }
+	    if (!inuse) {
+		if (w->flags & WIDGET_FREE)
+		    freewidget(w);
+		else
+		    w->flags &= ~WIDGET_INUSE;
+	    }
 	    if (!(wflags & ZLE_NOTCOMMAND))
 		lastcmd = wflags;
 	}
@@ -1387,6 +1395,8 @@ execzlefunc(Thingy func, char **args, int set_bindk)
 	    int osc = sfcontext, osi = movefd(0);
 	    int oxt = isset(XTRACE);
 	    LinkList largs = NULL;
+	    int inuse = w->flags & WIDGET_INUSE;
+	    w->flags |= WIDGET_INUSE;
 
 	    if (*args) {
 		largs = newlinklist();
@@ -1402,8 +1412,15 @@ execzlefunc(Thingy func, char **args, int set_bindk)
 	    opts[XTRACE] = oxt;
 	    sfcontext = osc;
 	    endparamscope();
-	    lastcmd = w->flags;
-	    w->flags = 0;
+	    lastcmd = w->flags & ~(WIDGET_INUSE|WIDGET_FREE);
+	    if (inuse) {
+		w->flags &= WIDGET_INUSE|WIDGET_FREE;
+	    } else {
+		if (w->flags & WIDGET_FREE)
+		    freewidget(w);
+		else
+		    w->flags = 0;
+	    }
 	    r = 1;
 	    redup(osi, 0);
 	}
diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c
index 271fd8e..21495b6 100644
--- a/Src/Zle/zle_thingy.c
+++ b/Src/Zle/zle_thingy.c
@@ -253,9 +253,14 @@ unbindwidget(Thingy t, int override)
 /* Free a widget. */
 
 /**/
-static void
+void
 freewidget(Widget w)
 {
+    if (w->flags & WIDGET_INUSE) {
+	w->flags |= WIDGET_FREE;
+	return;
+    }
+
     if (w->flags & WIDGET_NCOMP) {
 	zsfree(w->u.comp.wid);
 	zsfree(w->u.comp.func);

Re: Segfault with zsh 5.2

[Christian Neukirchen]

Peter Stephenson <p.stephenson@samsung.com> writes:

> On Mon, 7 Dec 2015 14:01:57 +0100
> Christian Neukirchen <chneukirchen@gmail.com> wrote:
>>   zle-line-init() {
>>     BUFFER="$ICMD"
>>     zle accept-line
>>     zle -D zle-line-init
>>   }
>
> I couldn't get your crash to happen easily, and the crash
> actually happened in a normal alloc high up in the execution tree so
> doesn't give us much direct help apart from pointing at memory
> management.  (The call was protected by signal queueing, by the way.)
>
> However, there's definitely something very dodgy in memory management
> for the code above.  It's always been this way, so I think the fact it's
> just shown up is an accident.  I couldn't get valgrind to show it up,
> for some reason, but the evidence from gdb is incontrovertible.

I have one valgrid run, I shall test your patch soon:

juno ~% valgrind zsh -is eval sleep 0 0 0 0        
==1389== Memcheck, a memory error detector
==1389== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==1389== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==1389== Command: zsh -is eval sleep 0 0 0 0
==1389== 
==1397== 
==1397== HEAP SUMMARY:
==1397==     in use at exit: 527,943 bytes in 10,860 blocks
==1397==   total heap usage: 16,675 allocs, 5,815 frees, 3,817,997 bytes allocated
==1397== 
==1397== LEAK SUMMARY:
==1397==    definitely lost: 0 bytes in 0 blocks
==1397==    indirectly lost: 0 bytes in 0 blocks
==1397==      possibly lost: 0 bytes in 0 blocks
==1397==    still reachable: 527,943 bytes in 10,860 blocks
==1397==         suppressed: 0 bytes in 0 blocks
==1397== Rerun with --leak-check=full to see details of leaked memory
==1397== 
==1397== For counts of detected and suppressed errors, rerun with: -v
==1397== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==1407==                                                                        
==1407== HEAP SUMMARY:
==1407==     in use at exit: 1,891,117 bytes in 47,441 blocks
==1407==   total heap usage: 56,449 allocs, 9,008 frees, 6,313,464 bytes allocated
==1407== 
==1407== LEAK SUMMARY:
==1407==    definitely lost: 0 bytes in 0 blocks
==1407==    indirectly lost: 0 bytes in 0 blocks
==1407==      possibly lost: 0 bytes in 0 blocks
==1407==    still reachable: 1,891,117 bytes in 47,441 blocks
==1407==         suppressed: 0 bytes in 0 blocks
==1407== Rerun with --leak-check=full to see details of leaked memory
==1407== 
==1407== For counts of detected and suppressed errors, rerun with: -v
==1407== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==1389== Invalid read of size 4
==1389==    at 0x6187FD9: execzlefunc (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x61A0C20: zlecallhook (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x6189097: zleread (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x1550A9: zleentry (in /usr/bin/zsh)
==1389==    by 0x156648: ingetc.part.1 (in /usr/bin/zsh)
==1389==    by 0x14E22C: ihgetc (in /usr/bin/zsh)
==1389==    by 0x16039E: zshlex.part.1 (in /usr/bin/zsh)
==1389==    by 0x17EB6E: parse_event (in /usr/bin/zsh)
==1389==    by 0x151C18: loop (in /usr/bin/zsh)
==1389==    by 0x155753: zsh_main (in /usr/bin/zsh)
==1389==    by 0x57D167F: (below main) (libc-start.c:289)
==1389==  Address 0x6edca70 is 0 bytes inside a block of size 40 free'd
==1389==    at 0x4C2AE10: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1389==    by 0x6196D67: unbindwidget (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x6196DB4: bin_zle_del (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x12CB14: execbuiltin (in /usr/bin/zsh)
==1389==    by 0x13ADEC: execcmd (in /usr/bin/zsh)
==1389==    by 0x13B94D: execpline2 (in /usr/bin/zsh)
==1389==    by 0x13BD3A: execpline (in /usr/bin/zsh)
==1389==    by 0x13D5F8: execlist (in /usr/bin/zsh)
==1389==    by 0x13D97C: execode (in /usr/bin/zsh)
==1389==    by 0x13E45A: runshfunc (in /usr/bin/zsh)
==1389==    by 0x13EDCF: doshfunc (in /usr/bin/zsh)
==1389==    by 0x6187FB1: execzlefunc (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==  Block was alloc'd at
==1389==    at 0x4C29BA0: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1389==    by 0x167C7B: zalloc (in /usr/bin/zsh)
==1389==    by 0x6197413: bin_zle_new (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x12CB14: execbuiltin (in /usr/bin/zsh)
==1389==    by 0x13ADEC: execcmd (in /usr/bin/zsh)
==1389==    by 0x13B94D: execpline2 (in /usr/bin/zsh)
==1389==    by 0x13BD3A: execpline (in /usr/bin/zsh)
==1389==    by 0x13D5F8: execlist (in /usr/bin/zsh)
==1389==    by 0x162AB1: execif (in /usr/bin/zsh)
==1389==    by 0x139E5C: execcmd (in /usr/bin/zsh)
==1389==    by 0x13B94D: execpline2 (in /usr/bin/zsh)
==1389==    by 0x13BD3A: execpline (in /usr/bin/zsh)
==1389== 
==1389== Invalid write of size 4
==1389==    at 0x6187FEA: execzlefunc (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x61A0C20: zlecallhook (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x6189097: zleread (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x1550A9: zleentry (in /usr/bin/zsh)
==1389==    by 0x156648: ingetc.part.1 (in /usr/bin/zsh)
==1389==    by 0x14E22C: ihgetc (in /usr/bin/zsh)
==1389==    by 0x16039E: zshlex.part.1 (in /usr/bin/zsh)
==1389==    by 0x17EB6E: parse_event (in /usr/bin/zsh)
==1389==    by 0x151C18: loop (in /usr/bin/zsh)
==1389==    by 0x155753: zsh_main (in /usr/bin/zsh)
==1389==    by 0x57D167F: (below main) (libc-start.c:289)
==1389==  Address 0x6edca70 is 0 bytes inside a block of size 40 free'd
==1389==    at 0x4C2AE10: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1389==    by 0x6196D67: unbindwidget (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x6196DB4: bin_zle_del (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x12CB14: execbuiltin (in /usr/bin/zsh)
==1389==    by 0x13ADEC: execcmd (in /usr/bin/zsh)
==1389==    by 0x13B94D: execpline2 (in /usr/bin/zsh)
==1389==    by 0x13BD3A: execpline (in /usr/bin/zsh)
==1389==    by 0x13D5F8: execlist (in /usr/bin/zsh)
==1389==    by 0x13D97C: execode (in /usr/bin/zsh)
==1389==    by 0x13E45A: runshfunc (in /usr/bin/zsh)
==1389==    by 0x13EDCF: doshfunc (in /usr/bin/zsh)
==1389==    by 0x6187FB1: execzlefunc (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==  Block was alloc'd at
==1389==    at 0x4C29BA0: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1389==    by 0x167C7B: zalloc (in /usr/bin/zsh)
==1389==    by 0x6197413: bin_zle_new (in /usr/lib/zsh/5.2/zsh/zle.so)
==1389==    by 0x12CB14: execbuiltin (in /usr/bin/zsh)
==1389==    by 0x13ADEC: execcmd (in /usr/bin/zsh)
==1389==    by 0x13B94D: execpline2 (in /usr/bin/zsh)
==1389==    by 0x13BD3A: execpline (in /usr/bin/zsh)
==1389==    by 0x13D5F8: execlist (in /usr/bin/zsh)
==1389==    by 0x162AB1: execif (in /usr/bin/zsh)
==1389==    by 0x139E5C: execcmd (in /usr/bin/zsh)
==1389==    by 0x13B94D: execpline2 (in /usr/bin/zsh)
==1389==    by 0x13BD3A: execpline (in /usr/bin/zsh)
==1389== 
sleep 0 0 0 0
==1410==                                                                        
==1410== HEAP SUMMARY:
==1410==     in use at exit: 1,895,679 bytes in 47,458 blocks
==1410==   total heap usage: 56,713 allocs, 9,255 frees, 6,340,861 bytes allocated
==1410== 
==1410== LEAK SUMMARY:
==1410==    definitely lost: 0 bytes in 0 blocks
==1410==    indirectly lost: 0 bytes in 0 blocks
==1410==      possibly lost: 0 bytes in 0 blocks
==1410==    still reachable: 1,895,679 bytes in 47,458 blocks
==1410==         suppressed: 0 bytes in 0 blocks
==1410== Rerun with --leak-check=full to see details of leaked memory
==1410== 
==1410== For counts of detected and suppressed errors, rerun with: -v
==1410== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)


-- 
Christian Neukirchen  <chneukirchen@gmail.com>  http://chneukirchen.org

Re: Segfault with zsh 5.2

[Peter Stephenson]

On Mon, 7 Dec 2015 15:31:32 +0100
Christian Neukirchen <chneukirchen@gmail.com> wrote:
> I have one valgrid run, I shall test your patch soon:

That certainly looks consistent with what I think I've fixed.

Change now pushed.

pws

Re: Segfault with zsh 5.2

[Christian Neukirchen]

Peter Stephenson <p.stephenson@samsung.com> writes:

> On Mon, 7 Dec 2015 15:31:32 +0100
> Christian Neukirchen <chneukirchen@gmail.com> wrote:
>> I have one valgrid run, I shall test your patch soon:
>
> That certainly looks consistent with what I think I've fixed.
>
> Change now pushed.

I could not reproduce it so far after applying the patch.

> pws

Thanks!
-- 
Christian Neukirchen  <chneukirchen@gmail.com>  http://chneukirchen.org

${path[@]} in sh mode [was: Segfault with zsh 5.2]

[Martijn Dekker]

Peter Stephenson schreef op 07-12-15 om 14:36:
> Change now pushed.

Can the ${path[@]} array be disabled in sh mode? I just noticed that it
auto-syncs with the system $PATH. Lowercase "path" is a perfectly normal
variable name in other shells and is a common English word, so conflicts
that result in corruption of $PATH are not unlikely when running scripts
written using other shells.

Thanks,

- M.

Re: ${path[@]} in sh mode [was: Segfault with zsh 5.2]

[Martijn Dekker]

Martijn Dekker schreef op 22-01-16 om 20:12:
> Peter Stephenson schreef op 07-12-15 om 14:36:
>> Change now pushed.
> 
> Can the ${path[@]} array be disabled in sh mode? I just noticed that it
> auto-syncs with the system $PATH. Lowercase "path" is a perfectly normal
> variable name in other shells and is a common English word, so conflicts
> that result in corruption of $PATH are not unlikely when running scripts
> written using other shells.

Never mind: it is disabled when zsh is launched as sh (but not when
launched as zsh and then executing 'emulate sh'). So I suppose it was
decided long before that disabling it post-launch is not feasible. Sorry
for the noise.

- M.

Re: ${path[@]} in sh mode [was: Segfault with zsh 5.2]

[Mikael Magnusson]

On Fri, Jan 22, 2016 at 9:45 PM, Martijn Dekker <martijn@inlv.org> wrote:
> Martijn Dekker schreef op 22-01-16 om 20:12:
>> Peter Stephenson schreef op 07-12-15 om 14:36:
>>> Change now pushed.
>>
>> Can the ${path[@]} array be disabled in sh mode? I just noticed that it
>> auto-syncs with the system $PATH. Lowercase "path" is a perfectly normal
>> variable name in other shells and is a common English word, so conflicts
>> that result in corruption of $PATH are not unlikely when running scripts
>> written using other shells.
>
> Never mind: it is disabled when zsh is launched as sh (but not when
> launched as zsh and then executing 'emulate sh'). So I suppose it was
> decided long before that disabling it post-launch is not feasible. Sorry
> for the noise.

If you're in a function context, you can say 'typeset -h path' to hide
the specialness of $path and declare a new local of the same name. Eg,
one that isn't connected to PATH.

-- 
Mikael Magnusson

Re: ${path[@]} in sh mode [was: Segfault with zsh 5.2]

[Martijn Dekker]

Mikael Magnusson schreef op 23-01-16 om 00:07:
> On Fri, Jan 22, 2016 at 9:45 PM, Martijn Dekker <martijn@inlv.org> wrote:
>> Martijn Dekker schreef op 22-01-16 om 20:12:
>>> Peter Stephenson schreef op 07-12-15 om 14:36:
>>>> Change now pushed.
>>>
>>> Can the ${path[@]} array be disabled in sh mode? I just noticed that it
>>> auto-syncs with the system $PATH. Lowercase "path" is a perfectly normal
>>> variable name in other shells and is a common English word, so conflicts
>>> that result in corruption of $PATH are not unlikely when running scripts
>>> written using other shells.
>>
>> Never mind: it is disabled when zsh is launched as sh (but not when
>> launched as zsh and then executing 'emulate sh'). So I suppose it was
>> decided long before that disabling it post-launch is not feasible. Sorry
>> for the noise.
> 
> If you're in a function context, you can say 'typeset -h path' to hide
> the specialness of $path and declare a new local of the same name. Eg,
> one that isn't connected to PATH.

It would be nice if that worked globally too, and even nicer if it were
automatically done by 'emulate sh' (or ksh for that matter) and undone
by 'emulate zsh'. But that might be non-trivial and the developers may
have other priorities.

- M.

Re: ${path[@]} in sh mode [was: Segfault with zsh 5.2]

[Bart Schaefer]

On Jan 23,  1:08am, Martijn Dekker wrote:
}
} > If you're in a function context, you can say 'typeset -h path' to hide
} > the specialness of $path and declare a new local of the same name. Eg,
} > one that isn't connected to PATH.
} 
} It would be nice if that worked globally too, and even nicer if it were
} automatically done by 'emulate sh' (or ksh for that matter) and undone
} by 'emulate zsh'. But that might be non-trivial and the developers may
} have other priorities.

It's non-trivial.  The emulate command only deals with setopts, not with
keywords, parameters, etc.

Re: ${path[@]} in sh mode [was: Segfault with zsh 5.2]

[Peter Stephenson]

On Fri, 22 Jan 2016 17:53:01 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Jan 23,  1:08am, Martijn Dekker wrote:
> }
> } > If you're in a function context, you can say 'typeset -h path' to hide
> } > the specialness of $path and declare a new local of the same name. Eg,
> } > one that isn't connected to PATH.
> } 
> } It would be nice if that worked globally too, and even nicer if it were
> } automatically done by 'emulate sh' (or ksh for that matter) and undone
> } by 'emulate zsh'. But that might be non-trivial and the developers may
> } have other priorities.
> 
> It's non-trivial.  The emulate command only deals with setopts, not with
> keywords, parameters, etc.

One minor bonus worth nothing is that you don't need to use the "-h" in
the function.  You can tell the special itself that it's to be hidden by
a local:

% typeset -h path
% foo() { integer path=43; print ${(t)path}; }
% foo
integer-local
% print ${(t)path}
array-unique-hide-special

pws