[bug] type -m '^foo'

[bug] type -m '^foo'

[Stephane Chazelas]

With latest CVS version as well as 4.2.0

$ type -m '(^a)'

segfaults.

It seems '(^a)' is matched against the keywords whose names are
defined as string constants. That may be why the *testptr =
'\0'; causes the segfaults (attempt to modify a readonly
variable).

Program received signal SIGSEGV, Segmentation fault.
0x000a7010 in patmatch (prog=0xfa2ec) at pattern.c:1849
1849                                *testptr = '\0';
(gdb) bt
#0  0x000a7010 in patmatch (prog=0xfa2ec) at pattern.c:1849
#1  0x000a57a4 in pattryrefs (prog=0xfa2c8, string=0xd1e08 "if", nump=0x0, begp=0x0, endp=0x0) at pattern.c:1404
#2  0x000a54c0 in pattry (prog=0xfa2c8, string=0xd1e08 "if") at pattern.c:1346
#3  0x0004f638 in scanmatchtable (ht=0xf3b30, pprog=0xfa2c8, flags1=0, flags2=1, scanfunc=0x50e60 <printreswdnode>, scanflags=64)
    at hashtable.c:435
#4  0x00026538 in bin_whence (nam=0xff3904d8 "type", argv=0xffbef2d4, ops=0xffbef2f8, func=0) at builtin.c:2762
#5  0x0001a530 in execbuiltin (args=0xff3904a8, bn=0xe7784) at builtin.c:439
#6  0x0003e414 in execcmd (state=0xffbef768, input=0, output=0, how=18, last1=2) at exec.c:2425
#7  0x00039158 in execpline2 (state=0xffbef768, pcode=131, how=18, input=0, output=0, last1=0) at exec.c:1276
#8  0x00037e9c in execpline (state=0xffbef768, slcode=5122, how=18, last1=0) at exec.c:1065
#9  0x0003729c in execlist (state=0xffbef768, dont_change_job=0, exiting=0) at exec.c:871
#10 0x00036d90 in execode (p=0xff390448, dont_change_job=0, exiting=0) at exec.c:771
#11 0x0005ce98 in loop (toplevel=1, justonce=0) at init.c:165
#12 0x00060fa8 in zsh_main (argc=1, argv=0xffbef964) at init.c:1274
#13 0x0001960c in main (argc=1, argv=0xffbef964) at ./main.c:93

(gdb) bt full
#0  0x000a7010 in patmatch (prog=0xfa2ec) at pattern.c:1849
        savpatinlen = 3
        syncpt = (unsigned char *) 0x113412 "\001ø"
        savchar = 0 '\0'
        testptr = 0xd1e0a ""
        savpatinstart = 0xd1e08 "if"
        savforce = -1
        savpatflags = 64
        syncstrp = 0xfa304
        matchpt = 0xd1e0a ""
        ret = 1
        savparsfound = 0
        oldsyncstr = (unsigned char *) 0x0
        savglobdots = 1
        matchederrs = 0
        scan = 0xfa2f4
        next = 0xfa300
        opnd = 0xfa308
        start = 0x7efefeff <Address 0x7efefeff out of bounds>
        save = 0xd1e08 "if"
        chrop = 0x1 <Address 0x1 out of bounds>
        savglobflags = 0
        op = -13041360
        no = 10
        min = 272
        nextch = 0
        fail = 0
        saverrsfound = 0
        from = 0
        to = -511101108225
        comp = 1074944
#1  0x000a57a4 in pattryrefs (prog=0xfa2c8, string=0xd1e08 "if", nump=0x0, begp=0x0, endp=0x0) at pattern.c:1404
        i = -4263548
        maxnpos = 0
        sp = (char **) 0x0
        ep = (char **) 0x0
        progstr = 0xfa2ec ""
#2  0x000a54c0 in pattry (prog=0xfa2c8, string=0xd1e08 "if") at pattern.c:1346
No locals.
#3  0x0004f638 in scanmatchtable (ht=0xf3b30, pprog=0xfa2c8, flags1=0, flags2=1, scanfunc=0x50e60 <printreswdnode>, scanflags=64)
    at hashtable.c:435
        hn = 0xe7b34
        i = 2
        hsize = 23
        nodes = (HashNode *) 0xf3b80
        match = 0
        st = {sorted = 0, u = {s = {tab = 0x0, ct = 0}, u = 0x0}}
