From 73dea411d32bd8cc4bc0a9e79710a00817b05970 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 22 Jan 2024 16:04:46 +0000 Subject: [PATCH] SHOPT_OPTIMIZE: Do not optimize namerefs in for loops Reproducer: one=1 bar=2 baz=3 nameref vv for vv in one bar baz do print -n -- "$vv" done Expected output: 123 Actual output: 111 This is another bug in the loop invariants optimiser. Research on the abandoned AT&T betas shows this bug was fixed in ksh 93v- 2013-02-13 (tagged as 2013-02-14 in the ksh93-history repo). The 93v- fix involves the sh_exec() TFOR code in xec.c temporarily setting the NV_TABLE attribute for namerefs, then adding a check for it in nv_create() so optimisation is disabled for namerefs. This commit backports that fix. For code legibility's sake, in this commit I'm aliasing NV_TABLE to NV_NOOPTIMIZE. This also allows us to define it as 0 when SHOPT_OPTIMIZE is disabled at compile time, so the compiler optimises out the new code. (It makes sense to (ab)use NV_TABLE for this purpose as it is never normally set for namerefs, plus, tables (as in .foo.bar-style variables, name starting with dot) aren't optimised either.) Thanks to @ormaaj for the bug report. Resolves: https://github.com/ksh93/ksh/issues/704 --- NEWS | 6 ++++ src/cmd/ksh93/include/nval.h | 5 ++++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/name.c | 2 +- src/cmd/ksh93/sh/xec.c | 7 ++++- src/cmd/ksh93/tests/nameref.sh | 14 +++++++++ src/cmd/ksh93/tests/types.sh | 50 +++++++++++++++++++++++++++++++++ 7 files changed, 83 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 81d5bb71705b..d2657ccb1d71 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,12 @@ This documents significant changes in the dev branch of ksh 93u+m. For full details, see the git log at: https://github.com/ksh93/ksh Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library. +2024-01-22: + +- Fixed a bug in the loop invariants optimizer (SHOPT_OPTIMIZE) that caused + expansions of name references to be incorrectly treated as invariant so they + yielded the wrong values. Fix backported from from ksh 93v- 2013-02-13. + 2024-01-21: - [v1.1] New feature added: SRANDOM is a secure random number generator. diff --git a/src/cmd/ksh93/include/nval.h b/src/cmd/ksh93/include/nval.h index 677199c608f2..969163ff137e 100644 --- a/src/cmd/ksh93/include/nval.h +++ b/src/cmd/ksh93/include/nval.h @@ -139,6 +139,11 @@ struct Namval #define NV_TABLE 0x800 /* node is a dictionary table */ #define NV_IMPORT 0x1000 /* value imported from environment */ #define NV_MINIMAL NV_IMPORT /* node does not contain all fields */ +#if SHOPT_OPTIMIZE +#define NV_NOOPTIMIZE NV_TABLE /* disable loop invariants optimizer */ +#else +#define NV_NOOPTIMIZE 0 +#endif #define NV_INTEGER 0x2 /* integer attribute */ /* The following attributes are valid only when NV_INTEGER is off */ diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 6b1fd8cf8fae..dc55f4278de9 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -18,7 +18,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.1.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2024-01-21" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2024-01-22" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2024 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/name.c b/src/cmd/ksh93/sh/name.c index 1470b66dee68..3555ce68e0e2 100644 --- a/src/cmd/ksh93/sh/name.c +++ b/src/cmd/ksh93/sh/name.c @@ -908,7 +908,7 @@ Namval_t *nv_create(const char *name, Dt_t *root, int flags, Namfun_t *dp) #if NVCACHE nvcache.ok = 0; #endif - if(c=='.') /* don't optimize */ + if(nv_isattr(np,NV_NOOPTIMIZE) || c=='.') /* don't optimize */ sh.argaddr = 0; else if((flags&NV_NOREF) && (c!='[' && *cp!='.')) { diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index dc2628ee7673..c04ba23102a4 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -2029,12 +2029,15 @@ int sh_exec(const Shnode_t *t, int flags) } } if(nameref) - nv_offattr(np,NV_REF); + nv_offattr(np,NV_REF|NV_NOOPTIMIZE); else if(nv_isattr(np, NV_ARRAY)) nv_putsub(np,NULL,0L); nv_putval(np,cp,0); if(nameref) + { nv_setref(np,NULL,NV_VARNAME); + nv_onattr(np,NV_NOOPTIMIZE); + } if(trap=sh.st.trap[SH_DEBUGTRAP]) { av[0] = (t->tre.tretyp&COMSCAN)?"select":"for"; @@ -2058,6 +2061,8 @@ int sh_exec(const Shnode_t *t, int flags) if(sh.st.breakcnt<0) sh.st.breakcnt++; } + if(nameref) + nv_offattr(np,NV_NOOPTIMIZE); #if SHOPT_OPTIMIZE endfor: sh_popcontext(buffp); diff --git a/src/cmd/ksh93/tests/nameref.sh b/src/cmd/ksh93/tests/nameref.sh index e48a0d28e06a..b8a89aefe29f 100755 --- a/src/cmd/ksh93/tests/nameref.sh +++ b/src/cmd/ksh93/tests/nameref.sh @@ -756,5 +756,19 @@ got=$("$SHELL" -c 'nameref unsetref; unsetref[1]=bar' 2>&1) "(expected status 1 and match of *$(printf %q "$exp")," \ "got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))" +# ====== +# https://github.com/ksh93/ksh/issues/704 +# test backported from ksh 93v- 2013-02-14 +unset one bar baz arr val vv +one=1 bar=2 baz=3 +arr=(one bar baz) +nameref vv +val=$( + for vv in "${arr[@]}" + do print -n -- "$vv" + done +) +[[ $val == 123 ]] || err_exit 'optimization bug with for loops with references' + # ====== exit $((Errors<125?Errors:125)) diff --git a/src/cmd/ksh93/tests/types.sh b/src/cmd/ksh93/tests/types.sh index 97f1f63d16ca..0853a58c164b 100755 --- a/src/cmd/ksh93/tests/types.sh +++ b/src/cmd/ksh93/tests/types.sh @@ -795,5 +795,55 @@ e=$? let "e==0" || err_exit 'crash involving short int as first type member' \ "(got status $e$( ((e>128)) && print -n /SIG && kill -l "$e"), $(printf %q "$got"))" +# ====== +# loop invariants optimizer bug +# reproducer by Daniel Douglas +# https://github.com/ksh93/ksh/issues/704 +typeset -T Thing=( + integer x + typeset y +) +function f +{ + Thing -a t=( + [0]=(x=1; y=hi) + [1]=(x=2; y=yo) + [2]=(x=3; y=moo) + [5]=(x=6; y=boo) + [9]=(x=10; y=boom) + ) + typeset -p t + g t +} +function g +{ + typeset -n ref=$1 d e + set -- "${!ref[@]}" + set -- "${@/*/ref[\0]}" + for e do + for d in e.x e.y + do printf '%s %s %-14s %s\n' "${@e}" "${!d}" "${@d}" "${d}" + done + done + echo +} +exp='Thing -a t=([0]=(typeset -l -i x=1;y=hi) [1]=(typeset -l -i x=2;y=yo) [2]=(typeset -l -i x=3;y=moo) [5]=(typeset -l -i x=6;y=boo) [9]=(typeset -l -i x=10;y=boom)) +Thing t[0].x typeset -l -i 1 +Thing t[0].y hi +Thing t[1].x typeset -l -i 2 +Thing t[1].y yo +Thing t[2].x typeset -l -i 3 +Thing t[2].y moo +Thing t[5].x typeset -l -i 6 +Thing t[5].y boo +Thing t[9].x typeset -l -i 10 +Thing t[9].y boom' +got=$(f 2>&1) +[[ $got == "$exp" ]] || +{ + err_exit "issue 704: expected '-' lines, got '+' lines:" + diff -U1 <(print "$exp") <(print "$got") | sed $'1,3 d; s,^,\t,' >&2 +} + # ====== exit $((Errors<125?Errors:125))