Fix subshell scoping of changes in shared command substitution

A ${ shared-state command substitution; } (internally called
subshare) is documented to share its state with the parent shell
environment, so all changes made within the command substitution
survive outside of it. However, when it is run within a
virtual/non-forked subshell, variables that are not already local
to that subshell will leak out of it into the grandparent state.
Reproducer:

	$ ksh -c '( v=${ bug=BAD; } ); echo "$bug"'
	BAD

If the variable pre-exists in the subshell, the bug does not occur:

	$ ksh -c '( bug=BAD1; v=${ bug=BAD2; } ); echo "$bug"'
	(empty line, as expected)

The problem is that the sh_assignok() function, which is
responsible for variable scoping in virtual subshells, does not
ever bother to create a virtual subshell scope for a subshare.
That is an error if a subshare's parent (or higher-up ancestor)
environment is a virtual subshell, because a scope needs to be
created in that parent environment if none exists.

To make this bugfix possible, first we need to get something out of
the way. nv_restore() temporarily sets the subshell's pointer to
the preesnt working directory, shpwd, to null. This causes
sh_assignok() to assume that the subshell is a subshare (because
subshares don't store their own PWD) and refuse to create a scope.
However, nv_restore() sets it to null for a different purpose: to
temporarily disable scoping for *all* virtual subshells, making
restoring possible. This is a good illustration of why it's often
not a good idea to use the same variable for unrelated purposes.

src/cmd/ksh93/sh/subshell.c:
- Add a global static subshell_noscope flag variable to replace the
  misuse of sh.shpwd described above.
- sh_assignok():
  . Check subshell_noscope instead of shpwd to see if scope
    creation is disabled. This makes it possible to distinguish
    between restoring scope and handling subshares.
  . If the current environment is a subshare that is in a virtual
    subshell, create a scope in the parent subshell. This is done
    by temporarily making the parent virtual subshell the current
    subshell (by setting the global subshell_data pointer to it)
    and calling sh_assignok() again, recursively.
- nv_restore(): To disable subshell scope creation while restoring,
  set subshell_noscope instead of saving and unsetting sh.shpwd.

src/cmd/ksh93/tests/subshell.sh:
- Add tests. I like tests. Tests are good.

Fixes: https://github.com/ksh93/ksh/issues/143

NEWS

@@ -10,6 +10,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
      extra characters.
   2. The backslash now escapes a subsequent interrupt (^C) as documented.
 
+- Fixed a longstanding bug with shared-state command substitutions of the form
+  ${ command; }. If these were executed in a subshell, changes made within
+  could survive not only the command substitution but also the parent subshell.
+
 2021-02-15:
 
 - Fixed a regression introduced by ksh93 (was not in ksh88): an empty 'case'

src/cmd/ksh93/sh/subshell.c

@@ -106,6 +106,8 @@ static struct subshell
 #endif /* _lib_fchdir */
 } *subshell_data;
 
+static char subshell_noscope;	/* for temporarily disabling all virtual subshell scope creation */
+
 static unsigned int subenv;
 
 