#4  0x00026538 in bin_whence (nam=0xff3904d8 "type", argv=0xffbef2d4, ops=0xffbef2f8, func=0) at builtin.c:2762
        hn = 0xff3904e8
        pprog = 0xfa2c8
        returnval = 0
        printflags = 64
        aliasflags = 64
        csh = 0
        all = 0
        v = 1
        wd = 0
        informed = -16954744
        cnam = 0xff390000 ""
#5  0x0001a530 in execbuiltin (args=0xff3904a8, bn=0xe7784) at builtin.c:439
        argarr = (char **) 0xffbef2d0
        argv = (char **) 0xffbef2d4
        pp = 0xd06b1 ""
        name = 0xff3904d8 "type"
        optstr = 0xd06b8 "ampfsw"
        flags = 8
        sense = 0
        argc = 1
        execop = 109
        xtr = 0
        ops = {ind = '\0' <repeats 109 times>, "\001\000\000\000\000\000\000\000\000\001\000\000\000\000\000\000\000\000",
  args = 0x0, argscount = 0, argsalloc = 0}
#6  0x0003e414 in execcmd (state=0xffbef768, input=0, output=0, how=18, last1=2) at exec.c:2425
        restorelist = 0x0
        removelist = 0x0
        hn = 0xe7784
        node = 0xff390490
        fn = 0xff39048c
        mfds = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}
        text = 0xebfe0 "type -m '(^a)'"
        save = {-2, -2, -2, -2, -2, -2, -2, -2, -2, -2}
        fil = 860513
        dfil = 1097904
        is_cursh = 1
        type = 6
        do_exec = 0
        i = 10
        htok = 1
        nullexec = 0
        assign = 0
        forked = 0
        is_shfunc = 0
        is_builtin = 1
        is_exec = 0
        use_defpath = 0
        cflags = 0
        checked = 1
        oautocont = 0
        redir = 0x0
        code = 102
        beg = 0xff39047c
        varspc = 0x0
        oxtrerr = (FILE *) 0xee5a8
#7  0x00039158 in execpline2 (state=0xffbef768, pcode=131, how=18, input=0, output=0, last1=0) at exec.c:1276
        pid = 1
        pipes = {860504, 60}
#8  0x00037e9c in execpline (state=0xffbef768, slcode=5122, how=18, last1=0) at exec.c:1065
        ipipe = {0, 0}
        opipe = {0, 0}
        pj = 0
        newjob = 1
        old_simple_pline = 0
        slflags = 0
        code = 131
        lastwj = 0
        lpforked = 0
#9  0x0003729c in execlist (state=0xffbef768, dont_change_job=0, exiting=0) at exec.c:871
        donetrap = 0
        next = 0xff39048c
        code = 5122
        ret = 0
        cj = 0
        csp = 0
        ltype = 18
        old_pline_level = 0
        old_list_pipe = 0
        oldlineno = 2
        oldnoerrexit = 0
#10 0x00036d90 in execode (p=0xff390448, dont_change_job=0, exiting=0) at exec.c:771
        s = {prog = 0xff390448, pc = 0xff39048c, strs = 0xff390490 "type"}
#11 0x0005ce98 in loop (toplevel=1, justonce=0) at init.c:165
        toksav = 1
        preprog = 0xf0178
        prog = 0xff390448
#12 0x00060fa8 in zsh_main (argc=1, argv=0xffbef964) at init.c:1274
        t = (char **) 0xffbef968
        t0 = 156
#13 0x0001960c in main (argc=1, argv=0xffbef964) at ./main.c:93
No locals.

-- 
Stephane

Re: [bug] type -m '^foo'

[DervishD]

    Hi Stephane :)

 * Stephane Chazelas <Stephane.Chazelas@morse.com> dixit:
> With latest CVS version as well as 4.2.0
> $ type -m '(^a)'
> segfaults.

    It does with 4.0.9 here.

    Raúl Núñez de Arenas Coronado

-- 
Linux Registered User 88736
http://www.dervishd.net & http://www.pleyades.net/

