1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00

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 <johnothanking@protonmail.com>
Resolves: https://github.com/ksh93/ksh/issues/285
This commit is contained in:
Martijn Dekker 2021-05-03 04:03:46 +01:00 committed by GitHub
parent f31e368795
commit af6a32d14f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 139 additions and 17 deletions

9
NEWS
View file

@ -5,6 +5,15 @@ Any uppercase BUG_* names are modernish shell bug IDs.
2021-05-03: 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 '-', '+', - Completed the 2021-04-30 fix for ${var<OP>'{}'} where <OP> is '-', '+',
':-' or ':+' by fixing a bug that caused an extra '}' to be output. ':-' or ':+' by fixing a bug that caused an extra '}' to be output.

View file

@ -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 24. The readonly attribute of ksh variables is no longer imported from
or exported to other ksh shell instances through the environment. 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 KSH-93 VS. KSH-88

View file

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

View file

@ -25,6 +25,16 @@
#include <option.h> #include <option.h>
#include "FEATURE/options" #include "FEATURE/options"
#include "FEATURE/dynamic" #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 */ /* The following defines must be kept synchronous with shtab_variables[] in data/variables.c */

View file

@ -34,6 +34,7 @@
#include <pwd.h> #include <pwd.h>
#include <tmx.h> #include <tmx.h>
#include <regex.h> #include <regex.h>
#include <math.h>
#include "variables.h" #include "variables.h"
#include "path.h" #include "path.h"
#include "fault.h" #include "fault.h"
@ -142,13 +143,6 @@ struct seconds
Shell_t *sh; Shell_t *sh;
}; };
struct rand
{
Namfun_t hdr;
Shell_t *sh;
int32_t rand_last;
};
struct ifs struct ifs
{ {
Namfun_t hdr; 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) if(flags&NV_INTEGER)
n = *(double*)val; n = *(double*)val;
else else
n = sh_arith(rp->sh,val); n = sh_arith(&sh,val);
srand((int)(n&RANDMASK)); srand(rp->rand_seed = (unsigned int)n);
rp->rand_last = -1; rp->rand_last = -1;
if(!np->nvalue.lp) if(!np->nvalue.lp)
np->nvalue.lp = &rp->rand_last; 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) 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; register long cur, last= *np->nvalue.lp;
NOT_USED(fp); NOT_USED(fp);
do do
cur = (rand()>>rand_shift)&RANDMASK; cur = (rand_r(&rp->rand_seed)>>rand_shift)&RANDMASK;
while(cur==last); while(cur==last);
*np->nvalue.lp = cur; *np->nvalue.lp = cur;
return((Sfdouble_t)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)); 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 * 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->SECONDS_init.hdr.nofree = 1;
ip->RAND_init.hdr.disc = &RAND_disc; ip->RAND_init.hdr.disc = &RAND_disc;
ip->RAND_init.hdr.nofree = 1; 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.disc = &SH_MATCH_disc;
ip->SH_MATCH_init.hdr.nofree = 1; ip->SH_MATCH_init.hdr.nofree = 1;
ip->SH_MATH_init.disc = &SH_MATH_disc; 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_stack(L_ARGNOD, &ip->L_ARG_init);
nv_putval(SECONDS, (char*)&d, NV_DOUBLE); nv_putval(SECONDS, (char*)&d, NV_DOUBLE);
nv_stack(RANDNOD, &ip->RAND_init.hdr); nv_stack(RANDNOD, &ip->RAND_init.hdr);
d = (shp->gd->pid&RANDMASK);
nv_putval(RANDNOD, (char*)&d, NV_DOUBLE); nv_putval(RANDNOD, (char*)&d, NV_DOUBLE);
sh_reseed_rand((struct rand *)RANDNOD->nvfun);
nv_stack(LINENO, &ip->LINENO_init); nv_stack(LINENO, &ip->LINENO_init);
SH_MATCHNOD->nvfun = &ip->SH_MATCH_init.hdr; SH_MATCHNOD->nvfun = &ip->SH_MATCH_init.hdr;
nv_putsub(SH_MATCHNOD,(char*)0,10); nv_putsub(SH_MATCHNOD,(char*)0,10);

View file

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

View file

