Fix $RANDOM to act consistently in subshells (#294)

This fixes the following:
1. Using $RANDOM in a virtual/non-forked subshell no longer
   influences the reproducible $RANDOM sequence in the parent
   environment.
2. When invoking a subshell $RANDOM is now re-seeded (as mksh and
   bash do) so that invocations in repeated subshells (including
   forked subshells) longer produce identical sequences by default.
3. Program flow corruption that occurred in scripts on executing
   ( ( simple_command & ) ).

src/cmd/ksh93/include/variables.h:
- Move 'struct rand' here as it will be needed in subshell.c. Add
  rand_seed member to save the pseudorandom generator seed. Remove
  the pointer to the shell state as it's redundant.

src/cmd/ksh93/sh/init.c:
- put_rand(): Store given seed in rand_seed while calling srand().
  No longer pointlessly limit the number of possible seeds with the
  RANDMASK bitmask (that mask is to limit the values to 0-32767,
  it should not limit the number of possible sequences to 32768).
- nget_rand(): Instead of using rand(), use rand_r() to update the
  random_seed value. This makes it possible to save/restore the
  current seed of the pseudorandom generator.
- Add sh_reseed_rand() function that reseeds the pseudorandom
  generator by calling srand() with a bitwise-xor combination of
  the current PID, the current time with a granularity of 1/10000
  seconds, and a sequence number that is increased on each
  invocation.
- nv_init(): Set the initial seed using sh_reseed_rand() here
  instead of in sh_main(), as this is where the other struct rand
  members are initialised.

src/cmd/ksh93/sh/main.c: sh_main():
- Remove the srand() call that was replaced by the sh_reseed_rand()
  call in init.c.

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Upon entering a virtual subshell, save the current $RANDOM seed
  and state, then reseed $RANDOM for the subshell.
- Upon exiting a virtual subshell, restore $RANDOM seed and state
  and reseed the generator using srand() with the restored seed.

src/cmd/ksh93/sh/xec.c: sh_exec():
- When optimizing out a subshell that is the last command, still
  act like a subshell: reseed $RANDOM and increase ${.sh.subshell}.
- Fix a separate bug discovered while implementing this. Do not
  optimize '( simple_command & )' when in a virtual subshell; doing
  this causes program flow corruption.
- When optimizing '( simple_command & )', also reseed $RANDOM and
  increment ${.sh.subshell}.

src/cmd/ksh93/tests/subshell.sh,
src/cmd/ksh93/tests/variables.sh:
- Add various tests for all of the above.

Co-authored-by: Johnothan King <[email protected]>
Resolves: #285

NEWS

@@ -5,6 +5,15 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 
 2021-05-03:
 
+- Subshells (even if non-forked) now keep a properly separated state of the
+  pseudorandom generator used for $RANDOM, so that using $RANDOM in a
+  non-forked subshell no longer influences a reproducible $RANDOM sequence in
+  the parent environment. In addition, upon invoking a subshell, $RANDOM is now
+  reseeded (as mksh and bash do).
+
+- Fixed program flow corruption that occurred in scripts on executing a
+  background job in a nested subshell, as in ( ( simple_command & ) ).
+
 - Completed the 2021-04-30 fix for ${var<OP>'{}'} where <OP> is '-', '+',
   ':-' or ':+' by fixing a bug that caused an extra '}' to be output.
 

src/cmd/ksh93/COMPATIBILITY

@@ -132,6 +132,12 @@ For more details, see the NEWS file and for complete details, see the git log.
 24.	The readonly attribute of ksh variables is no longer imported from
 	or exported to other ksh shell instances through the environment.
 
+25.	Subshells (even if non-forked) now keep a properly separated state
+	of the pseudorandom generator used for $RANDOM, so that using
+	$RANDOM in a non-forked subshell no longer influences a reproducible
+	$RANDOM sequence in the parent environment. In addition, upon
+	invoking a subshell, $RANDOM is now reseeded (as mksh and bash do).
+
 ____________________________________________________________________________
 
 		KSH-93 VS. KSH-88

src/cmd/ksh93/Mamfile

@@ -311,6 +311,7 @@ make install
 						prev FEATURE/dynamic implicit
 						prev FEATURE/options implicit
 						prev ${PACKAGE_ast_INCLUDE}/option.h implicit
+						prev include/nval.h implicit
 					done include/variables.h
 					prev ${PACKAGE_ast_INCLUDE}/error.h implicit
 					prev ${PACKAGE_ast_INCLUDE}/stak.h implicit

src/cmd/ksh93/include/variables.h

@@ -25,6 +25,16 @@
 #include        <option.h>
 #include        "FEATURE/options"
 #include        "FEATURE/dynamic"
+#include	<nval.h>
+
+/* used for RANDNOD ($RANDOM) */
+struct rand
+{
+	Namfun_t	hdr;
+	unsigned int	rand_seed;
+	int32_t		rand_last;
+};
+extern void sh_reseed_rand(struct rand *);
 
 /* The following defines must be kept synchronous with shtab_variables[] in data/variables.c */
 

src/cmd/ksh93/sh/init.c

@@ -34,6 +34,7 @@
 #include        <pwd.h>
 #include        <tmx.h>
 #include        <regex.h>
+#include	<math.h>
 #include        "variables.h"
 #include        "path.h"
 #include        "fault.h"
@@ -142,13 +143,6 @@ struct seconds
 	Shell_t		*sh;
 };
 