Re: [bug] type -m '^foo'

[Danek Duvall]

On Fri, Oct 01, 2004 at 03:50:47PM +0100, Stephane Chazelas wrote:

> With latest CVS version as well as 4.2.0
> 
> $ type -m '(^a)'
> 
> segfaults.

FWIW, it works just fine on Solaris SPARC and x86, both 4.2.0 and 4.2.1 +
patches.  It does crash on Linux, 4.0.6 and 4.2.0.

Danek

Re: [bug] type -m '^foo'

[Stephane Chazelas]

On Fri, Oct 01, 2004 at 08:29:22AM -0700, Danek Duvall wrote:
> On Fri, Oct 01, 2004 at 03:50:47PM +0100, Stephane Chazelas wrote:
> 
> > With latest CVS version as well as 4.2.0
> > 
> > $ type -m '(^a)'
> > 
> > segfaults.
> 
> FWIW, it works just fine on Solaris SPARC
[...]

I forgot to say that you need "setopt extendedglob" for the bug
to occur. That may be why you didn't get the bug (I did on
Solaris SPARC).


regards,
Stephane

Re: [bug] type -m '^foo'

[Danek Duvall]

On Fri, Oct 01, 2004 at 04:32:35PM +0100, Stephane Chazelas wrote:

> I forgot to say that you need "setopt extendedglob" for the bug
> to occur. That may be why you didn't get the bug (I did on
> Solaris SPARC).

Thanks for the heads-up, but I already have it turned on, so that's not it.

Danek

Re: [bug] type -m '^foo'

[Bart Schaefer]

On Fri, 1 Oct 2004, Stephane Chazelas wrote:

> With latest CVS version as well as 4.2.0
> 
> $ type -m '(^a)'
> 
> segfaults.

This fixes the crash, but it would be better if pattry() could be 
rewritten so as not to need to modify the string in place.

Index: Src/hashtable.c
===================================================================
RCS file: /extra/cvsroot/zsh/zsh-4.0/Src/hashtable.c,v
retrieving revision 1.10
diff -c -r1.10 hashtable.c
--- Src/hashtable.c	22 Jun 2004 07:53:05 -0000	1.10
+++ Src/hashtable.c	1 Oct 2004 16:53:31 -0000
@@ -433,7 +433,7 @@
 	    HashNode hn = st.u.u;
 	    st.u.u = st.u.u->next;
 	    if ((hn->flags & flags1) + !flags1 && !(hn->flags & flags2) &&
-		pattry(pprog, hn->nam)) {
+		pattry(pprog, dupstring(hn->nam))) {
 		scanfunc(hn, scanflags);
 		match++;
 	    }

Re: [bug] type -m '^foo'

[Peter Stephenson]

Bart Schaefer wrote:
> On Fri, 1 Oct 2004, Stephane Chazelas wrote:
> 
> > With latest CVS version as well as 4.2.0
> > 
> > $ type -m '(^a)'
> > 
> > segfaults.
> 
> This fixes the crash, but it would be better if pattry() could be 
> rewritten so as not to need to modify the string in place.

I think this fixes it, but it's horribly complicated around there.

