From 0d3aee68a1d754571fc44e69febf00589e2423cf Mon Sep 17 00:00:00 2001
From: Artem Zhmurov <zhmurov@gmail.com>
Date: Mon, 12 Oct 2020 08:50:01 +0000
Subject: [PATCH] Fix the seeding of PRNG when it is used instead of RG

Use the timer value to seed PRNG, also switch to MT generator.
---
 src/gromacs/random/seed.cpp       | 60 +++++++++++++++++--------------
 src/gromacs/utility/coolstuff.cpp |  4 ++-
 2 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/src/gromacs/random/seed.cpp b/src/gromacs/random/seed.cpp
index 62adc95afb..b1fc061fd1 100644
--- a/src/gromacs/random/seed.cpp
+++ b/src/gromacs/random/seed.cpp
@@ -37,6 +37,8 @@
 
 #include "seed.h"
 
+#include <time.h>
+
 #include "gromacs/utility/basedefinitions.h"
 #include "gromacs/utility/fatalerror.h"
 
@@ -49,14 +51,14 @@ namespace gmx
  * Due to a bug in AMD Ryzen microcode, RDRAND may always return -1 (0xFFFFFFFF).
  * To avoid that, fall back to using PRNG instead of RDRAND if this happens.
  *
- * \param[in] rd Random device to check.
- *
  * \returns The result of the checks.
  */
-static bool checkIfRandomDeviceIsFunctional(std::random_device* rd)
+static bool checkIfRandomDeviceIsFunctional()
 {
-    uint64_t randomNumber1 = static_cast<uint64_t>((*rd)());
-    uint64_t randomNumber2 = static_cast<uint64_t>((*rd)());
+    std::random_device rd;
+
+    uint64_t randomNumber1 = static_cast<uint64_t>(rd());
+    uint64_t randomNumber2 = static_cast<uint64_t>(rd());
 
     // Due to a bug in AMD Ryzen microcode, RDRAND may always return -1 (0xFFFFFFFF).
     // To avoid that, fall back to using PRNG instead of RDRAND if this happens.
@@ -75,40 +77,46 @@ static bool checkIfRandomDeviceIsFunctional(std::random_device* rd)
 
 /*! \brief Get the next pure or pseudo-random number
  *
- * Returns the next random number taken from the hardware generator or from standard PRNG.
+ * Returns the next random number taken from the hardware generator or from PRNG.
  *
- * \param[in] useRdrand  Whether the hardware source of random numbers should be used.
- * \param[in] rd         Pointer to the random device to use.
+ * \param[in] gen  Pseudo-random/random numbers generator/device to use.
  *
  * \return Random or pseudo-random number.
  */
-static inline uint64_t getNextRandomNumber(bool useRdrand, std::random_device* rd)
+template<typename GeneratorType>
+static uint64_t makeRandomSeedInternal(GeneratorType& gen)
 {
-    if (debug && !useRdrand)
+    constexpr std::size_t resultBits = std::numeric_limits<uint64_t>::digits;
+    constexpr std::size_t numBitsInRandomNumber =
+            std::numeric_limits<typename GeneratorType::result_type>::digits;
+
+    uint64_t result = static_cast<uint64_t>(gen());
+    // This conditional is needed so that compiler understands that what follows is a dead branch
+    // and not complains about shift larger than number of bits in the result.
+    if (resultBits < numBitsInRandomNumber)
     {
-        fprintf(debug,
-                "Will use pseudo-random number generator (PRNG) rather than hardware device.");
+        for (std::size_t bits = numBitsInRandomNumber; bits < resultBits; bits += numBitsInRandomNumber)
+        {
+            result = (result << numBitsInRandomNumber) | static_cast<uint64_t>(gen());
+        }
     }
-    return useRdrand ? static_cast<uint64_t>((*rd)()) : static_cast<uint64_t>(rand());
+    return result;
 }
 
 uint64_t makeRandomSeed()
 {
-    std::random_device rd;
-
-    bool useRdrand = checkIfRandomDeviceIsFunctional(&rd);
-
-    const std::size_t resultBits = std::numeric_limits<uint64_t>::digits;
-    const std::size_t numBitsInRandomNumber =
-            useRdrand ? std::numeric_limits<std::random_device::result_type>::digits
-                      : std::numeric_limits<int>::digits;
-
-    uint64_t result = getNextRandomNumber(useRdrand, &rd);
-    for (std::size_t bits = numBitsInRandomNumber; bits < resultBits; bits += numBitsInRandomNumber)
+    bool useRdrand = checkIfRandomDeviceIsFunctional();
+    if (useRdrand)
     {
-        result = (result << numBitsInRandomNumber) | getNextRandomNumber(useRdrand, &rd);
+        std::random_device rd;
+        return makeRandomSeedInternal(rd);
+    }
+    else
+    {
+        std::mt19937_64 prng(time(nullptr));
+        return makeRandomSeedInternal(prng);
     }
-    return result;
 }
 
+
 } // namespace gmx
diff --git a/src/gromacs/utility/coolstuff.cpp b/src/gromacs/utility/coolstuff.cpp
index 55c80cd1cd..65d438b1b6 100644
--- a/src/gromacs/utility/coolstuff.cpp
+++ b/src/gromacs/utility/coolstuff.cpp
@@ -43,6 +43,8 @@
 
 #include "config.h"
 
+#include <time.h>
+
 #include <random>
 #include <string>
 
@@ -76,7 +78,7 @@ bool beCool()
 template<typename T>
 const T& getPseudoRandomElement(gmx::ArrayRef<const T> arrayRef)
 {
-    std::mt19937_64                       generator;
+    std::mt19937_64                       generator(time(nullptr));
     std::uniform_int_distribution<size_t> distribution(0, arrayRef.size() - 1);
     return arrayRef[distribution(generator)];
 }
-- 
2.22.0