Re: [PATCH] Fix #87 - Segfault fault when autocompleting after ">" in, "!> ."

["Raphaël Jakse" <raphael.jakse@imag.fr>]

Bart, Peter,

Thanks for taking a look at it. Both of your patches seem more accurate 
than mine.
It would be nice if one of them could make it into zsh.

For your information, looking at this bug was / is part of my research. 
We try to combine monitoring techniques and interactive debugging. If 
you are interested, feel free to ask me questions and look at our our 
tool here: http://check-exec.forge.imag.fr/

Best,
Raphaël

Le 28/11/2016 à 17:57, Peter Stephenson a écrit :
> On Mon, 28 Nov 2016 14:06:54 +0100
> Raphaël Jakse <raphael.jakse@imag.fr> wrote:
>> Bug #87 in the SF bug tracker (which is not used anymore if I understood
>> correctly) claims that when hitting tab to complete after '>' in the
>> string "!> cmake ..", zsh crashes. I could reproduce the bug that also
>> appears with the string "!> .".
> Thanks, that's still the case and this is indeed a tricky piece of code.
> Sending patches here is the right thing to do.
>
> I think the following patch does a little better: I've tracked down the
> case where this is failing, and it's in handling something that looks
> like a redirection before the command word.  Here it looks a bit like a
> redirection because of the > but not enough because of the !.
>
> I haven't fully resolved redirection handling here (not sure exactly
> what this mixture should really do at this point), but I have made the
> particular aspect of it that was causing the problem a bit safer:
> because the code here didn't properly know about the redirection it
> thought it could reset the completion word when it found the command
> word.  But because the lexer had detected a redirection and we'd
> already passed the word where the cursor was, that was no longer safe.
> Detecting we've already found the completion word looks like an
> excellent reason for leaving the command line word array that we've
> built up so far alone whatever else is happening.
>
> I've also added a debug message if we find a string that's NULL, which
> should also make it safe in the way you intended but with debug info for
> developers if debugging is turned on.
>
> The middle hunk is a bit of extra safety / readability.  Possibly some
> extra DPUTS() would be a good idea there.
>
> With a bit of luck (we don't tend to get much in get_comp_string()), the
> condition is specific enough that this doesn't make anything else worse.
>
> pws
>
> diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
> index c8d3bb3..3d86791 100644
> --- a/Src/Zle/zle_tricky.c
> +++ b/Src/Zle/zle_tricky.c
> @@ -1305,8 +1305,12 @@ get_comp_string(void)
>   	    zsfree(cmdstr);
>   	    cmdstr = ztrdup(tokstr);
>   	    cmdtok = tok;
> -	    /* If everything before is a redirection, don't reset the index */
> -	    if (wordpos != redirpos)
> +	    /*
> +	     * If everything before is a redirection, or anything
> +	     * complicated enough that we've seen the word the
> +	     * cursor is on, don't reset the index.
> +	     */
> +	    if (wordpos != redirpos && clwpos == -1)
>   		wordpos = redirpos = 0;
>   	} else if (tok == SEPER) {
>   	    /*
> @@ -1414,9 +1418,17 @@ get_comp_string(void)
>   	/* If this is the word the cursor is in and we added a `x', *
>   	 * remove it.                                               */
>   	if (clwpos == wordpos++ && addedx) {
> +	    int chuck_at, word_diff;
>   	    zlemetacs_qsub = zlemetacs - qsub;
> -	    chuck(&clwords[wordpos - 1][((zlemetacs_qsub - wb) >= sl) ?
> -				 (sl - 1) : (zlemetacs_qsub - wb)]);
> +	    word_diff = zlemetacs_qsub - wb;
> +	    /* Ensure we chuck within the word... */
> +	    if (word_diff >= sl)
> +		chuck_at = sl -1;
> +	    else if (word_diff < 0)
> +		chuck_at = 0;
> +	    else
> +		chuck_at = word_diff;
> +	    chuck(&clwords[wordpos - 1][chuck_at]);
>   	}
>       } while (tok != LEXERR && tok != ENDINPUT &&
>   	     (tok != SEPER || (lexflags && tt0 == NULLTOK)));
> @@ -1464,7 +1476,9 @@ get_comp_string(void)
>   	t0 = STRING;
>       } else if (t0 == STRING || t0 == TYPESET) {
>   	/* We found a simple string. */
> -	s = ztrdup(clwords[clwpos]);
> +	s = clwords[clwpos];
> +	DPUTS(!s, "Completion word has disappeared!");
> +	s = ztrdup(s ? s : "");
>       } else if (t0 == ENVSTRING) {
>   	char sav;
>   	/* The cursor was inside a parameter assignment. */