From d50d3d7c4c408448ae1146e1e3d71409e4a326c7 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Sun, 11 Apr 2021 00:08:26 +0100 Subject: [PATCH] Reset arithmetic recursion level on all errors (re: 264ba48b) The recursion level for arithmetic expressions is kept track of in a static 'level' variable in streval.c. It is reset when arithmetic expressions throw an error. But an error for an arithmetic expression may also occur elsewhere -- at least in one case: when an arithmetic expression attempts to change a read-only variable. In that case, the recursion level is never reset because that code does not have access to the static 'level' variable. If many such conditions occur (as in the new readonly.sh regression tests), an arithmetic command like 'i++' may eventually fail with a 'recursion too deep' error. To mitigate the problem, MAXLEVEL in streval.c was changed from 9 to 1024 in 264ba48b (as in the ksh 93v- beta). This commit leaves that increase, but adds a proper fix. src/cmd/ksh93/include/defs.h: - Add global sh.arithrecursion (a.k.a. shp->arithrecursion) variable to keep track of the arithmetic recursion level, replacing the static 'level' variable in streval.c. src/cmd/ksh93/sh/xec.c: sh_exec(): - Reset sh.arithrecursion before starting a new simple command (TCOM), a new subshell with parentheses (TPAR), a new pipe (TFIL), or a new [[ ... ]] command (TTST). These are the same places where 'echeck' is set to 1 for --errexit and ERR trap checks, so it should cover everything. src/cmd/ksh93/sh/streval.c: - Change all uses of 'level' to sh.arithrecursion. - _seterror, aritherror(): No longer bother to reset the level to zero here; xec.c should have this covered for all cases now. src/cmd/ksh93/tests/arith.sh: - Add tests for main shell and subshell. --- NEWS | 7 +++++++ src/cmd/ksh93/include/defs.h | 1 + src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/streval.c | 13 ++++--------- src/cmd/ksh93/sh/xec.c | 4 ++++ src/cmd/ksh93/tests/arith.sh | 26 ++++++++++++++++++++++++++ 6 files changed, 43 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 1cf04f129..27f9c6c6b 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,13 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2021-04-10: + +- Fixed: the internal count of the recursion level for arithmetic expressions + was not reset when certain errors occurred in a virtual subshell. This could + cause an erroneous "recursion to deep" error when a loop executed many + subshells containing arithmetic expressions with errors, e.g. for testing. + 2021-04-09: - Fixed a bug that caused ksh to enable -c during the shell's initialization diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h index 84642e9c2..6ebc299cd 100644 --- a/src/cmd/ksh93/include/defs.h +++ b/src/cmd/ksh93/include/defs.h @@ -203,6 +203,7 @@ struct shared char universe; \ char winch; \ char inarith; /* set when in ((...)) */ \ + short arithrecursion; /* current arithmetic recursion level */ \ char indebug; /* set when in debug trap */ \ unsigned char ignsig; /* ignored signal in subshell */ \ unsigned char lastsig; /* last signal received */ \ diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index e62ff12e8..35529d808 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -20,7 +20,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2021-04-09" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2021-04-10" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ diff --git a/src/cmd/ksh93/sh/streval.c b/src/cmd/ksh93/sh/streval.c index fa94d863b..e2410cf1b 100644 --- a/src/cmd/ksh93/sh/streval.c +++ b/src/cmd/ksh93/sh/streval.c @@ -65,8 +65,6 @@ *((type*)stakptr((v)->offset)) = (val)),(v)->offset) #define roundptr(ep,cp,type) (((unsigned char*)(ep))+round(cp-((unsigned char*)(ep)),pow2size(sizeof(type)))) -static int level; - struct vars /* vars stacked per invocation */ { Shell_t *shp; @@ -125,14 +123,12 @@ static int _seterror(struct vars *vp,const char *msg) vp->errmsg.value = (char*)msg; vp->errchr = vp->nextchr; vp->nextchr = ""; - level = 0; return(0); } static void arith_error(const char *message,const char *expr, int mode) { - level = 0; mode = (mode&3)!=0; errormsg(SH_DICT,ERROR_exit(mode),message,expr); } @@ -177,7 +173,7 @@ Sfdouble_t arith_exec(Arith_t *ep) node.nosub = 0; node.ptr = 0; node.eflag = 0; - if(level++ >=MAXLEVEL) + if(shp->arithrecursion++ >= MAXLEVEL) { arith_error(e_recursive,ep->expr,ep->emode); return(0); @@ -249,7 +245,7 @@ Sfdouble_t arith_exec(Arith_t *ep) if(node.flag = c) lastval = 0; node.isfloat=0; - node.level = level; + node.level = shp->arithrecursion; node.nosub = 0; num = (*ep->fun)(&ptr,&node,VALUE,num); if(node.emode&ARITH_ASSIGNOP) @@ -497,8 +493,8 @@ Sfdouble_t arith_exec(Arith_t *ep) *sp = num; *tp = type; } - if(level>0) - level--; + if(shp->arithrecursion>0) + shp->arithrecursion--; if(type==0 && !num) num = 0; return(num); @@ -1022,7 +1018,6 @@ Sfdouble_t strval(Shell_t *shp,const char *s,char **end,Sfdouble_t(*conv)(const default: return(1); } - level=0; errormsg(SH_DICT,ERROR_exit(1),message,ep->name); UNREACHABLE(); } diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 5e85c68a8..69af7ffc6 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -1020,6 +1020,7 @@ int sh_exec(register const Shnode_t *t, int flags) error_info.line = t->com.comline-shp->st.firstline; com = sh_argbuild(shp,&argn,&(t->com),OPTIMIZE); echeck = 1; + shp->arithrecursion = 0; if(t->tre.tretyp&COMSCAN) { argp = t->com.comarg; @@ -1903,6 +1904,7 @@ int sh_exec(register const Shnode_t *t, int flags) case TPAR: echeck = 1; + shp->arithrecursion = 0; flags &= ~OPTIMIZE_FLAG; if(!shp->subshell && !shp->st.trapcom[0] && !shp->st.trap[SH_ERRTRAP] && (flags&sh_state(SH_NOFORK))) { @@ -1966,6 +1968,7 @@ int sh_exec(register const Shnode_t *t, int flags) int *exitval=0,*saveexitval = job.exitval; pid_t savepgid = job.curpgid; echeck = 1; + shp->arithrecursion = 0; job.exitval = 0; job.curjobid = 0; if(shp->subshell) @@ -2649,6 +2652,7 @@ int sh_exec(register const Shnode_t *t, int flags) skipexitset++; error_info.line = t->tst.tstline-shp->st.firstline; echeck = 1; + shp->arithrecursion = 0; if((type&TPAREN)==TPAREN) { sh_exec(t->lst.lstlef,OPTIMIZE); diff --git a/src/cmd/ksh93/tests/arith.sh b/src/cmd/ksh93/tests/arith.sh index 251d2c2bf..cfc479439 100755 --- a/src/cmd/ksh93/tests/arith.sh +++ b/src/cmd/ksh93/tests/arith.sh @@ -822,5 +822,31 @@ got=$(set +x; { var="x\]+b\[\$(echo INJECTION >&2)"; typeset -A a; ((a[\$var]++) got=$(set +x; { var="x\]+b\[\$(uname>&2)"; typeset -A a; ((a["\$var"]++)); typeset -p a; } 2>&1) [[ $got == "$exp" ]] || err_exit "Array subscript quoting test 5M: expected $(printf %q "$exp"), got $(printf %q "$got")" +# ====== +# Arithmetic recursion level was not reset on encountering readonly error in combination with a recursive arithmetic expression. +# (arithmetic array subscripts inside arithmetic expressions are one example of such recursion) +# https://github.com/ksh93/ksh/pull/239#discussion_r606874442 +srcdir=${SHTESTS_COMMON%/tests/*} +integer maxlevel=$(grep $'^#define MAXLEVEL\t' "$srcdir/sh/streval.c" | sed 's/.*MAXLEVEL//') +if ((!maxlevel)) +then err_exit "could not get maximum arithmetic recursion level from source code; update this test" + maxlevel=1024 +fi +integer loopcount=maxlevel+10 +got=$( + typeset -r -A -i ro_arr=([a]=10 [b]=20 [c]=30) + for ((i=0; i&1 +) +[[ $got == *recursion* ]] && err_exit "recursion level not reset on readonly error (main shell)" +got=$( + typeset -r -A -i ro_arr=([a]=10 [b]=20 [c]=30) + for ((i=0; i&1 +) +[[ $got == *recursion* ]] && err_exit "recursion level not reset on readonly error (subshell)" + # ====== exit $((Errors<125?Errors:125))