Index: Src/pattern.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/pattern.c,v
retrieving revision 1.19
diff -u -r1.19 pattern.c
--- Src/pattern.c	28 May 2004 19:21:05 -0000	1.19
+++ Src/pattern.c	1 Oct 2004 18:21:16 -0000
@@ -1818,10 +1818,9 @@
 			syncstrp->p = (unsigned char *)zshcalloc(patinlen);
 			while ((ret = patmatch(P_OPERAND(scan)))) {
 			    unsigned char *syncpt;
-			    char savchar, *testptr;
-			    char *savpatinstart = patinstart;
+			    char *savpatinstart, *origsave, *origpatinstart;
 			    int savforce = forceerrs, savpatinlen = patinlen;
-			    int savpatflags = patflags;
+			    int savpatflags = patflags, synclen;
 			    forceerrs = -1;
 			    savglobdots = globdots;
 			    matchederrs = errsfound;
@@ -1837,16 +1836,38 @@
 			     */
 			    for (syncpt = syncstrp->p; !*syncpt; syncpt++)
 				;
-			    testptr = patinstart + (syncpt - syncstrp->p);
-			    DPUTS(testptr > matchpt, "BUG: EXCSYNC failed");
-			    savchar = *testptr;
-			    /*
-			     * If this isn't really the end of the string,
-			     * remember this for the (#e) assertion.
-			     */
-			    if (savchar)
+			    synclen = syncpt - syncstrp->p;
+			    if (patinstart[synclen]) {
+				/*
+				 * We need to truncate the string at
+				 * this point.  Copy a whole load of
+				 * stuff to avoid modifying the string.
+				 * This includes (at least) patinstart,
+				 * patinput and save.
+				 */
+				origsave = save;
+				origpatinstart = patinstart;
+
+				DPUTS(patinstart + synclen > matchpt,
+				      "BUG: EXCSYNC failed");
+
+				savpatinstart = patinstart =
+				    ztrduppfx(patinstart, synclen);
+				patinput = patinstart +
+				    (patinput - origpatinstart);
+				save = patinstart + (save - origpatinstart);
+				/*
+				 * If this isn't really the end of the string,
+				 * remember this for the (#e) assertion.
+				 */
 				patflags |= PAT_NOTEND;
-			    *testptr = '\0';
+			    }
+			    else
+			    {
+				/* Don't need to copy, already right length */
+				origsave = origpatinstart = NULL;
+				savpatinstart = patinstart;
+			    }
 			    next = PATNEXT(scan);
 			    while (next && P_ISEXCLUDE(next)) {
 				char *buf = NULL;
@@ -1893,7 +1914,17 @@
 				    break;
 				next = PATNEXT(next);
 			    }
-			    *testptr = savchar;
+			    /*
+			     * Free copied string and restore if
+			     * we needed to truncate.
+			     */
+			    if (origpatinstart) {
+				patinput = origpatinstart +
+				    (patinput - patinstart);
+				zfree(patinstart, synclen+1);
+				patinstart = origpatinstart;
+				save = origsave;
+			    }
 			    patflags = savpatflags;
 			    globdots = savglobdots;
 			    forceerrs = savforce;

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: [bug] type -m '^foo'

[Bart Schaefer]

On Fri, 1 Oct 2004, Peter Stephenson wrote:

> > This fixes the crash, but it would be better if pattry() could be 
> > rewritten so as not to need to modify the string in place.
> 
> I think this fixes it, but it's horribly complicated around there.

Hmm, well, what I meant was that it would be better if it could be re- 
written so as not to need a null terminator at that point, i.e., to do 
everything with array indices or starting/ending pointers rather than rely 
on having a nul-terminated string.  Copying and freeing the string down in 
the recursive guts of matching is probably more expensive (for certain 
patterns, although far less expensive for other patterns) than copying the 
whole string beforehand.

On the other hand the code is already doing zshcalloc() in that vicinity,
so perhaps a few more allocs/frees is not enough to worry about.

Re: [bug] type -m '^foo'

[Peter Stephenson]

Bart Schaefer wrote:
> On Fri, 1 Oct 2004, Peter Stephenson wrote:
> 
> > > This fixes the crash, but it would be better if pattry() could be 
> > > rewritten so as not to need to modify the string in place.
> > 
> > I think this fixes it, but it's horribly complicated around there.
> 
> Hmm, well, what I meant was that it would be better if it could be re- 
> written so as not to need a null terminator at that point, i.e., to do 
> everything with array indices or starting/ending pointers rather than rely 
> on having a nul-terminated string.  Copying and freeing the string down in 
> the recursive guts of matching is probably more expensive (for certain 
> patterns, although far less expensive for other patterns) than copying the 
> whole string beforehand.
> 
> On the other hand the code is already doing zshcalloc() in that vicinity,
> so perhaps a few more allocs/frees is not enough to worry about.

Yes, I agree with all of that... however, rewriting the whole thing not
to require NULL-terminated strings is quite a big change (there's no
"memncmp" function, for example, and I'm not sure if you can guarantee
memcmp will stop comparing soon enough).  Only a few of the nasty
negative pattern matches need to rejig the string, so it would probably
make the vast majority of matches a bit slower.  I'll put it on the list
to look at.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************