Re: PATCH: pattern incremental search

[Mikael Magnusson <mikachu@xxxxxxxxx>]

[forgot to change the ml address from sunsite.dk]

On 12/20/21, Mikael Magnusson <mikachu@xxxxxxxxx> wrote:
> On 4/26/08, Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
>> I think this is now good enough for some beta testing, though I'm sure
>> there must be glitches.  It adds
>> history-incremental-pattern-search-backward and
>> history-incremental-pattern-search-forkward which are ridiculously long
>> names but nothing shorter really fits in with the names we have already.
>>
>> The limitation that only non-overlapping pattern matches on the same
>> line are found comes from the parameter substitution code I hijacked to
>> do this.  It's not needed in this case since there's no substitution.
>> If anyone can see a good reason I can alter it.
>>
>> I've also finished the work of the previous patch that put history
>> searching back to using unmetafied strings: it was no longer necessary
>> to allocate additional space for each history line at all (unless it was
>> modified), so now it doesn't.  This should be a significant saving for
>> searching large histories.
>>
>> Index: Src/glob.c
>> ===================================================================
>> RCS file: /cvsroot/zsh/zsh/Src/glob.c,v
>> retrieving revision 1.61
>> diff -u -r1.61 glob.c
>> --- Src/glob.c	1 Nov 2007 17:57:57 -0000	1.61
>> +++ Src/glob.c	26 Apr 2008 19:40:43 -0000
>> @@ -2050,12 +2050,6 @@
>>  /* do the ${foo%%bar}, ${foo#bar} stuff */
>>  /* please do not laugh at this code. */
>>
>> -struct repldata {
>> -    int b, e;			/* beginning and end of chunk to replace */
>> -    char *replstr;		/* replacement string to use */
>> -};
>> -typedef struct repldata *Repldata;
>> -
>>  /* Having found a match in getmatch, decide what part of string
>>   * to return.  The matched part starts b characters into string s
>>   * and finishes e characters in: 0 <= b <= e <= strlen(s)
>> @@ -2073,19 +2067,23 @@
>>      char buf[80], *r, *p, *rr;
>>      int ll = 0, l = strlen(s), bl = 0, t = 0, i;
>>
>> -    if (replstr) {
>> +    if (replstr || (fl & SUB_LIST)) {
>>  	if (fl & SUB_DOSUBST) {
>>  	    replstr = dupstring(replstr);
>>  	    singsub(&replstr);
>>  	    untokenize(replstr);
>>  	}
>> -	if ((fl & SUB_GLOBAL) && repllist) {
>> +	if ((fl & (SUB_GLOBAL|SUB_LIST)) && repllist) {
>>  	    /* We are replacing the chunk, just add this to the list */
>> -	    Repldata rd = (Repldata) zhalloc(sizeof(*rd));
>> +	    Repldata rd = (Repldata)
>> +		((fl & SUB_LIST) ? zalloc(sizeof(*rd)) : zhalloc(sizeof(*rd)));
>>  	    rd->b = b;
>>  	    rd->e = e;
>>  	    rd->replstr = replstr;
>> -	    addlinknode(repllist, rd);
>> +	    if (fl & SUB_LIST)
>> +		zaddlinknode(repllist, rd);
>> +	    else
>> +		addlinknode(repllist, rd);
>>  	    return s;
>>  	}
>>  	ll += strlen(replstr);
>
> Someone in the irc channel reported a crash on this strlen when doing
> history-incremental-pattern-search-backward with any search, and they
> can reproduce it with the latest git version too, they posted this
> backtrace:
>
> Program received signal SIGSEGV, Segmentation fault.
> __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
> 65              VPCMPEQ (%rdi), %ymm0, %ymm1
> (gdb) back
> #0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
> #1  0x0000000000436eaa in get_match_ret (imd=imd@entry=0x7fffffffd460,
>     b=<optimized out>, e=0) at glob.c:2571
> #2  0x000000000043742f in igetmatch (sp=sp@entry=0x7fffffffd4d8,
>     p=<optimized out>, fl=fl@entry=8710, n=<optimized out>, n@entry=0,
>     replstr=replstr@entry=0x0, repllistp=<optimized out>) at glob.c:3309
> #3  0x000000000043d7b0 in getmatchlist (str=<optimized out>,
>     p=<optimized out>, repllistp=<optimized out>) at glob.c:2743
> #4  0x00007ffff774d4f6 in ?? () from /usr/lib64/zsh/5.8.0.2-dev/zsh/zle.so
> #5  0x0000000000000001 in ?? ()
> #6  0x0000000000000000 in ?? ()
>
> I'm not very familiar with this code, but it looks like this patch
> allows going into this path with a NULL replstr on purpose, but it
> also looks like it shouldn't work... I never had any issues with it
> before though, and I do use pattern search on ^R.
>
> --
> Mikael Magnusson