Skip to content

Commit 817a95a

Browse files
kwvgkwvg
committed
merge bitcoin#24041: Restore GetIntArg saturating behavior
1 parent d451246 commit 817a95a

File tree

5 files changed

+66
-25
lines changed

5 files changed

+66
-25
lines changed

‎src/test/fuzz/string.cpp

+2-8
Original file line numberDiff line numberDiff line change
@@ -280,20 +280,14 @@ FUZZ_TARGET(string)
280280
}
281281

282282
{
283-
const int atoi_result = atoi(random_string_1.c_str());
284283
const int locale_independent_atoi_result = LocaleIndependentAtoi<int>(random_string_1);
285284
const int64_t atoi64_result = atoi64_legacy(random_string_1);
286-
const bool out_of_range = atoi64_result < std::numeric_limits<int>::min() || atoi64_result > std::numeric_limits<int>::max();
287-
if (out_of_range) {
288-
assert(locale_independent_atoi_result == 0);
289-
} else {
290-
assert(atoi_result == locale_independent_atoi_result);
291-
}
285+
assert(locale_independent_atoi_result == std::clamp<int64_t>(atoi64_result, std::numeric_limits<int>::min(), std::numeric_limits<int>::max()));
292286
}
293287

294288
{
295289
const int64_t atoi64_result = atoi64_legacy(random_string_1);
296290
const int64_t locale_independent_atoi_result = LocaleIndependentAtoi<int64_t>(random_string_1);
297-
assert(atoi64_result == locale_independent_atoi_result || locale_independent_atoi_result == 0);
291+
assert(atoi64_result == locale_independent_atoi_result);
298292
}
299293
}

‎src/test/getarg_tests.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <util/strencodings.h>
77
#include <util/system.h>
88

9+
#include <limits>
910
#include <string>
1011
#include <utility>
1112
#include <vector>
@@ -144,6 +145,11 @@ BOOST_AUTO_TEST_CASE(intarg)
144145
BOOST_CHECK_EQUAL(m_local_args.GetArg("-foo", 11), 0);
145146
BOOST_CHECK_EQUAL(m_local_args.GetArg("-bar", 11), 0);
146147

148+
// Check under-/overflow behavior.
149+
ResetArgs("-foo=-9223372036854775809 -bar=9223372036854775808");
150+
BOOST_CHECK_EQUAL(m_local_args.GetArg("-foo", 0), std::numeric_limits<int64_t>::min());
151+
BOOST_CHECK_EQUAL(m_local_args.GetArg("-bar", 0), std::numeric_limits<int64_t>::max());
152+
147153
ResetArgs("-foo=11 -bar=12");
148154
BOOST_CHECK_EQUAL(m_local_args.GetArg("-foo", 0), 11);
149155
BOOST_CHECK_EQUAL(m_local_args.GetArg("-bar", 11), 12);

‎src/test/util_tests.cpp

+41-14
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
#include <fstream>
3030
#include <deque>
3131
#include <optional>
32+
#include <limits>
33+
#include <map>
3234
#include <random>
3335
#include <stdint.h>
3436
#include <string.h>
@@ -1737,6 +1739,11 @@ BOOST_AUTO_TEST_CASE(test_ToIntegral)
17371739
BOOST_CHECK(!ToIntegral<uint8_t>("256"));
17381740
}
17391741

1742+
int64_t atoi64_legacy(const std::string& str)
1743+
{
1744+
return strtoll(str.c_str(), nullptr, 10);
1745+
}
1746+
17401747
BOOST_AUTO_TEST_CASE(test_LocaleIndependentAtoi)
17411748
{
17421749
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("1234"), 1'234);
@@ -1764,48 +1771,68 @@ BOOST_AUTO_TEST_CASE(test_LocaleIndependentAtoi)
17641771
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>(""), 0);
17651772
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("aap"), 0);
17661773
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("0x1"), 0);
1767-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-32482348723847471234"), 0);
1768-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("32482348723847471234"), 0);
1774+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-32482348723847471234"), -2'147'483'647 - 1);
1775+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("32482348723847471234"), 2'147'483'647);
17691776

1770-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("-9223372036854775809"), 0);
1777+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("-9223372036854775809"), -9'223'372'036'854'775'807LL - 1LL);
17711778
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("-9223372036854775808"), -9'223'372'036'854'775'807LL - 1LL);
17721779
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("9223372036854775807"), 9'223'372'036'854'775'807);
1773-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("9223372036854775808"), 0);
1780+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>("9223372036854775808"), 9'223'372'036'854'775'807);
1781+
1782+
std::map<std::string, int64_t> atoi64_test_pairs = {
1783+
{"-9223372036854775809", std::numeric_limits<int64_t>::min()},
1784+
{"-9223372036854775808", -9'223'372'036'854'775'807LL - 1LL},
1785+
{"9223372036854775807", 9'223'372'036'854'775'807},
1786+
{"9223372036854775808", std::numeric_limits<int64_t>::max()},
1787+
{"+-", 0},
1788+
{"0x1", 0},
1789+
{"ox1", 0},
1790+
{"", 0},
1791+
};
1792+
1793+
for (const auto& pair : atoi64_test_pairs) {
1794+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>(pair.first), pair.second);
1795+
}
1796+
1797+
// Ensure legacy compatibility with previous versions of Bitcoin Core's atoi64
1798+
for (const auto& pair : atoi64_test_pairs) {
1799+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int64_t>(pair.first), atoi64_legacy(pair.first));
1800+
}
17741801

