BUG: Zsh crashes

BUG: Zsh crashes

[Bjorn Baron]

Dear zsh developers,

I have found a bug in zsh. You can crash zsh with for example a 
segmentation fault by autocompleting `rustc --pretty flowgraph` two times

# Reproduction:
type `rustc --pretty flowgraph`
press tab 2x

# Actual result:
Zsh crashes.
```
$ zsh
  bjorn@pc-bjorn  ~  rustc --pretty flowgraph==......_values:147: 
command not found: opts
*** Error in `zsh': double free or corruption (fasttop): 0x083a23d8 ***
Afgebroken                 #MEANS ABORTED
$ zsh
  bjorn@pc-bjorn  ~  rustc --pretty flowgraph==......_values:147: 
command not found: garbage
Segmentatiefout            #MEANS SEGMENTATION FAULT
$ zsh
  bjorn@pc-bjorn  ~  rustc --pretty flowgraph==......*** Error in 
`zsh': munmap_chunk(): invalid pointer: 0x096daee8 *** expanded
```

# Expected result:
Doesn't crash.

More information can be found at 
https://github.com/robbyrussell/oh-my-zsh/issues/5780 where I originally 
posted this bug, because I couldn't find where this bug report to.

With kind regards,

Bjorn

Re: BUG: Zsh crashes

[Peter Stephenson]

On Fri, 13 Jan 2017 17:40:48 +0000
Bjorn Baron <bjornbaron@outlook.com> wrote:
> Dear zsh developers,
> 
> I have found a bug in zsh. You can crash zsh with for example a 
> segmentation fault by autocompleting `rustc --pretty flowgraph` two times
> 
> # Reproduction:
> type `rustc --pretty flowgraph`
> press tab 2x
> 
> # Actual result:
> Zsh crashes.

You always need to report both the zsh version and, in the case of
completion crashes, how this is set up (looks like in your case this is
down to oh-my-zsh so I guess you don't have explicit details of your
own setup beyond that).

However, you appear to be using zsh 5.0.2, if I've read you're
web report correctly --- that's several years out of date, so I'm
afraid you're not going to get any direct help with the version you're
using...

Obviously, if it still happens with 5.3.1 that's a different matter.

pws

Re: BUG: Zsh crashes

[Phil Pennock]

On 2017-01-13 at 17:52 +0000, Peter Stephenson wrote:
> You always need to report both the zsh version and, in the case of
> completion crashes, how this is set up (looks like in your case this is
> down to oh-my-zsh so I guess you don't have explicit details of your
> own setup beyond that).
> 
> However, you appear to be using zsh 5.0.2, if I've read you're
> web report correctly --- that's several years out of date, so I'm
> afraid you're not going to get any direct help with the version you're
> using...
> 
> Obviously, if it still happens with 5.3.1 that's a different matter.

It does.  Disclaimer: my oh-my-zsh experience is limited to having
helped former co-workers with their setups and writing a custom theme
for one who wanted some special employer-specific stuff in their prompt.

