crash in completing code with unicode support

crash in completing code with unicode support

[Mikael Magnusson]

I tried the recent developments with unicode support, and it's going
pretty well. I can write text in zle without it showing up as
^{saa'*^+{^ instaed of actual text. Backspacing over doublewidth
characters only steps back one cell, but i understand this has not
been worked on yet. One problem i have encountered though, is if i
have a couple file that starts with the same multibyte character (it
seems to be important that it is at the very beginning of the name),
and then different endings, zsh crashes when i try to tabcomplete
them. An obvious case where this happens is tabcompleting mp3 files
where the artist's name starts with Ø (a danish capital O with a /
through it, any of åäöæøÅÄÖÆØ seems to do the same thing too).
The simplest testcase i could find is to create an empty directory,
and in it creaty two files, Øa and Øb. Just typing ls <tab> gives ls
Ø, and the second <tab> crashes zsh (backtrace at the end of mail).
Zsh crashes both if in a singlebyte locale and a doublebyte locale,
the important thing is the actual filename is utf-8.

backtrace:
% touch Øa
% touch Øb
%  ls<tab>
Øa  Øb
% ls Ø<tab>
Program received signal SIGSEGV, Segmentation fault.
ztrsub (t=0x82badca "", s=0x82d6000 <Address 0x82d6000 out of bounds>)
at utils.c:3059
3059		if (*s++ == Meta) {
(gdb) bt
#0  ztrsub (t=0x82badca "", s=0x82d6000 <Address 0x82d6000 out of
bounds>) at utils.c:3059
#1  0x080c42a4 in pattryrefs (prog=0x82bb738, string=0x82badc8
"Ã\203", stringlen=2, unmetalen=-1,
    patoffset=0, nump=0x0, begp=0x0, endp=0x0) at pattern.c:1586
#2  0x080c4548 in pattry (prog=0x0, string=0x0) at pattern.c:1511
#3  0xa7d11f96 in do_comp_vars (test=184, na=1, sa=0xa7a27970 "\213",
nb=137080266,
    sb=0xa7a27970 "\213", mod=1) at complete.c:831
#4  0xa7d12334 in bin_compset (name=0xa7a27970 "\213",
argv=0xffffffe5, ops=0xaffee090, func=0)
    at complete.c:969
#5  0x08052fb4 in execbuiltin (args=0xaffee090, bn=0xa7d29ba4) at builtin.c:439
#6  0x0807054e in execcmd (state=0xafffddf0, input=0, output=0,
how=18, last1=2) at exec.c:2558
#7  0x08070e65 in execpline2 (state=0xafffddf0, pcode=0, how=18,
input=0, output=0, last1=0)
    at exec.c:1289
#8  0x08071433 in execpline (state=0xafffddf0, slcode=137191424,
how=18, last1=0) at exec.c:1075
#9  0x08072da3 in execlist (state=0xafffddf0, dont_change_job=1,
exiting=0) at exec.c:881
#10 0x08099440 in execif (state=0xafffddf0, do_exec=0) at loop.c:505
#11 0x0806f070 in execcmd (state=0xafffddf0, input=0, output=0,
how=18, last1=2) at exec.c:2505
#12 0x08070e65 in execpline2 (state=0xafffddf0, pcode=0, how=18,
input=0, output=0, last1=0)
    at exec.c:1289
#13 0x08071433 in execpline (state=0xafffddf0, slcode=137191424,
how=18, last1=0) at exec.c:1075
#14 0x08072da3 in execlist (state=0xafffddf0, dont_change_job=1,
exiting=0) at exec.c:881
#15 0x080994e5 in execif (state=0xafffddf0, do_exec=0) at loop.c:520
#16 0x0806f070 in execcmd (state=0xafffddf0, input=0, output=0, how=2,
last1=2) at exec.c:2505
#17 0x08070e65 in execpline2 (state=0xafffddf0, pcode=0, how=2,
input=0, output=0, last1=0)
    at exec.c:1289
