From adfb9de9b1548d61daa992d356e601223ca56f84 Mon Sep 17 00:00:00 2001 From: dnewhall Date: Wed, 15 Jan 2025 00:22:33 -0500 Subject: [PATCH] [v1.1] Replace usage of rand_r with nrand48 (#817) This replaces the call to rand_r with nrand48 and changes rand_seed to be an array instead of an unsigned int. With rand_r being deprecated in POSIX 2024, the best replacement is nrand48, which stores its intermediate values as a 48-bit integer in an array of shorts instead of an unsigned int and returns a 32-bit value not constrained by RAND_MAX. nrand48 has been in the POSIX spec. since 2004 and on the large UNIX systems since SVr4. Only things that could be affected are BSDs older than ~2004, but those are presumably old enough that we can ignore them. bin/package test returned with all tests passing (particularly tests/subshell.sh and tests/variables.sh) and monkey testing shows getting and setting RANDOM works. --- ANNOUNCE | 5 ++ NEWS | 7 +++ src/cmd/ksh93/include/variables.h | 2 +- src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/init.c | 51 ++++-------------- src/cmd/ksh93/sh/subshell.c | 6 +-- src/lib/libast/Mamfile | 15 +----- src/lib/libast/include/ast.h | 1 - src/lib/libast/man/ast.3 | 24 --------- src/lib/libast/port/astcopy.c | 86 ------------------------------- 10 files changed, 29 insertions(+), 170 deletions(-) delete mode 100644 src/lib/libast/port/astcopy.c diff --git a/ANNOUNCE b/ANNOUNCE index f3e95564e4cf..0748b6d8a64d 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -90,3 +90,8 @@ New features in shell variables: - TIMEFORMAT now supports a precision of up to six places after the decimal point. The default precision for format directives remains 3. + +- The $RANDOM pseudorandom numbers are now generated by nrand48(3) instead + of rand_r(3). Since POSIX specifies a particular pseudorandom generator + for nrand48, this means the same pseudorandom sequence should now be + generated on all platforms if RANDOM is assigned the same seed value. diff --git a/NEWS b/NEWS index aeb5d0744b2f..e8bd8cd94766 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,13 @@ This documents significant changes in the dev branch of ksh 93u+m. For full details, see the git log at: https://github.com/ksh93/ksh Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library. +2025-01-15: + +- [v1.1] The $RANDOM pseudorandom numbers are now generated by nrand48(3) + instead of rand_r(3). Since POSIX specifies a particular pseudorandom + generator for nrand48, this means the same pseudorandom sequence should now + be generated on all platforms if RANDOM is assigned the same seed value. + 2025-01-08: - Fixed buggy emacs mode behaviour on exceeding the maximum line length. diff --git a/src/cmd/ksh93/include/variables.h b/src/cmd/ksh93/include/variables.h index 67595088942a..0cdc1d6cacb1 100644 --- a/src/cmd/ksh93/include/variables.h +++ b/src/cmd/ksh93/include/variables.h @@ -26,7 +26,7 @@ struct rand { Namfun_t hdr; - unsigned int rand_seed; + unsigned short rand_seed[3]; int32_t rand_last; }; extern void sh_reseed_rand(struct rand *); diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 1a6ba7c6aa3a..d83f12e73a6a 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -18,7 +18,7 @@ #include #include "git.h" -#define SH_RELEASE_DATE "2025-01-08" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2025-01-15" /* must be in this format for $((.sh.version)) */ /* * This comment keeps SH_RELEASE_DATE a few lines away from SH_RELEASE_SVER to avoid * merge conflicts when cherry-picking dev branch commits onto a release branch. diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index 3ab349b01c19..ab251f3d08a6 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -623,38 +623,15 @@ static Sfdouble_t nget_seconds(Namval_t* np, Namfun_t *fp) return dtime(&tp) - offset; } -#if !_lib_rand_r -#undef RAND_MAX -#define RAND_MAX 0x7fffffff -#define rand_r _ksh_rand_r /* - * rand_r(3) fallback nicked from FreeBSD libc. - * License: BSD 3-clause. See the COPYRIGHT file. - * Not to be confused with actual randomness! - * - * Compute x = (7^5 * x) mod (2^31 - 1) - * without overflowing 31 bits: - * (2^31 - 1) = 127773 * (7^5) + 2836 - * From "Random number generators: good ones are hard to find", - * Park and Miller, Communications of the ACM, vol. 31, no. 10, - * October 1988, p. 1195. + * Seeds the rand stucture using the same algorithm as srand48() */ -static int rand_r(unsigned int *seed) -{ - long hi, lo, x; - /* Transform to [1, 0x7ffffffe] range. */ - x = (*seed % 0x7ffffffe) + 1; - hi = x / 127773; - lo = x % 127773; - x = 16807 * lo - 2836 * hi; - if (x < 0) - x += 0x7fffffff; - /* Transform to [0, 0x7ffffffd] range. */ - x--; - *seed = x; - return x; -} -#endif /* !_lib_rand_r */ +static void seed_rand_uint(struct rand *rp, unsigned int seed) +{ + rp->rand_seed[0] = 0x330e; /* Constant from POSIX spec. */ + rp->rand_seed[1] = (unsigned short)seed; + rp->rand_seed[2] = (unsigned short)(seed >> 16); +} /* * These four functions are used to get and set the RANDOM variable @@ -676,7 +653,7 @@ static void put_rand(Namval_t* np,const char *val,int flags,Namfun_t *fp) n = *(Sfdouble_t*)val; else n = sh_arith(val); - rp->rand_seed = (unsigned int)n; + seed_rand_uint(rp, (unsigned int)n); rp->rand_last = -1; if(!np->nvalue) np->nvalue = &rp->rand_last; @@ -694,14 +671,8 @@ static Sfdouble_t nget_rand(Namval_t* np, Namfun_t *fp) int32_t last = *lp; sh_save_rand_seed(rp, 1); do -#if RAND_MAX > (RANDMASK << 3) - /* don't use lower bits when rand_r() generates large numbers */ - cur = (rand_r(&rp->rand_seed) >> 3) & RANDMASK; -#elif RAND_MAX > RANDMASK - cur = rand_r(&rp->rand_seed) & RANDMASK; -#else - cur = rand_r(&rp->rand_seed); -#endif + /* don't use lower bits when generating large numbers */ + cur = (nrand48(rp->rand_seed) >> 3) & RANDMASK; while(cur==last); *lp = cur; return (Sfdouble_t)cur; @@ -715,7 +686,7 @@ static char* get_rand(Namval_t* np, Namfun_t *fp) void sh_reseed_rand(struct rand *rp) { - rp->rand_seed = arc4random(); + seed_rand_uint(rp, arc4random()); rp->rand_last = -1; } diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index dd730df599df..2929bf398849 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -80,7 +80,7 @@ static struct subshell int cpipe; char subshare; char comsub; - unsigned int rand_seed; /* parent shell $RANDOM seed */ + unsigned short rand_seed[3]; /* parent shell $RANDOM seed */ int rand_last; /* last random number from $RANDOM in parent shell */ char rand_state; /* 0 means sp->rand_seed hasn't been set, 1 is the opposite */ uint32_t srand_upper_bound; /* parent shell's upper bound for $SRANDOM */ @@ -226,7 +226,7 @@ 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; + memcpy(sp->rand_seed, rp->rand_seed, sizeof sp->rand_seed); sp->rand_last = rp->rand_last; sp->rand_state = 1; if(reseed) @@ -886,7 +886,7 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) rp = (struct rand*)RANDNOD->nvfun; if(sp->rand_state) { - rp->rand_seed = sp->rand_seed; + memcpy(rp->rand_seed, sp->rand_seed, sizeof rp->rand_seed); rp->rand_last = sp->rand_last; } /* restore $SRANDOM upper bound */ diff --git a/src/lib/libast/Mamfile b/src/lib/libast/Mamfile index 4627c65f5845..c6ce01be8afc 100644 --- a/src/lib/libast/Mamfile +++ b/src/lib/libast/Mamfile @@ -152,10 +152,6 @@ make install virtual prev FEATURE/lib exec - cp -f %{<} %{@} done - make ast_mmap.h - prev FEATURE/mmap - exec - cp -f %{<} %{@} - done make ast_fs.h prev FEATURE/fs exec - cp -f %{<} %{@} @@ -4295,15 +4291,6 @@ make install virtual exec - compile %{<} done - make astcopy.o - make port/astcopy.c - prev include/ls.h - prev ast_mmap.h - prev include/ast.h - done - exec - compile %{<} - done - make astconf.o make port/astconf.c prev FEATURE/libpath @@ -4480,7 +4467,7 @@ make install virtual done done note * ...generated headers - loop HDR ast_release ast_standards ast_common ast_lib ast_sys lc align sig tmx tv ast_api ast_ccode ast_fcntl ast_float ast_fs ast_map ast_mmap ast_mode ast_ndbm ast_param ast_random ast_time ast_tty ast_limits ast_sizeof ast_dirent ast_iconv ast_nl_types ast_stdio ast_wchar ast_wctype + loop HDR ast_release ast_standards ast_common ast_lib ast_sys lc align sig tmx tv ast_api ast_ccode ast_fcntl ast_float ast_fs ast_map ast_mode ast_ndbm ast_param ast_random ast_time ast_tty ast_limits ast_sizeof ast_dirent ast_iconv ast_nl_types ast_stdio ast_wchar ast_wctype make %{INCLUDE_AST}/%{HDR}.h prev %{HDR}.h exec - cp -f %{<} %{@} diff --git a/src/lib/libast/include/ast.h b/src/lib/libast/include/ast.h index 8c754c6a1ec2..81cc7adfdcbe 100644 --- a/src/lib/libast/include/ast.h +++ b/src/lib/libast/include/ast.h @@ -293,7 +293,6 @@ extern char* astgetconf(const char*, const char*, const char*, int, Error_f); extern char* astconf(const char*, const char*, const char*); extern Ast_confdisc_f astconfdisc(Ast_confdisc_f); extern void astconflist(Sfio_t*, const char*, int, const char*); -extern off_t astcopy(int, int, off_t); extern int astquery(int, const char*, ...); extern void astwinsize(int, int*, int*); #if _lib_sysconf diff --git a/src/lib/libast/man/ast.3 b/src/lib/libast/man/ast.3 index c88927aad512..54c32213c3fd 100644 --- a/src/lib/libast/man/ast.3 +++ b/src/lib/libast/man/ast.3 @@ -48,7 +48,6 @@ long astconf_long(\fIarg\fP); unsigned long astconf_ulong(\fIarg\fP); Ast_confdisc_t astconfdisc(Ast_confdisc_t new_notify); void astconflist(Sfio_t* stream, const char* path, int flags); -off_t astcopy(int \fIrfd\fP, int \fIwfd\fP, off_t \fIn\fP); int astquery(int \fIfd\fP, const char* \fIformat\fP , ...); .EE .SH DESCRIPTION @@ -223,29 +222,6 @@ snarfed for input to the .IR getconf (1) command. .PP -.L astcopy -efficiently copies up to -.I n -bytes from the file descriptor -.I rfd -to the file descriptor -.IR wfd . -The actual number of bytes copied is returned; \-1 is returned on error. -If -.I n -is 0 then an optimal number of bytes (with respect to both -.I rfd -and -.IR wfd ) -is copied. -.PP -If possible -.IR mmap (2) -is used to do the transfer. -Some implementations may bypass user buffer copies usually required by the -.IR read (2)- write (2) -paradigm. -.PP .L astquery outputs an .IR sfprintf (3) diff --git a/src/lib/libast/port/astcopy.c b/src/lib/libast/port/astcopy.c deleted file mode 100644 index 49d58366bdb2..000000000000 --- a/src/lib/libast/port/astcopy.c +++ /dev/null @@ -1,86 +0,0 @@ -/*********************************************************************** -* * -* This software is part of the ast package * -* Copyright (c) 1985-2011 AT&T Intellectual Property * -* Copyright (c) 2020-2023 Contributors to ksh 93u+m * -* and is licensed under the * -* Eclipse Public License, Version 2.0 * -* * -* A copy of the License is available at * -* https://www.eclipse.org/org/documents/epl-2.0/EPL-2.0.html * -* (with md5 checksum 84283fa8859daf213bdda5a9f8d1be1d) * -* * -* Glenn Fowler * -* David Korn * -* Phong Vo * -* Martijn Dekker * -* * -***********************************************************************/ -/* - * Glenn Fowler - * AT&T Bell Laboratories - * - * copy from rfd to wfd (with conditional mmap hacks) - */ - -#include -#include - -#if _mmap_worthy > 1 - -#include - -#define MAPSIZE (1024*256) - -#endif - -#undef BUFSIZ -#define BUFSIZ 4096 - -/* - * copy n bytes from rfd to wfd - * actual byte count returned - * if n<=0 then ``good'' size is used - */ - -off_t -astcopy(int rfd, int wfd, off_t n) -{ - off_t c; -#ifdef MAPSIZE - off_t pos; - off_t mapsize; - char* mapbuf; - struct stat st; -#endif - - static int bufsiz; - static char* buf; - - if (n <= 0 || n >= BUFSIZ * 2) - { -#if MAPSIZE - if (!fstat(rfd, &st) && S_ISREG(st.st_mode) && (pos = lseek(rfd, 0, 1)) != ((off_t)-1)) - { - if (pos >= st.st_size) return 0; - mapsize = st.st_size - pos; - if (mapsize > MAPSIZE) mapsize = (mapsize > n && n > 0) ? n : MAPSIZE; - if (mapsize >= BUFSIZ * 2 && (mapbuf = (char*)mmap(NULL, mapsize, PROT_READ, MAP_SHARED, rfd, pos)) != ((caddr_t)-1)) - { - if (write(wfd, mapbuf, mapsize) != mapsize || lseek(rfd, mapsize, 1) == ((off_t)-1)) return -1; - munmap((caddr_t)mapbuf, mapsize); - return mapsize; - } - } -#endif - if (n <= 0) n = BUFSIZ; - } - if (n > bufsiz) - { - if (buf) free(buf); - bufsiz = roundof(n, BUFSIZ); - if (!(buf = newof(0, char, bufsiz, 0))) return -1; - } - if ((c = read(rfd, buf, (size_t)n)) > 0 && write(wfd, buf, (size_t)c) != c) c = -1; - return c; -}