uname .. => Darwin 16.3.0  (it's MacOS 10.12.2)
zsh installed via brew, installed as a binary package ("bottle"), the
./configure flags can be seen in:
  https://github.com/Homebrew/homebrew-core/blob/master/Formula/zsh.rb


...% env - /usr/local/bin/zsh -f
tungsten% export TERM=xterm-256color; LC_CTYPE=en_US.UTF-8
tungsten% echo $ZSH_VERSION
5.3.1
tungsten% export ZSH=$HOME/.oh-my-zsh; ZSH_THEME="agnoster"; COMPLETION_WAITING_DOTS="true"
tungsten% plugins=(git rust)
tungsten% . $ZSH/oh-my-zsh.sh


This with having freshly cloned github.com:robbyrussell/oh-my-zsh.git to
~/.oh-my-zsh and no modifications locally; checked out at 0b4bba4c

No install steps for oh-my-zsh or curl|sh stuff done, it's purely a git
repo clone to that path.


...% rustc --pretty flowgraph==......zsh(41997,0x7fffdf23d3c0) malloc: ***
error for object 0x7fc75560ef60: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
 zsh: abort (core dumped)  env - /usr/local/bin/zsh -f


% lldb -c /cores/core.41997 /usr/local/bin/zsh
(lldb) target create "/usr/local/bin/zsh" --core "/cores/core.41997"
warning: (x86_64) /cores/core.41997 load command 386 LC_SEGMENT_64 has a fileoff + filesize (0x2d3ae000) that extends beyond the end of the file (0x2d3ad000), the segment will be truncated to match
warning: (x86_64) /cores/core.41997 load command 387 LC_SEGMENT_64 has a fileoff (0x2d3ae000) that extends beyond the end of the file (0x2d3ad000), ignoring this section
Core file '/cores/core.41997' (x86_64) was loaded.
(lldb) 

I've put the output of `bt` into:
  https://gist.github.com/philpennock/89531216fb7096c6906e5aa1f0fa8fc7
That URL does not require authentication to access, no GitHub account
needed, it's just a convenient paste-site.  There are 207 frames, which
is a little long for this email.

My lldb-fu is even weaker than my remnants of gdb-fu.  Not sure what's
useful to drill further.  My zsh allocator internals knowledge is almost
two decades rusty at this point.

Hopefully the above reproduction steps will help others.

-Phil

Re: BUG: Zsh crashes

[Daniel Shahaf]

Phil Pennock wrote on Sat, Jan 14, 2017 at 01:09:00 +0000:
> This with having freshly cloned github.com:robbyrussell/oh-my-zsh.git to
> ~/.oh-my-zsh and no modifications locally; checked out at 0b4bba4c
> 
> No install steps for oh-my-zsh or curl|sh stuff done, it's purely a git
> repo clone to that path.
> 
> 
> ...% rustc --pretty flowgraph==......zsh(41997,0x7fffdf23d3c0) malloc: ***
> error for object 0x7fc75560ef60: pointer being freed was not allocated
> *** set a breakpoint in malloc_error_break to debug
>  zsh: abort (core dumped)  env - /usr/local/bin/zsh -f

More minimal reproducer:

% git clone --depth 5 git://github.com/robbyrussell/oh-my-zsh.git
% mkdir d
% cp */plugins/rust/_rust d 
% fpath+=($PWD/d) 
% autoload compinit
% compinit 
% rustc --pretty flowgraph==<TAB><TAB>

This uses the _rust from omz but nothing else from it.

Re: BUG: Zsh crashes

[Peter Stephenson]

On Sat, 14 Jan 2017 02:48:33 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> More minimal reproducer:
> 
> % git clone --depth 5 gitn://github.com/robbyrussell/oh-my-zsh.git
> % mkdir d
> % cp */plugins/rust/_rust d 
> % fpath+=($PWD/d) 
> % autoload compinit
> % compinit 
> % rustc --pretty flowgraph==<TAB><TAB>
> 
> This uses the _rust from omz but nothing else from it.

Excellent, just what we need, thanks.

With ZSH_SECURE_FREE this goes wrong with the error message at line 1506
of mem.c.  That comes from a bad free of a parameter.  I'm guessing this
is (a) earlier than the crash, obviously (b) already somewhat later than
the fundamental problem, which is presumably memory related.

Interestingly, even with exactly the same steps I've seen two slightly
different versions:

- the free came from scanendscope for function _normal
- the free came from deleting a hashtable associated with _lastcomp.

and within those the symptoms aren't stable, either.

In both cases it's from a parameter, but it doesn't look like it's a
single particular erroneous parameter that's the problem; I suspect the
original error has come much earlier.  The relationship to the hash
table of a hash parameter could well be just because that has a
particular large memory footprint for the problem to hit.

Normal valgrind didn't show anything helfupl.  Valgrind integration with
heaps with --enble-zsh-valgrind hasn't been working properly for a while
now, but the circumstantial evidence points more to permanent allocation
problem anyway.

The fact that it's the second tab must be relevant in some way.  Some
state may not be being preserved properly.

This is going to take some lateral thinking.  Anyone who can think
laterally might be able to help.  Playing with the completer, for
example.

pws

Re: BUG: Zsh crashes

[Peter Stephenson]

On Sat, 14 Jan 2017 02:48:33 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> This uses the _rust from omz but nothing else from it.

The following completer is enough:


#compdef rustc

_rustc_pretty_types=(
    'flowgraph=[graphviz formatted flowgraph for node]:NODEID:'
)

_arguments -s :  \
    --pretty='[Pretty-print the input instead of compiling]::TYPE:_values "TYPES" "$_rustc_pretty_types[@]"'


Removing the "=" after flowgraph stops it ("it" includes any reports of
errors in mem.c, of course).  So this points at something to do with how
_values handles values.

Further intelligent suggestions?

pws

Re: BUG: Zsh crashes

[Peter Stephenson]

On Sat, 14 Jan 2017 20:36:24 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> Removing the "=" after flowgraph stops it ("it" includes any reports of
> errors in mem.c, of course).  So this points at something to do with how
> _values handles values.

Can someone else see if this helps?

I have a feeling _rust ought to be quoting the "=", too, but that's much
more minor.

pws

diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 7bf9535..5b9ceec 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -3569,8 +3569,8 @@ bin_compvalues(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	    Cvval val = cv_get_val(cv_laststate.d, args[1]);
 
 	    if (val && val->arg) {
-		setsparam(args[2], val->arg->descr);
-		setsparam(args[3], val->arg->action);
+		setsparam(args[2], ztrdup(val->arg->descr));
+		setsparam(args[3], ztrdup(val->arg->action));
 
 		if (args[4])
 		    setsparam(args[4], ztrdup(val->name));

Re: BUG: Zsh crashes

[Bart Schaefer]

On Sat, 14 Jan 2017, Peter Stephenson wrote:

> On Sat, 14 Jan 2017 20:36:24 +0000
> Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > Removing the "=" after flowgraph stops it ("it" includes any reports of
> > errors in mem.c, of course).  So this points at something to do with how
> > _values handles values.
>
> Can someone else see if this helps?

That's certainly the direction my investigation was going.  It almost
*has* to be *something* about bin_compvalues().

Re: BUG: Zsh crashes

[Daniel Shahaf]

Peter Stephenson wrote on Sat, Jan 14, 2017 at 22:10:52 +0000:
> On Sat, 14 Jan 2017 20:36:24 +0000
> Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > Removing the "=" after flowgraph stops it ("it" includes any reports of
> > errors in mem.c, of course).  So this points at something to do with how
> > _values handles values.
> 
> Can someone else see if this helps?

I can reproduce errors without the patch but not with it, using

_f() {
  .. the completer you posted ..
}
compdef _f f
f <TAB><TAB><TAB><TAB>

I applied the patch to 33799ae2b00c; the changes after that don't appear
to be relevant.

Re: BUG: Zsh crashes

[Eric Cook]

On 01/13/2017 12:52 PM, Peter Stephenson wrote:

> You always need to report both the zsh version and, in the case of
> completion crashes, how this is set up (looks like in your case this is
> down to oh-my-zsh so I guess you don't have explicit details of your
> own setup beyond that).


With less steps:

curl https://raw.githubusercontent.com/robbyrussell/oh-my-zsh/3705d47bb3f3229234cba992320eadc97a221caf/plugins/rust/_rust > /path/in/fpath/_rust
autoload -Uz _rust; compdef _rust rustc
rustc --pretty flowgraph==<^I>

The interesting parts of _complete_debug
```
+_description:109> return 0
+_next_label:19> set -A expl -M 'm:{[:lower:][:upper:]}={[:upper:][:lower:]} r:|[._-]=** r:|=* l:|=*' -J arguments -X 'Completing <F0>&8^A'
+_next_label:22> return 0
+_values:147> '`' -M 'm:{[:lower:][:upper:]}={[:upper:][:lower:]} r:|[._-]=** r:|=* l:|=*' -J arguments -X 'Completing <F0>&8^A' '>2^A'
_values:147: command not found: `
+_values:146> _next_label arguments expl '<F0>&8^A'
+_next_label:3> local __gopt __descr __spec
+_next_label:5> __gopt=( )
+_next_label:6> zparseopts -D -a __gopt 1 2 V J x
```

and

```
+_description:13> [[ -n files ]]
+_description:13> _lastdescr=( '' TYPE NODEID files directories '<F0>&8^A' files )
+_description:15> zstyle -s :completion::complete:rustc:argument-1:argument-1 group-name gname
+_description:16> [[ -z '' ]]
+_description:16> gname=argument-1
+_description:18> _setup argument-1 argument-1
```

There are a few other places where the control characters show up but i am not sure what is creating it.
I can get zsh to crash by adding a space before _values on line 52 of the completer.

Re: BUG: Zsh crashes

[Eric Cook]

On 01/13/2017 11:08 PM, Eric Cook wrote:
> I can get zsh to crash by adding a space before _values on line 52 of the completer.
> 
Should've read:
I can get zsh to crash in an slightly different way (free() invalid size) by adding a
space before _values on line 52 of the completer.