17751802
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("-1"), 0U);
17761803
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("0"), 0U);
17771804
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("18446744073709551615"), 18'446'744'073'709'551'615ULL);
1778-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("18446744073709551616"), 0U);
1805+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint64_t>("18446744073709551616"), 18'446'744'073'709'551'615ULL);
17791806

1780-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-2147483649"), 0);
1807+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-2147483649"), -2'147'483'648LL);
17811808
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("-2147483648"), -2'147'483'648LL);
17821809
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("2147483647"), 2'147'483'647);
1783-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("2147483648"), 0);
1810+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int32_t>("2147483648"), 2'147'483'647);
17841811

17851812
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("-1"), 0U);
17861813
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("0"), 0U);
17871814
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("4294967295"), 4'294'967'295U);
1788-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("4294967296"), 0U);
1815+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint32_t>("4294967296"), 4'294'967'295U);
17891816

1790-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("-32769"), 0);
1817+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("-32769"), -32'768);
17911818
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("-32768"), -32'768);
17921819
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("32767"), 32'767);
1793-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("32768"), 0);
1820+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int16_t>("32768"), 32'767);
17941821

17951822
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("-1"), 0U);
17961823
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("0"), 0U);
17971824
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("65535"), 65'535U);
1798-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("65536"), 0U);
1825+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint16_t>("65536"), 65'535U);
17991826

1800-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("-129"), 0);
1827+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("-129"), -128);
18011828
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("-128"), -128);
18021829
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("127"), 127);
1803-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("128"), 0);
1830+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<int8_t>("128"), 127);
18041831

18051832
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("-1"), 0U);
18061833
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("0"), 0U);
18071834
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("255"), 255U);
1808-
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("256"), 0U);
1835+
BOOST_CHECK_EQUAL(LocaleIndependentAtoi<uint8_t>("256"), 255U);
18091836
}
18101837

18111838
BOOST_AUTO_TEST_CASE(test_ParseInt64)

‎src/util/strencodings.h

+16-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include <charconv>
1717
#include <cstdint>
18+
#include <limits>
1819
#include <optional>
1920
#include <string>
2021
#include <vector>
@@ -91,8 +92,12 @@ void SplitHostPort(std::string_view in, uint16_t &portOut, std::string &hostOut)
9192
// New code should use ToIntegral or the ParseInt* functions
9293
// which provide parse error feedback.
9394
//
94-
// The goal of LocaleIndependentAtoi is to replicate the exact defined behaviour
95-
// of atoi and atoi64 as they behave under the "C" locale.
95+
// The goal of LocaleIndependentAtoi is to replicate the defined behaviour of
96+
// std::atoi as it behaves under the "C" locale, and remove some undefined
97+
// behavior. If the parsed value is bigger than the integer type's maximum
98+
// value, or smaller than the integer type's minimum value, std::atoi has
99+
// undefined behavior, while this function returns the maximum or minimum
100+
// values, respectively.
96101
template <typename T>
97102
T LocaleIndependentAtoi(std::string_view str)
98103
{
@@ -107,7 +112,15 @@ T LocaleIndependentAtoi(std::string_view str)
107112
s = s.substr(1);
108113
}
109114
auto [_, error_condition] = std::from_chars(s.data(), s.data() + s.size(), result);
110-
if (error_condition != std::errc{}) {
115+
if (error_condition == std::errc::result_out_of_range) {
116+
if (s.length() >= 1 && s[0] == '-') {
117+
// Saturate underflow, per strtoll's behavior.
118+
return std::numeric_limits<T>::min();
119+
} else {
120+
// Saturate overflow, per strtoll's behavior.
121+
return std::numeric_limits<T>::max();
122+
}
123+
} else if (error_condition != std::errc{}) {
111124
return 0;
112125
}
113126
return result;

‎test/lint/lint-locale-dependence.py

+1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
"src/dbwrapper.cpp:.*vsnprintf",
5050
"src/test/fuzz/locale.cpp",
5151
"src/test/fuzz/string.cpp",
52+
"src/test/util_tests.cpp",
5253
"src/util/strencodings.cpp:.*strtoll",
5354
"src/util/system.cpp:.*fprintf",
5455
"src/wallet/bdb.cpp:.*DbEnv::strerror", # False positive

0 commit comments

Comments
 (0)