Skip to content

Commit 055eeed

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 905e6e8 commit 055eeed

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
@@ -1759,7 +1759,23 @@ TEST(HashCombineDouble, NumericallyEqualConstantsHashEqual) {
17591759
// fractional constants distinct
17601760
auto h1 = make_expression<scalar_constant>(0.5);
17611761
auto h2 = make_expression<scalar_constant>(0.9);
1762-
EXPECT_NE(h1.get().hash_value(), h2.get().hash_value());}
1762+
EXPECT_NE(h1.get().hash_value(), h2.get().hash_value());
1763+
}
1764+
1765+
// Review on #361: the int64 cast in the value-normalizing hash ran before
1766+
// its range guard - UB for NaN, inf, and huge doubles.
1767+
TEST(HashCombineDouble, HugeAndNonFiniteConstantsHashSafely) {
1768+
auto big = make_expression<scalar_constant>(1e300);
1769+
auto nan = make_expression<scalar_constant>(
1770+
std::numeric_limits<double>::quiet_NaN());
1771+
auto inf =
1772+
make_expression<scalar_constant>(std::numeric_limits<double>::infinity());
1773+
// must be UB-free under -fsanitize=float-cast-overflow (CI leg, #356)
1774+
(void)big.get().hash_value();
1775+
(void)nan.get().hash_value();
1776+
(void)inf.get().hash_value();
1777+
EXPECT_NE(big.get().hash_value(), inf.get().hash_value());
1778+
}
17631779

17641780
} // namespace numsim::cas
17651781

0 commit comments

Comments
 (0)