[ruby-core:29291] [Bug #3104] Random: seeding issues

Bug #3104: Random: seeding issues
http://redmine.ruby-lang.org/issues/show/3104

Author: Marc-Andre Lafortune
Status: Open, Priority: Normal
Assigned to: Nobuyoshi Nakada, Category: core, Target version: 1.9.2
ruby -v: ruby 1.9.2dev (2010-04-03 trunk 27200) [x86_64-darwin10.2.0]

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.

   Random.new((1<<64)-1).rand == Random.new((1 << 65) -1).rand 
    # => true, should be false

Probably some leftover code?

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.

   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 --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 --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.


----------------------------------------
http://redmine.ruby-lang.org