-struct rand
-{
-	Namfun_t	hdr;
-	Shell_t		*sh;
-	int32_t		rand_last;
-};
-
 struct ifs
 {
 	Namfun_t	hdr;
@@ -658,8 +652,8 @@ static void put_rand(register Namval_t* np,const char *val,int flags,Namfun_t *f
 	if(flags&NV_INTEGER)
 		n = *(double*)val;
 	else
-		n = sh_arith(rp->sh,val);
-	srand((int)(n&RANDMASK));
+		n = sh_arith(&sh,val);
+	srand(rp->rand_seed = (unsigned int)n);
 	rp->rand_last = -1;
 	if(!np->nvalue.lp)
 		np->nvalue.lp = &rp->rand_last;
@@ -671,10 +665,11 @@ static void put_rand(register Namval_t* np,const char *val,int flags,Namfun_t *f
  */
 static Sfdouble_t nget_rand(register Namval_t* np, Namfun_t *fp)
 {
+	struct rand *rp = (struct rand*)fp;
 	register long cur, last= *np->nvalue.lp;
 	NOT_USED(fp);
 	do
-		cur = (rand()>>rand_shift)&RANDMASK;
+		cur = (rand_r(&rp->rand_seed)>>rand_shift)&RANDMASK;
 	while(cur==last);
 	*np->nvalue.lp = cur;
 	return((Sfdouble_t)cur);
@@ -686,6 +681,17 @@ static char* get_rand(register Namval_t* np, Namfun_t *fp)
 	return(fmtbase(n, 10, 0));
 }
 
+void sh_reseed_rand(struct rand *rp)
+{
+	struct tms		tp;
+	unsigned int		time;
+	static unsigned int	seq;
+	timeofday(&tp);
+	time = (unsigned int)remainder(dtime(&tp) * 10000.0, (double)UINT_MAX);
+	srand(rp->rand_seed = shgd->current_pid ^ time ^ ++seq);
+	rp->rand_last = -1;
+}
+
 /*
  * These three routines are for LINENO
  */
@@ -1748,7 +1754,6 @@ static Init_t *nv_init(Shell_t *shp)
 	ip->SECONDS_init.hdr.nofree = 1;
 	ip->RAND_init.hdr.disc = &RAND_disc;
 	ip->RAND_init.hdr.nofree = 1;
-	ip->RAND_init.sh = shp;
 	ip->SH_MATCH_init.hdr.disc = &SH_MATCH_disc;
 	ip->SH_MATCH_init.hdr.nofree = 1;
 	ip->SH_MATH_init.disc = &SH_MATH_disc;
@@ -1793,8 +1798,8 @@ static Init_t *nv_init(Shell_t *shp)
 	nv_stack(L_ARGNOD, &ip->L_ARG_init);
 	nv_putval(SECONDS, (char*)&d, NV_DOUBLE);
 	nv_stack(RANDNOD, &ip->RAND_init.hdr);
-	d = (shp->gd->pid&RANDMASK);
 	nv_putval(RANDNOD, (char*)&d, NV_DOUBLE);
+	sh_reseed_rand((struct rand *)RANDNOD->nvfun);
 	nv_stack(LINENO, &ip->LINENO_init);
 	SH_MATCHNOD->nvfun =  &ip->SH_MATCH_init.hdr;
 	nv_putsub(SH_MATCHNOD,(char*)0,10);

src/cmd/ksh93/sh/main.c

@@ -149,8 +149,6 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
 	}
 	shp->fn_depth = shp->dot_depth = 0;
 	command = error_info.id;
-	/* set pidname '$$' */
-	srand(shp->gd->pid&0x7fff);
 	if(nv_isnull(PS4NOD))
 		nv_putval(PS4NOD,e_traceprompt,NV_RDONLY);
 	path_pwd(shp,1);

src/cmd/ksh93/sh/subshell.c

@@ -488,6 +488,9 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 	struct sh_scoped savst;
 	struct dolnod   *argsav=0;
 	int argcnt;
+	struct rand *rp;		/* current $RANDOM discipline function data */
+	unsigned int save_rand_seed;	/* parent shell $RANDOM seed */
+	int save_rand_last;		/* last random number from $RANDOM in parent shell */
 	memset((char*)sp, 0, sizeof(*sp));
 	sfsync(shp->outpool);
 	sh_sigcheck(shp);
@@ -601,6 +604,11 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 		sp->cpipe = shp->cpipe[1];
 		shp->cpid = 0;
 		sh_sigreset(0);
+		/* save the current $RANDOM seed and state; reseed $RANDOM */
+		rp = (struct rand*)RANDNOD->nvfun;
+		save_rand_seed = rp->rand_seed;
+		save_rand_last = rp->rand_last;
+		sh_reseed_rand(rp);
 	}
 	jmpval = sigsetjmp(buff.buff,0);
 	if(jmpval==0)
@@ -856,6 +864,10 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 		shp->cpid = sp->cpid;
 		shp->cpipe[1] = sp->cpipe;
 		shp->coutpipe = sp->coutpipe;
+		/* restore $RANDOM seed and state */
+		rp = (struct rand*)RANDNOD->nvfun;
+		srand(rp->rand_seed = save_rand_seed);
+		rp->rand_last = save_rand_last;
 	}
 	shp->subshare = sp->subshare;
 	shp->subdup = sp->subdup;

