From ac18d082505f1dfd17a0e3b937b88e848f36d189 Mon Sep 17 00:00:00 2001 From: "svenpanne@chromium.org" Date: Fri, 13 Jun 2014 06:36:09 +0000 Subject: [PATCH] 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 R=dcarney@chromium.org Review URL: https://codereview.chromium.org/332733002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21828 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/utils/random-number-generator.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/utils/random-number-generator.cc b/src/utils/random-number-generator.cc index 21dd163435..cf71c6aa66 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(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((product + kAddend) & kMask); seed_ = seed; return static_cast(seed >> (48 - bits)); }