diff options
-rw-r--r-- | src/test/fuzz/string.cpp | 10 | ||||
-rw-r--r-- | src/test/getarg_tests.cpp | 6 | ||||
-rw-r--r-- | src/test/util_tests.cpp | 55 | ||||
-rw-r--r-- | src/util/strencodings.h | 19 | ||||
-rwxr-xr-x | test/lint/lint-locale-dependence.sh | 1 |
5 files changed, 66 insertions, 25 deletions
diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index ab646c68fc..8f071b71fe 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -276,20 +276,14 @@ FUZZ_TARGET(string) } { - const int atoi_result = atoi(random_string_1.c_str()); const int locale_independent_atoi_result = LocaleIndependentAtoi<int>(random_string_1); const int64_t atoi64_result = atoi64_legacy(random_string_1); - const bool out_of_range = atoi64_result < std::numeric_limits<int>::min() || atoi64_result > std::numeric_limits<int>::max(); - if (out_of_range) { - assert(locale_independent_atoi_result == 0); - } else { - assert(atoi_result == locale_independent_atoi_result); - } + assert(locale_independent_atoi_result == std::clamp<int64_t>(atoi64_result, std::numeric_limits<int>::min(), std::numeric_limits<int>::max())); } { const int64_t atoi64_result = atoi64_legacy(random_string_1); const int64_t locale_independent_atoi_result = LocaleIndependentAtoi<int64_t>(random_string_1); - assert(atoi64_result == locale_independent_atoi_result || locale_independent_atoi_result == 0); + assert(atoi64_result == locale_independent_atoi_result); } } diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp index 88ce9648bc..d5142c8d74 100644 --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -6,6 +6,7 @@ #include <util/strencodings.h> #include <util/system.h> +#include <limits> #include <string> #include <utility> #include <vector> @@ -144,6 +145,11 @@ BOOST_AUTO_TEST_CASE(intarg) BOOST_CHECK_EQUAL(m_local_args.GetIntArg("-foo", 11), 0); BOOST_CHECK_EQUAL(m_local_args.GetIntArg("-bar", 11), 0); + // Check under-/overflow behavior. + ResetArgs("-foo=-9223372036854775809 -bar=9223372036854775808"); + BOOST_CHECK_EQUAL(m_local_args.GetIntArg("-foo", 0), std::numeric_limits<int64_t>::min()); + BOOST_CHECK_EQUAL(m_local_args.GetIntArg("-bar", 0), std::numeric_limits<int64_t>::max()); + ResetArgs("-foo=11 -bar=12"); BOOST_CHECK_EQUAL(m_local_args.GetIntArg("-foo", 0), 11); BOOST_CHECK_EQUAL(m_local_args.GetIntArg("-bar", 11), 12); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 04c4524911..20d27a237d 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -24,6 +24,8 @@ #include <array> #include <optional> +#include <limits> +#include <map> #include <stdint.h> #include <string.h> #include <thread> @@ -1621,6 +1623,11 @@ BOOST_AUTO_TEST_CASE(test_ToIntegral) BOOST_CHECK(!ToIntegral<uint8_t>("256")); } +int64_t atoi64_legacy(const std::string& str) +{ + return strtoll(str.c_str(), nullptr, 10); +} + BOOST_AUTO_TEST_CASE(test_LocaleIndependentAtoi) { BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("1234"), 1'234); @@ -1648,48 +1655,68 @@ BOOST_AUTO_TEST_CASE(test_LocaleIndependentAtoi) BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>(""), 0); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("aap"), 0); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("0x1"), 0); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-32482348723847471234"), 0); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("32482348723847471234"), 0); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-32482348723847471234"), -2'147'483'647 - 1); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("32482348723847471234"), 2'147'483'647); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("-9223372036854775809"), 0); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("-9223372036854775809"), -9'223'372'036'854'775'807LL - 1LL); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("-9223372036854775808"), -9'223'372'036'854'775'807LL - 1LL); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("9223372036854775807"), 9'223'372'036'854'775'807); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("9223372036854775808"), 0); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("9223372036854775808"), 9'223'372'036'854'775'807); + + std::map<std::string, int64_t> atoi64_test_pairs = { + {"-9223372036854775809", std::numeric_limits<int64_t>::min()}, + {"-9223372036854775808", -9'223'372'036'854'775'807LL - 1LL}, + {"9223372036854775807", 9'223'372'036'854'775'807}, + {"9223372036854775808", std::numeric_limits<int64_t>::max()}, + {"+-", 0}, + {"0x1", 0}, + {"ox1", 0}, + {"", 0}, + }; + + for (const auto& pair : atoi64_test_pairs) { + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>(pair.first), pair.second); + } + + // Ensure legacy compatibility with previous versions of Bitcoin Core's atoi64 + for (const auto& pair : atoi64_test_pairs) { + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>(pair.first), atoi64_legacy(pair.first)); + } BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("-1"), 0U); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("0"), 0U); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("18446744073709551615"), 18'446'744'073'709'551'615ULL); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("18446744073709551616"), 0U); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("18446744073709551616"), 18'446'744'073'709'551'615ULL); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-2147483649"), 0); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-2147483649"), -2'147'483'648LL); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-2147483648"), -2'147'483'648LL); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("2147483647"), 2'147'483'647); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("2147483648"), 0); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("2147483648"), 2'147'483'647); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("-1"), 0U); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("0"), 0U); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("4294967295"), 4'294'967'295U); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("4294967296"), 0U); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("4294967296"), 4'294'967'295U); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("-32769"), 0); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("-32769"), -32'768); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("-32768"), -32'768); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("32767"), 32'767); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("32768"), 0); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("32768"), 32'767); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("-1"), 0U); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("0"), 0U); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("65535"), 65'535U); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("65536"), 0U); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("65536"), 65'535U); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("-129"), 0); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("-129"), -128); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("-128"), -128); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("127"), 127); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("128"), 0); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("128"), 127); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("-1"), 0U); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("0"), 0U); BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("255"), 255U); - BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("256"), 0U); + BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("256"), 255U); } BOOST_AUTO_TEST_CASE(test_ParseInt64) diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 0945556b5e..1f83fa3ffa 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -16,6 +16,7 @@ #include <charconv> #include <cstdint> #include <iterator> +#include <limits> #include <optional> #include <string> #include <vector> @@ -93,8 +94,12 @@ void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut); // New code should use ToIntegral or the ParseInt* functions // which provide parse error feedback. // -// The goal of LocaleIndependentAtoi is to replicate the exact defined behaviour -// of atoi and atoi64 as they behave under the "C" locale. +// The goal of LocaleIndependentAtoi is to replicate the defined behaviour of +// std::atoi as it behaves under the "C" locale, and remove some undefined +// behavior. If the parsed value is bigger than the integer type's maximum +// value, or smaller than the integer type's minimum value, std::atoi has +// undefined behavior, while this function returns the maximum or minimum +// values, respectively. template <typename T> T LocaleIndependentAtoi(const std::string& str) { @@ -109,7 +114,15 @@ T LocaleIndependentAtoi(const std::string& str) s = s.substr(1); } auto [_, error_condition] = std::from_chars(s.data(), s.data() + s.size(), result); - if (error_condition != std::errc{}) { + if (error_condition == std::errc::result_out_of_range) { + if (s.length() >= 1 && s[0] == '-') { + // Saturate underflow, per strtoll's behavior. + return std::numeric_limits<T>::min(); + } else { + // Saturate overflow, per strtoll's behavior. + return std::numeric_limits<T>::max(); + } + } else if (error_condition != std::errc{}) { return 0; } return result; diff --git a/test/lint/lint-locale-dependence.sh b/test/lint/lint-locale-dependence.sh index 661bc35175..7d608eed6a 100755 --- a/test/lint/lint-locale-dependence.sh +++ b/test/lint/lint-locale-dependence.sh @@ -43,6 +43,7 @@ KNOWN_VIOLATIONS=( "src/test/dbwrapper_tests.cpp:.*snprintf" "src/test/fuzz/locale.cpp" "src/test/fuzz/string.cpp" + "src/test/util_tests.cpp" ) REGEXP_IGNORE_EXTERNAL_DEPENDENCIES="^src/(crypto/ctaes/|leveldb/|secp256k1/|minisketch/|tinyformat.h|univalue/)" |