#18 0x08071433 in execpline (state=0xafffddf0, slcode=137191424,
how=2, last1=0) at exec.c:1075
#19 0x08072da3 in execlist (state=0xafffddf0, dont_change_job=1,
exiting=0) at exec.c:881
#20 0x0807308a in runshfunc (prog=0x82b71d0, wrap=0x82b71d0,
name=0xa7a26030 "_main_complete")
    at exec.c:781
#21 0xa7d12d7a in comp_wrapper (prog=0x0, w=0x0, name=0x0) at complete.c:1328
#22 0x08073482 in doshfunc (name=0x81451f0 "_main_complete",
prog=0x82b71d0, doshargs=0xa7a26030,
    flags=0, noreturnval=0) at exec.c:3823
#23 0xa7d1a866 in makecomplist (s=0x82bc0b0 "Ã\203¸", incmd=0, lst=0)
at compcore.c:818
#24 0xa7d1b918 in do_completion (dummy=0xa7d66e74, dat=0x82d6000) at
compcore.c:347
#25 0xa7d58604 in docomplete (lst=0) at zle_tricky.c:1899
#26 0xa7d59bbf in completeword (args=0x0) at zle_tricky.c:232
#27 0xa7d540b9 in completecall (args=0x82d6000) at zle_tricky.c:208
#28 0xa7d475c5 in execzlefunc (func=0xa7d64330, args=0xa7d6715c) at
zle_main.c:1063
#29 0xa7d47938 in zlecore () at zle_main.c:838
#30 0xa7d483bd in zleread (lp=0x8101b10, rp=0x0, flags=0, context=0)
at zle_main.c:992
#31 0x0808a054 in inputline () at input.c:278
#32 0x0808a69a in ingetc () at input.c:214
#33 0x08080bc7 in ihgetc () at hist.c:240
#34 0x08096076 in gettok () at lex.c:628
#35 0x08097d36 in yylex () at lex.c:344
#36 0x080b9dfa in parse_event () at parse.c:451
#37 0x08086eea in loop (toplevel=1, justonce=0) at init.c:128
#38 0x08089c04 in zsh_main (argc=1, argv=0xafffebd4) at init.c:1315
#39 0x0805292e in main (argc=0, argv=0x0) at main.c:93

-- 
Mikael Magnusson

Re: crash in completing code with unicode support

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1626 bytes --]

On Tuesday 16 August 2005 05:37, Mikael Magnusson wrote:
> I tried the recent developments with unicode support, and it's going
> pretty well. I can write text in zle without it showing up as
> ^{saa'*^+{^ instaed of actual text. Backspacing over doublewidth
> characters only steps back one cell, but i understand this has not
> been worked on yet.

this works for me using UTF-8 Russian. Backward word has problems but backword 
char works. Or do you mean, characters with double display width? I am not 
aware of these in European languages?

> One problem i have encountered though, is if i 
> have a couple file that starts with the same multibyte character (it
> seems to be important that it is at the very beginning of the name),
> and then different endings, zsh crashes when i try to tabcomplete
> them. An obvious case where this happens is tabcompleting mp3 files
> where the artist's name starts with Ø (a danish capital O with a /
> through it, any of åäöæøÅÄÖÆØ seems to do the same thing too).

Does not crash here but looks pretty weird:

{pts/1}% pkg/bin/zsh -f
localhost% cd tmp/foo
localhost% autoload -U compinit; compinit
localhost% la Ф
\M-P\M-$1  \M-P\M-$2
localhost% ls
Ф1  Ф2

When I continue to complete it inserts correct string in command line so it is 
just listing that is wrong.

Using complist/menuselect works just nicely. Could you please try to reproduce 
it using zsh -f with the smallest set of extra options. And please, show 
UTF-8 (or UNICDOE) representation of characters that crash zsh and yuor OS 
and locale settings.

-andrey


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: crash in completing code with unicode support

[Clint Adams]

> localhost% la Ф
> \M-P\M-$1  \M-P\M-$2

setopt printeightbit

Re: crash in completing code with unicode support

[Mikael Magnusson]

