Fix a few issues with $RANDOM seeding in subshells (#339)

This commit fixes an issue I found in the subshell $RANDOM
reseeding code.

The main issue is a performance regression in the shbench fibonacci
benchmark, introduced in commit af6a32d1. Performance dropped in
this benchmark because $RANDOM is always reseeded and restored,
even when it's never used in a subshell. Performance results from
before and after this performance fix (results are on Linux with
CC=gcc and CCFLAGS='-O2 -D_std_malloc'):

  $ ./shbench -b bench/fibonacci.ksh -l 100 ./ksh-0f06a2e ./ksh-af6a32d ./ksh-f31e368 ./ksh-randfix

  benchmarking ./ksh-0f06a2e, ./ksh-af6a32d, ./ksh-f31e368, ./ksh-randfix ...
  *** fibonacci.ksh ***
  # ./ksh-0f06a2e  # Recent version of ksh93u+m
  # ./ksh-af6a32d  # Commit that introduced the regression
  # ./ksh-f31e368  # Commit without the regression
  # ./ksh-randfix  # Ksh93u+m with this patch applied

  -------------------------------------------------------------------------------------------------
  name           ./ksh-0f06a2e        ./ksh-af6a32d        ./ksh-f31e368        ./ksh-randfix
  -------------------------------------------------------------------------------------------------
  fibonacci.ksh  0.481 [0.459-0.515]  0.472 [0.455-0.504]  0.396 [0.380-0.442]  0.407 [0.385-0.439]
  -------------------------------------------------------------------------------------------------

src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/sh/{init,subshell}.c:
- Rather than reseed $RANDOM every time a subshell is created, add
  a sh_save_rand_seed() function that does this only when the
  $RANDOM variable is used in a subshell. This function is called
  by the $RANDOM discipline functions nget_rand() and put_rand().
  As a minor optimization, sh_save_rand_seed doesn't reseed if it's
  called from put_rand().
- Because $RANDOM may have a seed of zero (i.e., RANDOM=0),
  sp->rand_seed isn't enough to tell if $RANDOM has been reseeded.
  Add sp->rand_state for this purpose.
- sh_subshell(): Only restore the former $RANDOM seed and state if
  it is necessary to prevent a subshell leak.

src/cmd/ksh93/tests/variables.sh:
- Add two regression tests for bugs I ran into while making this
  patch.

NEWS

@@ -8,6 +8,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 - The printf built-in command now supports a -v option as on bash and zsh.
   This allows you to assign formatted output directly to a variable.
 
+- Fixed a performance regression introduced on 2021-05-03 that caused
+  the shbench[*] fibonacci benchmark to run slower.
+  [*]: https://github.com/ksh-community/shbench
+
 2021-11-16:
 
 - By default, arithmetic expressions in ksh no longer interpret a number

src/cmd/ksh93/include/variables.h

@@ -35,6 +35,7 @@ struct rand
 	int32_t		rand_last;
 };
 extern void sh_reseed_rand(struct rand *);
+extern void sh_save_rand_seed(struct rand *, int);
 
 /* The following defines must be kept synchronous with shtab_variables[] in data/variables.c */
 

src/cmd/ksh93/sh/init.c

