diff --git a/absl/random/log_uniform_int_distribution.h b/absl/random/log_uniform_int_distribution.h index cbd5e0ca2ba..4871f0e8f60 100644 --- a/absl/random/log_uniform_int_distribution.h +++ b/absl/random/log_uniform_int_distribution.h @@ -16,13 +16,13 @@ #define ABSL_RANDOM_LOG_UNIFORM_INT_DISTRIBUTION_H_ #include -#include #include #include #include #include #include "absl/base/config.h" +#include "absl/base/macros.h" #include "absl/random/internal/iostream_state_saver.h" #include "absl/random/internal/traits.h" #include "absl/random/uniform_int_distribution.h" @@ -60,15 +60,18 @@ class log_uniform_int_distribution { range_(static_cast(max_) - static_cast(min_)), log_range_(0) { - assert(max_ >= min_); - assert(base_ > 1); + ABSL_HARDENING_ASSERT(max_ >= min_); + ABSL_HARDENING_ASSERT(base_ > 1); if (base_ == 2) { // Determine where the first set bit is on range(), giving a log2(range) // value which can be used to construct bounds. log_range_ = (std::min)(random_internal::BitWidth(range()), std::numeric_limits::digits); - } else { + } else if (base_ > 2) { + // An out-of-contract base_ (<= 1) skips this branch entirely so that + // no floating-point undefined behavior is reached. + // // NOTE: Computing the logN(x) introduces error from 2 sources: // 1. Conversion of int to double loses precision for values >= // 2^53, which may cause some log() computations to operate on @@ -240,9 +243,16 @@ std::basic_istream& operator>>( auto saver = random_internal::make_istream_state_saver(is); is >> min >> max >> base; if (!is.fail()) { - x.param(param_type(static_cast(min), - static_cast(max), - static_cast(base))); + const result_type min_val = static_cast(min); + const result_type max_val = static_cast(max); + const result_type base_val = static_cast(base); + if (max_val < min_val || base_val <= 1) { + // The input violates the param_type preconditions; signal failure by + // setting the failbit instead of constructing an invalid param_type. + is.setstate(is.rdstate() | std::ios_base::failbit); + } else { + x.param(param_type(min_val, max_val, base_val)); + } } return is; } diff --git a/absl/random/log_uniform_int_distribution_test.cc b/absl/random/log_uniform_int_distribution_test.cc index 591b5b37e48..4404111b1f4 100644 --- a/absl/random/log_uniform_int_distribution_test.cc +++ b/absl/random/log_uniform_int_distribution_test.cc @@ -116,6 +116,26 @@ TYPED_TEST(LogUniformIntDistributionTypeTest, SerializeTest) { } } +// operator>> must reject input that violates the param_type preconditions +// (max >= min and base > 1) by setting failbit and leaving the distribution +// unchanged, rather than constructing an out-of-contract param_type. +TYPED_TEST(LogUniformIntDistributionTypeTest, DeserializeRejectsInvalidParams) { + for (const char* input : { + "0 100 1", // base == 1 + "0 100 0", // base == 0 + "100 0 2", // max < min + }) { + absl::log_uniform_int_distribution dist(3, 6, 17); + const auto before = dist.param(); + + std::istringstream is(input); + is >> dist; + + EXPECT_TRUE(is.fail()) << input; + EXPECT_EQ(dist.param(), before) << input; + } +} + using log_uniform_i32 = absl::log_uniform_int_distribution; class LogUniformIntChiSquaredTest