[PATCH] fix core dump by reverse-menu-complete

[Jun T <takimoto-j@xxxxxxxxxxxxxxxxx>]

The patch is at the end of this post, but I will not push it
unless it gets some positive feedback.

Steps to reproduce the core dump:

$ zsh -f
% autoload -Uz compinit
% compinit
% zstyle ':completion:*' verbose true
% bindkey '^K' menu-complete
% bindkey '^L' reverse-menu-complete
% AAa=foo; AAb=foo
% echo $AA<^L>

Hitting ^K works, but ^L causes core dump.

Backtrace:
  * frame #0: 0x00007ff80fb28f12 libsystem_platform.dylib`_platform_strlen + 18
    frame #1: 0x000000010315a0c5 complete.so`do_single(m=0x0000600000adc5a0) at compresult.c:1069:7
    frame #2: 0x000000010315ac6b complete.so`do_ambig_menu at compresult.c:1429:9
    frame #3: 0x0000000103159609 complete.so`do_ambiguous at compresult.c:773:2
(--snip--)
(lldb) up
frame #1: 0x000000010315a0c5 complete.so`do_single(m=0x0000600000adc5a0) at compresult.c:1069:7
   1066 
   1067			    p = (char *) zhalloc(strlen((m->flags & CMF_ISPAR) ?
   1068							parpre : m->ripre) +
-> 1069						 strlen(str) + 2);
   1070			    sprintf(p, "%s%s%c",
   1071				    ((m->flags & CMF_ISPAR) ? parpre : m->ripre), str,
   1072				    ((m->flags & CMF_PARBR) ? '}' : '\0'));
(lldb) p str
(char *) 0x0000000000000000

'str' is set (at compresult.c:968) to the (*mc)->orig of the caller:

(lldb) up
frame #2: 0x000000010315ac6b complete.so`do_ambig_menu at compresult.c:1429:9
   1426	#endif
   1427	    mc = (minfo.group)->matches + insmnum;
   1428	    if (iforcemenu != -1)
-> 1429	        do_single(*mc);
   1430	    minfo.cur = mc;
   1431	}
   1432	
(lldb) p **mc
(cmatch) {
  str = 0x00006000039c1560 ""
  orig = 0x0000000000000000
  disp = 0x0000600001af4b90 "-- foo                                                              "
  flags = 16900
  (--snip--)
}

'flags' has the CMF_DUMMY bit, indicating that this is a dummy match
for handling the duplicated 'disp' (display string).

I found a loop for skipping "unusable" matches in do_menucmp(), lines
1230-1249 of compresult.c. In the patch below, I just blindly moved
the loop into a new function valid_match() and used it in do_menucmp()
and do_ambig_menu(). It seems to work, but I don't understand the
tests in the loop, especially
"(menuacc && !hasbrpsfx(*m, minfo.prebr, minfo.postbr))".

Related question:
zshcompsys(1) says:
   verbose
      If set, as it is by default, the completion listing is more
      verbose. ..
But if I don't use "zstyle ':completion:*' verbose true" then I get
no core dump. Is the verbose style set by default?


diff --git a/Src/Zle/compresult.c b/Src/Zle/compresult.c
index 46de4466f..5b0889523 100644
--- a/Src/Zle/compresult.c
+++ b/Src/Zle/compresult.c
@@ -1201,6 +1201,48 @@ do_single(Cmatch m)
     }
 }
 
+/*
+ * get a "valid" (= need not be skipped) match from minfo.group.
+ * if next==1, get the next valid match even if *m is already valid.
+ */
+
+static Cmatch*
+valid_match(Cmatch* m, int next)
+{
+    while (next ||
+	    (menuacc && !hasbrpsfx(*m, minfo.prebr, minfo.postbr)) ||
+	    ((*m)->flags & CMF_DUMMY) ||
+	    (((*m)->flags & (CMF_NOLIST | CMF_MULT)) &&
+	    (!(*m)->str || !*(*m)->str))) {
+	if (zmult > 0) {
+	    if (!*++(m)) {
+		do {
+		    if (!(minfo.group = (minfo.group)->next)) {
+			minfo.group = amatches;
+#ifdef ZSH_HEAP_DEBUG
+			if (memory_validate(minfo.group->heap_id)) {
+			    HEAP_ERROR(minfo.group->heap_id);
+			}
+#endif
+		    }
+		} while (!(minfo.group)->mcount);
+		m = minfo.group->matches;
+	    }
+	} else {
+	    if (m == (minfo.group)->matches) {
+		do {
+		    if (!(minfo.group = (minfo.group)->prev))
+			minfo.group = lmatches;
+		} while (!(minfo.group)->mcount);
+		m = (minfo.group)->matches + (minfo.group)->mcount - 1;
+	    } else
+		m--;
+	}
+	next = 0;
+    }
+    return m;
+}
+
 /* Do completion, given that we are in the middle of a menu completion.  We *
  * don't need to generate a list of matches, because that's already been    *
  * done by previous commands.  We will either list the completions, or      *
@@ -1227,36 +1269,7 @@ do_menucmp(int lst)
 
     /* Otherwise go to the next match in the array... */
     while (zmult) {
-	do {
-	    if (zmult > 0) {
-		if (!*++(minfo.cur)) {
-		    do {
-			if (!(minfo.group = (minfo.group)->next)) {
-			    minfo.group = amatches;
-#ifdef ZSH_HEAP_DEBUG
-			    if (memory_validate(minfo.group->heap_id)) {
-				HEAP_ERROR(minfo.group->heap_id);
-			    }
-#endif
-			}
-		    } while (!(minfo.group)->mcount);
-		    minfo.cur = minfo.group->matches;
-		}
-	    } else {
-		if (minfo.cur == (minfo.group)->matches) {
-		    do {
-			if (!(minfo.group = (minfo.group)->prev))
-			    minfo.group = lmatches;
-		    } while (!(minfo.group)->mcount);
-		    minfo.cur = (minfo.group)->matches + (minfo.group)->mcount - 1;
-		} else
-		    minfo.cur--;
-	    }
-	} while ((menuacc &&
-		!hasbrpsfx(*(minfo.cur), minfo.prebr, minfo.postbr)) ||
-		((*minfo.cur)->flags & CMF_DUMMY) ||
-		(((*minfo.cur)->flags & (CMF_NOLIST | CMF_MULT)) &&
-		(!(*minfo.cur)->str || !*(*minfo.cur)->str)));
+	minfo.cur = valid_match(minfo.cur, 1);
 	zmult -= (0 < zmult) - (zmult < 0);
     }
     /* ... and insert it into the command line. */
@@ -1424,7 +1437,7 @@ do_ambig_menu(void)
 	/* group-numbers in compstate[insert] */
     }
 #endif
-    mc = (minfo.group)->matches + insmnum;
+    mc = valid_match((minfo.group)->matches + insmnum, 0);
     if (iforcemenu != -1)
         do_single(*mc);
     minfo.cur = mc;