SERVER-43032 simplify overflow_arithmetic.h

BillyDonahue · evergreen · commit d6f11844f1c1 · 2019-08-28T18:26:07.000Z
diff --git a/src/mongo/base/parse_number.cpp b/src/mongo/base/parse_number.cpp
@@ -142,9 +142,9 @@ inline StatusWith<uint64_t> parseMagnitudeFromStringWithBase(uint64_t base,
 
         // This block is (n = (n * base) + digitValue) with overflow checking at each step.
         uint64_t multiplied;
-        if (mongoUnsignedMultiplyOverflow64(n, base, &multiplied))
+        if (overflow::mul(n, base, &multiplied))
             return Status(ErrorCodes::Overflow, "Overflow");
-        if (mongoUnsignedAddOverflow64(multiplied, digitValue, &n))
+        if (overflow::add(multiplied, digitValue, &n))
             return Status(ErrorCodes::Overflow, "Overflow");
         ++charsConsumed;
     }
diff --git a/src/mongo/db/pipeline/document_source_sort.cpp b/src/mongo/db/pipeline/document_source_sort.cpp
@@ -130,11 +130,10 @@ Pipeline::SourceContainer::iterator DocumentSourceSort::doOptimizeAt(
 
         // The skip and limit values can be very large, so we need to make sure the sum doesn't
         // overflow before applying an optimization to pull the limit into the sort stage.
-        if (nextSkip && !mongoSignedAddOverflow64(skipSum, nextSkip->getSkip(), &safeSum)) {
+        if (nextSkip && !overflow::add(skipSum, nextSkip->getSkip(), &safeSum)) {
             skipSum = safeSum;
             ++stageItr;
-        } else if (nextLimit &&
-                   !mongoSignedAddOverflow64(nextLimit->getLimit(), skipSum, &safeSum)) {
+        } else if (nextLimit && !overflow::add(nextLimit->getLimit(), skipSum, &safeSum)) {
             _sortExecutor->setLimit(safeSum);
             container->erase(stageItr);
             stageItr = std::next(itr);
diff --git a/src/mongo/db/pipeline/expression.cpp b/src/mongo/db/pipeline/expression.cpp
@@ -2724,8 +2724,9 @@ Value ExpressionMultiply::evaluate(const Document& root, Variables* variables) c
                 decimalProduct = decimalProduct.multiply(val.coerceToDecimal());
             } else {
                 doubleProduct *= val.coerceToDouble();
+
                 if (!std::isfinite(val.coerceToDouble()) ||
-                    mongoSignedMultiplyOverflow64(longProduct, val.coerceToLong(), &longProduct)) {
+                    overflow::mul(longProduct, val.coerceToLong(), &longProduct)) {
                     // The number is either Infinity or NaN, or the 'longProduct' would have
                     // overflowed, so we're abandoning it.
                     productType = NumberDouble;
diff --git a/src/mongo/platform/overflow_arithmetic.h b/src/mongo/platform/overflow_arithmetic.h
@@ -35,123 +35,50 @@
 #include <SafeInt.hpp>
 #endif
 
-namespace mongo {
+#include "mongo/stdx/type_traits.h"
 
-/**
- * Returns true if multiplying lhs by rhs would overflow. Otherwise, multiplies 64-bit signed
- * or unsigned integers lhs by rhs and stores the result in *product.
- */
-constexpr bool mongoSignedMultiplyOverflow64(int64_t lhs, int64_t rhs, int64_t* product);
-constexpr bool mongoUnsignedMultiplyOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* product);
+namespace mongo::overflow {
 
 /**
- * Returns true if adding lhs and rhs would overflow. Otherwise, adds 64-bit signed or unsigned
- * integers lhs and rhs and stores the result in *sum.
- */
-constexpr bool mongoSignedAddOverflow64(int64_t lhs, int64_t rhs, int64_t* sum);
-constexpr bool mongoUnsignedAddOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* sum);
-
-/**
- * Returns true if subtracting rhs from lhs would overflow. Otherwise, subtracts 64-bit signed or
- * unsigned integers rhs from lhs and stores the result in *difference.
+ * Synopsis:
+ *
+ *   bool mul(A a, A b, T* r);
+ *   bool add(A a, A b, T* r);
+ *   bool sub(A a, A b, T* r);
+ *
+ * The domain type `A` evaluates to `T`, which is deduced from the `r` parameter.
+ * That is, the input parameters are coerced into the type accepted by the output parameter.
+ * All functions return true if operation would overflow, otherwise they store result in `*r`.
  */
-constexpr bool mongoSignedSubtractOverflow64(int64_t lhs, int64_t rhs, int64_t* difference);
-constexpr bool mongoUnsignedSubtractOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* difference);
 
+// MSVC : The SafeInt functions return false on overflow.
+// GCC, Clang: The __builtin_*_overflow functions return true on overflow.
 
+template <typename T>
+constexpr bool mul(stdx::type_identity_t<T> a, stdx::type_identity_t<T> b, T* r) {
 #ifdef _MSC_VER
-
-// The SafeInt functions return true on success, false on overflow.
-
-constexpr bool mongoSignedMultiplyOverflow64(int64_t lhs, int64_t rhs, int64_t* product) {
-    return !SafeMultiply(lhs, rhs, *product);
-}
-
-constexpr bool mongoUnsignedMultiplyOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* product) {
-    return !SafeMultiply(lhs, rhs, *product);
-}
-
-constexpr bool mongoSignedAddOverflow64(int64_t lhs, int64_t rhs, int64_t* sum) {
-    return !SafeAdd(lhs, rhs, *sum);
-}
-
-constexpr bool mongoUnsignedAddOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* sum) {
-    return !SafeAdd(lhs, rhs, *sum);
-}
-
-constexpr bool mongoSignedSubtractOverflow64(int64_t lhs, int64_t rhs, int64_t* difference) {
-    return !SafeSubtract(lhs, rhs, *difference);
-}
-
-constexpr bool mongoUnsignedSubtractOverflow64(uint64_t lhs, uint64_t rhs, uint64_t* difference) {
-    return !SafeSubtract(lhs, rhs, *difference);
-}
-
+    return !SafeMultiply(a, b, *r);
 #else
-
-// On GCC and CLANG we can use __builtin functions to perform these calculations. These return true
-// on overflow and false on success.
-
-constexpr bool mongoSignedMultiplyOverflow64(long lhs, long rhs, long* product) {
-    return __builtin_mul_overflow(lhs, rhs, product);
-}
-
-constexpr bool mongoSignedMultiplyOverflow64(long long lhs, long long rhs, long long* product) {
-    return __builtin_mul_overflow(lhs, rhs, product);
-}
-
-constexpr bool mongoUnsignedMultiplyOverflow64(unsigned long lhs,
-                                               unsigned long rhs,
-                                               unsigned long* product) {
-    return __builtin_mul_overflow(lhs, rhs, product);
-}
-
-constexpr bool mongoUnsignedMultiplyOverflow64(unsigned long long lhs,
-                                               unsigned long long rhs,
-                                               unsigned long long* product) {
-    return __builtin_mul_overflow(lhs, rhs, product);
-}
-
-constexpr bool mongoSignedAddOverflow64(long lhs, long rhs, long* sum) {
-    return __builtin_add_overflow(lhs, rhs, sum);
-}
-
-constexpr bool mongoSignedAddOverflow64(long long lhs, long long rhs, long long* sum) {
-    return __builtin_add_overflow(lhs, rhs, sum);
-}
-
-constexpr bool mongoUnsignedAddOverflow64(unsigned long lhs,
-                                          unsigned long rhs,
-                                          unsigned long* sum) {
-    return __builtin_add_overflow(lhs, rhs, sum);
-}
-
-constexpr bool mongoUnsignedAddOverflow64(unsigned long long lhs,
-                                          unsigned long long rhs,
-                                          unsigned long long* sum) {
-    return __builtin_add_overflow(lhs, rhs, sum);
-}
-
-constexpr bool mongoSignedSubtractOverflow64(long lhs, long rhs, long* difference) {
-    return __builtin_sub_overflow(lhs, rhs, difference);
-}
-
-constexpr bool mongoSignedSubtractOverflow64(long long lhs, long long rhs, long long* difference) {
-    return __builtin_sub_overflow(lhs, rhs, difference);
-}
-
-constexpr bool mongoUnsignedSubtractOverflow64(unsigned long lhs,
-                                               unsigned long rhs,
-                                               unsigned long* sum) {
-    return __builtin_sub_overflow(lhs, rhs, sum);
+    return __builtin_mul_overflow(a, b, r);
+#endif
 }
 
-constexpr bool mongoUnsignedSubtractOverflow64(unsigned long long lhs,
-                                               unsigned long long rhs,
-                                               unsigned long long* sum) {
-    return __builtin_sub_overflow(lhs, rhs, sum);
+template <typename T>
+constexpr bool add(stdx::type_identity_t<T> a, stdx::type_identity_t<T> b, T* r) {
+#ifdef _MSC_VER
+    return !SafeAdd(a, b, *r);
+#else
+    return __builtin_add_overflow(a, b, r);
+#endif
 }
 
+template <typename T>
+constexpr bool sub(stdx::type_identity_t<T> a, stdx::type_identity_t<T> b, T* r) {
+#ifdef _MSC_VER
+    return !SafeSubtract(a, b, *r);
+#else
+    return __builtin_sub_overflow(a, b, r);
 #endif
+}
 
-}  // namespace mongo
+}  // namespace mongo::overflow
diff --git a/src/mongo/platform/overflow_arithmetic_test.cpp b/src/mongo/platform/overflow_arithmetic_test.cpp
@@ -49,34 +49,34 @@ namespace {
     } while (false)
 
 #define assertSignedMultiplyNoOverflow(LHS, RHS, EXPECTED) \
-    assertArithOverflow(int64_t, mongoSignedMultiplyOverflow64, LHS, RHS, false, EXPECTED)
+    assertArithOverflow(int64_t, overflow::mul, LHS, RHS, false, EXPECTED)
 #define assertSignedMultiplyWithOverflow(LHS, RHS) \
-    assertArithOverflow(int64_t, mongoSignedMultiplyOverflow64, LHS, RHS, true, 0)
+    assertArithOverflow(int64_t, overflow::mul, LHS, RHS, true, 0)
 
 #define assertUnsignedMultiplyNoOverflow(LHS, RHS, EXPECTED) \
-    assertArithOverflow(uint64_t, mongoUnsignedMultiplyOverflow64, LHS, RHS, false, EXPECTED)
+    assertArithOverflow(uint64_t, overflow::mul, LHS, RHS, false, EXPECTED)
 #define assertUnsignedMultiplyWithOverflow(LHS, RHS) \
-    assertArithOverflow(uint64_t, mongoUnsignedMultiplyOverflow64, LHS, RHS, true, 0)
+    assertArithOverflow(uint64_t, overflow::mul, LHS, RHS, true, 0)
 
 #define assertSignedAddNoOverflow(LHS, RHS, EXPECTED) \
-    assertArithOverflow(int64_t, mongoSignedAddOverflow64, LHS, RHS, false, EXPECTED)
+    assertArithOverflow(int64_t, overflow::add, LHS, RHS, false, EXPECTED)
 #define assertSignedAddWithOverflow(LHS, RHS) \
-    assertArithOverflow(int64_t, mongoSignedAddOverflow64, LHS, RHS, true, 0)
+    assertArithOverflow(int64_t, overflow::add, LHS, RHS, true, 0)
 
 #define assertUnsignedAddNoOverflow(LHS, RHS, EXPECTED) \
-    assertArithOverflow(uint64_t, mongoUnsignedAddOverflow64, LHS, RHS, false, EXPECTED)
+    assertArithOverflow(uint64_t, overflow::add, LHS, RHS, false, EXPECTED)
 #define assertUnsignedAddWithOverflow(LHS, RHS) \
-    assertArithOverflow(uint64_t, mongoUnsignedAddOverflow64, LHS, RHS, true, 0)
+    assertArithOverflow(uint64_t, overflow::add, LHS, RHS, true, 0)
 
 #define assertSignedSubtractNoOverflow(LHS, RHS, EXPECTED) \
-    assertArithOverflow(int64_t, mongoSignedSubtractOverflow64, LHS, RHS, false, EXPECTED)
+    assertArithOverflow(int64_t, overflow::sub, LHS, RHS, false, EXPECTED)
 #define assertSignedSubtractWithOverflow(LHS, RHS) \
-    assertArithOverflow(int64_t, mongoSignedSubtractOverflow64, LHS, RHS, true, 0)
+    assertArithOverflow(int64_t, overflow::sub, LHS, RHS, true, 0)
 
 #define assertUnsignedSubtractNoOverflow(LHS, RHS, EXPECTED) \
-    assertArithOverflow(uint64_t, mongoUnsignedSubtractOverflow64, LHS, RHS, false, EXPECTED)
+    assertArithOverflow(uint64_t, overflow::sub, LHS, RHS, false, EXPECTED)
 #define assertUnsignedSubtractWithOverflow(LHS, RHS) \
-    assertArithOverflow(uint64_t, mongoUnsignedSubtractOverflow64, LHS, RHS, true, 0)
+    assertArithOverflow(uint64_t, overflow::sub, LHS, RHS, true, 0)
 
 TEST(OverflowArithmetic, SignedMultiplicationTests) {
     using limits = std::numeric_limits<int64_t>;
diff --git a/src/mongo/s/query/cluster_find.cpp b/src/mongo/s/query/cluster_find.cpp
@@ -94,7 +94,7 @@ StatusWith<std::unique_ptr<QueryRequest>> transformQueryForShards(
     boost::optional<long long> newLimit;
     if (qr.getLimit()) {
         long long newLimitValue;
-        if (mongoSignedAddOverflow64(*qr.getLimit(), qr.getSkip().value_or(0), &newLimitValue)) {
+        if (overflow::add(*qr.getLimit(), qr.getSkip().value_or(0), &newLimitValue)) {
             return Status(
                 ErrorCodes::Overflow,
                 str::stream()
@@ -110,8 +110,7 @@ StatusWith<std::unique_ptr<QueryRequest>> transformQueryForShards(
         // !wantMore and ntoreturn mean the same as !wantMore and limit, so perform the conversion.
         if (!qr.wantMore()) {
             long long newLimitValue;
-            if (mongoSignedAddOverflow64(
-                    *qr.getNToReturn(), qr.getSkip().value_or(0), &newLimitValue)) {
+            if (overflow::add(*qr.getNToReturn(), qr.getSkip().value_or(0), &newLimitValue)) {
                 return Status(ErrorCodes::Overflow,
                               str::stream()
                                   << "sum of ntoreturn and skip cannot be represented as a 64-bit "
@@ -121,8 +120,7 @@ StatusWith<std::unique_ptr<QueryRequest>> transformQueryForShards(
             newLimit = newLimitValue;
         } else {
             long long newNToReturnValue;
-            if (mongoSignedAddOverflow64(
-                    *qr.getNToReturn(), qr.getSkip().value_or(0), &newNToReturnValue)) {
+            if (overflow::add(*qr.getNToReturn(), qr.getSkip().value_or(0), &newNToReturnValue)) {
                 return Status(ErrorCodes::Overflow,
                               str::stream()
                                   << "sum of ntoreturn and skip cannot be represented as a 64-bit "
diff --git a/src/mongo/util/duration.h b/src/mongo/util/duration.h
@@ -110,7 +110,7 @@ constexpr ToDuration duration_cast(const Duration<FromPeriod>& from) {
         typename ToDuration::rep toCount = 0;
         uassert(ErrorCodes::DurationOverflow,
                 "Overflow casting from a lower-precision duration to a higher-precision duration",
-                !mongoSignedMultiplyOverflow64(from.count(), FromOverTo::num, &toCount));
+                !overflow::mul(from.count(), FromOverTo::num, &toCount));
         return ToDuration{toCount};
     }
     return ToDuration{from.count() / FromOverTo::den};
@@ -281,7 +281,7 @@ class Duration {
         }
         using OtherOverThis = std::ratio_divide<OtherPeriod, period>;
         rep otherCount;
-        if (mongoSignedMultiplyOverflow64(other.count(), OtherOverThis::num, &otherCount)) {
+        if (overflow::mul(other.count(), OtherOverThis::num, &otherCount)) {
             return other.count() < 0 ? 1 : -1;
         }
         if (count() < otherCount) {
@@ -329,14 +329,14 @@ class Duration {
     Duration& operator+=(const Duration& other) {
         uassert(ErrorCodes::DurationOverflow,
                 str::stream() << "Overflow while adding " << other << " to " << *this,
-                !mongoSignedAddOverflow64(count(), other.count(), &_count));
+                !overflow::add(count(), other.count(), &_count));
         return *this;
     }
 
     Duration& operator-=(const Duration& other) {
         uassert(ErrorCodes::DurationOverflow,
                 str::stream() << "Overflow while subtracting " << other << " from " << *this,
-                !mongoSignedSubtractOverflow64(count(), other.count(), &_count));
+                !overflow::sub(count(), other.count(), &_count));
         return *this;
     }
 
@@ -347,7 +347,7 @@ class Duration {
             "Durations may only be multiplied by values of signed integral type");
         uassert(ErrorCodes::DurationOverflow,
                 str::stream() << "Overflow while multiplying " << *this << " by " << scale,
-                !mongoSignedMultiplyOverflow64(count(), scale, &_count));
+                !overflow::mul(count(), scale, &_count));
         return *this;
     }
 
diff --git a/src/mongo/util/net/ssl_manager.cpp b/src/mongo/util/net/ssl_manager.cpp
@@ -938,8 +938,7 @@ StatusWith<DERToken> DERToken::parse(ConstDataRange cdr, size_t* outLength) {
     const uint64_t tagAndLengthByteCount = kTagLength + encodedLengthBytesCount;
 
     // This may overflow since derLength is from user data so check our arithmetic carefully.
-    if (mongoUnsignedAddOverflow64(tagAndLengthByteCount, derLength, outLength) ||
-        *outLength > cdr.length()) {
+    if (overflow::add(tagAndLengthByteCount, derLength, outLength) || *outLength > cdr.length()) {
         return Status(ErrorCodes::InvalidSSLConfiguration, "Invalid DER length");
     }
 

Original file line number	Diff line number	Diff line change
`@@ -938,8 +938,7 @@ StatusWith<DERToken> DERToken::parse(ConstDataRange cdr, size_t* outLength) {`
`938`	`938`	`const uint64_t tagAndLengthByteCount = kTagLength + encodedLengthBytesCount;`
`939`	`939`
`940`	`940`	`// This may overflow since derLength is from user data so check our arithmetic carefully.`
`941`		`- if (mongoUnsignedAddOverflow64(tagAndLengthByteCount, derLength, outLength) \|\|`
`942`		`- *outLength > cdr.length()) {`
	`941`	`+ if (overflow::add(tagAndLengthByteCount, derLength, outLength) \|\| *outLength > cdr.length()) {`
`943`	`942`	`return Status(ErrorCodes::InvalidSSLConfiguration, "Invalid DER length");`
`944`	`943`	`}`
`945`	`944`