From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27236 invoked by alias); 29 Sep 2017 14:16:35 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 41787 Received: (qmail 10538 invoked by uid 1010); 29 Sep 2017 14:16:35 -0000 X-Qmail-Scanner-Diagnostics: from mailout1.w1.samsung.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(210.118.77.11):SA:0(-6.9/5.0):. Processed in 4.32447 secs); 29 Sep 2017 14:16:35 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham autolearn_force=no version=3.4.1 X-Envelope-From: p.stephenson@samsung.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | X-AuditID: cbfec7f1-f793a6d00000326b-72-59ce55b18ee4 Date: Fri, 29 Sep 2017 15:16:14 +0100 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: Crash when completion script call itself. Message-id: <20170929151614.56fd9cff@pwslap01u.europe.root.pri> In-reply-to: <4913136.yYypKkW7sH@kdudka-nb> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset="US-ASCII" Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrDIsWRmVeSWpSXmKPExsWy7djP87obQ89FGjycoGlxsPkhkwOjx6qD H5gCGKO4bFJSczLLUov07RK4Mpb1ahc8Mq/YuHM2SwNjs24XIyeHhICJxJmel4wQtpjEhXvr 2boYuTiEBJYySqz4cYwJwullkvj+5DwzTEfr1NOsEIlljBJt81YxQjjTmCQu3lnNDOGcYZS4 tfMe2GAhgbOMEst6CkBsFgFVie/nPrOD2GwChhJTN80GqxEREJc4u/Y8C4gtLGAm0XV+Adg6 XgF7iR0froLVcApoS0xbshAszi+gL3H17ycmiJPsJWZeOcMIUS8o8WPyPbA5zAI6Etu2PWaH sOUlNq95C3achMAcNon/z+6yQjS7SCx4Oh/KFpZ4dXwLO4QtI9HZcRBqQT+jxJNuX4jmGYwS p8/sYINIWEv03b7ICLGBT2LStulAGziA4rwSHW1CEKaHxL4WU4hqR4krZ2awQcLkN6PEsek5 ExgVZiE5exaSs2chOXsBI/MqRpHU0uLc9NRiI73ixNzi0rx0veT83E2MwDRw+t/xjzsY35+w OsQowMGoxMPb4HEuUog1say4MvcQowQHs5IIb7cbUIg3JbGyKrUoP76oNCe1+BCjNAeLkjiv bVRbpJBAemJJanZqakFqEUyWiYNTqoHxrOrVt5v9PiStf9B/1qsxgc3XtXrt4T6TJi2WIx/m +PMoC0x5rvXR/rBgv8lsAS/7meV5PtbCRQEsDr889jY1v7u74CqzbUPRjCT1Zf4rXgZ0/vih 3tB6rP6R5Q3RM197H2+TvJsblxrALepQ+29VxlMTtmwei592B66v0LZYuf5OiJRAXZASS3FG oqEWc1FxIgB9h7SY/wIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrALMWRmVeSWpSXmKPExsVy+t/xq7obQ89FGny9JGRxsPkhkwOjx6qD H5gCGKO4bFJSczLLUov07RK4Mpb1ahc8Mq/YuHM2SwNjs24XIyeHhICJROvU06wQtpjEhXvr 2boYuTiEBJYwSqzePp8FwpnBJPF7+mWozDlGiS9b/7FDOGcZJe582gTWzyKgKvH93Gd2EJtN wFBi6qbZjCC2iIC4xNm151lAbGEBM4mu8wuYQWxeAXuJHR+ugtVwCmhLTFuykBli6F9Gid4r r5hAEvwC+hJX/35igjjQXmLmlTOMEM2CEj8m3wMbyiygJbF5WxMrhC0vsXnNW7AFQgLqEjfu 7mafwCg8C0nLLCQts5C0LGBkXsUoklpanJueW2yoV5yYW1yal66XnJ+7iREYztuO/dy8g/HS xuBDjAIcjEo8vA0e5yKFWBPLiitzDzFKcDArifB2uwGFeFMSK6tSi/Lji0pzUosPMUpzsCiJ 8/buWR0pJJCeWJKanZpakFoEk2Xi4JRqYOy0uKtUFbTjUHTArc287zbK8u6fd/1DqPyXvgrx qXWO83om3NOtW98zOfNIRdpzpoJMyTDOgqQonovJ05+7FBQ6yZy/LrDJj/fB1GlJ77kXWXrt EDhv4qusKlAZIdOtqx48626/trKO75PAThX7AHNG+wMqjVEijvP4X3z1lpw2/8adazySSizF GYmGWsxFxYkA5Cd2qmMCAAA= X-CMS-MailID: 20170929141617eucas1p2ecb68d258f8067f94821a4eeb0cbe9c4 X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUb?= =?UTF-8?B?7IK87ISx7KCE7J6QG1ByaW5jaXBhbCBFbmdpbmVlciwgU29mdHdhcmU=?= X-Global-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUbU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtQcmluY2lwYWwgRW5naW5lZXIsIFNvZnR3YXJl?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDA1Q0QwNTAwNTg=?= CMS-TYPE: 201P X-CMS-RootMailID: 20170929072715epcas1p4171c28e9b82f94d79796ecca7e564ec3 X-RootMTR: 20170929072715epcas1p4171c28e9b82f94d79796ecca7e564ec3 References: <20170929121008.3da15b34@pwslap01u.europe.root.pri> <4913136.yYypKkW7sH@kdudka-nb> This is the proposed change. I think the compromise of reducing the value but making it more easily configurable is probably a good one. It currently makes the new variable appear in all emulations, but I can change that. On Fri, 29 Sep 2017 13:27:58 +0200 Kamil Dudka wrote: > Recompiling zsh with the -fconserve-stack option of GCC made the default > nesting limit 1000 reachable again on Fedora: This is where it gets interesting owing to trade offs. People do care about function execution time --- not necessarily the same people that care as much about crashing if they introduce an accidental infinite recursion. I see this flag introduces a heuristic based on frame size, so tweaking memory management internally might have a similar but more controllable effect; on the other hand, it simultaneously removes any easy trade-off you can make directly using compiler flags, and I'm not keen on introducing #ifdef's where one branch would be underused and rot. Obviously we can document the gcc tweak but I don't think all that many people will notice. pws diff --git a/Doc/Zsh/params.yo b/Doc/Zsh/params.yo index 05ea45f..5757111 100644 --- a/Doc/Zsh/params.yo +++ b/Doc/Zsh/params.yo @@ -731,6 +731,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/README b/README index 7373306..6fad1d5 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 0d2dc4e..2a95528 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5505,9 +5505,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 */ @@ -5637,13 +5635,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"); + lastval = 1; + goto undoshfunc; + } fstack.name = dupstring(name); /* * The caller is whatever is immediately before on the stack, @@ -5682,10 +5679,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 3236f71..507d069 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,9 @@ 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), +#ifdef MAX_FUNCTION_DEPTH +IPDEF5("FUNCNEST", &zsh_funcnest, varinteger_gsu), +#endif /* 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..90b77ab 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 + %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