From aecd7ab4b1b29aed27d32605428414aa81786b4c Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 9 Dec 2025 19:53:27 -0500 Subject: [PATCH] Handle the edge case of constructing a Number from int64_t::min() --- include/xrpl/basics/Number.h | 13 ++++++++----- src/libxrpl/basics/Number.cpp | 27 ++++++++++++++++++++++++++- src/test/basics/Number_test.cpp | 31 ++++++++++++++++++++++++++----- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 66f9490bde..f83c9f7bf5 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -336,6 +336,13 @@ private: Number shiftExponent(int exponentDelta) const; + // Safely convert rep (int64) mantissa to internalrep (uint64). If the rep + // is negative, returns the positive value. This takes a little extra work + // because converting std::numeric_limits::min() flirts with + // UB, and can vary across compilers. + static internalrep + externalToInternal(rep mantissa); + class Guard; }; @@ -374,11 +381,7 @@ inline Number::Number(internalrep mantissa, int exponent, normalized) } inline Number::Number(rep mantissa, int exponent) - : Number( - mantissa < 0, - mantissa < 0 ? -mantissa : mantissa, - exponent, - normalized{}) + : Number(mantissa < 0, externalToInternal(mantissa), exponent, normalized{}) { } diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 00a079ed37..65edcd6d49 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -15,11 +15,13 @@ #include #ifdef _MSC_VER -#pragma message("Using boost::multiprecision::uint128_t") +#pragma message("Using boost::multiprecision::uint128_t and int128_t") #include using uint128_t = boost::multiprecision::uint128_t; +using int128_t = boost::multiprecision::int128_t; #else // !defined(_MSC_VER) using uint128_t = __uint128_t; +using int128_t = __int128_t; #endif // !defined(_MSC_VER) namespace ripple { @@ -183,6 +185,29 @@ Number::Guard::round() noexcept // Number +// Safely convert rep (int64) mantissa to internalrep (uint64). If the rep is +// negative, returns the positive value. This takes a little extra work because +// converting std::numeric_limits::min() flirts with UB, and can +// vary across compilers. +Number::internalrep +Number::externalToInternal(rep mantissa) +{ + // If the mantissa is already positive, just return it + if (mantissa >= 0) + return mantissa; + // If the mantissa is negative, but fits within the positive range of rep, + // return it negated + if (mantissa >= -std::numeric_limits::max()) + return -mantissa; + + // If the mantissa doesn't fit within the positive range, convert to + // int128_t, negate that, and cast it back down to the internalrep + // In practice, this is only going to cover the case of + // std::numeric_limits::min(). + int128_t temp = mantissa; + return static_cast(-temp); +} + constexpr Number Number::oneSmall() { diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 29523236d4..f581e2b182 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -70,11 +70,14 @@ public: test( Number{std::numeric_limits::min()}, - Number{ - scale == MantissaRange::small - ? -9'223'372'036'854'776 - : std::numeric_limits::min(), - 18 - Number::mantissaLog()}, + scale == MantissaRange::small + ? Number{-9'223'372'036'854'776, 3} + : Number{true, 9'223'372'036'854'775'808ULL, 0, Number::normalized{}}, + __LINE__); + test( + Number{std::numeric_limits::min() + 1}, + scale == MantissaRange::small ? Number{-9'223'372'036'854'776, 3} + : Number{-9'223'372'036'854'775'807}, __LINE__); test( Number{std::numeric_limits::max()}, @@ -1303,6 +1306,7 @@ public: test(Number(-2, 10), "-20000000000"); test(Number(-2, 11), "-2e11"); + switch (scale) { case MantissaRange::small: @@ -1336,6 +1340,13 @@ public: test( -(Number{std::numeric_limits::max(), -3}), "-9223372036854775"); + + test( + Number{std::numeric_limits::min(), 0}, + "-9223372036854775e3"); + test( + -(Number{std::numeric_limits::min(), 0}), + "9223372036854775e3"); } break; case MantissaRange::large: @@ -1362,6 +1373,16 @@ public: test( -(Number{std::numeric_limits::max(), 0}), "-9223372036854775807"); + + // Because the absolute value of min is larger than max, it + // will be scaled down to fit under max. Since we're + // rounding towards zero, the 8 at the end is dropped. + test( + Number{std::numeric_limits::min(), 0}, + "-9223372036854775800"); + test( + -(Number{std::numeric_limits::min(), 0}), + "9223372036854775800"); } break; default: