Subject: Re: Completion in empty double-quotes generates error
From: Daniel Shahaf <[email-masked]>
To: Bart Schaefer <[email-masked]>
Cc: [email-masked]
Date: Tue, 10 May 2016 18:40:31 +0000
List-Id: Zsh Workers List <zsh-workers.zsh.org>

Bart Schaefer wrote on Wed, Apr 06, 2016 at 12:10:53 -0700:
> On Apr 2, 11:18am, Bart Schaefer wrote:
> } Subject: Re: Completion in empty double-quotes generates error
> }
> } What would one expect to be completed with the cursor on the "2"?  The
> } -redirect- special context is for what comes *after* the operator.  I
> } can think of only two choices:
> } 
> } 1.  Complete as if there were an implicit space to the right of the
> }     cursor.  This makes some sense because "2>" is treated as the same
> }     single token as ">" all alone.
> } 
> } 2.  Treat it as an error and simply fail.
> } 
> } Patch below does this
> 
> More specifically, it attempted to do (1).  However, this situation is
> really nasty -- get_comp_string() repeatedly calls the lexer looking for
> the word containing the cursor, but because the lexer treats redirections
> as positionally insignficant, there literally is no word containing the
> cursor, so get_comp_string() keeps looking and finds the NEXT word (or
> the end of the input), which places both wb and we beyond zlemetacs.
> 
> It gets worse still with "{myfd}>", and worse again when completing in
> the whitespace between a word and a redirection (because in that last
> situation it's correct for the redirection token to be ignored).
> 
> I'm still not entirely sure that the below is correct, i.e. that the
> right things would happen if the "Should" comments were replaced with
> the appropriate actual code.  But this at least handles more cases than
> the previous situation.

'tmux new <TAB>' segfaults:

    $ zsh -f
    % echo $ZSH_PATCHLEVEL
    zsh-5.2-dev-1-130-g98f670c
    % autoload compinit
    % compinit
    % tmux new <TAB>5: compcore.c:1657: expecting 'x' at offset -7 of "x"
    
    Program received signal SIGSEGV, Segmentation fault.
    0x00007ffff5a030da in check_param (s=0x7ffff7f90188 "x", set=0, test=1) at compcore.c:1108
    1108		if (*p == String || *p == Qstring) {
    (gdb) p p
    $1 = 0x7ffff7f8ffff <error: Cannot access memory at address 0x7ffff7f8ffff>
    (gdb) bt
    #0  0x00007ffff5a030da in check_param (s=0x7ffff7f90188 "x", set=0, test=1) at compcore.c:1108
    #1  0x00007ffff5a0451f in set_comp_sep () at compcore.c:1677
    #2  0x00007ffff59fece5 in bin_compset (name=0x7ffff7f900d0 "compset", argv=0x7ffff7f900e0, ops=0x7ffffffe4160, func=0) at complete.c:1031

It bisects to 38248 (which I'm replying to).

I didn't have any tmux sessions/processes running while I was
reproducing the bug.

Cheers,

Daniel

P.S. With latest master, «sh -c <TAB>» also segfaults; on the other
hand, after «compdef _cmdstring f», «f <TAB>» doesn't segfault.

> The change in gotword() is necessary but of a little concern.  AFAICT
> we only care about wb when examining the word under the cursor, so it
> shouldn't hurt to skip updating it when doing so would violate the
> wb <= zlemetacs <= we constraint, but there may be some corner case I
> haven't tried that depends on the old behavior.
> 
> 
> diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
> index b1709c1..1d4e1d2 100644
> --- a/Src/Zle/zle_tricky.c
> +++ b/Src/Zle/zle_tricky.c
> @@ -1161,6 +1161,7 @@ get_comp_string(void)
>      inpush(dupstrspace(linptr), 0, NULL);
>      strinbeg(0);
>      wordpos = cp = rd = ins = oins = linarr = parct = ia = redirpos = 0;
> +    we = wb = zlemetacs;
>      tt0 = NULLTOK;
>  
>      /* This loop is possibly the wrong way to do this.  It goes through *
> @@ -1238,6 +1239,20 @@ get_comp_string(void)
>  	    /* Record if we haven't had the command word yet */
>  	    if (wordpos == redirpos)
>  		redirpos++;
> +	    if (zlemetacs < (zlemetall - inbufct) &&
> +		zlemetacs >= wordbeg && wb == we) {
> +		/* Cursor is in the middle of a redirection, treat as a word */
> +		we = zlemetall - (inbufct + addedx);
> +		if (addedx && we > wb) {
> +		    /* Assume we are in {param}> form, wb points at "{" */
> +		    wb++;
> +		    /* Should complete parameter names here */
> +		} else {
> +		    /* In "2>" form, zlemetacs points at "2" */
> +		    wb = zlemetacs;
> +		    /* Should insert a space under cursor here */
> +		}
> +	    }
>          }
>  	if (tok == DINPAR)
>  	    tokstr = NULL;
> diff --git a/Src/lex.c b/Src/lex.c
> index d4132fe..25b372a 100644
> --- a/Src/lex.c
> +++ b/Src/lex.c
> @@ -1789,9 +1789,13 @@ parse_subst_string(char *s)
>  static void
>  gotword(void)
>  {
> -    we = zlemetall + 1 - inbufct + (addedx == 2 ? 1 : 0);
> -    if (zlemetacs <= we) {
> -	wb = zlemetall - wordbeg + addedx;
> +    int nwe = zlemetall + 1 - inbufct + (addedx == 2 ? 1 : 0);
> +    if (zlemetacs <= nwe) {
> +	int nwb = zlemetall - wordbeg + addedx;
> +	if (zlemetacs >= nwb) {
> +	    wb = nwb;
> +	    we = nwe;
> +	}
>  	lexflags = 0;
>      }
>  }
>