Re: zsh 5.3.1 crashes on completion

[Peter Stephenson <p.stephenson@samsung.com>]

On Fri, 26 May 2017 01:01:16 +0900
Jun T. <takimoto-j@kba.biglobe.ne.jp> wrote:
> With the latest git master, expand-history doesn't work correctly:
> 
> [1]% echo one
> [2]% echo two
> [3]% !!<TAB>   (or the key bound to expand-history)
> 
> then !! expands to 'echo one' (!!<Return> works as expected).

There are two obvious ways to go here.

First, link and unlink the current history line into the ring at
additional points when doing interactive expansion or completion.
Although this might well be safer in terms of structure it's quite hard
to track down the right points.  I haven't looked further.

Second, instead of using link / unlink, directly mark the current
history in some way so that it doesn't get freed.  As curline's not on
the saved history stack, the assumption (whether safe or not is an
entirely different matter) is that it's unique, i.e. there are not going
to be different notions of it at different lexical level. In that case
simply comparing against it so we don't free it will fix the immediate
problem.  This doesn't keep the structure at different nesting levels
as separate, but it looks like we can't. Here's a possible minimal fix
to the original problem based on that --- that means I really don't
know if there are other ramifications that may turn up, but it looks
like we need to track them down one by one rather than assuming
structure the shell doesn't have.

The first change is the actual fix, making the DPUTS as well as
the other new framework redundant.

pws

diff --git a/Src/hashtable.c b/Src/hashtable.c
index c34744c..6ec2ed2 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -1445,10 +1445,12 @@ freehistdata(Histent he, int unlink)
     if (!he)
 	return;
 
+    if (he == &curline)
+	return;
+
     if (!(he->node.flags & (HIST_DUP | HIST_TMPSTORE)))
 	removehashnode(histtab, he->node.nam);
 
-    DPUTS(he->node.nam == chline, "Attempt to free chline in history data");
     zsfree(he->node.nam);
     if (he->nwords)
 	zfree(he->words, he->nwords*2*sizeof(short));
diff --git a/Src/hist.c b/Src/hist.c
index 4c1039b..350688d 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -87,9 +87,6 @@ mod_export zlong curhist;
 /**/
 struct histent curline;
 
-/***/
-int curline_linked;
-
 /* current line count of allocated history entries */
 
 /**/
@@ -264,9 +261,6 @@ hist_context_save(struct hist_stack *hs, int toplevel)
      */
     hs->cstack = cmdstack;
     hs->csp = cmdsp;
-    hs->curline_linked = curline_linked;
-
-    unlinkcurline();
 
     stophist = 0;
     chline = NULL;
@@ -306,9 +300,6 @@ hist_context_restore(const struct hist_stack *hs, int toplevel)
 	zfree(cmdstack, CMDSTACKSZ);
     cmdstack = hs->cstack;
     cmdsp = hs->csp;
-    unlinkcurline();
-    if (hs->curline_linked)
-	linkcurline();
 }
 
 /*
@@ -999,7 +990,6 @@ nohwe(void)
 
 /* these functions handle adding/removing curline to/from the hist_ring */
 
-/**/
 static void
 linkcurline(void)
 {
@@ -1012,15 +1002,11 @@ linkcurline(void)
 	hist_ring = &curline;
     }
     curline.histnum = ++curhist;
-    curline_linked = 1;
 }
 
-/**/
 static void
 unlinkcurline(void)
 {
-    if (!curline_linked)
-	return;
     curline.up->down = curline.down;
     curline.down->up = curline.up;
     if (hist_ring == &curline) {
@@ -1030,7 +1016,6 @@ unlinkcurline(void)
 	    hist_ring = curline.up;
     }
     curhist--;
-    curline_linked = 0;
 }
 
 /* initialize the history mechanism */
@@ -1050,7 +1035,6 @@ hbegin(int dohist)
 	stophist = (!interact || unset(SHINSTDIN)) ? 2 : 0;
     else
 	stophist = 0;
-    DPUTS(chline != NULL, "chline set at start of history");
     /*
      * pws: We used to test for "|| (inbufflags & INP_ALIAS)"
      * in this test, but at this point we don't have input
@@ -1308,10 +1292,11 @@ putoldhistentryontop(short keep_going)
 Histent
 prepnexthistent(void)
 {
-    Histent he;
-    int relink_curline = curline_linked;
+    Histent he; 
+    int curline_in_ring = hist_ring == &curline;
 
-    unlinkcurline();
+    if (curline_in_ring)
+	unlinkcurline();
     if (hist_ring && hist_ring->node.flags & HIST_TMPSTORE) {
 	curhist--;
 	freehistnode(&hist_ring->node);
@@ -1335,7 +1320,7 @@ prepnexthistent(void)
 	he = hist_ring;
     }
     he->histnum = ++curhist;
-    if (relink_curline)
+    if (curline_in_ring)
 	linkcurline();
     return he;
 }
@@ -1410,7 +1395,8 @@ hend(Eprog prog)
     queue_signals();
     if (histdone & HISTFLAG_SETTY)
 	settyinfo(&shttyinfo);
-    unlinkcurline();
+    if (!(histactive & HA_NOINC))
+	unlinkcurline();
     if (histactive & HA_NOINC) {
 	zfree(chline, hlinesz);
 	zfree(chwords, chwordlen*sizeof(short));
@@ -3638,7 +3624,7 @@ int
 pushhiststack(char *hf, zlong hs, zlong shs, int level)
 {
     struct histsave *h;
-    int relink_curline = curline_linked;
+    int curline_in_ring = (histactive & HA_ACTIVE) && hist_ring == &curline;
 
     if (histsave_stack_pos == histsave_stack_size) {
 	histsave_stack_size += 5;
@@ -3646,7 +3632,8 @@ pushhiststack(char *hf, zlong hs, zlong shs, int level)
 			    histsave_stack_size * sizeof (struct histsave));
     }
 
-    unlinkcurline();
+    if (curline_in_ring)
+	unlinkcurline();
 
     h = &histsave_stack[histsave_stack_pos++];
 
@@ -3681,7 +3668,7 @@ pushhiststack(char *hf, zlong hs, zlong shs, int level)
     savehistsiz = shs;
     inithist(); /* sets histtab */
 
-    if (relink_curline)
+    if (curline_in_ring)
 	linkcurline();
 
     return histsave_stack_pos;
@@ -3693,12 +3680,13 @@ int
 pophiststack(void)
 {
     struct histsave *h;
-    int relink_curline = curline_linked;
+    int curline_in_ring = (histactive & HA_ACTIVE) && hist_ring == &curline;
 
     if (histsave_stack_pos == 0)
 	return 0;
 
-    unlinkcurline();
+    if (curline_in_ring)
+	unlinkcurline();
 
     deletehashtable(histtab);
     zsfree(lasthist.text);
@@ -3721,7 +3709,7 @@ pophiststack(void)
     histsiz = h->histsiz;
     savehistsiz = h->savehistsiz;
 
-    if (relink_curline)
+    if (curline_in_ring)
 	linkcurline();
 
     return histsave_stack_pos + 1;
diff --git a/Src/zsh.h b/Src/zsh.h
index 405b274..22f73f8 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2931,7 +2931,6 @@ struct hist_stack {
     void (*addtoline) _((int));
     unsigned char *cstack;
     int csp;
-    int curline_linked;
 };
 
 /*