Re: unbounded recursive call in a shell script crashes zsh

[Kamil Dudka <kdudka@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]

On Thursday, April 13, 2017 18:01:13 Jérémie Roquet wrote:
> 2017-04-13 17:21 GMT+02:00 Jérémie Roquet <jroquet@arkanosis.net>:
> > Hence a total of 5856 bytes per recursion, or 5719 kiB for 10000
> > recursions.
> Sorry, I meant 1000 recursions, obviously.
> 
> Here are the numbers when compiling using -O3 instead of -O0 -ggdb —
> probably more useful for optimization:
> 
> execlist: 400 bytes
> execpline: 416 bytes
> execpline2: 224 bytes
> execcmd_exec: 4864 bytes
> execshfunc: 336 bytes
> doshfunc: 704 bytes
> runshfunc: 336 bytes
> execode: 80 bytes
> execlist: 400 bytes
> execpline: 416 bytes
> execpline2: 224 bytes
> execcmd_exec: 4864 bytes
> execif: 80 bytes
> 
> Aggregated:
> 
> execlist: 800 bytes
> execpline: 832 bytes
> execpline2: 448 bytes
> execcmd_exec: 9728 bytes
> execshfunc: 336 bytes
> doshfunc: 704 bytes
> runshfunc: 336 bytes
> execode: 80 bytes
> execif: 80 bytes
> 
> Hence an even higher total of 13344 bytes per recursion, or 13032 kiB
> for 1000 recursions.
> 
> If I'm not mistaken, execcmd_exec seems to account for 73% of the stack
> usage.

Thanks for the analysis!  I tried to apply the attached patch on top of my 
previous patch but the total saving of stack allocation was only up to 10%, 
depending on the compiler flags.  So it is not worth the troubles.  What 
helped significantly to make the default shell call nesting limit reachable 
again was the -fconserve-stack option of GCC.

Kamil

[-- Attachment #2: 0001-execcmd_exec-reduce-stack-allocation-in-favour-of-he.patch --]
[-- Type: text/x-patch, Size: 2154 bytes --]

>From 98fb1642f4b809f4984390871b982cc37155d9ed Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Tue, 18 Apr 2017 15:03:55 +0200
Subject: [PATCH] execcmd_exec: reduce stack allocation in favour of heap

---
 Src/exec.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/Src/exec.c b/Src/exec.c
index cde549e..b28db61 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2654,9 +2654,9 @@ execcmd_exec(Estate state, Execcmd_params eparams,
     LinkList filelist = NULL;
     LinkNode node;
     Redir fn;
-    struct multio *mfds[10];
+    struct multio **mfds;
     char *text;
-    int save[10];
+    int *save;
     int fil, dfil, is_cursh, do_exec = 0, redir_err = 0, i;
     int nullexec = 0, magic_assign = 0, forked = 0;
     int is_shfunc = 0, is_builtin = 0, is_exec = 0, use_defpath = 0;
@@ -2689,11 +2689,6 @@ execcmd_exec(Estate state, Execcmd_params eparams,
      */
     use_cmdoutval = !args;
 
-    for (i = 0; i < 10; i++) {
-	save[i] = -2;
-	mfds[i] = NULL;
-    }
-
     /* If the command begins with `%', then assume it is a *
      * reference to a job in the job table.                */
     if ((type == WC_SIMPLE || type == WC_TYPESET) && args && nonempty(args) &&
@@ -3370,6 +3365,13 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	}
     }
 
+    save = zalloc(10 * sizeof(int));
+    mfds = zalloc(10 * sizeof(struct multio *));
+    for (i = 0; i < 10; i++) {
+	save[i] = -2;
+	mfds[i] = NULL;
+    }
+
     /* Add pipeline input/output to mnodes */
     if (input)
 	addfd(forked, save, mfds, 0, input, 0, NULL);
@@ -3616,6 +3618,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	if (mfds[i] && mfds[i]->ct >= 2)
 	    closemn(mfds, i, REDIR_CLOSE);
 
+    zfree(mfds, 10 * sizeof(struct multio *));
+
     if (nullexec) {
 	/*
 	 * If nullexec is 2, we have variables to add with the redirections
@@ -4003,6 +4007,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
     fixfds(save);
 
  done:
+    zfree(save, 10 * sizeof(int));
     if (isset(POSIXBUILTINS) &&
 	(cflags & (BINF_PSPECIAL|BINF_EXEC)) &&
 	!(orig_cflags & BINF_COMMAND)) {
-- 
2.10.2