src/cmd/ksh93/sh/xec.c

@@ -1937,6 +1937,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 			flags &= ~OPTIMIZE_FLAG;
 			if(!shp->subshell && !shp->st.trapcom[0] && !shp->st.trap[SH_ERRTRAP] && (flags&sh_state(SH_NOFORK)))
 			{
+				/* This is the last command, so avoid creating a subshell */
 				char *savsig;
 				int nsig,jmpval;
 				struct checkpt *buffp = (struct checkpt*)stkalloc(shp->stk,sizeof(struct checkpt));
@@ -1948,6 +1949,9 @@ int sh_exec(register const Shnode_t *t, int flags)
 					memcpy(savsig,(char*)&shp->st.trapcom[0],nsig);
 					shp->st.otrapcom = (char**)savsig;
 				}
+				/* Still act like a subshell: reseed $RANDOM and increment ${.sh.subshell} */
+				sh_reseed_rand((struct rand*)RANDNOD->nvfun);
+				shgd->realsubshell++;
 				sh_sigreset(0);
 				sh_pushcontext(shp,buffp,SH_JMPEXIT);
 				jmpval = sigsetjmp(buffp->buff,0);
@@ -1961,7 +1965,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 				sh_done(shp,0);
 			}
 			else if(((type=t->par.partre->tre.tretyp)&FAMP) && ((type&COMMSK)==TFORK)
-			&& !sh_isoption(SH_INTERACTIVE))
+			&& !job.jobcontrol && !shp->subshell)
 			{
 				/* Optimize '( simple_command & )' */
 				pid_t	pid;
@@ -1970,7 +1974,8 @@ int sh_exec(register const Shnode_t *t, int flags)
 					_sh_fork(shp,pid,0,0);
 				if(pid==0)
 				{
-					shgd->current_pid = getpid();
+					sh_reseed_rand((struct rand*)RANDNOD->nvfun);
+					shgd->realsubshell++;
 					sh_exec(t->par.partre,flags);
 					shp->st.trapcom[0]=0;
 					sh_done(shp,0);

src/cmd/ksh93/tests/subshell.sh

@@ -1034,5 +1034,14 @@ got=$(_AST_FEATURES="TEST_TMP_VAR - $$" "$SHELL" -c '(d=${ builtin getconf;}); g
 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"
 
+# ======
+# https://github.com/ksh93/ksh/pull/294#discussion_r624627501
+exp='this should be run once'
+$SHELL -c '( ( : & ) ); echo "this should be run once"' >r624627501.out
+sleep .01
+got=$(<r624627501.out)
+[[ $got == "$exp" ]] || err_exit 'background job optimization within virtual subshell causes program flow corruption' \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+
 # ======
 exit $((Errors<125?Errors:125))

src/cmd/ksh93/tests/variables.sh

@@ -26,10 +26,55 @@ unset ss
 [[ ${@ss} ]] && err_exit '${@ss} should be empty string when ss is unset'
 [[ ${!ss} == ss ]] ||  err_exit '${!ss} should be ss when ss is unset'
 [[ ${#ss} == 0 ]] ||  err_exit '${#ss} should be 0 when ss is unset'
+
 # RANDOM
 if	(( RANDOM==RANDOM || $RANDOM==$RANDOM ))
 then	err_exit RANDOM variable not working
 fi
+# When the $RANDOM variable is used in a forked subshell, it shouldn't
+# use the same pseudorandom seed as the main shell.
+# https://github.com/ksh93/ksh/issues/285
+RANDOM=123
+function rand_print {
+	ulimit -t unlimited 2> /dev/null
+	print $RANDOM
+}
+integer rand1=$(rand_print)
+integer rand2=$(rand_print)
+(( rand1 == rand2 )) && err_exit "Test 1: \$RANDOM seed in subshell doesn't change" \
+	"(both results are $rand1)"
+# Make sure we're actually using a different pseudorandom seed
+integer rand1=$(
+	ulimit -t unlimited 2> /dev/null
+	test $RANDOM
+	print $RANDOM
+)
+integer rand2=${ print $RANDOM ;}
+(( rand1 == rand2 )) && err_exit "Test 2: \$RANDOM seed in subshell doesn't change" \
+	"(both results are $rand1)"
+# $RANDOM should be reseeded when the final command is inside of a subshell
+rand1=$($SHELL -c 'RANDOM=1; (echo $RANDOM)')
+rand2=$($SHELL -c 'RANDOM=1; (echo $RANDOM)')
+(( rand1 == rand2 )) && err_exit "Test 3: \$RANDOM seed in subshell doesn't change" \
+	"(both results are $rand1)"
+# $RANDOM should be reseeded for the ( simple_command & ) optimization
+( echo $RANDOM & ) >r1
+( echo $RANDOM & ) >r2
+sleep .01
+(( $(<r1) == $(<r2) )) && err_exit "Test 4: \$RANDOM seed in ( simple_command & ) doesn't change" \
+	"(both results are $(<r1))"
+# Virtual subshells should not influence the parent shell's RANDOM sequence
+RANDOM=456
+exp="$RANDOM $RANDOM $RANDOM $RANDOM $RANDOM"
+RANDOM=456
+got=
+for((i=0; i<5; i++))
+do	: $( : $RANDOM $RANDOM $RANDOM )
+	got+=${got:+ }$RANDOM
+done
+[[ $got == "$exp" ]] || err_exit 'Using $RANDOM in subshell influences reproducible sequence in parent environment' \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+
 # SECONDS
 float secElapsed=0.0 secSleep=0.001
 let SECONDS=$secElapsed
@@ -165,7 +210,6 @@ if	[[ $LANG != "$save_LANG" ]]
 then	err_exit "$save_LANG locale not working"
 fi
 
-unset RANDOM
 unset -n foo
 foo=junk
 function foo.get
@@ -703,6 +747,12 @@ actual=$(
 expect=$'4\n3\n3\n2\n1'
 [[ $actual == "$expect" ]] || err_exit "\${.sh.subshell} failure (expected $(printf %q "$expect"), got $(printf %q "$actual"))"
 
+# ${.sh.subshell} should increment when the final command is inside of a subshell
+exp=1
+got=$($SHELL -c '(echo ${.sh.subshell})')
+[[ $exp == $got ]] || err_exit '${.sh.subshell} fails to increment when the final command is inside of a subshell' \
+	"(expected '$exp', got '$got')"
+
 unset IFS
 if	((SHOPT_BRACEPAT)) && command set -o braceexpand
 then	set -- {1..32768}
@@ -1067,6 +1117,23 @@ $SHELL -c '
 (((e = $?) == 1)) || err_exit "typeset -l/-u doesn't work on special variables" \
 	"(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))"
 
+# ... unset followed by launching a forked subshell
+$SHELL -c '
+	errors=0
+	unset -v "$@" || let errors++
+	(
+		ulimit -t unlimited 2>/dev/null
+		for var do
+			[[ $var == _ ]] && continue	# only makes sense that $_ is immediately set again
+			[[ -v $var ]] && let errors++
+		done
+		exit $((errors + 1))
+	)
+	exit $?
+' unset_to_fork_test "$@"
+(((e = $?) == 1)) || err_exit "Failure in unsetting one or more special variables followed by launching forked subshell" \
+	"(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))"
+
 # ======
 # ${.sh.pid} should be the forked subshell's PID
 (