Skip to content

Commit

Permalink
Remove a buggy optimization for variables in subshells
Browse files Browse the repository at this point in the history
This bug was originally reported by @lijog in att#7 and has been
reported again in ksh93#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.
  • Loading branch information
JohnoKing committed Jun 15, 2020
1 parent ef1621c commit 3d38270
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 3 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions src/cmd/ksh93/sh/subshell.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 13 additions & 0 deletions src/cmd/ksh93/tests/subshell.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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))

0 comments on commit 3d38270

Please sign in to comment.