Reviewers: dcarney,

Description:
Fixed undefined behavior in RNG.

We're basically trading undefined behavior for implementation defined
behavior, which should be OK for UBSan. :-) The generated code should
be identical, at least I checked that for GCC 4.6.3 on x64.

BUG=377790
LOG=y

Please review this at https://codereview.chromium.org/332733002/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+7, -1 lines):
  M src/utils/random-number-generator.cc


Index: src/utils/random-number-generator.cc
diff --git a/src/utils/random-number-generator.cc b/src/utils/random-number-generator.cc index 21dd16343595e3d1c5ac09accbca7ce1b33de495..cf71c6aa66497ad10b4ae7b582c83d6b777c2320 100644
--- a/src/utils/random-number-generator.cc
+++ b/src/utils/random-number-generator.cc
@@ -117,7 +117,13 @@ void RandomNumberGenerator::NextBytes(void* buffer, size_t buflen) {
 int RandomNumberGenerator::Next(int bits) {
   ASSERT_LT(0, bits);
   ASSERT_GE(32, bits);
-  int64_t seed = (seed_ * kMultiplier + kAddend) & kMask;
+ // Do unsigned multiplication, which has the intended modulo semantics, while
+  // signed multiplication would expose undefined behavior.
+  uint64_t product = static_cast<uint64_t>(seed_) * kMultiplier;
+  // Assigning a uint64_t to an int64_t is implementation defined, but this
+ // should be OK. Use a static_cast to explicitly state that we know what we're
+  // doing. (Famous last words...)
+  int64_t seed = static_cast<int64_t>((product + kAddend) & kMask);
   seed_ = seed;
   return static_cast<int>(seed >> (48 - bits));
 }


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to