On Tue, 05 Aug 2025 at 09:21 +0000, 1nfocalypse wrote:
Implements Philox Engine (P2075R6) and associated tests.
Implements additional feedback from v5 from Patrick Palka.
Also cut some trailing whitespace from the limb propagation
fix in v5.
Apologies for the delay from v5 - I had to finish writing some
CTF challenges for DEFCON this week. Thank you for the
feedback, and please let me know if anything further needs
to be changed.
Built/tested on x86_64-pc-linux-gnu.
From 23ffd0ca595c27e8721d6d73ac746293aace7771 Mon Sep 17 00:00:00 2001
From: 1nfocalypse <[email protected]>
Date: Tue, 5 Aug 2025 01:37:18 +0000
Subject: [PATCH] [PATCH v6] libstdc++: Implement Philox Engine (PR119794)
Implemented additional changes from Patrick Palka.
Also removed some trailing whitespace in random.tcc.
replaced alias of __ios_base with ios_base
included <iomanip> in random.h
altered line number in pr60037-neg.cc to be accurate
Refactored iterator/index loops to be range-based for
in istream/ostream operator overloads
include <random> in philox_engine/cons/seed_seq.cc
Conforms with errata LWG4143, LWG4153 for Philox Engine.
PR libstdc++/119794
---
libstdc++-v3/include/bits/random.h | 283 ++++++++++++++++++
libstdc++-v3/include/bits/random.tcc | 191 ++++++++++++
libstdc++-v3/include/bits/version.def | 9 +
libstdc++-v3/include/bits/version.h | 10 +
libstdc++-v3/include/std/random | 3 +
.../26_numerics/random/philox4x32.cc | 23 ++
.../26_numerics/random/philox4x64.cc | 23 ++
.../random/philox_engine/cons/119794.cc | 39 +++
.../random/philox_engine/cons/copy.cc | 25 ++
.../random/philox_engine/cons/default.cc | 27 ++
.../random/philox_engine/cons/seed.cc | 20 ++
.../random/philox_engine/cons/seed_seq.cc | 24 ++
.../random/philox_engine/operators/equal.cc | 30 ++
.../random/philox_engine/operators/inequal.cc | 30 ++
.../philox_engine/operators/serialize.cc | 49 +++
.../philox_engine/requirements/constants.cc | 26 ++
.../requirements/constexpr_data.cc | 50 ++++
.../requirements/constexpr_functions.cc | 41 +++
.../philox_engine/requirements/typedefs.cc | 26 ++
.../26_numerics/random/pr60037-neg.cc | 4 +-
20 files changed, 931 insertions(+), 2 deletions(-)
create mode 100644 libstdc++-v3/testsuite/26_numerics/random/philox4x32.cc
create mode 100644 libstdc++-v3/testsuite/26_numerics/random/philox4x64.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/cons/119794.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/cons/copy.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/cons/default.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/cons/seed.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/cons/seed_seq.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/operators/equal.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/operators/inequal.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/operators/serialize.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/requirements/constants.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/requirements/constexpr_data.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/requirements/constexpr_functions.cc
create mode 100644
libstdc++-v3/testsuite/26_numerics/random/philox_engine/requirements/typedefs.cc
diff --git a/libstdc++-v3/include/bits/random.h
b/libstdc++-v3/include/bits/random.h
index 1fdaf51934f..e74b7bd75c0 100644
--- a/libstdc++-v3/include/bits/random.h
+++ b/libstdc++-v3/include/bits/random.h
@@ -33,6 +33,7 @@
#include <vector>
#include <bits/uniform_int_dist.h>
+#include <iomanip>
namespace std _GLIBCXX_VISIBILITY(default)
{
@@ -1688,6 +1689,270 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ return !(__lhs == __rhs); }
#endif
+#if __cpp_lib_philox_engine
This should check __glibcxx_philox_engine (just in case this code is
moved to a separate header that ends up being included somewhere
withou all of <random>).
+ /**
+ * @brief: A discrete pseudorandom number generator based off of weakened
+ * cryptographic primitives.
Can we say "based on" instead of "based off of" which is an informal
American English term that sounds wrong to me.
I'm also not sure what this is telling me as a non-export user. Would
it be clearer to talk about it having weak cryptographic properties,
rather than being based on weakened cryptographic primitives?
+ *
+ * This algorithm was intended to be used for highly parallel random number
"was intended to be used" sounds like it refers to something in the
past that is no longer true. Maybe "was designed to be used" instead?
+ * generation, and is capable of immensely long periods. It provides "Crush-
I was going to ask whether immensely is a bit hyperbolic and could be
just "very long periods (e.g. ...)" but since we're talking at least
2^128 I think "immensely" is fine :-)
+ * resistance", denoting an ability to pass the TestU01 Suite's "Big Crush"
+ * test, demonstrating significant apparent entropy. It is not intended for
+ * cryptographic use and should not be used for such, despite being based on
+ * cryptographic primitives.
+ *
+ * The two four-word definitions are likely the best use for this algorithm,
+ * and are given below as defaults.
Can we refer to those as aliases or typedefs rather than definitions?
Or specializations?
Maybe even say it like this:
* The typedefs `philox4x32` and `philox4x64` are provided as
* suitable defaults for most use cases, providing high-quality
* random numbers at reasonable performance.
+ *
+ * This algorithm was created by John Salmon, Mark Moraes, Ron Dror, and
+ * David Shaw as a product of D.E. Shaw Research.
+ *
+ * @tparam __w Word size
+ * @tparam __n Buffer size
+ * @tparam __r Rounds
+ * @tparam __consts Multiplication and round constant pack, ordered as
+ * M_{0}, C_{0}, M_{1}, C_{1}, ... , M_{N/2-1}, C_{N/2-1}
+ *
+ * @headerfile random
+ * @since C++26
+ */
+ template<class _UIntType, size_t __w,
Our convention in libstdc++ code is to type 'typename' for template
parameters instead of 'class'
+ size_t __n, size_t __r,
+ _UIntType... __consts>
+ class philox_engine
+ {
+ static_assert(__n == 2 || __n == 4,
+ "template argument N must be either 2 or 4");
+ static_assert(sizeof...(__consts) == __n,
+ "length of consts array must match specified N");
+ static_assert(0 < __r, "a number of rounds must be specified");
+ static_assert((0 < __w && __w <= numeric_limits<_UIntType>::digits),
+ "specified bitlength must match input type");
The static assert messages all use the preferred "must" form, thanks!
+ template<typename _Sseq>
+ using _If_seed_seq
+ = __detail::_If_seed_seq_for<_Sseq, philox_engine, _UIntType>;
+
+ private:
This access specifier is redundant because everything above it is
already private.
+ // the ordering here is essential to functionality.
Does this refer to the ordering of the _S_popArray member in the
class, because it needs to be declared before it's used?
If so, I think we can drop the comment. If somebody fails to notice
that when reading the class definition, they'd find out pretty quickly
if they tried to reorder it and got errors.
The style recommendations to put private internal members last is not
a hard requirement, for cases like this where the order is significant
putting them first is obviously fine (since you have no choice! :-)
+ /** @brief an internal unpacking function for %philox_engine. */
We don't need a doxygen comment for private member functions, they
won't get included in the docs anyway.
+ template <size_t __ind0, size_t __ind1>
+ static constexpr
+ array<_UIntType, __n / 2>
+ _S_popArray()
+ {
+ if constexpr (__n == 4)
+ return {__consts...[__ind0], __consts...[__ind1]};
+ else
+ return {__consts...[__ind0]};
+ }
+
+ public:
+ /** Type of template param. */
+ using result_type = _UIntType;
+ // public members
+ static constexpr size_t word_size = __w;
+ static constexpr size_t word_count = __n;
+ static constexpr size_t round_count = __r;
+ static constexpr array<result_type, __n / 2> multipliers =
+ philox_engine::_S_popArray<0,2>();
+ static constexpr array<result_type, __n / 2> round_consts =
+ philox_engine::_S_popArray<1,3>();
+
+ /** @brief returns the minimum value possible. */
+ static constexpr result_type
+ min()
+ { return 0; }
+
+ /** @brief returns the maximum value possible. */
+ static constexpr result_type
+ max()
+ {
+ return ((1ull << (__w - 1)) | ((1ull << (__w - 1)) - 1));
+ }
+ // default key value
+ static constexpr result_type default_seed = 20111115u;
I wonder if we have a defect in the C++26 draft here, because
20111115u won't fit in a 16-bit int. We fixed that for
std::subtract_with_carry_engine via:
https://cplusplus.github.io/LWG/issue3809
and then fixed what I broke via:
https://cplusplus.github.io/LWG/issue4014
Since the value of default_seed is only used as the argument to the
default ctor (or the equivalent of calling seed() with no argument)
and the ctor reduces value by mod 2^w, I think we could define the
default seed as 20111115u & max().
But let's leave it for now, I'll raise this with LWG.
+
+ // constructors
+ philox_engine()
+ : philox_engine(default_seed)
+ {}
+
+ explicit
+ philox_engine(result_type __value);
+
+ /** @brief seed sequence constructor for %philox_engine
+ *
+ * @params __q the seed sequence
