From 3d38270b323dbb78d62772995143c7a001217893 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Mon, 15 Jun 2020 06:43:37 -0700 Subject: [PATCH] Remove a buggy optimization for variables in subshells This bug was originally reported by @lijog in att/ast#7 and has been reported again in #15. KSH does not save the state of a variable if it is in a newer scope. This is because of an optimization in sh_assignok first introduced in ksh93t+ 2010-05-24. Here is the code change in that version: return(np); /* don't bother to save if in newer scope */ - if(!(rp=shp->st.real_fun) || !(dp=rp->sdict)) - dp = sp->var; - if(np->nvenv && !nv_isattr(np,NV_MINIMAL|NV_EXPORT) && shp->last_root) - dp = shp->last_root; - if((mp=nv_search((char*)np,dp,HASH_BUCKET))!=np) - { - if(mp || !np->nvfun || np->nvfun->subshell>=sh.subshell) - return(np); - } + if(sp->var!=shp->var_tree && shp->last_root==shp->var_tree) + return(np); if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np))) { This change was originally made to replace a buggier optimization. However, the current optimization causes variables set in subshells to wrongly affect the environment outside of the subshell, as the variable does not get set back to its original value. This patch simply removes the buggy optimization to fix this problem. src/cmd/ksh93/sh/subshell.c: - Remove a buggy optimization that caused variables set in subshells to affect the environment outside of the subshell. src/cmd/ksh93/tests/subshell.sh: - Add a regression test for setting variables in subshells. This test has to be run from the disk after being created with a here document because it always returns the expected result when run directly in the regression test script. --- NEWS | 3 +++ src/cmd/ksh93/sh/subshell.c | 3 --- src/cmd/ksh93/tests/subshell.sh | 13 +++++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 49be082ca..fa5a5b99c 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,9 @@ Any uppercase BUG_* names are modernish shell bug IDs. - The 'source' alias has been converted into a regular built-in command. +- Functions that set variables in a virtual subshell will no longer affect + variables of the same name outside of the virtual subshell's environment. + 2020-06-14: - 'read -S' is now able to correctly handle strings with double quotes diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index 0aa1ceb4f..ef33fb3b7 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -251,9 +251,6 @@ Namval_t *sh_assignok(register Namval_t *np,int add) /* don't bother with this */ if(!sp->shpwd || np==SH_LEVELNOD || np==L_ARGNOD || np==SH_SUBSCRNOD || np==SH_NAMENOD) return(np); - /* don't bother to save if in newer scope */ - if(sp->var!=shp->var_tree && sp->var!=shp->var_base && shp->last_root==shp->var_tree) - return(np); if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np))) { shp->last_root = ap->table; diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 1a676f67d..9098e91c3 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/src/cmd/ksh93/tests/subshell.sh @@ -726,5 +726,18 @@ check_hash_table() (hash cat) [[ $(hash) == "chmod=$(whence -p chmod)" ]] || err_exit $'changes to a subshell\'s hash table affect the parent shell' +# ====== +# Variables set in functions inside of a virtual subshell should not affect the +# outside environment. This regression test must be run from the disk. +testvars=$tmp/testvars.sh +cat >| "$testvars" << 'EOF' +c=0 +function set_ac { a=1; c=1; } +function set_abc { ( set_ac ; b=1 ) } +set_abc +echo "a=$a b=$b c=$c" +EOF +v=$($SHELL $testvars) && [[ "$v" == "a= b= c=0" ]] || err_exit 'variables set in subshells are not confined to the subshell' + # ====== exit $((Errors<125?Errors:125))