Merge #20406: util: Avoid invalid integer negation in FormatMoney and ValueFromAmount

1f05dbd06d util: Avoid invalid integer negation in ValueFromAmount: make ValueFromAmount(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() (practicalswift)
7cc75c9ba3 util: Avoid invalid integer negation in FormatMoney: make FormatMoney(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() (practicalswift)

Pull request description:

  Avoid invalid integer negation in `FormatMoney` and `ValueFromAmount`.

  Fixes #20402.

  Before this patch:

  ```
  $ CC=clang CXX=clang++ ./configure --with-sanitizers=undefined
  $ make -C src/ test/test_bitcoin
  $ src/test/test_bitcoin -t rpc_tests/rpc_format_monetary_values -t util_tests/util_FormatMoney
  core_write.cpp:21:29: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount'
    (aka 'long'); cast to an unsigned type to negate this value to itself
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior core_write.cpp:21:29 in
  test/rpc_tests.cpp(186): error: in "rpc_tests/rpc_format_monetary_values":
    check ValueFromAmount(std::numeric_limits<CAmount>::min()).write() == "-92233720368.54775808" has failed
    [--92233720368.-54775808 != -92233720368.54775808]
  util/moneystr.cpp:16:34: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount'
    (aka 'long'); cast to an unsigned type to negate this value to itself
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior util/moneystr.cpp:16:34 in
  test/util_tests.cpp(1188): error: in "util_tests/util_FormatMoney":
    check FormatMoney(std::numeric_limits<CAmount>::min()) == "-92233720368.54775808" has failed
    [--92233720368.-54775808 != -92233720368.54775808]
  ```

  After this patch:

  ```
  $ CC=clang CXX=clang++ ./configure --with-sanitizers=undefined
  $ make -C src/ test/test_bitcoin
  $ src/test/test_bitcoin -t rpc_tests/rpc_format_monetary_values -t util_tests/util_FormatMoney
  ```

ACKs for top commit:
  laanwj:
    re-ACK 1f05dbd06d

Tree-SHA512: 5aaeb8e2178f1597921f53c12bdfc2f3d5993d10c41658dcd25943e54e8cc2116a411bc71d928f890b33bc0b3761a8ee4449b0532bce41125b6c60692808c8c3
This commit is contained in:
Wladimir J. van der Laan
2021-03-03 19:04:21 +01:00
8 changed files with 45 additions and 29 deletions

View File

@@ -84,8 +84,7 @@ FUZZ_TARGET_INIT(integer, initialize_integer)
(void)DecompressAmount(u64);
(void)FormatISO8601Date(i64);
(void)FormatISO8601DateTime(i64);
// FormatMoney(i) not defined when i == std::numeric_limits<int64_t>::min()
if (i64 != std::numeric_limits<int64_t>::min()) {
{
int64_t parsed_money;
if (ParseMoney(FormatMoney(i64), parsed_money)) {
assert(parsed_money == i64);
@@ -132,8 +131,7 @@ FUZZ_TARGET_INIT(integer, initialize_integer)
(void)SipHashUint256Extra(u64, u64, u256, u32);
(void)ToLower(ch);
(void)ToUpper(ch);
// ValueFromAmount(i) not defined when i == std::numeric_limits<int64_t>::min()
if (i64 != std::numeric_limits<int64_t>::min()) {
{
int64_t parsed_money;
if (ParseMoney(ValueFromAmount(i64).getValStr(), parsed_money)) {
assert(parsed_money == i64);

View File

@@ -100,16 +100,7 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction)
(void)IsWitnessStandard(tx, coins_view_cache);
UniValue u(UniValue::VOBJ);
// ValueFromAmount(i) not defined when i == std::numeric_limits<int64_t>::min()
bool skip_tx_to_univ = false;
for (const CTxOut& txout : tx.vout) {
if (txout.nValue == std::numeric_limits<int64_t>::min()) {
skip_tx_to_univ = true;
}
}
if (!skip_tx_to_univ) {
TxToUniv(tx, /* hashBlock */ {}, u);
static const uint256 u256_max(uint256S("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"));
TxToUniv(tx, u256_max, u);
}
TxToUniv(tx, /* hashBlock */ {}, u);
static const uint256 u256_max(uint256S("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"));
TxToUniv(tx, u256_max, u);
}