Re: PATCH: Re: Seg fault in matcher-list matching

Re: PATCH: Re: Seg fault in matcher-list matching

[Sven Wischnowsky]

Bart Schaefer wrote:

> ...
> 
> It doesn't dump for me any more, but I'm still nervous about line 1767 of
> compcore.c:
> 
> #1  0x80bd798 in addmatches (dat=0xbfffa854, argv=0xbfffa8d8)
>     at ../../../zsh-3.1.6/Src/Zle/compcore.c:1768
> 1768                if ((ml = match_str(lsuf, s, &bsl, 0, NULL, 1, 0, 1)) >= 0) {
> (gdb) l 
> 1763                    else
> 1764                        *argv = NULL;
> 1765                    bcp = lpl;
> 1766                }
> 1767                s = dat->psuf ? dat->psuf : "";
> 1768                if ((ml = match_str(lsuf, s, &bsl, 0, NULL, 1, 0, 1)) >= 0) {
> 1769                    if (matchsubs) {
> 1770                        Cline tmp = get_cline(NULL, 0, NULL, 0, NULL, 0, CLF_SUF);
> 1771
> 1772                        tmp->suffix = matchsubs;
> 
> The reported core dump was caused because match_str() wrote a '\0' byte into
> the string pointed to by its second argument [`s' above, `w' in match_str()]
> which is being passed as a string constant when dat->psuf == 0.

Yes, I know.

>  Is that a
> potential bug, still?

I'm pretty sure I made sure that we don't try to write into strings we 
can't write into with the patch I sent. But I'll also commit the one
below for some extra savety.

Bye
 Sven

Index: Src/Zle/compcore.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compcore.c,v
retrieving revision 1.19
diff -u -r1.19 compcore.c
--- Src/Zle/compcore.c	2000/05/12 07:03:41	1.19
+++ Src/Zle/compcore.c	2000/05/15 11:52:03
@@ -1739,7 +1739,7 @@
 		    llpl -= gfl;
 		}
 	    }
-	    s = dat->ppre ? dat->ppre : "";
+	    s = dat->ppre ? dat->ppre : dupstring("");
 	    if ((ml = match_str(lpre, s, &bpl, 0, NULL, 0, 0, 1)) >= 0) {
 		if (matchsubs) {
 		    Cline tmp = get_cline(NULL, 0, NULL, 0, NULL, 0, 0);
@@ -1757,14 +1757,14 @@
 		bpadd = strlen(s) - ml;
 	    } else {
 		if (llpl <= lpl && strpfx(lpre, s))
-		    lpre = "";
+		    lpre = dupstring("");
 		else if (llpl > lpl && strpfx(s, lpre))
 		    lpre += lpl;
 		else
 		    *argv = NULL;
 		bcp = lpl;
 	    }
-	    s = dat->psuf ? dat->psuf : "";
+	    s = dat->psuf ? dat->psuf : dupstring("");
 	    if ((ml = match_str(lsuf, s, &bsl, 0, NULL, 1, 0, 1)) >= 0) {
 		if (matchsubs) {
 		    Cline tmp = get_cline(NULL, 0, NULL, 0, NULL, 0, CLF_SUF);
@@ -1782,7 +1782,7 @@
 		bsadd = strlen(s) - ml;
 	    } else {
 		if (llsl <= lsl && strsfx(lsuf, s))
-		    lsuf = "";
+		    lsuf = dupstring("");
 		else if (llsl > lsl && strsfx(s, lsuf))
 		    lsuf[llsl - lsl] = '\0';
 		else

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: Seg fault in matcher-list matching

[Tanaka Akira]

In article <200005161048.MAA26740@beta.informatik.hu-berlin.de>,
  Sven Wischnowsky <wischnow@informatik.hu-berlin.de> writes:

> The new behaviour is the correct one: with `*', the `*' must not match 
> anchors and that's what it does now (it would have to match the `.',
> of course). With `**' you get the old behaviour, which is correct, too.

I see.  

This is a patch for _cvs to follow the new behaviour.

Index: Completion/User/_cvs
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/User/_cvs,v
retrieving revision 1.7
diff -u -r1.7 _cvs
--- Completion/User/_cvs	2000/05/16 00:07:06	1.7
+++ Completion/User/_cvs	2000/05/16 15:59:19
@@ -584,7 +584,7 @@
   fi
 
   _tags files && {
-    compadd -M 'r:|[:@./]=* r:|=*' "$@" $_cvs_roots || _files "$@" -/
+    compadd -M 'r:|[:@./]=** r:|=**' "$@" $_cvs_roots || _files "$@" -/
   }
 }
 
-- 
Tanaka Akira

Re: PATCH: Re: Seg fault in matcher-list matching

[Sven Wischnowsky]

Tanaka Akira wrote:

> In article <200005150926.LAA17371@beta.informatik.hu-berlin.de>,
>   Sven Wischnowsky <wischnow@informatik.hu-berlin.de> writes:
> 
> > I couldn't get it to seg-fault, but there was something broken. Matching 
> > of the suffix, for example, and that both in C and shell code.
> 
> I found another problem about matching control.
> 
> Z(4):akr@serein% Src/zsh -f
> serein% bindkey -e; autoload -U compinit; compinit -D; compdef _tst tst
> serein% _tst () { compadd -M 'r:|[:.]=* r:|=*' a.b:0 c.d:1 }
> serein% tst :<TAB>
> 
> This inserts nothing now.
> 
> But 3.1.7-pre-2 inserts `.' as follows (and place the cursor on the
> `.').
> 
> % tst .:

The new behaviour is the correct one: with `*', the `*' must not match 
anchors and that's what it does now (it would have to match the `.',
of course). With `**' you get the old behaviour, which is correct, too.

;-)

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: Seg fault in matcher-list matching

[Tanaka Akira]

In article <200005150926.LAA17371@beta.informatik.hu-berlin.de>,
  Sven Wischnowsky <wischnow@informatik.hu-berlin.de> writes:

> I couldn't get it to seg-fault, but there was something broken. Matching 
> of the suffix, for example, and that both in C and shell code.

I found another problem about matching control.

Z(4):akr@serein% Src/zsh -f
serein% bindkey -e; autoload -U compinit; compinit -D; compdef _tst tst
serein% _tst () { compadd -M 'r:|[:.]=* r:|=*' a.b:0 c.d:1 }
serein% tst :<TAB>

This inserts nothing now.

But 3.1.7-pre-2 inserts `.' as follows (and place the cursor on the
`.').

% tst .:
-- 
Tanaka Akira

Re: PATCH: Re: Seg fault in matcher-list matching

[Bart Schaefer]

On May 15, 11:26am, Sven Wischnowsky wrote:
} Subject: PATCH: Re: Seg fault in matcher-list matching
}
} 
} Bart Schaefer wrote:
} 
} > I did this:
} > 
} > zagzig[41] /u/s/l/z/z4/s/zsh
} >                      ^cursor over the 4, press TAB
} > 
} > The path this was intended to match was /usr/src/local/zsh/zsh-2.4/src/zsh.
} > I had first tried TAB at the end of the line and gotten a feep, in case
} > that matters.
} 
} I couldn't get it to seg-fault, but there was something broken. Matching 
} of the suffix, for example, and that both in C and shell code.

It doesn't dump for me any more, but I'm still nervous about line 1767 of
compcore.c:

#1  0x80bd798 in addmatches (dat=0xbfffa854, argv=0xbfffa8d8)
    at ../../../zsh-3.1.6/Src/Zle/compcore.c:1768
1768                if ((ml = match_str(lsuf, s, &bsl, 0, NULL, 1, 0, 1)) >= 0) {
(gdb) l 
1763                    else
1764                        *argv = NULL;
1765                    bcp = lpl;
1766                }
1767                s = dat->psuf ? dat->psuf : "";
1768                if ((ml = match_str(lsuf, s, &bsl, 0, NULL, 1, 0, 1)) >= 0) {
1769                    if (matchsubs) {
1770                        Cline tmp = get_cline(NULL, 0, NULL, 0, NULL, 0, CLF_SUF);
1771
1772                        tmp->suffix = matchsubs;

The reported core dump was caused because match_str() wrote a '\0' byte into
the string pointed to by its second argument [`s' above, `w' in match_str()]
which is being passed as a string constant when dat->psuf == 0.  Is that a
potential bug, still?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: PATCH: Re: Seg fault in matcher-list matching

[Sven Wischnowsky]

I wrote:

> Bart Schaefer wrote:
> 
> > I did this:
> > 
> > zagzig[41] /u/s/l/z/z4/s/zsh
> >                      ^cursor over the 4, press TAB
> > 
> > The path this was intended to match was /usr/src/local/zsh/zsh-2.4/src/zsh.
> > I had first tried TAB at the end of the line and gotten a feep, in case
> > that matters.
> 
> I couldn't get it to seg-fault, but there was something broken. Matching 
> of the suffix, for example, and that both in C and shell code.

Small followup... let's try to do that as seldom as possible.

Bye
 Sven

Index: Completion/Core/_path_files
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Core/_path_files,v
retrieving revision 1.12
diff -u -r1.12 _path_files
--- Completion/Core/_path_files	2000/05/15 09:34:12	1.12
+++ Completion/Core/_path_files	2000/05/15 10:41:52
@@ -529,7 +529,7 @@
       cpre="${cpre}${tpre%%/*}/"
       tpre="${tpre#*/}"
     elif [[ "$tsuf" = */* ]]; then
-      mid="$testpath"
+      [[ "$tsuf" != /* ]] && mid="$testpath"
       cpre="${cpre}${tpre}/"
       tpre="${tsuf#*/}"
       tsuf=

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

PATCH: Re: Seg fault in matcher-list matching

[Sven Wischnowsky]

Bart Schaefer wrote:

> I did this:
> 
> zagzig[41] /u/s/l/z/z4/s/zsh
>                      ^cursor over the 4, press TAB
> 
> The path this was intended to match was /usr/src/local/zsh/zsh-2.4/src/zsh.
> I had first tried TAB at the end of the line and gotten a feep, in case
> that matters.

I couldn't get it to seg-fault, but there was something broken. Matching 
of the suffix, for example, and that both in C and shell code.


Oliver Kiddle wrote:

> ...
> 
> It's too late too look at this in any more detail but I've just found that
> I get a seg fault when I do this:
> diff  .c<tab>
>      ^cursor here.

I couldn't reproduce this (neither before nor after this patch -- your 
match specs would have helped), but this looks suspiciously similar to 
the one above... could you try with this patch?

Bye
 Sven

Index: Completion/Core/_path_files
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Core/_path_files,v
retrieving revision 1.11
diff -u -r1.11 _path_files
--- Completion/Core/_path_files	2000/05/02 08:18:54	1.11
+++ Completion/Core/_path_files	2000/05/15 09:25:35
@@ -6,7 +6,7 @@
 local linepath realpath donepath prepath testpath exppath skips skipped
 local tmp1 tmp2 tmp3 tmp4 i orig eorig pre suf tpre tsuf opre osuf cpre
 local pats haspats ignore pfxsfx remt sopt gopt opt sdirs ignpar
-local nm=$compstate[nmatches] menu matcher mopts sort match
+local nm=$compstate[nmatches] menu matcher mopts sort match mid
 
 typeset -U prepaths exppaths
 
@@ -529,6 +529,7 @@
       cpre="${cpre}${tpre%%/*}/"
       tpre="${tpre#*/}"
     elif [[ "$tsuf" = */* ]]; then
+      mid="$testpath"
       cpre="${cpre}${tpre}/"
       tpre="${tsuf#*/}"
       tsuf=
@@ -539,17 +540,32 @@
   done
 
   if [[ -z "$tmp4" ]]; then
-    if [[ "$osuf" = */* ]]; then
-      PREFIX="${opre}${osuf}"
-      SUFFIX=
-    else
+    if [[ "$mid" = */ ]]; then
       PREFIX="${opre}"
       SUFFIX="${osuf}"
+
+      tmp4="${testpath#${mid}}"
+      tmp3="${mid%/*/}"
+      tmp2="${${mid%/}##*/}"
+      compquote tmp4 tmp3 tmp2 tmp1
+      for i in "$tmp1[@]"; do
+        compadd -Qf "$mopts[@]" -p "$linepath$tmp3/" -s "/$tmp4$i" \
+                -W "$prepath$realpath${mid%/*/}/" \
+	        "$pfxsfx[@]" -M "r:|/=* r:|=*" - "$tmp2"
+      done
+    else
+      if [[ "$osuf" = */* ]]; then
+        PREFIX="${opre}${osuf}"
+        SUFFIX=
+      else
+        PREFIX="${opre}"
+        SUFFIX="${osuf}"
+      fi
+      tmp4="$testpath"
+      compquote tmp4 tmp1
+      compadd -Qf "$mopts[@]" -p "$linepath$tmp4" -W "$prepath$realpath$testpath" \
+	      "$pfxsfx[@]" -M "r:|/=* r:|=*" - "$tmp1[@]"
     fi
-    tmp4="$testpath"
-    compquote tmp4 tmp1
-    compadd -Qf "$mopts[@]" -p "$linepath$tmp4" -W "$prepath$realpath$testpath" \
-	    "$pfxsfx[@]" -M "r:|/=* r:|=*" - "$tmp1[@]"
   fi
 done
 
Index: Src/Zle/compmatch.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compmatch.c,v
retrieving revision 1.12
diff -u -r1.12 compmatch.c
--- Src/Zle/compmatch.c	2000/05/12 11:52:30	1.12
+++ Src/Zle/compmatch.c	2000/05/15 09:25:37
@@ -482,9 +482,10 @@
 	 */
 
 	bslash = 0;
-	if (test && (l[ind] == w[ind] ||
-		     (bslash = (lw > 1 && w[ind] == '\\' &&
-				(ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
+	if (test && !sfx &&
+	    (l[ind] == w[ind] ||
+	     (bslash = (lw > 1 && w[ind] == '\\' &&
+			(ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
 	    /* No matcher could be used, but the strings have the same
 	     * character here, skip over it. */
 	    l += add; w += (bslash ? (add + add ) : add);
@@ -562,10 +563,8 @@
 
 		    /* Fine, now we call ourselves recursively to find the
 		     * string matched by the `*'. */
-		    if (sfx) {
-			savl = l[-(llen + zoff)];
+		    if (sfx && (savl = l[-(llen + zoff)]))
 			l[-(llen + zoff)] = '\0';
-		    }
 		    for (t = 0, tp = w, ct = 0, ict = lw - alen + 1;
 			 ict;
 			 tp += add, ct++, ict--) {
@@ -585,11 +584,12 @@
 				!match_parts(l + aoff , tp - moff, alen, part))
 				break;
 			    if (sfx) {
-				savw = tp[-zoff];
-				tp[-zoff] = '\0';
+				if ((savw = tp[-zoff]))
+				    tp[-zoff] = '\0';
 				t = match_str(l - ll, w - lw,
 					      NULL, 0, NULL, 1, 2, part);
-				tp[-zoff] = savw;
+				if (savw)
+				    tp[-zoff] = savw;
 			    } else
 				t = match_str(l + llen + moff, tp + moff,
 					      NULL, 0, NULL, 0, 1, part);
@@ -598,7 +598,7 @@
 			}
 		    }
 		    ict = ct;
-		    if (sfx)
+		    if (sfx && savl)
 			l[-(llen + zoff)] = savl;
 
 		    /* Have we found a position in w where the rest of l
@@ -794,9 +794,10 @@
 	/* Same code as at the beginning, used in top-level calls. */
 
 	bslash = 0;
-	if (!test && (l[ind] == w[ind] ||
-		      (bslash = (lw > 1 && w[ind] == '\\' &&
-				 (ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
+	if ((!test || sfx) &&
+	    (l[ind] == w[ind] ||
+	     (bslash = (lw > 1 && w[ind] == '\\' &&
+			(ind ? (w[0] == l[0]) : (w[1] == l[0])))))) {
 	    /* No matcher could be used, but the strings have the same
 	     * character here, skip over it. */
 	    l += add; w += (bslash ? (add + add ) : add);
@@ -811,6 +812,8 @@
 	    lm = NULL;
 	    he = 0;
 	} else {
+	    if (!lw)
+		break;
 	    /* No matcher and different characters: l does not match w. */
 	    if (test)
 		return 0;
@@ -873,10 +876,15 @@
     char lsav = l[n], wsav = w[n];
     int ret;
 
-    l[n] = w[n] = '\0';
+    if (lsav)
+	l[n] = '\0';
+    if (wsav)
+	w[n] = '\0';
     ret = match_str(l, w, NULL, 0, NULL, 0, 1, part);
-    l[n] = lsav;
-    w[n] = wsav;
+    if (lsav)
+	l[n] = lsav;
+    if (wsav)
+	w[n] = wsav;
 
     return ret;
 }

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de