Rework NonsecureURBGBase seed sequence.

Decompose RandenPoolSeedSeq from NonsecureURBGBase.

Adjust how the RandenPoolSeedSeq detects contiguous buffers passed to the generate function. Previously it made incorrect assumptions regarding the contiguous concept, which have been replaced with some type-based tests for a small number of known contiguous random access iterator types, including raw pointers.

PiperOrigin-RevId: 452564114
Change-Id: Idab1df9dd078d8e5c565c7fa7ccb9c0d3d392ad2

absl/random/CMakeLists.txt

@@ -727,7 +727,7 @@ absl_cc_library(
     ${ABSL_DEFAULT_LINKOPTS}
     ${ABSL_DEFAULT_LINKOPTS}
   DEPS
   DEPS
     absl::core_headers
     absl::core_headers
-    absl::optional
+    absl::inlined_vector
     absl::random_internal_pool_urbg
     absl::random_internal_pool_urbg
     absl::random_internal_salted_seed_seq
     absl::random_internal_salted_seed_seq
     absl::random_internal_seed_material
     absl::random_internal_seed_material

absl/random/internal/BUILD.bazel

@@ -222,8 +222,8 @@ cc_library(
         ":salted_seed_seq",
         ":salted_seed_seq",
         ":seed_material",
         ":seed_material",
         "//absl/base:core_headers",
         "//absl/base:core_headers",
+        "//absl/container:inlined_vector",
         "//absl/meta:type_traits",
         "//absl/meta:type_traits",
-        "//absl/types:optional",
         "//absl/types:span",
         "//absl/types:span",
     ],
     ],
 )
 )

absl/random/internal/nonsecure_base.h

@@ -17,28 +17,82 @@
 
 
 #include <algorithm>
 #include <algorithm>
 #include <cstdint>
 #include <cstdint>
-#include <iostream>
 #include <iterator>
 #include <iterator>
-#include <random>
-#include <string>
 #include <type_traits>
 #include <type_traits>
+#include <utility>
 #include <vector>
 #include <vector>
 
 
 #include "absl/base/macros.h"
 #include "absl/base/macros.h"
+#include "absl/container/inlined_vector.h"
 #include "absl/meta/type_traits.h"
 #include "absl/meta/type_traits.h"
 #include "absl/random/internal/pool_urbg.h"
 #include "absl/random/internal/pool_urbg.h"
 #include "absl/random/internal/salted_seed_seq.h"
 #include "absl/random/internal/salted_seed_seq.h"
 #include "absl/random/internal/seed_material.h"
 #include "absl/random/internal/seed_material.h"
