diff --git a/NEWS b/NEWS index 9d21d6a4daf8..047b3c3a34df 100644 --- a/NEWS +++ b/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'{}'} where is '-', '+', ':-' or ':+' by fixing a bug that caused an extra '}' to be output. diff --git a/src/cmd/ksh93/COMPATIBILITY b/src/cmd/ksh93/COMPATIBILITY index a4e1c7705adf..e1c180f1c1d4 100644 --- a/src/cmd/ksh93/COMPATIBILITY +++ b/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 diff --git a/src/cmd/ksh93/Mamfile b/src/cmd/ksh93/Mamfile index c8207ff4c3e7..5490c6b59ab4 100644 --- a/src/cmd/ksh93/Mamfile +++ b/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 diff --git a/src/cmd/ksh93/include/variables.h b/src/cmd/ksh93/include/variables.h index f045d3518618..3c55d426039a 100644 --- a/src/cmd/ksh93/include/variables.h +++ b/src/cmd/ksh93/include/variables.h @@ -25,6 +25,16 @@ #include #include "FEATURE/options" #include "FEATURE/dynamic" +#include + +/* 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 */ diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index 1c450eaa71ee..5b7dcdcff05d 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -34,6 +34,7 @@ #include #include #include +#include #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); diff --git a/src/cmd/ksh93/sh/main.c b/src/cmd/ksh93/sh/main.c index c24379795ec2..c23b7fdbe0cb 100644 --- a/src/cmd/ksh93/sh/main.c +++ b/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); diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index c3196251fbb3..1cc96d388951 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/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; diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 1c0ebfca8930..e8913d81fb2c 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/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); diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 85ea3e928e17..edba4a8ebbf3 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/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=$( /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 +(( $(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 (