From 932bb5a32c7b9a477c5209e62363920efdbc71d1 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Fri, 29 Sep 2017 16:46:01 +0100 Subject: [PATCH 1/2] 41787 (plus minor tweaks): use $FUNCSTACK for function nesting depth. Initialised from existing configuration value. Upstream-commit: 174e560a23e40725cd0b50669a52d831342e5246 Signed-off-by: Kamil Dudka --- Doc/Zsh/params.yo | 10 ++++++++++ Doc/zshparam.1 | 9 +++++++++ README | 26 +++++++++++++++++++++++++- Src/exec.c | 17 ++++++----------- Src/params.c | 14 ++++++++++++++ Test/C04funcdef.ztst | 8 ++++++++ configure.ac | 6 +++--- 7 files changed, 75 insertions(+), 15 deletions(-) diff --git a/Doc/Zsh/params.yo b/Doc/Zsh/params.yo index 817496b..26d5039 100644 --- a/Doc/Zsh/params.yo +++ b/Doc/Zsh/params.yo @@ -668,6 +668,16 @@ This value is system dependent and is intended for debugging purposes. It is also useful with the tt(zsh/system) module which allows the number to be turned into a name or message. ) +vindex(FUNCNEST) +item(tt(FUNCNEST) )( +Integer. If greater than or equal to zero, the maximum nesting depth of +shell functions. When it is exceeded, an error is raised at the point +where a function is called. The default value is determined when +the shell is configured, but is typically 500. Increasing +the value increases the danger of a runaway function recursion +causing the shell to crash. Setting a negative value turns off +the check. +) vindex(GID) item(tt(GID) )( The real group ID of the shell process. If you have sufficient privileges, diff --git a/Doc/zshparam.1 b/Doc/zshparam.1 index 44d8629..c7188cb 100644 --- a/Doc/zshparam.1 +++ b/Doc/zshparam.1 @@ -701,6 +701,15 @@ This value is system dependent and is intended for debugging purposes\&. It is also useful with the \fBzsh/system\fP module which allows the number to be turned into a name or message\&. .TP +\fBFUNCNEST\fP +Integer\&. If greater than or equal to zero, the maximum nesting depth of +shell functions\&. When it is exceeded, an error is raised at the point +where a function is called\&. The default value is determined when +the shell is configured, but is typically 500\&. Increasing +the value increases the danger of a runaway function recursion +causing the shell to crash\&. Setting a negative value turns off +the check\&. +.TP \fBGID\fP The real group ID of the shell process\&. If you have sufficient privileges, you may change the group ID of the shell process by assigning to this diff --git a/README b/README index e22cfc1..b5c0769 100644 --- a/README +++ b/README @@ -30,9 +30,33 @@ Zsh is a shell with lots of features. For a list of some of these, see the file FEATURES, and for the latest changes see NEWS. For more details, see the documentation. -Incompatibilities since 5.3.1 +Incompatibilities since 5.4.2 ----------------------------- +1) The default build-time maximum nested function depth has been +decreased from 1000 to 500 based on user experience. However, +it can now be changed at run time via the variable FUNCNEST. +If you previously configured the shell to set a different value, +or to remove the check, this is now reflected in the default +value of the variable. + +2) The syntax + +foo=([key]=value) + +can be used to set elements of arrays and associative arrays. In the +unlikely event that you need to set an array by matching files using a +pattern that starts with a character range followed by '=', you need to +quote the '=', e.g.: + +foo=([aeiou]\=vowel) + +This is only required for array values contained within parentheses; +command line expansion for normal arguments has not changed. + +Incompatibilities between 5.3.1 and 5.4.2 +----------------------------------------- + 1) The default behaviour of code like the following has changed: alias foo='noglob foo' diff --git a/Src/exec.c b/Src/exec.c index cd99733..0f2bb4a 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5475,9 +5475,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) struct funcstack fstack; static int oflags; Emulation_options save_sticky = NULL; -#ifdef MAX_FUNCTION_DEPTH static int funcdepth; -#endif Heap funcheap; queue_signals(); /* Lots of memory and global state changes coming */ @@ -5607,13 +5605,12 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) argzero = ztrdup(argzero); } } -#ifdef MAX_FUNCTION_DEPTH - if(++funcdepth > MAX_FUNCTION_DEPTH) - { - zerr("maximum nested function level reached"); - goto undoshfunc; - } -#endif + ++funcdepth; + if (zsh_funcnest >= 0 && funcdepth > zsh_funcnest) { + zerr("maximum nested function level reached; increase FUNCNEST?"); + lastval = 1; + goto undoshfunc; + } fstack.name = dupstring(name); /* * The caller is whatever is immediately before on the stack, @@ -5652,10 +5649,8 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) runshfunc(prog, wrappers, fstack.name); doneshfunc: funcstack = fstack.prev; -#ifdef MAX_FUNCTION_DEPTH undoshfunc: --funcdepth; -#endif if (retflag) { retflag = 0; breaks = obreaks; diff --git a/Src/params.c b/Src/params.c index 6fbee88..9c7833f 100644 --- a/Src/params.c +++ b/Src/params.c @@ -101,6 +101,19 @@ zlong lastval, /* $? */ rprompt_indent, /* $ZLE_RPROMPT_INDENT */ ppid, /* $PPID */ zsh_subshell; /* $ZSH_SUBSHELL */ + +/* $FUNCNEST */ +/**/ +mod_export +zlong zsh_funcnest = +#ifdef MAX_FUNCTION_DEPTH + MAX_FUNCTION_DEPTH +#else + /* Disabled by default but can be enabled at run time */ + -1 +#endif + ; + /**/ zlong lineno, /* $LINENO */ zoptind, /* $OPTIND */ @@ -337,6 +350,7 @@ IPDEF5("COLUMNS", &zterm_columns, zlevar_gsu), IPDEF5("LINES", &zterm_lines, zlevar_gsu), IPDEF5U("ZLE_RPROMPT_INDENT", &rprompt_indent, rprompt_indent_gsu), IPDEF5("SHLVL", &shlvl, varinteger_gsu), +IPDEF5("FUNCNEST", &zsh_funcnest, varinteger_gsu), /* Don't import internal integer status variables. */ #define IPDEF6(A,B,F) {{NULL,A,PM_INTEGER|PM_SPECIAL|PM_DONTIMPORT},BR((void *)B),GSU(F),10,0,NULL,NULL,NULL,0} diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst index 6a675e0..0c00a04 100644 --- a/Test/C04funcdef.ztst +++ b/Test/C04funcdef.ztst @@ -515,6 +515,14 @@ 0:autoload with absolute path not cancelled by bare autoload >I have been loaded by explicit path. + ( + FUNCNEST=0 + fn() { true; } + fn + ) +1: +?fn:4: maximum nested function level reached; increase FUNCNEST? + %clean rm -f file.in file.out diff --git a/configure.ac b/configure.ac index ec0bdae..1a498f8 100644 --- a/configure.ac +++ b/configure.ac @@ -415,13 +415,13 @@ ifdef([max_function_depth],[undefine([max_function_depth])])dnl AH_TEMPLATE([MAX_FUNCTION_DEPTH], [Define for function depth limits]) AC_ARG_ENABLE(max-function-depth, -AC_HELP_STRING([--enable-max-function-depth=MAX], [limit function depth to MAX, default 1000]), +AC_HELP_STRING([--enable-max-function-depth=MAX], [limit function depth to MAX, default 500]), [if test x$enableval = xyes; then - AC_DEFINE(MAX_FUNCTION_DEPTH, 1000) + AC_DEFINE(MAX_FUNCTION_DEPTH, 500) elif test x$enableval != xno; then AC_DEFINE_UNQUOTED(MAX_FUNCTION_DEPTH, $enableval) fi], -[AC_DEFINE(MAX_FUNCTION_DEPTH, 1000)] +[AC_DEFINE(MAX_FUNCTION_DEPTH, 500)] ) ifdef([default_readnullcmd],[undefine([default_readnullcmd])])dnl -- 2.13.6 From aee6a617b5769ee6224165560db43bb8b8ee64ba Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Wed, 4 Oct 2017 09:18:51 +0100 Subject: [PATCH 2/2] 41802 (minor tweaks): use heap during shell function call. Replaces stack for more efficient memory management. Also fix debug message when FUNCNEST is increased. Upstream-commit: e573857a033504f7918c4a2309151329820f115f Signed-off-by: Kamil Dudka --- Src/exec.c | 154 +++++++++++++++++++++++++++++++++--------------------------- Src/parse.c | 3 +- 2 files changed, 87 insertions(+), 70 deletions(-) diff --git a/Src/exec.c b/Src/exec.c index 0f2bb4a..db77303 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -41,6 +41,20 @@ enum { ADDVAR_RESTORE = 1 << 2 }; +/* Structure in which to save values around shell function call */ + +struct funcsave { + char opts[OPT_SIZE]; + char *argv0; + int zoptind, lastval, optcind, numpipestats; + int *pipestats; + char *scriptname; + int breaks, contflag, loops, emulation, noerrexit, oflags, restore_sticky; + Emulation_options sticky; + struct funcstack fstack; +}; +typedef struct funcsave *Funcsave; + /* * used to suppress ERREXIT and trapping of SIGZERR, SIGEXIT. * Bits from noerrexit_bits. @@ -5462,34 +5476,36 @@ int sticky_emulation_differs(Emulation_options sticky2) mod_export int 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 **pptab, **x; + int ret; char *name = shfunc->node.nam; - int flags = shfunc->node.flags, ooflags; - int savnoerrexit; + int flags = shfunc->node.flags; char *fname = dupstring(name); - int obreaks, ocontflag, oloops, saveemulation, restore_sticky; Eprog prog; - struct funcstack fstack; static int oflags; - Emulation_options save_sticky = NULL; static int funcdepth; Heap funcheap; queue_signals(); /* Lots of memory and global state changes coming */ NEWHEAPS(funcheap) { - oargv0 = NULL; - obreaks = breaks; - ocontflag = contflag; - oloops = loops; + /* + * Save data in heap rather than on stack to keep recursive + * function cost down --- use of heap memory should be efficient + * at this point. Saving is not actually massive. + */ + Funcsave funcsave = zhalloc(sizeof(struct funcsave)); + funcsave->scriptname = scriptname; + funcsave->argv0 = NULL; + funcsave->breaks = breaks; + funcsave->contflag = contflag; + funcsave->loops = loops; + funcsave->lastval = lastval; + funcsave->pipestats = NULL; + funcsave->numpipestats = numpipestats; + funcsave->noerrexit = noerrexit; if (trap_state == TRAP_STATE_PRIMED) trap_return--; - oldlastval = lastval; - oldnumpipestats = numpipestats; - savnoerrexit = noerrexit; /* * Suppression of ERR_RETURN is turned off in function scope. */ @@ -5500,8 +5516,8 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) * immediately by a pushheap/popheap pair. */ size_t bytes = sizeof(int)*numpipestats; - oldpipestats = (int *)zhalloc(bytes); - memcpy(oldpipestats, pipestats, bytes); + funcsave->pipestats = (int *)zhalloc(bytes); + memcpy(funcsave->pipestats, pipestats, bytes); } starttrapscope(); @@ -5510,8 +5526,8 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) pptab = pparams; if (!(flags & PM_UNDEFINED)) scriptname = dupstring(name); - oldzoptind = zoptind; - oldoptcind = optcind; + funcsave->zoptind = zoptind; + funcsave->optcind = optcind; if (!isset(POSIXBUILTINS)) { zoptind = 1; optcind = 0; @@ -5520,9 +5536,9 @@ 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. */ - memcpy(saveopts, opts, sizeof(opts)); - saveemulation = emulation; - save_sticky = sticky; + memcpy(funcsave->opts, opts, sizeof(opts)); + funcsave->emulation = emulation; + funcsave->sticky = sticky; if (sticky_emulation_differs(shfunc->sticky)) { /* @@ -5539,7 +5555,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) */ sticky = sticky_emulation_dup(shfunc->sticky, 1); emulation = sticky->emulation; - restore_sticky = 1; + funcsave->restore_sticky = 1; installemulation(emulation, opts); if (sticky->n_on_opts) { OptIndex *onptr; @@ -5558,7 +5574,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) /* All emulations start with pattern disables clear */ clearpatterndisables(); } else - restore_sticky = 0; + funcsave->restore_sticky = 0; if (flags & (PM_TAGGED|PM_TAGGED_LOCAL)) opts[XTRACE] = 1; @@ -5576,11 +5592,11 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) else opts[WARNNESTEDVAR] = 0; } - ooflags = oflags; + funcsave->oflags = oflags; /* * oflags is static, because we compare it on the next recursive - * call. Hence also we maintain ooflags for restoring the previous - * value of oflags after the call. + * call. Hence also we maintain a saved version for restoring + * the previous value of oflags after the call. */ oflags = flags; opts[PRINTEXITVALUE] = 0; @@ -5591,7 +5607,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) pparams = x = (char **) zshcalloc(((sizeof *x) * (1 + countlinknodes(doshargs)))); if (isset(FUNCTIONARGZERO)) { - oargv0 = argzero; + funcsave->argv0 = argzero; argzero = ztrdup(getdata(node)); } /* first node contains name regardless of option */ @@ -5601,7 +5617,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) } else { pparams = (char **) zshcalloc(sizeof *pparams); if (isset(FUNCTIONARGZERO)) { - oargv0 = argzero; + funcsave->argv0 = argzero; argzero = ztrdup(argzero); } } @@ -5611,21 +5627,21 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) lastval = 1; goto undoshfunc; } - fstack.name = dupstring(name); + funcsave->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 : - dupstring(oargv0 ? oargv0 : argzero); - fstack.lineno = lineno; - fstack.prev = funcstack; - fstack.tp = FS_FUNC; - funcstack = &fstack; + funcsave->fstack.caller = funcstack ? funcstack->name : + dupstring(funcsave->argv0 ? funcsave->argv0 : argzero); + funcsave->fstack.lineno = lineno; + funcsave->fstack.prev = funcstack; + funcsave->fstack.tp = FS_FUNC; + funcstack = &funcsave->fstack; - fstack.flineno = shfunc->lineno; - fstack.filename = getshfuncfile(shfunc); + funcsave->fstack.flineno = shfunc->lineno; + funcsave->fstack.filename = getshfuncfile(shfunc); prog = shfunc->funcdef; if (prog->flags & EF_RUN) { @@ -5633,7 +5649,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) prog->flags &= ~EF_RUN; - runshfunc(prog, NULL, fstack.name); + runshfunc(prog, NULL, funcsave->fstack.name); if (!(shf = (Shfunc) shfunctab->getnode(shfunctab, (name = fname)))) { @@ -5646,52 +5662,52 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) } prog = shf->funcdef; } - runshfunc(prog, wrappers, fstack.name); + runshfunc(prog, wrappers, funcsave->fstack.name); doneshfunc: - funcstack = fstack.prev; + funcstack = funcsave->fstack.prev; undoshfunc: --funcdepth; if (retflag) { retflag = 0; - breaks = obreaks; + breaks = funcsave->breaks; } freearray(pparams); - if (oargv0) { + if (funcsave->argv0) { zsfree(argzero); - argzero = oargv0; + argzero = funcsave->argv0; } pparams = pptab; if (!isset(POSIXBUILTINS)) { - zoptind = oldzoptind; - optcind = oldoptcind; + zoptind = funcsave->zoptind; + optcind = funcsave->optcind; } - scriptname = oldscriptname; - oflags = ooflags; + scriptname = funcsave->scriptname; + oflags = funcsave->oflags; endpatternscope(); /* before restoring old LOCALPATTERNS */ - if (restore_sticky) { + if (funcsave->restore_sticky) { /* * If we switched to an emulation environment just for * this function, we interpret the option and emulation * switch as being a firewall between environments. */ - memcpy(opts, saveopts, sizeof(opts)); - emulation = saveemulation; - sticky = save_sticky; + memcpy(opts, funcsave->opts, sizeof(opts)); + emulation = funcsave->emulation; + sticky = funcsave->sticky; } else if (isset(LOCALOPTIONS)) { /* restore all shell options except PRIVILEGED and RESTRICTED */ - saveopts[PRIVILEGED] = opts[PRIVILEGED]; - saveopts[RESTRICTED] = opts[RESTRICTED]; - memcpy(opts, saveopts, sizeof(opts)); - emulation = saveemulation; + funcsave->opts[PRIVILEGED] = opts[PRIVILEGED]; + funcsave->opts[RESTRICTED] = opts[RESTRICTED]; + memcpy(opts, funcsave->opts, sizeof(opts)); + emulation = funcsave->emulation; } else { /* just restore a couple. */ - opts[XTRACE] = saveopts[XTRACE]; - opts[PRINTEXITVALUE] = saveopts[PRINTEXITVALUE]; - opts[LOCALOPTIONS] = saveopts[LOCALOPTIONS]; - opts[LOCALLOOPS] = saveopts[LOCALLOOPS]; - opts[WARNNESTEDVAR] = saveopts[WARNNESTEDVAR]; + opts[XTRACE] = funcsave->opts[XTRACE]; + opts[PRINTEXITVALUE] = funcsave->opts[PRINTEXITVALUE]; + opts[LOCALOPTIONS] = funcsave->opts[LOCALOPTIONS]; + opts[LOCALLOOPS] = funcsave->opts[LOCALLOOPS]; + opts[WARNNESTEDVAR] = funcsave->opts[WARNNESTEDVAR]; } if (opts[LOCALLOOPS]) { @@ -5699,9 +5715,9 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) zwarn("`continue' active at end of function scope"); if (breaks) zwarn("`break' active at end of function scope"); - breaks = obreaks; - contflag = ocontflag; - loops = oloops; + breaks = funcsave->breaks; + contflag = funcsave->contflag; + loops = funcsave->loops; } endtrapscope(); @@ -5709,11 +5725,11 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) if (trap_state == TRAP_STATE_PRIMED) trap_return++; ret = lastval; - noerrexit = savnoerrexit; + noerrexit = funcsave->noerrexit; if (noreturnval) { - lastval = oldlastval; - numpipestats = oldnumpipestats; - memcpy(pipestats, oldpipestats, sizeof(int)*numpipestats); + lastval = funcsave->lastval; + numpipestats = funcsave->numpipestats; + memcpy(pipestats, funcsave->pipestats, sizeof(int)*numpipestats); } } OLDHEAPS; diff --git a/Src/parse.c b/Src/parse.c index 2705252..d94ee99 100644 --- a/Src/parse.c +++ b/Src/parse.c @@ -2737,7 +2737,8 @@ freeeprog(Eprog p) DPUTS(p->nref < 0 && !(p->flags & EF_HEAP), "Real EPROG has nref < 0"); DPUTS(p->nref < -1, "Uninitialised EPROG nref"); #ifdef MAX_FUNCTION_DEPTH - DPUTS(p->nref > MAX_FUNCTION_DEPTH + 10, "Overlarge EPROG nref"); + DPUTS(zsh_funcnest >=0 && p->nref > zsh_funcnest + 10, + "Overlarge EPROG nref"); #endif if (p->nref > 0 && !--p->nref) { for (i = p->npats, pp = p->pats; i--; pp++) -- 2.13.6