Skip to content

Commit 2005761

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 3af6dfe commit 2005761

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
@@ -1401,7 +1401,23 @@ TEST(HashCombineDouble, NumericallyEqualConstantsHashEqual) {
14011401
// fractional constants distinct
14021402
auto h1 = make_expression<scalar_constant>(0.5);
14031403
auto h2 = make_expression<scalar_constant>(0.9);
1404-
EXPECT_NE(h1.get().hash_value(), h2.get().hash_value());}
1404+
EXPECT_NE(h1.get().hash_value(), h2.get().hash_value());
1405+
}
1406+
1407+
// Review on #361: the int64 cast in the value-normalizing hash ran before
1408+
// its range guard - UB for NaN, inf, and huge doubles.
1409+
TEST(HashCombineDouble, HugeAndNonFiniteConstantsHashSafely) {
1410+
auto big = make_expression<scalar_constant>(1e300);
1411+
auto nan = make_expression<scalar_constant>(
1412+
std::numeric_limits<double>::quiet_NaN());
1413+
auto inf =
1414+
make_expression<scalar_constant>(std::numeric_limits<double>::infinity());
1415+
// must be UB-free under -fsanitize=float-cast-overflow (CI leg, #356)
1416+
(void)big.get().hash_value();
1417+
(void)nan.get().hash_value();
1418+
(void)inf.get().hash_value();
1419+
EXPECT_NE(big.get().hash_value(), inf.get().hash_value());
1420+
}
14051421

14061422
} // namespace numsim::cas
14071423

0 commit comments

Comments
 (0)