-#include "absl/types/optional.h"
 #include "absl/types/span.h"
 #include "absl/types/span.h"
 
 
 namespace absl {
 namespace absl {
 ABSL_NAMESPACE_BEGIN
 ABSL_NAMESPACE_BEGIN
 namespace random_internal {
 namespace random_internal {
 
 
+// RandenPoolSeedSeq is a custom seed sequence type where generate() fills the
+// provided buffer via the RandenPool entropy source.
+class RandenPoolSeedSeq {
+ private:
+  struct ContiguousTag {};
+  struct BufferTag {};
+
+  // Generate random unsigned values directly into the buffer.
+  template <typename Contiguous>
+  void generate_impl(ContiguousTag, Contiguous begin, Contiguous end) {
+    const size_t n = std::distance(begin, end);
+    auto* a = &(*begin);
+    RandenPool<uint8_t>::Fill(
+        absl::MakeSpan(reinterpret_cast<uint8_t*>(a), sizeof(*a) * n));
+  }
+
+  // Construct a buffer of size n and fill it with values, then copy
+  // those values into the seed iterators.
+  template <typename RandomAccessIterator>
+  void generate_impl(BufferTag, RandomAccessIterator begin,
+                     RandomAccessIterator end) {
+    const size_t n = std::distance(begin, end);
+    absl::InlinedVector<uint32_t, 8> data(n, 0);
+    RandenPool<uint32_t>::Fill(absl::MakeSpan(data.begin(), data.end()));
+    std::copy(std::begin(data), std::end(data), begin);
+  }
+
+ public:
+  using result_type = uint32_t;
+
+  size_t size() { return 0; }
+
+  template <typename OutIterator>
+  void param(OutIterator) const {}
+
+  template <typename RandomAccessIterator>
+  void generate(RandomAccessIterator begin, RandomAccessIterator end) {
+    // RandomAccessIterator must be assignable from uint32_t
+    if (begin != end) {
+      using U = typename std::iterator_traits<RandomAccessIterator>::value_type;
+      // ContiguousTag indicates the common case of a known contiguous buffer,
+      // which allows directly filling the buffer. In C++20,
+      // std::contiguous_iterator_tag provides a mechanism for testing this
+      // capability, however until Abseil's support requirements allow us to
+      // assume C++20, limit checks to a few common cases.
+      using TagType = absl::conditional_t<
+          (std::is_pointer<RandomAccessIterator>::value ||
+           std::is_same<RandomAccessIterator,
+                        typename std::vector<U>::iterator>::value),
+          ContiguousTag, BufferTag>;
+
+      generate_impl(TagType{}, begin, end);
+    }
+  }
+};
+
 // Each instance of NonsecureURBGBase<URBG> will be seeded by variates produced
 // Each instance of NonsecureURBGBase<URBG> will be seeded by variates produced
 // by a thread-unique URBG-instance.
 // by a thread-unique URBG-instance.
-template <typename URBG>
+template <typename URBG, typename Seeder = RandenPoolSeedSeq>
 class NonsecureURBGBase {
 class NonsecureURBGBase {
  public:
  public:
   using result_type = typename URBG::result_type;
   using result_type = typename URBG::result_type;
@@ -85,49 +139,6 @@ class NonsecureURBGBase {
   }
   }
 
 
  private:
  private:
-  // Seeder is a custom seed sequence type where generate() fills the provided
-  // buffer via the RandenPool entropy source.
-  struct Seeder {
-    using result_type = uint32_t;
-
-    size_t size() { return 0; }
-
-    template <typename OutIterator>
-    void param(OutIterator) const {}
-
-    template <typename RandomAccessIterator>
-    void generate(RandomAccessIterator begin, RandomAccessIterator end) {
-      if (begin != end) {
-        // begin, end must be random access iterators assignable from uint32_t.
-        generate_impl(
-            std::integral_constant<bool, sizeof(*begin) == sizeof(uint32_t)>{},
-            begin, end);
-      }
-    }
-
-    // Commonly, generate is invoked with a pointer to a buffer which
-    // can be cast to a uint32_t.
-    template <typename RandomAccessIterator>
-    void generate_impl(std::integral_constant<bool, true>,
-                       RandomAccessIterator begin, RandomAccessIterator end) {
-      auto buffer = absl::MakeSpan(begin, end);
-      auto target = absl::MakeSpan(reinterpret_cast<uint32_t*>(buffer.data()),
-                                   buffer.size());
-      RandenPool<uint32_t>::Fill(target);
-    }
-
-    // The non-uint32_t case should be uncommon, and involves an extra copy,
-    // filling the uint32_t buffer and then mixing into the output.
-    template <typename RandomAccessIterator>
-    void generate_impl(std::integral_constant<bool, false>,
-                       RandomAccessIterator begin, RandomAccessIterator end) {
-      const size_t n = std::distance(begin, end);
-      absl::InlinedVector<uint32_t, 8> data(n, 0);
-      RandenPool<uint32_t>::Fill(absl::MakeSpan(data.begin(), data.end()));
-      std::copy(std::begin(data), std::end(data), begin);
-    }
-  };
-
   static URBG ConstructURBG() {
   static URBG ConstructURBG() {
     Seeder seeder;
     Seeder seeder;
     return URBG(seeder);
     return URBG(seeder);

absl/random/internal/nonsecure_base_test.cc

@@ -15,6 +15,7 @@
 #include "absl/random/internal/nonsecure_base.h"
 #include "absl/random/internal/nonsecure_base.h"
 
 
 #include <algorithm>
 #include <algorithm>
+#include <cstdint>
 #include <iostream>
 #include <iostream>
 #include <memory>
 #include <memory>
 #include <random>
 #include <random>
@@ -192,54 +193,35 @@ TEST(NonsecureURBGBase, EqualSeedSequencesYieldEqualVariates) {
   }
   }
 }
 }
 
 
-// This is a PRNG-compatible type specifically designed to test
-// that NonsecureURBGBase::Seeder can correctly handle iterators
-// to arbitrary non-uint32_t size types.
-template <typename T>
-struct SeederTestEngine {
-  using result_type = T;
+TEST(RandenPoolSeedSeqTest, SeederWorksForU32) {
+  absl::random_internal::RandenPoolSeedSeq seeder;
 
 
-  static constexpr result_type(min)() {
-    return (std::numeric_limits<result_type>::min)();
-  }
-  static constexpr result_type(max)() {
-    return (std::numeric_limits<result_type>::max)();
-  }
-
-  template <class SeedSequence,
-            typename = typename absl::enable_if_t<
-                !std::is_same<SeedSequence, SeederTestEngine>::value>>
-  explicit SeederTestEngine(SeedSequence&& seq) {
-    seed(seq);
-  }
-
-  SeederTestEngine(const SeederTestEngine&) = default;
-  SeederTestEngine& operator=(const SeederTestEngine&) = default;
-  SeederTestEngine(SeederTestEngine&&) = default;
-  SeederTestEngine& operator=(SeederTestEngine&&) = default;
+  uint32_t state[2] = {0, 0};
+  seeder.generate(std::begin(state), std::end(state));
+  EXPECT_FALSE(state[0] == 0 && state[1] == 0);
+}
 
 
-  result_type operator()() { return state[0]; }
+TEST(RandenPoolSeedSeqTest, SeederWorksForU64) {
+  absl::random_internal::RandenPoolSeedSeq seeder;
 
 
-  template <class SeedSequence>
-  void seed(SeedSequence&& seq) {
-    std::fill(std::begin(state), std::end(state), T(0));
-    seq.generate(std::begin(state), std::end(state));
-  }
+  uint64_t state[2] = {0, 0};
+  seeder.generate(std::begin(state), std::end(state));
+  EXPECT_FALSE(state[0] == 0 && state[1] == 0);
+  EXPECT_FALSE((state[0] >> 32) == 0 && (state[1] >> 32) == 0);
+}
 
 
-  T state[2];
-};
+TEST(RandenPoolSeedSeqTest, SeederWorksForS32) {
+  absl::random_internal::RandenPoolSeedSeq seeder;
 
 
-TEST(NonsecureURBGBase, SeederWorksForU32) {
-  using U32 =
-      absl::random_internal::NonsecureURBGBase<SeederTestEngine<uint32_t>>;
-  U32 x;
-  EXPECT_NE(0, x());
+  int32_t state[2] = {0, 0};
+  seeder.generate(std::begin(state), std::end(state));
+  EXPECT_FALSE(state[0] == 0 && state[1] == 0);
 }
 }
 
 
-TEST(NonsecureURBGBase, SeederWorksForU64) {
-  using U64 =
-      absl::random_internal::NonsecureURBGBase<SeederTestEngine<uint64_t>>;
+TEST(RandenPoolSeedSeqTest, SeederWorksForVector) {
+  absl::random_internal::RandenPoolSeedSeq seeder;
 
 
-  U64 x;
-  EXPECT_NE(0, x());
+  std::vector<uint32_t> state(2);
+  seeder.generate(std::begin(state), std::end(state));
+  EXPECT_FALSE(state[0] == 0 && state[1] == 0);
 }
 }

absl/random/internal/salted_seed_seq.h

@@ -22,6 +22,7 @@
 #include <memory>
 #include <memory>
 #include <type_traits>
 #include <type_traits>
 #include <utility>
 #include <utility>
+#include <vector>
 
 
 #include "absl/container/inlined_vector.h"
 #include "absl/container/inlined_vector.h"
 #include "absl/meta/type_traits.h"
 #include "absl/meta/type_traits.h"
@@ -65,15 +66,19 @@ class SaltedSeedSeq {
 
 
   template <typename RandomAccessIterator>
   template <typename RandomAccessIterator>
   void generate(RandomAccessIterator begin, RandomAccessIterator end) {
   void generate(RandomAccessIterator begin, RandomAccessIterator end) {
+    using U = typename std::iterator_traits<RandomAccessIterator>::value_type;
+
     // The common case is that generate is called with ContiguousIterators
     // The common case is that generate is called with ContiguousIterators
     // to uint arrays. Such contiguous memory regions may be optimized,
     // to uint arrays. Such contiguous memory regions may be optimized,
     // which we detect here.
     // which we detect here.
-    using tag = absl::conditional_t<
-        (std::is_pointer<RandomAccessIterator>::value &&
-         std::is_same<absl::decay_t<decltype(*begin)>, uint32_t>::value),
+    using TagType = absl::conditional_t<
+        (std::is_same<U, uint32_t>::value &&
+         (std::is_pointer<RandomAccessIterator>::value ||
+          std::is_same<RandomAccessIterator,
+                       typename std::vector<U>::iterator>::value)),
         ContiguousAndUint32Tag, DefaultTag>;
         ContiguousAndUint32Tag, DefaultTag>;
     if (begin != end) {
     if (begin != end) {
-      generate_impl(begin, end, tag{});
+      generate_impl(TagType{}, begin, end, std::distance(begin, end));
     }
     }
   }
   }
 
 
@@ -89,8 +94,15 @@ class SaltedSeedSeq {
   struct DefaultTag {};
   struct DefaultTag {};
 
 
   // Generate which requires the iterators are contiguous pointers to uint32_t.
   // Generate which requires the iterators are contiguous pointers to uint32_t.
-  void generate_impl(uint32_t* begin, uint32_t* end, ContiguousAndUint32Tag) {
-    generate_contiguous(absl::MakeSpan(begin, end));
+  // Fills the initial seed buffer the underlying SSeq::generate() call,
+  // then mixes in the salt material.
+  template <typename Contiguous>
+  void generate_impl(ContiguousAndUint32Tag, Contiguous begin, Contiguous end,
+                     size_t n) {
+    seq_->generate(begin, end);
+    const uint32_t salt = absl::random_internal::GetSaltMaterial().value_or(0);
+    auto span = absl::Span<uint32_t>(&*begin, n);
+    MixIntoSeedMaterial(absl::MakeConstSpan(&salt, 1), span);
   }
   }
 
 
   // The uncommon case for generate is that it is called with iterators over
   // The uncommon case for generate is that it is called with iterators over
@@ -98,27 +110,13 @@ class SaltedSeedSeq {
   // case we allocate a temporary 32-bit buffer and then copy-assign back
   // case we allocate a temporary 32-bit buffer and then copy-assign back
   // to the initial inputs.
   // to the initial inputs.
   template <typename RandomAccessIterator>
   template <typename RandomAccessIterator>
-  void generate_impl(RandomAccessIterator begin, RandomAccessIterator end,
-                     DefaultTag) {
-    return generate_and_copy(std::distance(begin, end), begin);
-  }
-
-  // Fills the initial seed buffer the underlying SSeq::generate() call,
-  // mixing in the salt material.
-  void generate_contiguous(absl::Span<uint32_t> buffer) {
-    seq_->generate(buffer.begin(), buffer.end());
-    const uint32_t salt = absl::random_internal::GetSaltMaterial().value_or(0);
-    MixIntoSeedMaterial(absl::MakeConstSpan(&salt, 1), buffer);
-  }
-
-  // Allocates a seed buffer of `n` elements, generates the seed, then
-  // copies the result into the `out` iterator.
-  template <typename Iterator>
-  void generate_and_copy(size_t n, Iterator out) {
-    // Allocate a temporary buffer, generate, and then copy.
+  void generate_impl(DefaultTag, RandomAccessIterator begin,
+                     RandomAccessIterator, size_t n) {
+    // Allocates a seed buffer of `n` elements, generates the seed, then
+    // copies the result into the `out` iterator.
     absl::InlinedVector<uint32_t, 8> data(n, 0);
     absl::InlinedVector<uint32_t, 8> data(n, 0);
-    generate_contiguous(absl::MakeSpan(data.data(), data.size()));
-    std::copy(data.begin(), data.end(), out);
+    generate_impl(ContiguousAndUint32Tag{}, data.begin(), data.end(), n);
+    std::copy(data.begin(), data.end(), begin);
   }
   }
 
 
   // Because [rand.req.seedseq] is not required to be copy-constructible,
   // Because [rand.req.seedseq] is not required to be copy-constructible,