QHash: double the size of the stored seed

There's now another half of the seed which will be used by the hashers.
This is not stored in QHash, so it is never changed for the lifetime of
the application (not even when QHashSeed::setDeterministicGlobalSeed()
is called). However, we will not use it when we're in deterministic
mode.

This commit uses the compiler thread-safe statics to implement the
initialization of more than one atomic word, thus freeing us from having
to have a reserved value. As a bonus, the QT_HASH_SEED warning will only
be printed once.

Change-Id: Id2983978ad544ff79911fffd16723f1673f9a5b4
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/corelib/tools/qhash.cpp

@@ -68,14 +68,125 @@
 #include <qrandom.h>
 #endif // QT_BOOTSTRAPPED
 
+#include <array>
 #include <limits.h>
 
+#ifdef Q_CC_GNU
+#  define Q_DECL_HOT_FUNCTION       __attribute__((hot))
+#else
+#  define Q_DECL_HOT_FUNCTION
+#endif
+
 QT_BEGIN_NAMESPACE
 
 // We assume that pointers and size_t have the same size. If that assumption should fail
 // on a platform the code selecting the different methods below needs to be fixed.
 static_assert(sizeof(size_t) == QT_POINTER_SIZE, "size_t and pointers have different size.");
 
+namespace {
+struct HashSeedStorage
+{
+    static constexpr int SeedCount = 2;
+    QBasicAtomicInteger<quintptr> seeds[SeedCount] = { Q_BASIC_ATOMIC_INITIALIZER(0), Q_BASIC_ATOMIC_INITIALIZER(0) };
+
+    constexpr HashSeedStorage() = default;
+
+    enum State {
+        OverriddenByEnvironment = -1,
+        JustInitialized,
+        AlreadyInitialized
+    };
+    struct StateResult {
+        quintptr requestedSeed;
+        State state;
+    };
+
+    StateResult state(int which = -1);
+    Q_DECL_HOT_FUNCTION QHashSeed currentSeed(int which)
+    {
+        return { state(which).requestedSeed };
+    }
+
+    void resetSeed()
+    {
+        if (state().state < AlreadyInitialized)
+            return;
+
+        // update the public seed
+        QRandomGenerator *generator = QRandomGenerator::system();
+        seeds[0].storeRelaxed(sizeof(size_t) > sizeof(quint32)
+                              ? generator->generate64() : generator->generate());
+    }
+
+    void clearSeed()
+    {
+        state();
+        seeds[0].storeRelaxed(0);   // always write (smaller code)
+    }
+
+private:
+    Q_DECL_COLD_FUNCTION Q_NEVER_INLINE StateResult initialize(int which) noexcept;
+    [[maybe_unused]] static void ensureConstexprConstructibility()
+    {
+        static_assert(std::is_trivially_destructible_v<HashSeedStorage>);
+        static constexpr HashSeedStorage dummy {};
+        Q_UNUSED(dummy);
+    }
+};
+
+[[maybe_unused]] HashSeedStorage::StateResult HashSeedStorage::initialize(int which) noexcept
+{
+    StateResult result = { 0, OverriddenByEnvironment };
+    bool ok;
+    int seed = qEnvironmentVariableIntValue("QT_HASH_SEED", &ok);
+    if (ok) {
+        if (seed) {
+            // can't use qWarning here (reentrancy)
+            fprintf(stderr, "QT_HASH_SEED: forced seed value is not 0; ignored.\n");
+        }
+
+        // we don't have to store to the seed, since it's pre-initialized by
+        // the compiler to zero
+        return result;
+    }
+
+    // update the full seed
+    auto x = qt_initial_random_value();
+    for (int i = 0; i < SeedCount; ++i) {
+        seeds[i].storeRelaxed(x.data[i]);
+        if (which == i)
+            result.requestedSeed = x.data[i];
+    }
+    result.state = JustInitialized;
+    return result;
+}
+
+inline HashSeedStorage::StateResult HashSeedStorage::state(int which)
+{
+    constexpr quintptr BadSeed = quintptr(Q_UINT64_C(0x5555'5555'5555'5555));
+    StateResult result = { BadSeed, AlreadyInitialized };
+
+#ifndef QT_BOOTSTRAPPED
+    static auto once = [&]() {
+        result = initialize(which);
+        return true;
+    }();
+    Q_UNUSED(once);
+#else
+    result = { 0, OverriddenByEnvironment };
+#endif
+
+    if (result.state == AlreadyInitialized && which >= 0)
+        return { seeds[which].loadRelaxed(), AlreadyInitialized };
+    return result;
+}
+} // unnamed namespace
+
+/*
+    The QHash seed itself.
+*/
+static HashSeedStorage qt_qhash_seed;
+
 /*
  * Hashing for memory segments is based on the public domain MurmurHash2 by
  * Austin Appleby. See http://murmurhash.googlepages.com/
@@ -720,64 +831,6 @@ size_t qHash(QLatin1String key, size_t seed) noexcept
     return qHashBits(reinterpret_cast<const uchar *>(key.data()), size_t(key.size()), seed);
 }
 
-/*!
-    \internal
-
-    Note: not \c{noexcept}, but called from a \c{noexcept} function and thus
-    will cause termination if any of the functions here throw.
-*/
-enum HashCreationMode { Initial, Reseed };
-static size_t qt_create_qhash_seed(HashCreationMode mode)
-{
-    size_t seed = 0;
-
-#ifdef QT_BOOTSTRAPPED
-    Q_UNUSED(mode)
-#else
-    bool ok;
-    seed = qEnvironmentVariableIntValue("QT_HASH_SEED", &ok);
-    if (ok) {
-        if (seed) {
-            // can't use qWarning here (reentrancy)
-            fprintf(stderr, "QT_HASH_SEED: forced seed value is not 0; ignored.\n");
-        }
-        seed = 1;   // QHashSeed::globalSeed subtracts 1
-    } else if (mode == Initial) {
-        auto data = qt_initial_random_value();
-        seed = data.data[0] ^ data.data[1];
-    } else if (sizeof(seed) > sizeof(uint)) {
-        seed = QRandomGenerator::system()->generate64();
-    } else {
-        seed = QRandomGenerator::system()->generate();
-    }
-#endif // QT_BOOTSTRAPPED
-
-    return seed;
-}
-
-/*
-    The QHash seed itself.
-
-    We store the seed value plus one, so the value zero is used to indicate the
-    seed is not initialized. This is corrected before passing to the user.
-*/
-static QBasicAtomicInteger<size_t> qt_qhash_seed = Q_BASIC_ATOMIC_INITIALIZER(0);
-
-/*!
-    \internal
-    \threadsafe
-
-    Initializes the seed and returns it.
-*/
-static size_t qt_initialize_qhash_seed()
-{
-    size_t theirSeed;               // another thread's seed
-    size_t ourSeed = qt_create_qhash_seed(Initial);
-    if (qt_qhash_seed.testAndSetRelaxed(0, ourSeed, theirSeed))
-        return ourSeed;
-    return theirSeed;
-}
-
 /*!
     \class QHashSeed
     \since 6.2
@@ -832,11 +885,7 @@ static size_t qt_initialize_qhash_seed()
  */
 QHashSeed QHashSeed::globalSeed() noexcept
 {
-    size_t seed = qt_qhash_seed.loadRelaxed();
-    if (Q_UNLIKELY(seed == 0))
-        seed = qt_initialize_qhash_seed();
-
-    return { seed - 1 };
+    return qt_qhash_seed.currentSeed(0);
 }
 
 /*!
@@ -850,7 +899,7 @@ QHashSeed QHashSeed::globalSeed() noexcept
  */
 void QHashSeed::setDeterministicGlobalSeed()
 {
-    qt_qhash_seed.storeRelease(1);
+    qt_qhash_seed.clearSeed();
 }
 
 /*!
@@ -870,8 +919,7 @@ void QHashSeed::setDeterministicGlobalSeed()
  */
 void QHashSeed::resetRandomGlobalSeed()
 {
-    size_t seed = qt_create_qhash_seed(Reseed);
-    qt_qhash_seed.storeRelaxed(seed + 1);
+    qt_qhash_seed.resetSeed();
 }
 
 #if QT_DEPRECATED_SINCE(6,6)

tests/auto/corelib/tools/qhashseed/tst_qhashseed.cpp

@@ -134,8 +134,12 @@ void tst_QHashSeed::reseeding()
 
 void tst_QHashSeed::quality()
 {
+    // this "bad seed" is used internally in qhash.cpp and should never leak!
+    constexpr size_t BadSeed = size_t(Q_UINT64_C(0x5555'5555'5555'5555));
+
     constexpr int Iterations = 16;
     int oneThird = 0;
+    int badSeeds = 0;
     size_t ored = 0;
 
     for (int i = 0; i < Iterations; ++i) {
@@ -146,6 +150,8 @@ void tst_QHashSeed::quality()
 
         if (bits >= std::numeric_limits<size_t>::digits / 3)
             ++oneThird;
+        if (seed == BadSeed)
+            ++badSeeds;
     }
 
     // report out
@@ -161,6 +167,12 @@ void tst_QHashSeed::quality()
 
     // at least one third of the seeds must have one third of all the bits set
     QVERIFY(oneThird > (16/3));
+
+    // at most one seed can be the bad seed, if 32-bit, none on 64-bit
+    if (std::numeric_limits<size_t>::digits > 32)
+        QCOMPARE(badSeeds, 0);
+    else
+        QVERIFY(badSeeds <= 1);
 }
 
 #if QT_VERSION < QT_VERSION_CHECK(7, 0, 0)