On 8/16/05, Andrey Borzenkov <arvidjaar@newmail.ru> wrote:
> On Tuesday 16 August 2005 05:37, Mikael Magnusson wrote:
> > I tried the recent developments with unicode support, and it's going
> > pretty well. I can write text in zle without it showing up as
> > ^{saa'*^+{^ instaed of actual text. Backspacing over doublewidth
> > characters only steps back one cell, but i understand this has not
> > been worked on yet.
> 
> this works for me using UTF-8 Russian. Backward word has problems but backword
> char works. Or do you mean, characters with double display width? I am not
> aware of these in European languages?

No, they are single display width. Your russion character works fine
for me too, only crashes with the characters i listed.

> > One problem i have encountered though, is if i
> > have a couple file that starts with the same multibyte character (it
> > seems to be important that it is at the very beginning of the name),
> > and then different endings, zsh crashes when i try to tabcomplete
> > them. An obvious case where this happens is tabcompleting mp3 files
> > where the artist's name starts with Ø (a danish capital O with a /
> > through it, any of åäöæøÅÄÖÆØ seems to do the same thing too).
> 
> Does not crash here but looks pretty weird:
> 
> {pts/1}% pkg/bin/zsh -f
> localhost% cd tmp/foo
> localhost% autoload -U compinit; compinit
> localhost% la Ф
> \M-P\M-$1  \M-P\M-$2
> localhost% ls
> Ф1  Ф2
> 
> When I continue to complete it inserts correct string in command line so it is
> just listing that is wrong.
> 
> Using complist/menuselect works just nicely. Could you please try to reproduce
> it using zsh -f with the smallest set of extra options. And please, show
> UTF-8 (or UNICDOE) representation of characters that crash zsh and yuor OS
> and locale settings.

This should create the files i have,

mkdir /tmp/foo
cd /tmp/foo
a=`echo +ANg- | iconv -f utf-7 -t utf-8`
touch ${a}a ${a}b
zsh -f
autoload -U compinit; compinit
ls <tab><tab>

That is all i have to do to get the crash. (I didn't compile with
--enable-zsh-debug, because then i only get an error message like
"BUG: substring ends in the middle of a metachar in ztrsub()", instead
i specified the flags manually)

> -andrey

-- 
Mikael Magnusson

Re: crash in completing code with unicode support

[Nikolai Weibull]

Clint Adams wrote:

> > localhost% la Ф
> > \M-P\M-$1  \M-P\M-$2

> setopt printeightbit

Perhaps force this option on in UTF-8 locales?,
        nikolai

-- 
Nikolai Weibull: now available free of charge at http://bitwi.se/!
Born in Chicago, IL USA; currently residing in Gothenburg, Sweden.
main(){printf(&linux["\021%six\012\0"],(linux)["have"]+"fun"-97);}

Re: crash in completing code with unicode support

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 336 bytes --]

On Tuesday 16 August 2005 16:09, Nikolai Weibull wrote:
> Clint Adams wrote:
> > > localhost% la Ф
> > > \M-P\M-$1  \M-P\M-$2
> >
> > setopt printeightbit
>

blushed. I have it on from the day one so I just forgot about it existence,

> Perhaps force this option on in UTF-8 locales?,

Any reason it is not on by default?

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: crash in completing code with unicode support

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

On Tuesday 16 August 2005 16:01, Mikael Magnusson wrote:
>
> mkdir /tmp/foo
> cd /tmp/foo
> a=`echo +ANg- | iconv -f utf-7 -t utf-8`
> touch ${a}a ${a}b
> zsh -f
> autoload -U compinit; compinit
> ls <tab><tab>
>
> That is all i have to do to get the crash. (I didn't compile with
> --enable-zsh-debug, because then i only get an error message like
> "BUG: substring ends in the middle of a metachar in ztrsub()", instead
> i specified the flags manually)

I still cannot make it crash but I get BUG in ztrsub too.

The bug was always there it is just that it is much more difficult to create 
such case in single byte encoding. Does the following patch helps? (somewhat 
unexpectedly all regression tests still pass here)

