SERVER-38623 Make safeNumberLongForHash consistent on all platforms

Previously, the hash function for double values relied on undefined
behavior when computing the hash for 2^63, meaning that it could give
different results on different platforms. Almost all platforms we
support returned -2^63 as the result of safeNumberLongForHash when the
input was 2^63, so this patch makes that the official result. Now,
safeNumberLongForHash will explicitly convert 2^63 to -2^63 on _all_
platforms without invoking any undefined behavior.

Note that this does not change our guidance that using a hashed index
for a field containing a floating point value greater than 2^53 is
unsupported:
https://docs.mongodb.com/manual/core/index-hashed/#considerations

2 files changed, 18 insertions, 19 deletions

diff --git a/src/mongo/bson/bsonelement.h b/src/mongo/bson/bsonelement.h
index a42f993c4a5..09ecf8b9d4b 100644
--- a/src/mongo/bson/bsonelement.h
+++ b/src/mongo/bson/bsonelement.h
@@ -893,28 +893,20 @@ inline long long BSONElement::safeNumberLong() const {
  * same behavior.
  *
  * Historically, safeNumberLong() used a check that would consider 2^63 to be safe to cast to
- * int64_t, but that value actually overflows. That overflow is preserved here.
+ * int64_t, but that cast actually overflows. On most platforms, the undefined cast of 2^63 to
+ * int64_t would roll over to -2^63, and that's the behavior we preserve here explicitly.
  *
  * The new safeNumberLong() function uses a tight bound, allowing it to correctly clamp double 2^63
  * to the max 64-bit int (2^63 - 1).
  */
 inline long long BSONElement::safeNumberLongForHash() const {
-    if (NumberDouble == type()) {
-        double d = numberDouble();
-        if (std::isnan(d)) {
-            return 0;
-        }
-        if (d > (double)std::numeric_limits<long long>::max()) {
-            return std::numeric_limits<long long>::max();
-        }
-        if (d < std::numeric_limits<long long>::min()) {
-            return std::numeric_limits<long long>::min();
-        }
-        return (long long)d;
+    // Rather than relying on the undefined overflow conversion, we maintain compatibility by
+    // explicitly checking for a 2^63 double value and returning -2^63.
+    if (NumberDouble == type() && numberDouble() == BSONElement::kLongLongMaxPlusOneAsDouble) {
+        return std::numeric_limits<long long>::lowest();
+    } else {
+        return safeNumberLong();
     }
-
-    // safeNumberLong() and safeNumberLongForHash() have identical behavior for non-long value.
-    return safeNumberLong();
 }
 
 inline BSONElement::BSONElement() {
diff --git a/src/mongo/bson/bsonelement_test.cpp b/src/mongo/bson/bsonelement_test.cpp
index 27f51039245..d8f311450fd 100644
--- a/src/mongo/bson/bsonelement_test.cpp
+++ b/src/mongo/bson/bsonelement_test.cpp
@@ -138,12 +138,19 @@ TEST(BSONElement, SafeNumberLongPositiveBound) {
                        << "positiveInfinity"
                        << std::numeric_limits<double>::infinity());
 
-    // The numberLongForHash() function clamps kLongLongMaxPlusOneAsDouble to the max 64-bit value
+    // kLongLongMaxPlusOneAsDouble is the least double value that will overflow a 64-bit signed
+    // two's-complement integer. Historically, converting this value with safeNumberLong() would
+    // return the result of casting to double with a C-style cast. That operation is undefined
+    // because of the overflow, but on most platforms we support, it returned the min 64-bit value
+    // (-2^63). The safeNumberLongForHash() function should preserve that behavior indefinitely for
+    // compatibility with on-disk data.
+    ASSERT_EQ(obj["kLongLongMaxPlusOneAsDouble"].safeNumberLongForHash(),
+              std::numeric_limits<long long>::lowest());
+
+    // The safeNumberLong() function clamps kLongLongMaxPlusOneAsDouble to the max 64-bit value
     // (2^63 - 1).
     ASSERT_EQ(obj["kLongLongMaxPlusOneAsDouble"].safeNumberLong(),
               std::numeric_limits<long long>::max());
-    // The safeNumberLongForHash() function will return -2^63, but we don't test it here, because
-    // the overflowing cast that it uses can trigger UBSan
 
     // One quantum below kLongLongMaxPlusOneAsDouble is the largest double that safely converts to a
     // 64-bit signed two-s complement integer. Both safeNumberLong() and safeNumberLongForHash()