Fork before entering shared-state command substitution

The code contains various checks to see if a subshell needs to
fork, like this one in the ulimit builtin:

	if(shp->subshell && !shp->subshare)
		sh_subfork();

All checks of this form are fatally broken, as each one of them
causes shared-state command substitutions to ignore parent virtual
subshells.

Currently the only feasible way to fix this is to fork a virtual
subshell before executing a shared-state command substitution in
it. In the long term I think shared-state command substitutions
should probably be redesigned to disassociate them completely from
the virtual subshell mechanism.

src/cmd/ksh93/sh/macro.c: comsubst():
- If we're in a non-subshare virtual subshell, fork it before
  entering a type 2 (subshare) command substitution.

src/cmd/ksh93/sh/subshell.c:
- sh_assignok(): Remove subshare fix from 911d6b06 as it's
  redundant now that the parent of a subshare is never a virtual
  subshell. Go back to not doing anything if the current "subshell"
  is a subshare.
- sh_subtracktree(), sh_subfuntree(): Similarly, remove the
  now-redundant subshare fixes from 13c57e4b.

src/cmd/ksh93/sh/xec.c: sh_exec():
- Fix a separate bug: only fork a virtual subshell before running a
  background job if that "subshell" is not a subshare.

src/cmd/ksh93/tests/subshell.sh:
- Add test for bug fixed in xec.c.
- Add tests for 'ulimit', 'builtin' and 'exec' run in subshare
  within subshell -- all commands that use checks of the form
  'if(sh.subshell && !sh.subshare) sh_subfork();'.

Resolves: https://github.com/ksh93/ksh/issues/289

NEWS

@@ -13,6 +13,9 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 - Fixed a bug introduced on 2020-09-05 that caused "echo ${var:+'{}'}"
   to be misparsed.
 
+- Fixed: the effects of 'builtin', 'exec' and 'ulimit' leaked out of a parent
+  virtual subshell if run from a ${ shared-state; } command substitution.
+
 2021-04-26:
 
 - Fixed a bug introduced on 2021-02-20 in which a shared-state command

src/cmd/ksh93/sh/macro.c

@@ -2075,6 +2075,7 @@ nosub:
 /*
  * This routine handles command substitution
  * <type> is 0 for older `...` version
+ * 1 for $(...) or 2 for ${ subshare; }
  */
 static void comsubst(Mac_t *mp,register Shnode_t* t, int type)
 {
@@ -2194,7 +2195,11 @@ static void comsubst(Mac_t *mp,register Shnode_t* t, int type)
 			type = 3;
 		}
 		else
+		{
+			if(type==2 && sh.subshell && !sh.subshare)
+				sh_subfork();	/* subshares within virtual subshells are broken, so fork first */
 			sp = sh_subshell(mp->shp,t,sh_isstate(SH_ERREXIT),type);
+		}
 		fcrestore(&save);
 	}
 	else

src/cmd/ksh93/sh/subshell.c

@@ -273,32 +273,11 @@ Namval_t *sh_assignok(register Namval_t *np,int add)
 	Namarr_t		*ap;
 	unsigned int		save;
 	/*
-	 * Don't create a scope if told not to (see nv_restore()).
+	 * Don't create a scope if told not to (see nv_restore()) or if this is a subshare.
 	 * Also, moving/copying ${.sh.level} (SH_LEVELNOD) may crash the shell.
 	 */
-	if(subshell_noscope || add<2 && np==SH_LEVELNOD)
+	if(subshell_noscope || sh.subshare || 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;
@@ -427,14 +406,8 @@ static void nv_restore(struct subshell *sp)
 Dt_t *sh_subtracktree(int create)
 {
 	register struct subshell *sp = subshell_data;
-	if(create && sh.subshell)
+	if(create && sh.subshell && !sh.subshare)
 	{
-		if(sh.subshare)
-		{
-			while(sp->subshare)	/* move up as long as parent is also a ${ subshare; } */
-				sp = sp->prev;
-			sp = sp->prev;		/* move to first non-subshare parent */
-		}
 		if(sp && !sp->strack)
 		{
 			sp->strack = dtopen(&_Nvdisc,Dtset);
@@ -453,14 +426,8 @@ Dt_t *sh_subtracktree(int create)
 Dt_t *sh_subfuntree(int create)
 {
 	register struct subshell *sp = subshell_data;
-	if(create && sh.subshell)
+	if(create && sh.subshell && !sh.subshare)
 	{
-		if(sh.subshare)
-		{
-			while(sp->subshare)	/* move up as long as parent is also a ${ subshare; } */
-				sp = sp->prev;
-			sp = sp->prev;		/* move to first non-subshare parent */
-		}
 		if(sp && !sp->sfun)
 		{
 			sp->sfun = dtopen(&_Nvdisc,Dtoset);

src/cmd/ksh93/sh/xec.c

@@ -1599,7 +1599,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 				if((shp->comsub && (type&(FAMP|TFORK))==(FAMP|TFORK) || shp->comsub==1) &&
 				!(shp->fdstatus[1]&IONOSEEK))
 					unpipe = iousepipe(shp);
-				if((type&(FAMP|TFORK))==(FAMP|TFORK))
+				if((type&(FAMP|TFORK))==(FAMP|TFORK) && !shp->subshare)
 					sh_subfork();
 			}
 			no_fork = !ntflag && !(type&(FAMP|FPOU)) && !shp->subshell &&

src/cmd/ksh93/tests/subshell.sh

@@ -1018,5 +1018,21 @@ dummy=${ : >&2; got='good'; }
 [[ $got == "$exp" ]] || err_exit 'subshare stopped sharing state after command that redirects stdout' \
 	"(expected '$exp', got '$got')"
 
+# ======
+unset d x
+exp='end 1'
+got=$(d=${ true & x=1; echo end; }; echo $d $x)
+[[ $got == "$exp" ]] || err_exit 'subshare forks when running background job' \
+	"(expected '$exp', got '$got')"
+
+# ======
+# https://github.com/ksh93/ksh/issues/289
+got=$(ulimit -t unlimited 2>/dev/null; (dummy=${ ulimit -t 1; }); ulimit -t)
+[[ $got == 1 ]] && err_exit "'ulimit' command run in subshare leaks out of parent virtual subshell"
+got=$(_AST_FEATURES="TEST_TMP_VAR - $$" "$SHELL" -c '(d=${ builtin getconf;}); getconf TEST_TMP_VAR' 2>&1)
+[[ $got == $$ ]] && err_exit "'builtin' command run in subshare leaks out of parent virtual subshell"
+got=$(ulimit -t unlimited 2>/dev/null; (dummy=${ exec true; }); echo ok)
+[[ $got == ok ]] || err_exit "'exec' command run in subshare disregards parent virtual subshell"
+
 # ======
 exit $((Errors<125?Errors:125))