Re: Complex config triggering Segfault in pattern matching code.

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Mon, 15 Dec 2014 15:39:36 -0800
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Dec 15,  1:20pm, Jonathan H wrote:
> }
> } It usually is, but for some reason I can't get it to crash in
> } valgrind.
> 
> That'll happen sometimes if the error is related to a signal being
> handled or something like that.  Zsh isn't internally "multi-threaded"
> so the only source of race conditions is signals (including child
> process exits).

I'm not sure we've really identified a solid strategy for this problem.

Maybe this suggests there's something to do with region highlighting
that's particular sensitive.  I didn't see anything of that kind --- I
don't know why it would be different in terms of memory management from
any other array stored by length --- though I did see a couple of
instances of somewhat incautious programming.  I can't believe the new
debug test will pick up anything.
 
> ==7806== 2 errors in context 2 of 2:
> ==7806== Conditional jump or move depends on uninitialised value(s)
> ==7806==    at 0x65A79E7: execzlefunc (zle_main.c:1360)
> ==7806==    by 0x65B8B2C: bin_zle_call (zle_thingy.c:711)
> 
> I have no idea what to do with that one; that line is:
> 
> 	Shfunc shf = (Shfunc) shfunctab->getnode(shfunctab, w->u.fnnam);

You'd expect use of shf to fall over horribly if there was really
something nasty in that line, which presumably it doesn't.

pws

diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c
index 467629d..415fee6 100644
--- a/Src/Zle/zle_refresh.c
+++ b/Src/Zle/zle_refresh.c
@@ -396,8 +396,9 @@ get_region_highlight(UNUSED(Param pm))
     struct region_highlight *rhp;
 
     /* region_highlights may not have been set yet */
-    if (arrsize)
-	arrsize -= N_SPECIAL_HIGHLIGHTS;
+    if (!arrsize)
+	return hmkarray(NULL);
+    arrsize -= N_SPECIAL_HIGHLIGHTS;
     arrp = retarr = (char **)zhalloc((arrsize+1)*sizeof(char *));
 
     /* ignore special highlighting */
@@ -1028,6 +1029,8 @@ zrefresh(void)
     /* this will create region_highlights if it's still NULL */
     zle_set_highlight();
 
+    DPUTS(!region_highlight, "region_highlight not created");
+
     /* check for region between point ($CURSOR) and mark ($MARK) */
     if (region_active) {
 	if (zlecs <= mark) {
diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c
index de91182..e361e5e 100644
--- a/Src/Zle/zle_utils.c
+++ b/Src/Zle/zle_utils.c
@@ -675,35 +675,42 @@ zle_restore_positions(void)
 	zlell = oldpos->ll;
     }
 
-    /* Count number of regions and see if the array needs resizing */
-    for (nreg = 0, oldrhp = oldpos->regions;
-	 oldrhp;
-	 nreg++, oldrhp = oldrhp->next)
-	;
-    if (nreg + N_SPECIAL_HIGHLIGHTS != n_region_highlights) {
-	n_region_highlights = nreg + N_SPECIAL_HIGHLIGHTS;
-	region_highlights = (struct region_highlight *)
-	    zrealloc(region_highlights,
-		     sizeof(struct region_highlight) * n_region_highlights);
-    }
-    oldrhp = oldpos->regions;
-    rhp = region_highlights + N_SPECIAL_HIGHLIGHTS;
-    while (oldrhp) {
-	struct zle_region *nextrhp = oldrhp->next;
-
-	rhp->atr = oldrhp->atr;
-	rhp->flags = oldrhp->flags;
-	if (zlemetaline) {
-	    rhp->start_meta = oldrhp->start;
-	    rhp->end_meta = oldrhp->end;
-	} else {
-	    rhp->start = oldrhp->start;
-	    rhp->end = oldrhp->end;
+    if (oldpos->regions) {
+	/* Count number of regions and see if the array needs resizing */
+	for (nreg = 0, oldrhp = oldpos->regions;
+	     oldrhp;
+	     nreg++, oldrhp = oldrhp->next)
+	    ;
+	if (nreg + N_SPECIAL_HIGHLIGHTS != n_region_highlights) {
+	    n_region_highlights = nreg + N_SPECIAL_HIGHLIGHTS;
+	    region_highlights = (struct region_highlight *)
+		zrealloc(region_highlights,
+			 sizeof(struct region_highlight) * n_region_highlights);
 	}
+	oldrhp = oldpos->regions;
+	rhp = region_highlights + N_SPECIAL_HIGHLIGHTS;
+	while (oldrhp) {
+	    struct zle_region *nextrhp = oldrhp->next;
 
-	zfree(oldrhp, sizeof(*oldrhp));
-	oldrhp = nextrhp;
-	rhp++;
+	    rhp->atr = oldrhp->atr;
+	    rhp->flags = oldrhp->flags;
+	    if (zlemetaline) {
+		rhp->start_meta = oldrhp->start;
+		rhp->end_meta = oldrhp->end;
+	    } else {
+		rhp->start = oldrhp->start;
+		rhp->end = oldrhp->end;
+	    }
+
+	    zfree(oldrhp, sizeof(*oldrhp));
+	    oldrhp = nextrhp;
+	    rhp++;
+	}
+    } else if (region_highlights) {
+	zfree(region_highlights, sizeof(struct region_highlight) *
+	      n_region_highlights);
+	region_highlights  = NULL;
+	n_region_highlights = 0;
     }
 
     zfree(oldpos, sizeof(*oldpos));

pws