@ -488,6 +488,9 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
struct sh_scoped savst; struct sh_scoped savst;
struct dolnod *argsav=0; struct dolnod *argsav=0;
int argcnt; 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)); memset((char*)sp, 0, sizeof(*sp));
sfsync(shp->outpool); sfsync(shp->outpool);
sh_sigcheck(shp); 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]; sp->cpipe = shp->cpipe[1];
shp->cpid = 0; shp->cpid = 0;
sh_sigreset(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); jmpval = sigsetjmp(buff.buff,0);
if(jmpval==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->cpid = sp->cpid;
shp->cpipe[1] = sp->cpipe; shp->cpipe[1] = sp->cpipe;
shp->coutpipe = sp->coutpipe; 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->subshare = sp->subshare;
shp->subdup = sp->subdup; shp->subdup = sp->subdup;

View file

@ -1937,6 +1937,7 @@ int sh_exec(register const Shnode_t *t, int flags)
flags &= ~OPTIMIZE_FLAG; flags &= ~OPTIMIZE_FLAG;
if(!shp->subshell && !shp->st.trapcom[0] && !shp->st.trap[SH_ERRTRAP] && (flags&sh_state(SH_NOFORK))) 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; char *savsig;
int nsig,jmpval; int nsig,jmpval;
struct checkpt *buffp = (struct checkpt*)stkalloc(shp->stk,sizeof(struct checkpt)); 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); memcpy(savsig,(char*)&shp->st.trapcom[0],nsig);
shp->st.otrapcom = (char**)savsig; 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_sigreset(0);
sh_pushcontext(shp,buffp,SH_JMPEXIT); sh_pushcontext(shp,buffp,SH_JMPEXIT);
jmpval = sigsetjmp(buffp->buff,0); jmpval = sigsetjmp(buffp->buff,0);
@ -1961,7 +1965,7 @@ int sh_exec(register const Shnode_t *t, int flags)
sh_done(shp,0); sh_done(shp,0);
} }
else if(((type=t->par.partre->tre.tretyp)&FAMP) && ((type&COMMSK)==TFORK) else if(((type=t->par.partre->tre.tretyp)&FAMP) && ((type&COMMSK)==TFORK)
&& !sh_isoption(SH_INTERACTIVE)) && !job.jobcontrol && !shp->subshell)
{ {
/* Optimize '( simple_command & )' */ /* Optimize '( simple_command & )' */
pid_t pid; pid_t pid;
@ -1970,7 +1974,8 @@ int sh_exec(register const Shnode_t *t, int flags)
_sh_fork(shp,pid,0,0); _sh_fork(shp,pid,0,0);
if(pid==0) if(pid==0)
{ {
shgd->current_pid = getpid(); sh_reseed_rand((struct rand*)RANDNOD->nvfun);
shgd->realsubshell++;
sh_exec(t->par.partre,flags); sh_exec(t->par.partre,flags);
shp->st.trapcom[0]=0; shp->st.trapcom[0]=0;
sh_done(shp,0); sh_done(shp,0);

View file

@ -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=$(ulimit -t unlimited 2>/dev/null; (dummy=${ exec true; }); echo ok)
[[ $got == ok ]] || err_exit "'exec' command run in subshare disregards parent virtual subshell" [[ $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)) exit $((Errors<125?Errors:125))

View file

@ -26,10 +26,55 @@ unset ss
[[ ${@ss} ]] && err_exit '${@ss} should be empty string when ss is unset' [[ ${@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} == ss ]] || err_exit '${!ss} should be ss when ss is unset'
[[ ${#ss} == 0 ]] || err_exit '${#ss} should be 0 when ss is unset' [[ ${#ss} == 0 ]] || err_exit '${#ss} should be 0 when ss is unset'
# RANDOM # RANDOM
if (( RANDOM==RANDOM || $RANDOM==$RANDOM )) if (( RANDOM==RANDOM || $RANDOM==$RANDOM ))
then err_exit RANDOM variable not working then err_exit RANDOM variable not working
fi 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 # SECONDS
float secElapsed=0.0 secSleep=0.001 float secElapsed=0.0 secSleep=0.001
let SECONDS=$secElapsed let SECONDS=$secElapsed
@ -165,7 +210,6 @@ if [[ $LANG != "$save_LANG" ]]
then err_exit "$save_LANG locale not working" then err_exit "$save_LANG locale not working"
fi fi
unset RANDOM
unset -n foo unset -n foo
foo=junk foo=junk
function foo.get function foo.get
@ -703,6 +747,12 @@ actual=$(
expect=$'4\n3\n3\n2\n1' expect=$'4\n3\n3\n2\n1'
[[ $actual == "$expect" ]] || err_exit "\${.sh.subshell} failure (expected $(printf %q "$expect"), got $(printf %q "$actual"))" [[ $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 unset IFS
if ((SHOPT_BRACEPAT)) && command set -o braceexpand if ((SHOPT_BRACEPAT)) && command set -o braceexpand
then set -- {1..32768} then set -- {1..32768}
@ -1067,6 +1117,23 @@ $SHELL -c '
(((e = $?) == 1)) || err_exit "typeset -l/-u doesn't work on special variables" \ (((e = $?) == 1)) || err_exit "typeset -l/-u doesn't work on special variables" \
"(exit status $e$( ((e>128)) && print -n / && kill -l "$e"))" "(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 # ${.sh.pid} should be the forked subshell's PID
( (