crash/memory corruption when completing dynamic named directory

crash/memory corruption when completing dynamic named directory

[Mikael Magnusson]

I can't reproduce this in zsh -f yet, all that happens is that the
completion just fails quietly without doing anything.
% echo ネネネネネ ~[<tab>
results in
% echo <e3><ad><e3><ad><e3><ad><e3><ad><e3><ad> ~[

any idea what could be wrong or what I can try to narrow it down? The
problem doesn't seem to depend on the zsh_directory_name function, I
get the same result with it unfunctioned. Incidentally after the above
tab, zsh tries to complete parameter names.

-- 
Mikael Magnusson

Re: crash/memory corruption when completing dynamic named directory

[Johan Sundström]

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

On Sat, Mar 26, 2011 at 05:54, Mikael Magnusson <mikachu@gmail.com> wrote:

> I can't reproduce this in zsh -f yet, all that happens is that the
> completion just fails quietly without doing anything.
> % echo ネネネネネ ~[<tab>
> results in
> % echo <e3><ad><e3><ad><e3><ad><e3><ad><e3><ad> ~[
>
> any idea what could be wrong or what I can try to narrow it down?


Not an answer per se, but maybe bits of data giving someone else a lead: ネ
is Unicode code point 12493, or "\u30CD", which normalizes to itself in all
four Unicode normalization forms NFC, NFD, NFKC and NFKD, and which UTF-8
encodes to 0xE3, 0x83, 0x8D. I have no ideas about what the pretty-printing
or <ad>:s could be related to.

-- 
 / Johan Sundström, http://ecmanaut.blogspot.com/

The problem doesn't seem to depend on the zsh_directory_name function,
> I get the same result with it unfunctioned. Incidentally after the above
> tab, zsh tries to complete parameter names.
>
> --
> Mikael Magnusson
>

Re: crash/memory corruption when completing dynamic named directory

[Benjamin R. Haskell]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1389 bytes --]

On Sat, 26 Mar 2011, Johan Sundström wrote:

> On Sat, Mar 26, 2011 at 05:54, Mikael Magnusson wrote:
>
>> I can't reproduce this in zsh -f yet, all that happens is that the 
>> completion just fails quietly without doing anything.
>> % echo ネネネネネ ~[<tab>
>> results in
>> % echo <e3><ad><e3><ad><e3><ad><e3><ad><e3><ad> ~[
>>
>> any idea what could be wrong or what I can try to narrow it down?
>
>
> Not an answer per se, but maybe bits of data giving someone else a 
> lead: ネ is Unicode code point 12493, or "\u30CD", which normalizes to 
> itself in all four Unicode normalization forms NFC, NFD, NFKC and 
> NFKD, and which UTF-8 encodes to 0xE3, 0x83, 0x8D. I have no ideas 
> about what the pretty-printing or <ad>:s could be related to.
>

The problem is that the string of 'ne's is being unmetafy'ed when it 
shouldn't be.  I don't know where.  But that's what's happening.

Not sure if there's programmatic access to the metafy and unmetafy 
functions from within zsh.  I verified with the attached program.  To 
compile it, first compile zsh.  Then compile the test program via:

$ gcc -o zsh-metafy zsh-metafy.c -lm -lcurses /path/to/zsh/compilation/*.o~*/main.o
$ print -l ネネネネネ | ./zsh-metafy -u | myod
00000000 e3 ad e3 ad e3 ad e3 ad e3 ad 0a -- -- -- -- --  »..........¬«

(myod is, as the name implies, just 'my' version of 'od')

-- 
Best,
Ben

[-- Attachment #2: Type: APPLICATION/octet-stream, Size: 703449 bytes --]

Re: crash/memory corruption when completing dynamic named directory

[Mikael Magnusson]

2011/3/26 Benjamin R. Haskell <zsh@benizi.com>:
> On Sat, 26 Mar 2011, Johan Sundström wrote:
>
>> On Sat, Mar 26, 2011 at 05:54, Mikael Magnusson wrote:
>>
>>> I can't reproduce this in zsh -f yet, all that happens is that the
>>> completion just fails quietly without doing anything.
>>> % echo ネネネネネ ~[<tab>
>>> results in
>>> % echo <e3><ad><e3><ad><e3><ad><e3><ad><e3><ad> ~[
>>>
>>> any idea what could be wrong or what I can try to narrow it down?
>>
>>
>> Not an answer per se, but maybe bits of data giving someone else a lead: ネ
>> is Unicode code point 12493, or "\u30CD", which normalizes to itself in all
>> four Unicode normalization forms NFC, NFD, NFKC and NFKD, and which UTF-8
>> encodes to 0xE3, 0x83, 0x8D. I have no ideas about what the pretty-printing
>> or <ad>:s could be related to.
>>
>
> The problem is that the string of 'ne's is being unmetafy'ed when it
> shouldn't be.  I don't know where.  But that's what's happening.
>
> Not sure if there's programmatic access to the metafy and unmetafy functions
> from within zsh.  I verified with the attached program.  To compile it,
> first compile zsh.  Then compile the test program via:
>
> $ gcc -o zsh-metafy zsh-metafy.c -lm -lcurses
> /path/to/zsh/compilation/*.o~*/main.o
> $ print -l ネネネネネ | ./zsh-metafy -u | myod
> 00000000 e3 ad e3 ad e3 ad e3 ad e3 ad 0a -- -- -- -- --  »..........¬«
>
> (myod is, as the name implies, just 'my' version of 'od')

Would you believe that accessing $CURSOR from within a completer will
actually unmetafy zlemetaline in place? :)

static zlong
get_cursor(UNUSED(Param pm))
{
    if (zlemetaline != NULL) {
	/* A lot of work for one number, but still... */
	ZLE_STRING_T tmpline;
	int tmpcs, tmpll, tmpsz;
	tmpline = stringaszleline(zlemetaline, zlemetacs,
				  &tmpll, &tmpsz, &tmpcs);
	free(tmpline);
	return tmpcs;
    }
    return zlecs;
}

calling stringaszleline with zlemetaline only occurs in one other
place: unmetafy_line :).

pws, afaict, only the first little bit of zlemetaline is needed for
this, ie the outcs calculation. Would it be okay to split this out to
a separate function and call that from get_cursor and stringaszleline?
Otherwise we have to copy zlemetaline first and then run
stringaszleline, and discard all that work, which seems silly. Or does
outcs actually depend on the conversion too? There's a comment that
suggests it doesn't, but there are some assignments to it further
down.

-- 
Mikael Magnusson

Re: crash/memory corruption when completing dynamic named directory

[Mikael Magnusson]

On 27 March 2011 13:32, Mikael Magnusson <mikachu@gmail.com> wrote:
> 2011/3/26 Benjamin R. Haskell <zsh@benizi.com>:
>> On Sat, 26 Mar 2011, Johan Sundström wrote:
>>
>>> On Sat, Mar 26, 2011 at 05:54, Mikael Magnusson wrote:
>>>
>>>> I can't reproduce this in zsh -f yet, all that happens is that the
>>>> completion just fails quietly without doing anything.
>>>> % echo ネネネネネ ~[<tab>
>>>> results in
>>>> % echo <e3><ad><e3><ad><e3><ad><e3><ad><e3><ad> ~[
>>>>
>>>> any idea what could be wrong or what I can try to narrow it down?
>>
>> The problem is that the string of 'ne's is being unmetafy'ed when it
>> shouldn't be.  I don't know where.  But that's what's happening.
>
> calling stringaszleline with zlemetaline only occurs in one other
> place: unmetafy_line :).
>
> pws, afaict, only the first little bit of zlemetaline is needed for
> this, ie the outcs calculation. Would it be okay to split this out to
> a separate function and call that from get_cursor and stringaszleline?
> Otherwise we have to copy zlemetaline first and then run
> stringaszleline, and discard all that work, which seems silly. Or does
> outcs actually depend on the conversion too? There's a comment that
> suggests it doesn't, but there are some assignments to it further
> down.

Here's a patch for copying the string and doing all the work, which
I've confirmed fixes the issue.

diff --git a/Src/Zle/zle_params.c b/Src/Zle/zle_params.c
index 98091b2..2c35874 100644
--- a/Src/Zle/zle_params.c
+++ b/Src/Zle/zle_params.c
@@ -233,8 +233,10 @@ get_cursor(UNUSED(Param pm))
 	/* A lot of work for one number, but still... */
 	ZLE_STRING_T tmpline;
 	int tmpcs, tmpll, tmpsz;
-	tmpline = stringaszleline(zlemetaline, zlemetacs,
+	char *tmpmetaline = ztrdup(zlemetaline);
+	tmpline = stringaszleline(tmpmetaline, zlemetacs,
 				  &tmpll, &tmpsz, &tmpcs);
+	free(tmpmetaline);
 	free(tmpline);
 	return tmpcs;
     }


-- 
Mikael Magnusson

Re: crash/memory corruption when completing dynamic named directory

[Peter Stephenson]

On Sun, 27 Mar 2011 13:37:01 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> > pws, afaict, only the first little bit of zlemetaline is needed for
> > this, ie the outcs calculation. Would it be okay to split this out to
> > a separate function and call that from get_cursor and stringaszleline?
> > Otherwise we have to copy zlemetaline first and then run
> > stringaszleline, and discard all that work, which seems silly. Or does
> > outcs actually depend on the conversion too? There's a comment that
> > suggests it doesn't, but there are some assignments to it further
> > down.

I think that would be OK.  The focus of the conversion stuff when it was
written was to be effective rather than efficient, so there are no doubt
lots of optimisations of this sort.

> Here's a patch for copying the string and doing all the work, which
> I've confirmed fixes the issue.
> 
> diff --git a/Src/Zle/zle_params.c b/Src/Zle/zle_params.c
> index 98091b2..2c35874 100644
> --- a/Src/Zle/zle_params.c
> +++ b/Src/Zle/zle_params.c
> @@ -233,8 +233,10 @@ get_cursor(UNUSED(Param pm))
>  	/* A lot of work for one number, but still... */
>  	ZLE_STRING_T tmpline;
>  	int tmpcs, tmpll, tmpsz;
> -	tmpline = stringaszleline(zlemetaline, zlemetacs,
> +	char *tmpmetaline = ztrdup(zlemetaline);
> +	tmpline = stringaszleline(tmpmetaline, zlemetacs,
>  				  &tmpll, &tmpsz, &tmpcs);
> +	free(tmpmetaline);
>  	free(tmpline);
>  	return tmpcs;
>      }

Thanks, I'll commit this.

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: crash/memory corruption when completing dynamic named directory

[Bart Schaefer]

On Mar 27,  1:32pm, Mikael Magnusson wrote:
} Subject: Re: crash/memory corruption when completing dynamic named directo
}
} Would you believe that accessing $CURSOR from within a completer will
} actually unmetafy zlemetaline in place? :)

I don't disbelieve it.

} pws, afaict, only the first little bit of zlemetaline is needed for
} this, ie the outcs calculation. Would it be okay to split this out to
} a separate function and call that from get_cursor and stringaszleline?
} [...]  Or does outcs actually depend on the conversion too?

I see PWS already replied, but:  It appears to me that to compute outcs
it's necessary to unmetafy the line in the MULTIBYTE_SUPPORT case, so we
are probably stuck with your patch from 28941.

} Otherwise we have to copy zlemetaline first and then run
} stringaszleline, and discard all that work, which seems silly.

It'd probably be possible to write a routine that decodes metatifed
characters on the fly and counts what they decode to, without updating
the buffer in place, but that's not what's available now.