Skip to content

Commit 20a97a7

Browse files
committed
Review fix on #361: range-guard before the int64 cast in the hash normalizer
The value-normalizing hash cast the double to int64 in the FIRST conjunct, before the range checks - UB for NaN, inf, and |x| >= 2^63 (UBSan float-cast-overflow abort at scalar_number.h:117, the very class this PR removes elsewhere). NaN/inf now short-circuit via the range comparisons (false for NaN) before any cast. Regression test hashes 1e300/NaN/inf constants. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
1 parent 9e430f8 commit 20a97a7

2 files changed

Lines changed: 21 additions & 3 deletions

File tree

include/numsim_cas/core/scalar_number.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,10 @@ inline void hash_combine(std::size_t &seed, scalar_number const &value) {
114114
[&](auto const &x) {
115115
using T = std::decay_t<decltype(x)>;
116116
if constexpr (std::is_same_v<T, double>) {
117-
if (x == static_cast<double>(static_cast<std::int64_t>(x)) &&
118-
x >= -9.2e18 && x <= 9.2e18) {
117+
// guard BEFORE casting: the int64 cast is UB for NaN/inf and
118+
// |x| >= 2^63 (review on #361)
119+
if (x >= -9.2e18 && x <= 9.2e18 &&
120+
x == static_cast<double>(static_cast<std::int64_t>(x))) {
119121
hash_combine(seed, static_cast<std::int64_t>(x));
120122
return;
121123
}

tests/CoreBugFixTest.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1553,7 +1553,23 @@ TEST(HashCombineDouble, NumericallyEqualConstantsHashEqual) {
15531553
// fractional constants distinct
15541554
auto h1 = make_expression<scalar_constant>(0.5);
15551555
auto h2 = make_expression<scalar_constant>(0.9);
1556-
EXPECT_NE(h1.get().hash_value(), h2.get().hash_value());}
1556+
EXPECT_NE(h1.get().hash_value(), h2.get().hash_value());
1557+
}
1558+
1559+
// Review on #361: the int64 cast in the value-normalizing hash ran before
1560+
// its range guard - UB for NaN, inf, and huge doubles.
1561+
TEST(HashCombineDouble, HugeAndNonFiniteConstantsHashSafely) {
1562+
auto big = make_expression<scalar_constant>(1e300);
1563+
auto nan = make_expression<scalar_constant>(
1564+
std::numeric_limits<double>::quiet_NaN());
1565+
auto inf =
1566+
make_expression<scalar_constant>(std::numeric_limits<double>::infinity());
1567+
// must be UB-free under -fsanitize=float-cast-overflow (CI leg, #356)
1568+
(void)big.get().hash_value();
1569+
(void)nan.get().hash_value();
1570+
(void)inf.get().hash_value();
1571+
EXPECT_NE(big.get().hash_value(), inf.get().hash_value());
1572+
}
15571573

15581574
} // namespace numsim::cas
15591575

0 commit comments

Comments
 (0)