Index: complete.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/complete.c,v
retrieving revision 1.28
diff -u -p -r1.28 complete.c
--- complete.c  21 Mar 2005 18:49:41 -0000      1.28
+++ complete.c  16 Aug 2005 19:24:00 -0000
@@ -821,8 +821,9 @@ do_comp_vars(int test, int na, char *sa,
                    add = -1;
                } else {
                    p = compprefix + 1;
+                   /* FIXME check EOS */
                    if (*p == Meta)
-                       p++;
+                       p += 2;
                    add = 1;
                }
                for (;;) {
@@ -835,6 +836,7 @@ do_comp_vars(int test, int na, char *sa,
                    if (add > 0) {
                        if (p == compprefix + l)
                            return 0;
+                       /* FIXME check EOS */
                        if (*p == Meta)
                            p += 2;
                        else

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: crash in completing code with unicode support

[Wayne Davison]

On Tue, Aug 16, 2005 at 11:27:49PM +0400, Andrey Borzenkov wrote:
>                     p = compprefix + 1;
>                     if (*p == Meta)
>                         p += 2;

I haven't tried to fully grok that code yet, but it looks to me like the
non-meta case results in just the character at compprefix being sent to
pattry(), so shouldn't the code really be checking if the compprefix
character is metified or not?

    p = compprefix + 1 + (*compprefix == Meta);

Also, the following check (from slightly lower down) needs to be changed
to handle the potential for the pointer to be bumped further forward:

    if (p == compprefix + l)

If the above is right, then this line should probably be changed to be:

    if (p == compprefix + l + (*compprefix == Meta))

..wayne..

Re: crash in completing code with unicode support

[Wayne Davison]

On Tue, Aug 16, 2005 at 02:37:13PM -0700, Wayne Davison wrote:
> If the above is right, then this line should probably be changed to be:
> 
>     if (p == compprefix + l + (*compprefix == Meta))

Oops, no -- that's an ell (l), not a one (1), so that change is wrong.
I'm sure now that the first change in my email was right, though, so
I went ahead and checked it in.

..wayne..

crash fixed, other minor issue though was: crash in completing code with unicode support

[Mikael Magnusson]

On 8/16/05, Wayne Davison <wayned@users.sourceforge.net> wrote:
> On Tue, Aug 16, 2005 at 02:37:13PM -0700, Wayne Davison wrote:
> > If the above is right, then this line should probably be changed to be:
> >
> >     if (p == compprefix + l + (*compprefix == Meta))
> 
> Oops, no -- that's an ell (l), not a one (1), so that change is wrong.
> I'm sure now that the first change in my email was right, though, so
> I went ahead and checked it in.
> 
> ..wayne..

I cvs update'd and now i'm not getting any more crashes. However, when
i make these things directories instead of files, cd <tab><tab>
doesn't add any trailing slashes. If i mkdir Øa Øb aa ab, then cd
<tab><tab><tab> and so on adds trailing slashes after aa and ab, but
not the first two. (with the same zsh -f sequence as earlier)

-- 
Mikael Magnusson

Re: crash in completing code with unicode support

[Peter Stephenson]

Andrey Borzenkov wrote:
> > setopt printeightbit
> 
> Any reason it is not on by default?

The test should be done by using iswprint(), although that may mean
yet another conversion.  It's probably harmless with UTF-8 but isn't
strictly the correct thing to do.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, 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.

**********************************************************************

Re: crash in completing code with unicode support

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]

On Wednesday 17 August 2005 01:49, Wayne Davison wrote:
> On Tue, Aug 16, 2005 at 02:37:13PM -0700, Wayne Davison wrote:
> > If the above is right, then this line should probably be changed to be:
> >
> >     if (p == compprefix + l + (*compprefix == Meta))
>

Of course I went blind.

The patch adds check for EOS, it intentionally leaves pointer at the end to 
let ztrsub() complaint later.

Index: Src/Zle/complete.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/complete.c,v
retrieving revision 1.29
diff -u -p -r1.29 complete.c
--- Src/Zle/complete.c  16 Aug 2005 21:49:33 -0000      1.29
+++ Src/Zle/complete.c  18 Aug 2005 16:30:36 -0000
@@ -821,6 +821,8 @@ do_comp_vars(int test, int na, char *sa,
                    add = -1;
                } else {
                    p = compprefix + 1 + (*compprefix == Meta);
+                   if (p > compprefix + l)
+                       p = compprefix + l;
                    add = 1;
                }
                for (;;) {
@@ -833,10 +835,9 @@ do_comp_vars(int test, int na, char *sa,
                    if (add > 0) {
                        if (p == compprefix + l)
                            return 0;
-                       if (*p == Meta)
-                           p += 2;
-                       else
-                           p++;
+                       p = p + 1 + (*p == Meta);
+                       if (p > compprefix + l)
+                           p = compprefix + l;
                    } else {
                        if (p == compprefix)
                            return 0;



[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: crash fixed, other minor issue though was: crash in completing code with unicode support

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 3454 bytes --]

On Wednesday 17 August 2005 04:22, Mikael Magnusson wrote:
> However, when
> i make these things directories instead of files, cd <tab><tab>
> doesn't add any trailing slashes. If i mkdir Øa Øb aa ab, then cd
> <tab><tab><tab> and so on adds trailing slashes after aa and ab, but
> not the first two. (with the same zsh -f sequence as earlier)

This makes ztat() unmetafy its argument; it effectively reverts 21631 (ztat() 
was called in several places). This fixes this issue for me (also some others 
like zstyle ignore-parents parent).

Index: Src/Zle/compcore.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compcore.c,v
retrieving revision 1.75
diff -u -p -r1.75 compcore.c
--- Src/Zle/compcore.c  16 Aug 2005 17:25:49 -0000      1.75
+++ Src/Zle/compcore.c  18 Aug 2005 16:44:07 -0000
@@ -2471,24 +2471,17 @@ add_match_data(int alt, char *str, char
     cm->modec = '\0';
     if ((flags & CMF_FILE) && orig[0] && orig[strlen(orig) - 1] != '/') {
         struct stat buf;
-       char *pb, *blah;
-       int blahl;
+       char *pb;

         pb = (char *) zhalloc((cm->prpre ? strlen(cm->prpre) : 0) +
                               3 + strlen(orig));
         sprintf(pb, "%s%s", (cm->prpre ? cm->prpre : "./"), orig);

-       blah = ztrdup(pb);
-
-       unmetafy(blah, &blahl);
-
-        if (!ztat(blah, &buf, 1)) {
+        if (!ztat(pb, &buf, 1)) {
             cm->mode = buf.st_mode;
             if ((cm->modec = file_type(buf.st_mode)) == ' ')
                 cm->modec = '\0';
         }
-
-       free(blah);
     }
     if ((*compqstack == '\\' && compqstack[1]) ||
        (autoq && *compqstack && compqstack[1] == '\\'))
Index: Src/Zle/compresult.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compresult.c,v
retrieving revision 1.56
diff -u -p -r1.56 compresult.c
--- Src/Zle/compresult.c        10 Aug 2005 19:51:30 -0000      1.56
+++ Src/Zle/compresult.c        18 Aug 2005 16:44:08 -0000
@@ -849,27 +849,33 @@ do_ambiguous(void)
  * parameter says if we have to do lstat() or stat().  I think this   *
  * should instead be done by use of a general function to expand a    *
  * filename (stripping backslashes), combined with the actual         *
- * (l)stat().                                                         */
+ * (l)stat().                                                         *
+ * Make sure input is unmetafied                                      */

 /**/
 mod_export int
 ztat(char *nam, struct stat *buf, int ls)
 {
-    if (!(ls ? lstat(nam, buf) : stat(nam, buf)))
-       return 0;
-    else {
-       char *p;
-       VARARR(char, b, strlen(nam) + 1);
+    int ret;

-       for (p = b; *nam; nam++)
-           if (*nam == '\\' && nam[1])
-               *p++ = *++nam;
+    nam = unmeta(nam);
+    if (!nam)
+       return -1;
+
+    if ((ret = ls ? lstat(nam, buf) : stat(nam, buf))) {
+       char *p, *q;
+
+       for (p = q = nam; *q; q++)
+           if (*q == '\\' && q[1])
+               *p++ = *++q;
            else
-               *p++ = *nam;
+               *p++ = *q;
        *p = '\0';

-       return ls ? lstat(b, buf) : stat(b, buf);
+       ret = ls ? lstat(nam, buf) : stat(nam, buf);
     }
+
+    return ret;
 }

 /* Insert all matches in the command line. */

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]