From b9b854663a3b23b1b62077df9f6bbff47918d93d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 21 Feb 2022 15:50:59 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#24224: util: Add SaturatingAdd helper faa7d8a3f7cba02eca7e247108a6b98ea9daf373 util: Add SaturatingAdd helper (MarcoFalke) Pull request description: Seems good to have this in the repo, as it might be needed to write cleaner code. For example: * https://github.com/bitcoin/bitcoin/pull/24090#issuecomment-1019948200 * https://github.com/bitcoin/bitcoin/pull/23418#discussion_r744953272 * ... ACKs for top commit: MarcoFalke: Added a test. Should be trivial to re-ACK with `git range-diff bitcoin-core/master fa90189cbf faa7d8a3f7` klementtan: reACK faa7d8a3f7 vasild: ACK faa7d8a3f7cba02eca7e247108a6b98ea9daf373 Tree-SHA512: d0e6efdba7dfcbdd16ab4539a7f5e45a97d17792e42586c3c52caaae3fc70612dc9e364359658de5de5718fb8c2a765a59ceb2230098355394fa067a9732bc2a --- src/test/fuzz/addition_overflow.cpp | 17 +++++++++++++---- src/test/util_tests.cpp | 16 ++++++++++++++++ src/util/overflow.h | 20 +++++++++++++++++++- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/test/fuzz/addition_overflow.cpp b/src/test/fuzz/addition_overflow.cpp index 8b435431dd..71bda68a7a 100644 --- a/src/test/fuzz/addition_overflow.cpp +++ b/src/test/fuzz/addition_overflow.cpp @@ -26,6 +26,12 @@ void TestAdditionOverflow(FuzzedDataProvider& fuzzed_data_provider) const T i = fuzzed_data_provider.ConsumeIntegral(); const T j = fuzzed_data_provider.ConsumeIntegral(); const bool is_addition_overflow_custom = AdditionOverflow(i, j); + const auto maybe_add{CheckedAdd(i, j)}; + const auto sat_add{SaturatingAdd(i, j)}; + assert(is_addition_overflow_custom == !maybe_add.has_value()); + assert(is_addition_overflow_custom == AdditionOverflow(j, i)); + assert(maybe_add == CheckedAdd(j, i)); + assert(sat_add == SaturatingAdd(j, i)); #if defined(HAVE_BUILTIN_ADD_OVERFLOW) T result_builtin; const bool is_addition_overflow_builtin = __builtin_add_overflow(i, j, &result_builtin); @@ -33,11 +39,14 @@ void TestAdditionOverflow(FuzzedDataProvider& fuzzed_data_provider) if (!is_addition_overflow_custom) { assert(i + j == result_builtin); } -#else - if (!is_addition_overflow_custom) { - (void)(i + j); - } #endif + if (is_addition_overflow_custom) { + assert(sat_add == std::numeric_limits::min() || sat_add == std::numeric_limits::max()); + } else { + const auto add{i + j}; + assert(add == maybe_add.value()); + assert(add == sat_add); + } } } // namespace diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 7e46108116..114fd4b870 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1506,9 +1506,17 @@ static void TestAddMatrixOverflow() constexpr T MAXI{std::numeric_limits::max()}; BOOST_CHECK(!CheckedAdd(T{1}, MAXI)); BOOST_CHECK(!CheckedAdd(MAXI, MAXI)); + BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(T{1}, MAXI)); + BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(MAXI, MAXI)); + BOOST_CHECK_EQUAL(0, CheckedAdd(T{0}, T{0}).value()); BOOST_CHECK_EQUAL(MAXI, CheckedAdd(T{0}, MAXI).value()); BOOST_CHECK_EQUAL(MAXI, CheckedAdd(T{1}, MAXI - 1).value()); + BOOST_CHECK_EQUAL(MAXI - 1, CheckedAdd(T{1}, MAXI - 2).value()); + BOOST_CHECK_EQUAL(0, SaturatingAdd(T{0}, T{0})); + BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(T{0}, MAXI)); + BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(T{1}, MAXI - 1)); + BOOST_CHECK_EQUAL(MAXI - 1, SaturatingAdd(T{1}, MAXI - 2)); } /* Check for overflow or underflow */ @@ -1520,9 +1528,17 @@ static void TestAddMatrix() constexpr T MAXI{std::numeric_limits::max()}; BOOST_CHECK(!CheckedAdd(T{-1}, MINI)); BOOST_CHECK(!CheckedAdd(MINI, MINI)); + BOOST_CHECK_EQUAL(MINI, SaturatingAdd(T{-1}, MINI)); + BOOST_CHECK_EQUAL(MINI, SaturatingAdd(MINI, MINI)); + BOOST_CHECK_EQUAL(MINI, CheckedAdd(T{0}, MINI).value()); BOOST_CHECK_EQUAL(MINI, CheckedAdd(T{-1}, MINI + 1).value()); BOOST_CHECK_EQUAL(-1, CheckedAdd(MINI, MAXI).value()); + BOOST_CHECK_EQUAL(MINI + 1, CheckedAdd(T{-1}, MINI + 2).value()); + BOOST_CHECK_EQUAL(MINI, SaturatingAdd(T{0}, MINI)); + BOOST_CHECK_EQUAL(MINI, SaturatingAdd(T{-1}, MINI + 1)); + BOOST_CHECK_EQUAL(MINI + 1, SaturatingAdd(T{-1}, MINI + 2)); + BOOST_CHECK_EQUAL(-1, SaturatingAdd(MINI, MAXI)); } BOOST_AUTO_TEST_CASE(util_overflow) diff --git a/src/util/overflow.h b/src/util/overflow.h index c70a8b663e..6b7dd1e8fd 100644 --- a/src/util/overflow.h +++ b/src/util/overflow.h @@ -13,7 +13,7 @@ template [[nodiscard]] bool AdditionOverflow(const T i, const T j) noexcept { static_assert(std::is_integral::value, "Integral required."); - if (std::numeric_limits::is_signed) { + if constexpr (std::numeric_limits::is_signed) { return (i > 0 && j > std::numeric_limits::max() - i) || (i < 0 && j < std::numeric_limits::min() - i); } @@ -29,4 +29,22 @@ template return i + j; } +template +[[nodiscard]] T SaturatingAdd(const T i, const T j) noexcept +{ + if constexpr (std::numeric_limits::is_signed) { + if (i > 0 && j > std::numeric_limits::max() - i) { + return std::numeric_limits::max(); + } + if (i < 0 && j < std::numeric_limits::min() - i) { + return std::numeric_limits::min(); + } + } else { + if (std::numeric_limits::max() - i < j) { + return std::numeric_limits::max(); + } + } + return i + j; +} + #endif // BITCOIN_UTIL_OVERFLOW_H