From c30af8f3db1722a687fb8452f0038f56682636d4 Mon Sep 17 00:00:00 2001 From: Charles Schlosser Date: Wed, 31 Dec 2025 03:57:04 +0000 Subject: [PATCH] fix UB in random implementation and tests libeigen/eigen!2102 --- Eigen/src/Core/RandomImpl.h | 19 +++++++++++-------- Eigen/src/Core/arch/Default/Half.h | 2 +- test/rand.cpp | 4 +++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Eigen/src/Core/RandomImpl.h b/Eigen/src/Core/RandomImpl.h index 1a82e6253..4a622fc37 100644 --- a/Eigen/src/Core/RandomImpl.h +++ b/Eigen/src/Core/RandomImpl.h @@ -56,19 +56,21 @@ struct random_bits_impl { EIGEN_STATIC_ASSERT(std::is_unsigned::value, SCALAR MUST BE A BUILT - IN UNSIGNED INTEGER) using RandomDevice = eigen_random_device; using RandomReturnType = typename RandomDevice::ReturnType; - static constexpr int kEntropy = RandomDevice::Entropy; static constexpr int kTotalBits = sizeof(Scalar) * CHAR_BIT; + static constexpr int kEntropy = plain_enum_min(kTotalBits, RandomDevice::Entropy); // return a Scalar filled with numRandomBits beginning from the least significant bit static EIGEN_DEVICE_FUNC inline Scalar run(int numRandomBits) { eigen_assert((numRandomBits >= 0) && (numRandomBits <= kTotalBits)); - const Scalar mask = Scalar(-1) >> ((kTotalBits - numRandomBits) & (kTotalBits - 1)); Scalar randomBits = 0; - for (int shift = 0; shift < numRandomBits; shift += kEntropy) { - RandomReturnType r = RandomDevice::run(); - randomBits |= static_cast(r) << shift; + for (int filledBits = 0; filledBits < numRandomBits; filledBits += kEntropy) { + Scalar r = static_cast(RandomDevice::run()); + int remainingBits = numRandomBits - filledBits; + if (remainingBits < kEntropy) { + // clear the excess bits to avoid UB and rounding bias + r >>= kEntropy - remainingBits; + } + randomBits |= r << filledBits; } - // clear the excess bits - randomBits &= mask; return randomBits; } }; @@ -204,7 +206,8 @@ struct random_int_impl { template struct random_int_impl { static constexpr int kTotalBits = sizeof(Scalar) * CHAR_BIT; - using BitsType = typename make_unsigned::type; + // avoid implicit integral promotion to `int` + using BitsType = std::conditional_t<(sizeof(Scalar) < sizeof(int)), unsigned int, std::make_unsigned_t >; static EIGEN_DEVICE_FUNC inline Scalar run(const Scalar& x, const Scalar& y) { if (y <= x) return x; // Avoid overflow by representing `range` as an unsigned type diff --git a/Eigen/src/Core/arch/Default/Half.h b/Eigen/src/Core/arch/Default/Half.h index 210dfff1e..47832a4df 100644 --- a/Eigen/src/Core/arch/Default/Half.h +++ b/Eigen/src/Core/arch/Default/Half.h @@ -721,7 +721,7 @@ EIGEN_STRONG_INLINE EIGEN_DEVICE_FUNC float half_to_float(__half_raw h) { o_bits = Eigen::numext::bit_cast(Eigen::numext::bit_cast(o_bits) - magic); } - o_bits |= (h.x & 0x8000) << 16; // sign bit + o_bits |= (h.x & 0x8000u) << 16; // sign bit return Eigen::numext::bit_cast(o_bits); #endif } diff --git a/test/rand.cpp b/test/rand.cpp index 69e7cf93e..0145299a7 100644 --- a/test/rand.cpp +++ b/test/rand.cpp @@ -83,7 +83,9 @@ class HistogramHelper { // helper class to avoid extending std:: namespace template -struct get_range_type : internal::make_unsigned {}; +struct get_range_type { + using type = std::conditional_t<(sizeof(T) < sizeof(int)), unsigned int, std::make_unsigned_t>; +}; template struct get_range_type> : internal::make_unsigned {};