@@ -259,18 +261,39 @@ Namval_t *sh_assignok(register Namval_t *np,int add)
 {
 	register Namval_t	*mp;
 	register struct Link	*lp;
-	register struct subshell *sp = (struct subshell*)subshell_data;
+	struct subshell		*sp = subshell_data;
 	Shell_t			*shp = sp->shp;
 	Dt_t			*dp= shp->var_tree;
 	Namval_t		*mpnext;
 	Namarr_t		*ap;
 	unsigned int		save;
 	/*
-	 * Don't save if told not to (see nv_restore()) or if we're in a ${ subshare; }.
+	 * Don't create a scope if told not to (see nv_restore()).
 	 * Also, moving/copying ${.sh.level} (SH_LEVELNOD) may crash the shell.
 	 */
-	if(!sp->shpwd || shp->subshare || add<2 && np==SH_LEVELNOD)
+	if(subshell_noscope || add<2 && np==SH_LEVELNOD)
 		return(np);
+	/*
+	 * Don't create a scope for a ${ shared-state command substitution; } (a.k.a. subshare).
+	 * However, if the subshare is in a virtual subshell, we need to create a scope in that.
+	 * If so, temporarily make that the current subshell and call this function recursively.
+	 */
+	if(shp->subshare)
+	{
+		while(subshell_data->subshare)		/* move up as long as parent is also a subshare */
+			subshell_data = subshell_data->prev;
+		subshell_data = subshell_data->prev;	/* move to first non-subshare parent */
+		if(!subshell_data)			/* if that's not a virtual subshell, don't create a scope */
+		{
+			subshell_data = sp;
+			return(np);
+		}
+		shp->subshare = 0;
+		np = sh_assignok(np,add);
+		shp->subshare = 1;
+		subshell_data = sp;
+		return(np);
+	}
 	if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np)))
 	{
 		shp->last_root = ap->table;
@@ -329,10 +352,9 @@ static void nv_restore(struct subshell *sp)
 {
 	register struct Link *lp, *lq;
 	register Namval_t *mp, *np;
-	const char *save = sp->shpwd;
 	Namval_t	*mpnext;
 	int		flags,nofree;
-	sp->shpwd = 0;	/* make sure sh_assignok doesn't save with nv_unset() */
+	subshell_noscope = 1;
 	for(lp=sp->svar; lp; lp=lq)
 	{
 		np = (Namval_t*)&lp->dict;
@@ -389,7 +411,7 @@ static void nv_restore(struct subshell *sp)
 		free((void*)lp);
 		sp->svar = lq;
 	}
-	sp->shpwd=save;
+	subshell_noscope = 0;
 }
 
 /*

src/cmd/ksh93/tests/subshell.sh

@@ -914,5 +914,41 @@ got=$( { "$SHELL" -c '(cd /); print -r -- "PWD=$PWD"'; } 2>&1 )
 	"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
 cd "$tmp"
 
+# ======
+# Before 93u+m 2021-02-17, the state of ${ shared-state command substitutions; } leaked out of parent virtual subshells.
+# https://github.com/ksh93/ksh/issues/143
+
+v=main
+d=${ v=shared; }
+[[ $v == shared ]] || err_exit "shared comsub in main shell wrongly scoped"
+
+v=main
+(d=${ v=shared; }; [[ $v == shared ]]) || err_exit "shared comsub in subshell wrongly scoped (1)"
+[[ $v == main ]] || err_exit "shared comsub leaks out of subshell (1)"
+
+v=main
+(v=sub; d=${ v=shared; }; [[ $v == shared ]]) || err_exit "shared comsub in subshell wrongly scoped (2)"
+[[ $v == main ]] || err_exit "shared comsub leaks out of subshell (2)"
+
+v=main
+(v=sub; (d=${ v=shared; }); [[ $v == sub ]]) || err_exit "shared comsub leaks out of nested subshell"
+[[ $v == main ]] || err_exit "shared comsub leaks out of subshell (3)"
+
+v=main
+(d=${ d=${ v=shared; }; }; [[ $v == shared ]]) || err_exit "nested shared comsub in subshell wrongly scoped (1)"
+[[ $v == main ]] || err_exit "shared comsub leaks out of subshell (4)"
+
+v=main
+(v=sub; d=${ d=${ v=shared; }; }; [[ $v == shared ]]) || err_exit "nested shared comsub in subshell wrongly scoped (2)"
+[[ $v == main ]] || err_exit "shared comsub leaks out of subshell (5)"
+
+v=main
+( (d=${ v=shared; }; [[ $v == shared ]]) ) || err_exit "shared comsub in nested subshell wrongly scoped (1)"
+[[ $v == main ]] || err_exit "shared comsub leaks out of subshell (6)"
+
+v=main
+(v=sub; (d=${ v=shared; }; [[ $v == shared ]]) ) || err_exit "shared comsub in nested subshell wrongly scoped (2)"
+[[ $v == main ]] || err_exit "shared comsub leaks out of subshell (7)"
+
 # ======
 exit $((Errors<125?Errors:125))