Bug #3104
Updated by mame (Yusuke Endoh) 18 days ago
=begin I think there are a couple of very small errors with the seeding of the Random class. 1) Seeding sometimes ignores the high bit of seed data. ```ruby Random.new((1<<64)-1).rand == Random.new((1 << 65) -1).rand # => true, should be false ``` Probably some leftover code? ```diff diff --git a/random.c b/random.c index 02d081c..447e59f 100644 --- a/random.c +++ b/random.c @@ -411,8 +411,6 @@ rand_init(struct MT *mt, VALUE vseed) init_genrand(mt, buf[0]); } else { - if (buf[len-1] == 1) /* remove leading-zero-guard */ - len--; init_by_array(mt, buf, len); } if (buf != buf0) xfree(buf); ``` 2) Treatment of negative seed values is currently platform dependent for all negative fixnums. From the same negative seed, the random sequence can be different on 32 bit platforms than on 64 bit ones. ```ruby Random.new(-1).rand == Random.new((1<<63) -1).rand # => true on 64 bit platform, false on 32 bit Random.new(-1).rand == Random.new((1<<31) -1).rand # => false on 64 bit platform, true on 32 bit ``` The simple solution below ignores the sign (which is what's done in case of negative Bignum). This means that the same values will result from seeding with x or -x. Another solution would be to use the lowest bit for the sign (for both fixnums and bignums) so as to avoid this collision, or raise an error in case of negative seeds. ```diff diff --git a/random.c b/random.c index 02d081c..272f7b5 100644 --- a/random.c +++ b/random.c @@ -367,6 +367,7 @@ rand_init(struct MT *mt, VALUE vseed) { volatile VALUE seed; long blen = 0; + long fixnum_seed; int i, j, len; unsigned int buf0[SIZEOF_LONG / SIZEOF_INT32 * 4], *buf = buf0; @@ -374,9 +375,11 @@ rand_init(struct MT *mt, VALUE vseed) switch (TYPE(seed)) { case T_FIXNUM: len = 1; - buf[0] = (unsigned int)(FIX2ULONG(seed) & 0xffffffff); + fixnum_seed = FIX2LONG(seed); + if (fixnum_seed < 0) fixnum_seed = -fixnum_seed; + buf[0] = (unsigned int)(fixnum_seed & 0xffffffff); #if SIZEOF_LONG > SIZEOF_INT32 - if ((buf[1] = (unsigned int)(FIX2ULONG(seed) >> 32)) != 0) ++len; + if ((buf[1] = (unsigned int)(fixnum_seed >> 32)) != 0) ++len; #endif break; case T_BIGNUM: ``` 3) Finally, I was wondering if there wasn't a typo trashing some of the bits in the initial states when seeding with an array: ```diff diff --git a/random.c b/random.c index 02d081c..f3db095 100644 --- a/random.c +++ b/random.c @@ -151,7 +151,7 @@ init_by_array(struct MT *mt, unsigned int init_key[], int key_length) if (i>=N) { mt->state[0] = mt->state[N-1]; i=1; } } - mt->state[0] = 0x80000000U; /* MSB is 1; assuring non-zero initial array */ + mt->state[0] |= 0x80000000U; /* MSB is 1; assuring non-zero initial array */ } static void ``` These changes will surely create test failures which should be trivial to fix. =end