@@ -645,12 +645,13 @@ static Sfdouble_t nget_seconds(register Namval_t* np, Namfun_t *fp)
 }
 
 /*
- * These three functions are used to get and set the RANDOM variable
+ * These four functions are used to get and set the RANDOM variable
  */
 static void put_rand(register Namval_t* np,const char *val,int flags,Namfun_t *fp)
 {
 	struct rand *rp = (struct rand*)fp;
 	register long n;
+	sh_save_rand_seed(rp, 0);
 	if(!val)
 	{
 		fp = nv_stack(np, NIL(Namfun_t*));
@@ -677,7 +678,7 @@ 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);
+	sh_save_rand_seed(rp, 1);
 	do
 		cur = (rand_r(&rp->rand_seed)>>rand_shift)&RANDMASK;
 	while(cur==last);

src/cmd/ksh93/sh/subshell.c

@@ -100,6 +100,9 @@ static struct subshell
 	int		subdup;
 	char		subshare;
 	char		comsub;
+	unsigned int	rand_seed;  /* parent shell $RANDOM seed */
+	int		rand_last;  /* last random number from $RANDOM in parent shell */
+	int		rand_state; /* 0 means sp->rand_seed hasn't been set, 1 is the opposite */
 #if _lib_fchdir
 	int		pwdfd;	/* file descriptor for PWD */
 	char		pwdclose;
@@ -193,6 +196,17 @@ void sh_subfork(void)
 	{
 		/* this is the child part of the fork */
 		sh_onstate(SH_FORKED);
+		/*
+		 * $RANDOM is only reseeded when it's used in a subshell, so if $RANDOM hasn't
+		 * been reseeded yet set rp->rand_last to -2. This allows sh_save_rand_seed()
+		 * to reseed $RANDOM later.
+		 */
+		if(!sp->rand_state)
+		{
+			struct rand *rp;
+			rp = (struct rand*)RANDNOD->nvfun;
+			rp->rand_last = -2;
+		}
 		subshell_data = 0;
 		shp->subshell = 0;
 		shp->comsub = 0;
@@ -231,6 +245,24 @@ int nv_subsaved(register Namval_t *np, int flags)
 	return(0);
 }
 
+/*
+ * Save the current $RANDOM seed and state, then reseed $RANDOM.
+ */
+void sh_save_rand_seed(struct rand *rp, int reseed)
+{
+	struct subshell	*sp = subshell_data;
+	if(!sh.subshare && sp && !sp->rand_state)
+	{
+		sp->rand_seed = rp->rand_seed;
+		sp->rand_last = rp->rand_last;
+		sp->rand_state = 1;
+		if(reseed)
+			sh_reseed_rand(rp);
+	}
+	else if(reseed && rp->rand_last == -2)
+		sh_reseed_rand(rp);
+}
+
 /*
  * This routine will make a copy of the given node in the
  * layer created by the most recent virtual subshell if the
@@ -467,9 +499,6 @@ 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);
@@ -580,11 +609,6 @@ 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)
@@ -748,6 +772,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 	if(!shp->subshare)	/* restore environment if saved */
 	{
 		int n;
+		struct rand *rp;
 		shp->options = sp->options;
 		/* Clean up subshell hash table. */
 		if(sp->strack)
@@ -841,8 +866,11 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 		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;
+		if(sp->rand_state)
+		{
+			srand(rp->rand_seed = sp->rand_seed);
+			rp->rand_last = sp->rand_last;
+		}
 	}
 	shp->subshare = sp->subshare;
 	shp->subdup = sp->subdup;

src/cmd/ksh93/tests/variables.sh

@@ -99,6 +99,19 @@ do	: $( : $RANDOM $RANDOM $RANDOM )
 done
 [[ $got == "$exp" ]] || err_exit 'Using $RANDOM in subshell influences reproducible sequence in parent environment' \
 	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+# Forking a subshell shouldn't throw away the $RANDOM seed in the main shell
+exp=$(ulimit -t unlimited; RANDOM=123; echo $RANDOM)
+RANDOM=123
+(ulimit -t unlimited; true)
+got=${ echo $RANDOM ;}
+[[ $got == "$exp" ]] || err_exit "Forking a subshell resets the parent shell's \$RANDOM seed" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+# Similarly, forking a subshell shouldn't throw away a seed
+# previously set inside of the subshell
+exp=$(ulimit -t unlimited; RANDOM=789; echo $RANDOM)
+got=$(RANDOM=789; ulimit -t unlimited; echo $RANDOM)
+[[ $got == "$exp" ]] || err_exit "Forking a subshell resets the subshell's \$RANDOM seed" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 unset N i rand1 rand2
 
 # SECONDS