Re: zsh segfaults with lots of data in one variable

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

[This seemed to disappear the first time I sent it but it may be queued
somewhere and sendmail is lying to me.]

On Tue, 3 Mar 2009 22:07:29 +0100
Frank Terbeck <ft@bewatermyfriend.org> wrote:
> I just wanted to see if lines="$(<&0)" plays nice with binary input
> data, when zsh crashed on me.
> 
> What I did is this:
> 
> [snip]
> zsh% slurp() { lines="$(<&0)" }
> zsh% slurp < ./a-big-71MiB-mp3-file.mp3
> zsh% printf '%s' $lines > foo.mp3  

This is down to a combination of implementation issues including (I
suspect, depending on your system) how gcc works.

For every command the shell saves the previous last argument in $_.
This happens after expansion, so in that last line it was the complete
value of "$lines".  (This indicates how inefficient it can be to get the
shell to do things that aren't really it's job...)

When executing a shell function, $_ is saved and restored.  If gcc or
some other compiler that supports variable length arrays is around,
we'll use that.  This memory then goes on the stack.  In the case of the
multimegabyte file this caused bad karma.

valgrind gave me an odd message about the stack being switched around
when this happened, so I can't exclude the possibility that the compiler or
the system is getting unnecessarily muddled.  Someone more au fait with
the internals might want to try this with an unpatched version of the
shell.

However, in our case it's easy to fix just using ordindary malloc memory
for saving $_.  It would be much more efficient to mark underscore as
"push old value to stack on write", or something similar, but if I try
that at ten o'clock in the evening I'll get it wrong.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.163
diff -u -r1.163 exec.c
--- Src/exec.c	11 Feb 2009 20:42:16 -0000	1.163
+++ Src/exec.c	3 Mar 2009 21:54:34 -0000
@@ -4431,10 +4431,12 @@
 mod_export void
 runshfunc(Eprog prog, FuncWrap wrap, char *name)
 {
-    int cont;
-    VARARR(char, ou, underscoreused);
+    int cont, ouu;
+    char *ou;
 
-    memcpy(ou, underscore, underscoreused);
+    ou = zalloc(ouu = underscoreused);
+    if (ou)
+	memcpy(ou, underscore, underscoreused);
 
     while (wrap) {
 	wrap->module->wrapper++;
@@ -4445,13 +4447,19 @@
 	    (wrap->module->node.flags & MOD_UNLOAD))
 	    unload_module(wrap->module);
 
-	if (!cont)
+	if (!cont) {
+	    if (ou)
+		zfree(ou, ouu);
 	    return;
+	}
 	wrap = wrap->next;
     }
     startparamscope();
     execode(prog, 1, 0);
-    setunderscore(ou);
+    if (ou) {
+	setunderscore(ou);
+	zfree(ou, ouu);
+    }
     endparamscope();
 }
 
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.212
diff -u -r1.212 utils.c
--- Src/utils.c	3 Mar 2009 05:22:57 -0000	1.212
+++ Src/utils.c	3 Mar 2009 21:54:35 -0000
@@ -1340,9 +1340,13 @@
 		    fprintf(shout, "You have new mail.\n");
 		    fflush(shout);
 		} else {
-		    VARARR(char, usav, underscoreused);
+		    char *usav;
+		    int uusav = underscoreused;
 
-		    memcpy(usav, underscore, underscoreused);
+		    usav = zalloc(underscoreused);
+
+		    if (usav)
+			memcpy(usav, underscore, underscoreused);
 
 		    setunderscore(*s);
 
@@ -1353,7 +1357,10 @@
 			fputc('\n', shout);
 			fflush(shout);
 		    }
-		    setunderscore(usav);
+		    if (usav) {
+			setunderscore(usav);
+			zfree(usav, uusav);
+		    }
 		}
 	    }
 	    if (isset(MAILWARNING) && st.st_atime > st.st_mtime &&


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/