Re: unbounded recursive call in a shell script crashes zsh

[Kamil Dudka <kdudka@xxxxxxxxxx>]

On Wednesday, April 12, 2017 15:11:05 Bart Schaefer wrote:
> On Wed, Apr 12, 2017 at 12:30 AM, Kamil Dudka <kdudka@xxxxxxxxxx> wrote:
> > On Tuesday, April 11, 2017 19:12:41 Bart Schaefer wrote:
> >> In fact 4.3.11 running out of job table space means that it consumed
> >> all malloc() memory before consuming all of the stack;
> > 
> > I cannot confirm your hypothesis.
> 
> Sorry, I forgot about (and missed because I was scanning for diffs in
> job handling and that part hasn't changed) MAX_MAXJOBS.  Either way it
> means 4.3.11 ran out of job table space and never had a chance to
> exercise the function call recursion limit.

Still the question remains:  Is it expected to run out of stack after <1000 
nested calls of a trivial shell function when 1000 is the default limit for
function call depth?

I was trying to reduce the stack usage of zsh but was not really successful, 
mainly because I do not know how to efficiently find the automatic variables 
that consumed the biggest portion of the stack.  The attached patch reduces 
the peak stack allocation on my example from 5.558 MB to 5.345 MB, so it is 
probably not worth applying at all.  Do you have any estimation about where 
else the stack allocation could be reduced?

Kamil

From 758a8ee2b36f73766b1387974e0b3bf4bc87d1d8 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@xxxxxxxxxx>
Date: Thu, 13 Apr 2017 14:08:49 +0200
Subject: [PATCH] doshfunc: reduce stack allocation in favour of heap

---
 Src/exec.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/Src/exec.c b/Src/exec.c
index f021a08..cde549e 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5358,13 +5358,13 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
     char **pptab, **x, *oargv0;
     int oldzoptind, oldlastval, oldoptcind, oldnumpipestats, ret;
     int *oldpipestats = NULL;
-    char saveopts[OPT_SIZE], *oldscriptname = scriptname;
+    char *saveopts, *oldscriptname = scriptname;
     char *name = shfunc->node.nam;
     int flags = shfunc->node.flags, ooflags;
     char *fname = dupstring(name);
     int obreaks, ocontflag, oloops, saveemulation, restore_sticky;
     Eprog prog;
-    struct funcstack fstack;
+    struct funcstack *fstack;
     static int oflags;
     Emulation_options save_sticky = NULL;
 #ifdef MAX_FUNCTION_DEPTH
@@ -5409,6 +5409,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	/* We need to save the current options even if LOCALOPTIONS is *
 	 * not currently set.  That's because if it gets set in the    *
 	 * function we need to restore the original options on exit.   */
+	saveopts = (char *)zalloc(OPT_SIZE * sizeof(char));
 	memcpy(saveopts, opts, sizeof(opts));
 	saveemulation = emulation;
 	save_sticky = sticky;
@@ -5501,21 +5502,22 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 		goto undoshfunc;
 	    }
 #endif
-	fstack.name = dupstring(name);
+	fstack = zalloc(sizeof(struct funcstack));
+	fstack->name = dupstring(name);
 	/*
 	 * The caller is whatever is immediately before on the stack,
 	 * unless we're at the top, in which case it's the script
 	 * or interactive shell name.
 	 */
-	fstack.caller = funcstack ? funcstack->name :
+	fstack->caller = funcstack ? funcstack->name :
 	    dupstring(oargv0 ? oargv0 : argzero);
-	fstack.lineno = lineno;
-	fstack.prev = funcstack;
-	fstack.tp = FS_FUNC;
-	funcstack = &fstack;
+	fstack->lineno = lineno;
+	fstack->prev = funcstack;
+	fstack->tp = FS_FUNC;
+	funcstack = fstack;
 
-	fstack.flineno = shfunc->lineno;
-	fstack.filename = getshfuncfile(shfunc);
+	fstack->flineno = shfunc->lineno;
+	fstack->filename = getshfuncfile(shfunc);
 
 	prog = shfunc->funcdef;
 	if (prog->flags & EF_RUN) {
@@ -5523,7 +5525,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 
 	    prog->flags &= ~EF_RUN;
 
-	    runshfunc(prog, NULL, fstack.name);
+	    runshfunc(prog, NULL, fstack->name);
 
 	    if (!(shf = (Shfunc) shfunctab->getnode(shfunctab,
 						    (name = fname)))) {
@@ -5536,9 +5538,10 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	    }
 	    prog = shf->funcdef;
 	}
-	runshfunc(prog, wrappers, fstack.name);
+	runshfunc(prog, wrappers, fstack->name);
     doneshfunc:
-	funcstack = fstack.prev;
+	funcstack = fstack->prev;
+	zfree(fstack, sizeof(struct funcstack));
 #ifdef MAX_FUNCTION_DEPTH
     undoshfunc:
 	--funcdepth;
@@ -5586,6 +5589,8 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	    opts[WARNNESTEDVAR] = saveopts[WARNNESTEDVAR];
 	}
 
+	zfree(saveopts, OPT_SIZE * sizeof(char));
+
 	if (opts[LOCALLOOPS]) {
 	    if (contflag)
 		zwarn("`continue' active at end of function scope");
-- 
2.10.2