* Re: Segfault on completion with interactive mode
2016-02-10 16:40 Segfault on completion with interactive mode Óscar García Amor
@ 2016-02-10 20:03 ` Bart Schaefer
2016-02-11 4:01 ` Bart Schaefer
2016-02-13 22:28 ` Bart Schaefer
2 siblings, 0 replies; 6+ messages in thread
From: Bart Schaefer @ 2016-02-10 20:03 UTC (permalink / raw)
To: Óscar García Amor; +Cc: Zsh hackers list
[-- Attachment #1: Type: text/plain, Size: 651 bytes --]
On Feb 10, 2016 8:40 AM, "Óscar García Amor" <ogarcia@connectical.com>
wrote:
>
> 4. Press tab to show options (interactive mode appears)
> 5. Write a letter that not mach with any option (to generate a completion
> warning)
> 6. Press backspace
>
> Tachan! zsh: segmentation fault (core dumped)
Hmm. So, complistmatches() is called with an empty list of completions to
cause the message to be displayed. It's then called again on backspace to
erase the message and draw a new list, but it chokes on the empty listing.
I can stop the coredump by doing nothing when the list is empty, but then
the message isn't displayed either.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Segfault on completion with interactive mode
2016-02-10 16:40 Segfault on completion with interactive mode Óscar García Amor
2016-02-10 20:03 ` Bart Schaefer
@ 2016-02-11 4:01 ` Bart Schaefer
2016-02-11 12:49 ` Peter Stephenson
2016-02-13 22:28 ` Bart Schaefer
2 siblings, 1 reply; 6+ messages in thread
From: Bart Schaefer @ 2016-02-11 4:01 UTC (permalink / raw)
To: zsh-workers
[ Long diagnosis follows; read just the first and last paragraphs for
the short version. ]
I think the problem is that "compadd -x" (which is used to show the
warnings) updates the current group of completions to have an empty
match with the warning as its explanation string. Because interactive
mode is doing its own loop on getkeycmd(), the listing data is not
restored after "compadd -x" clobbers it, and so the next time around
the interactive mode loop, its idea of what is in the set of matches
does not correspond, and chaos ensues.
It comes down to this block of code in domenuselect():
if (nmessages) {
showinglist = -2;
zrefresh();
} else {
trashzle();
zsetterm();
if (tccan(TCCLEAREOD))
tcout(TCCLEAREOD);
fputs("no matches\r", shout);
fflush(shout);
tcmultout(TCUP, TCMULTUP, nlnct);
showinglist = clearlist = 0;
clearflag = 1;
zrefresh();
showinglist = clearlist = 0;
}
statusline = NULL;
goto getk;
And earlier:
getk:
if (!do_last_key) {
zmult = 1;
cmd = getkeycmd();
/*
* On interrupt, we'll exit due to cmd being empty.
* Don't propagate the interrupt any further, which
* can screw up redrawing.
*/
errflag &= ~ERRFLAG_INT;
if (mtab_been_reallocated) {
do_last_key = 1;
continue;
}
}
do_last_key = 0;
When (nmessages) is true, we jump to getk:, do_last_key is false so
we arrive at getkeycmd(), and mtab_been_reallocated is true so after
reading the delete-char we continue back to the top of the "for (;;)"
loop and finally crash inside the call to zrefresh() at line 2550 --
which I *think* is intended to redraw the old list before highlighing
the desired item within it, but the old list has been replaced by the
warning message.
If there are no matches and (nmessages == 0), we go through the other
branch [beginning with trashzle()]. Thereafter everything is the
same except that we successfully redraw the list in zrefresh() and
go on as we were.
If showinglist is set to anything other than -2 in the (nmessages)
true branch, there's no crash but the messages aren't shown either.
If the list is invalidated (force listdat.valid = 0) then there also
isn't a crash but menu selection exits.
The thing is, I think domenuselect() has pushed all the right things
onto its linked list of Menustack structures, it just hasn't ever
restored it again. But I'm not sure and I haven't figured out the
right place to restore it if so. Other eyeballs?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Segfault on completion with interactive mode
2016-02-11 4:01 ` Bart Schaefer
@ 2016-02-11 12:49 ` Peter Stephenson
2016-02-12 10:17 ` Bart Schaefer
0 siblings, 1 reply; 6+ messages in thread
From: Peter Stephenson @ 2016-02-11 12:49 UTC (permalink / raw)
To: zsh-workers
What I saw when I tried this was a crash in singledraw() because
mgtab[0] was 1, suggesting it as a "marked" null pointer. So presumably
this state is incompatible with state required when we call
singledraw(),
if (!mnew && inselect && onlnct == nlnct && mlbeg >= 0 && mlbeg == molbeg)
singledraw();
This appears to be a lot later than anything you're talking about.
Preumably that pile of stuff in complistmatches inside "if (mnew) {" is
the key to what should be consistent. It sounds like in the code you're
talking about, that code had just run, because mtab_been_reallocated was
1, whereas this time round, when it crashes, it hadn't.
("mtab_been_reallocated" seems to be associated with the 'if you've got
global variables you don't really need a calling convention' programming
style.)
So at the level I'm looking at it, it looks vaguely like it's in a state
as if "mnew" has just been set, in which case we'd skip the
singledraw(), and somehow we've got back too soon before all the stuff
that happens after to fix it up, whatever that is.
Anyway, the summary is I've completely failed to understand even the
most basic points of the logic that sets up the completion list
consistently.
pws
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Segfault on completion with interactive mode
2016-02-11 12:49 ` Peter Stephenson
@ 2016-02-12 10:17 ` Bart Schaefer
0 siblings, 0 replies; 6+ messages in thread
From: Bart Schaefer @ 2016-02-12 10:17 UTC (permalink / raw)
To: zsh-workers
On Feb 11, 12:49pm, Peter Stephenson wrote:
} Subject: Re: Segfault on completion with interactive mode
}
} What I saw when I tried this was a crash in singledraw() because
} mgtab[0] was 1, suggesting it as a "marked" null pointer. So presumably
} this state is incompatible with state required when we call
} singledraw(),
Yes, I saw that as well.
} This appears to be a lot later than anything you're talking about.
The long description in my previous message was the result of several
(dozen) passes trying to track down WHY we arrive in singledraw() with
mgtab[0] = 1 in the case of
zstyle ':completion:*:warnings' format '%BNo matching %b%d'
when the analogous case WITHOUT that style works exactly as designed.
The answer to "why?" appears to be that "compadd -x" has clobbered the
completion groups when displaying the warning. Thus I believe that we
somehow need to save and restore state around the "compadd -x". The
right place to do that seems to be in domenuselect() in the branch
that uses showinglist = -2, because it's that call to zrefresh() that
ultimately calls the hookdef that eventually does the "compadd -x".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Segfault on completion with interactive mode
2016-02-10 16:40 Segfault on completion with interactive mode Óscar García Amor
2016-02-10 20:03 ` Bart Schaefer
2016-02-11 4:01 ` Bart Schaefer
@ 2016-02-13 22:28 ` Bart Schaefer
2 siblings, 0 replies; 6+ messages in thread
From: Bart Schaefer @ 2016-02-13 22:28 UTC (permalink / raw)
To: zsh-workers
This seems to do it. It may be possible to further reconcile the
(nmessages) branch with the print-our-own "no matches" branch, but
I haven't tried yet. The idea is:
- there's no match for the current input, so there's nothing to be
selected in the previously displayed set of possible matches
- the listing has been trashed anyway by displaying the warning
- so, don't bother attempting to redraw or to find the (nonexistent)
selected item in the (trashed) list until after the next keystroke
has been processed
- at which point the original listing has been redrawn and all is well.
diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c
index 937e1d1..162436b 100644
--- a/Src/Zle/complist.c
+++ b/Src/Zle/complist.c
@@ -34,8 +34,9 @@
/* Information about the list shown. */
/*
- * noselect: 1 if complistmatches indicated we shouldn't do selection.
- * Tested in domenuselect.
+ * noselect: 1 if complistmatches indicated we shouldn't do selection;
+ * -1 if interactive mode needs to reset the selection list.
+ * Tested in domenuselect, and in complistmatches to skip redraw.
* mselect: Local copy of the index of the currently selected match.
* Initialised to the gnum entry of the current match for
* each completion.
@@ -1980,7 +1981,8 @@ complistmatches(UNUSED(Hookdef dummy), Chdata dat)
}
#endif
- noselect = 0;
+ if (noselect > 0)
+ noselect = 0;
if ((minfo.asked == 2 && mselect < 0) || nlnct >= zterm_lines) {
showinglist = 0;
@@ -2078,9 +2080,11 @@ complistmatches(UNUSED(Hookdef dummy), Chdata dat)
last_cap = (char *) zhalloc(max_caplen + 1);
*last_cap = '\0';
- if (!mnew && inselect && onlnct == nlnct && mlbeg >= 0 && mlbeg == molbeg)
- singledraw();
- else if (!compprintlist(mselect >= 0) || !clearflag)
+ if (!mnew && inselect &&
+ onlnct == nlnct && mlbeg >= 0 && mlbeg == molbeg) {
+ if (!noselect)
+ singledraw();
+ } else if (!compprintlist(mselect >= 0) || !clearflag)
noselect = 1;
onlnct = nlnct;
@@ -2093,7 +2097,7 @@ complistmatches(UNUSED(Hookdef dummy), Chdata dat)
popheap();
opts[EXTENDEDGLOB] = extendedglob;
- return noselect;
+ return (noselect < 0 ? 0 : noselect);
}
static int
@@ -2547,14 +2551,23 @@ domenuselect(Hookdef dummy, Chdata dat)
} else {
statusline = NULL;
}
+ if (noselect < 0) {
+ showinglist = clearlist = 0;
+ clearflag = 1;
+ }
zrefresh();
statusline = NULL;
inselect = 1;
+ selected = 1;
if (noselect) {
+ if (noselect < 0) {
+ /* no selection until after processing keystroke */
+ noselect = 0;
+ goto getk;
+ }
broken = 1;
break;
}
- selected = 1;
if (!i) {
i = mcols * mlines;
while (i--)
@@ -2752,6 +2765,7 @@ domenuselect(Hookdef dummy, Chdata dat)
if (nmessages) {
showinglist = -2;
zrefresh();
+ noselect = -1;
} else {
trashzle();
zsetterm();
^ permalink raw reply [flat|nested] 6+ messages in thread