Segfault in hist.c

Segfault in hist.c

[Frank Terbeck]

This just came up on IRC:

zsh% alias foo=bar
zsh% foo\<RET>
> <C-c>
zsh% <RET>
 hist.c:1165: BUG: chline is NULL in hend()
[1] 8401 segmentation fault (core dumped)  PS1="zsh%# " zsh -f

Here's a backtrace:

[snip]
Core was generated by `zsh -f'.
Program terminated with signal 11, Segmentation fault.
#0  0x0807f8b5 in hend (prog=0x0) at hist.c:1232
1232               if (save || *chline == ' ') {
(gdb)
#0  0x0807f8b5 in hend (prog=0x0) at hist.c:1232
        hookargs = 0xb7242738
        flag = 8
        save = 0
        hookret = 0
        stack_pos = 0
        hf = 0x0
#1  0x08083bc9 in loop (toplevel=1, justonce=0) at init.c:133
        prog = 0x0
        err = 50
        non_empty = 1
#2  0x08086b9b in zsh_main (argc=2, argv=0xbfe56854) at init.c:1528
        t = 0xbfe5685c
        runscript = 0x0
        t0 = 158
#3  0x0805505f in main (argc=2, argv=0xbfe56854) at ./main.c:93
No locals.
[snap]

Maybe someone here immediately knows what's going on.

Regards, Frank

Re: Segfault in hist.c

[Peter Stephenson]

On Mon, 11 Jul 2011 13:56:55 +0200
Frank Terbeck <ft@bewatermyfriend.org> wrote:
> This just came up on IRC:
> 
> zsh% alias foo=bar
> zsh% foo\<RET>
> > <C-c>
> zsh% <RET>
>  hist.c:1165: BUG: chline is NULL in hend()
> [1] 8401 segmentation fault (core dumped)  PS1="zsh%# " zsh -f

Hmm.

I've tracked it down to a test in hbegin() when initialising the command
line after the aborted one.  At this point inbufflags hasn't been
initialised and it misfires by testing the "alias" flag still in effect
from the failed line and is tricked into thinking the history mechanism
shouldn't be used.

I can't see why that test could be useful, and taking it out didn't seem
to break anything.  It comes from:

revision 1.2
date: 2000/04/12 08:24:16;  author: wischnow;  state: Exp;  lines: +718 -291
new widget copy-prev-shell-word, like copy-prev-word but uses shell parsing to find word (10685)
----------------------------

so presumably is related to some grungy undocumented zle interaction.

About the only course of action I can see is to take it out and see if
something possibly in copy-prev-shell-word breaks; whatever it's for, it
can't possibly be the right way of doing it.  I tried that ZLE function
but didn't see anything odd.  My hope is that if it was ever needed it
was a nasty hack to cover up the fact that we didn't properly save and
restore as many things when calling into the main shell for input in
ZLE.

The zle hunk is just paranoia:  I don't like the idea of ZLE returning a
non-zero command line if there's been an error.  I think this just fixes
an inconsistency.

Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.112
diff -p -u -r1.112 hist.c
--- Src/hist.c	3 Jun 2011 19:54:44 -0000	1.112
+++ Src/hist.c	11 Jul 2011 13:38:07 -0000
@@ -876,7 +876,18 @@ hbegin(int dohist)
 	stophist = (!interact || unset(SHINSTDIN)) ? 2 : 0;
     else
 	stophist = 0;
-    if (stophist == 2 || (inbufflags & INP_ALIAS)) {
+    /*
+     * pws: We used to test for "|| (inbufflags & INP_ALIAS)"
+     * in this test, but at this point we don't have input
+     * set up up so this can trigger unnecessarily.
+     * I don't see how the test at this point could ever be
+     * useful, since we only get here when we're initialising
+     * the history mechanism, before we've done any input.
+     *
+     * (I also don't see any point where this function is called with
+     * dohist=0.)
+     */
+    if (stophist == 2) {
 	chline = hptr = NULL;
 	hlinesz = 0;
 	chwords = NULL;
Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.128
diff -p -u -r1.128 zle_main.c
--- Src/Zle/zle_main.c	26 May 2011 09:19:11 -0000	1.128
+++ Src/Zle/zle_main.c	11 Jul 2011 13:38:07 -0000
@@ -1233,7 +1233,7 @@ zleread(char **lp, char **rp, int flags,
     alarm(0);
 
     freeundo();
-    if (eofsent) {
+    if (eofsent || errflag) {
 	s = NULL;
     } else {
 	zleline[zlell++] = ZWC('\n');

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

Re: Segfault in hist.c

[Bart Schaefer]

On Jul 11,  1:56pm, Frank Terbeck wrote:
} Subject: Segfault in hist.c
}
} This just came up on IRC:
} 
} zsh% alias foo=bar
} zsh% foo\<RET>
} > <C-c>
} zsh% <RET>
}  hist.c:1165: BUG: chline is NULL in hend()
} [1] 8401 segmentation fault (core dumped)  PS1="zsh%# " zsh -f

Well ... the problem is that the global inbufflags == INP_ALIAS which
means the shell believes it is still expanding "foo".  This causes it
to zero out everything when hbegin() is called, instead of allocating
a new chline etc., so when hend() is finally called ... boom.

What I don't follow is why inbufflags isn't getting cleared properly
on send-break ... because I can't figure out where it's ever cleared
at all.  I'm not very familiar with teh flow of control in input.c.

Re: Segfault in hist.c

[Bart Schaefer]

On Jul 11,  2:46pm, Peter Stephenson wrote:
}
} I've tracked it down to a test in hbegin() when initialising the command
} line after the aborted one.  At this point inbufflags hasn't been
} initialised and it misfires by testing the "alias" flag still in effect
} from the failed line and is tricked into thinking the history mechanism
} shouldn't be used.
} 
} I can't see why that test could be useful, and taking it out didn't seem
} to break anything.  It comes from:
} 
} revision 1.2
} date: 2000/04/12 08:24:16;  author: wischnow;  state: Exp;  lines: +718 -291
} new widget copy-prev-shell-word, like copy-prev-word but uses shell parsing to find word (10685)
} ----------------------------

That's what the commit log says, but it actually appears to have come
from ten months earlier in the (reversed) patch here:

http://www.zsh.org/mla/workers/1999/msg01579.html

Which means Sven probably committed it by accident!

} so presumably is related to some grungy undocumented zle interaction.

The intention appears to have been to optimize when it was OK to NOT
allocate a history line, because it was never actually going to be
used.  But either part of the patch has eroded away over the years or
it was never fully committed (the latter might make sense if it were
an accidental commit).