Skip to content

Commit

Permalink
Merge #2214: [Tests] Fix various things -fsanitize complains about
Browse files Browse the repository at this point in the history
921c4a4 script: prevent UB when computing abs value for num opcode serialize (pierrenn)
58ebadb Fix memory leak in net_tests (Pieter Wuille)
786f396 Fix memory leak in wallet tests (random-zebra)
d8b125d Avoid integer overflows in scriptnum tests (Pieter Wuille)

Pull request description:

  Fix `scriptnum_tests` currently failing with debug enabled, due to integer overflow, backporting bitcoin#9512.
  Last commit coming from bitcoin#18413.

ACKs for top commit:
  Fuzzbawls:
    ACK 921c4a4
  furszy:
    utACK 921c4a4 and merging..

Tree-SHA512: 07827bfda430e0704b427e373700ff37049bddaa59a56d415618441cb63fb8500bdb19f9df834574ade4618f0825853e44255f3a304486b4bbcb60dbbb382938
  • Loading branch information
furszy committed Mar 1, 2021
2 parents 6fc6208 + 921c4a4 commit 99f478b
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/script/script.h
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ class CScriptNum

std::vector<unsigned char> result;
const bool neg = value < 0;
uint64_t absvalue = neg ? -value : value;
uint64_t absvalue = neg ? ~static_cast<uint64_t>(value) + 1 : static_cast<uint64_t>(value);

while(absvalue)
{
Expand Down
7 changes: 2 additions & 5 deletions src/test/net_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,17 +166,14 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
bool fInboundIn = false;

// Test that fFeeler is false by default.
CNode* pnode1 = new CNode(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, pszDest, fInboundIn);
std::unique_ptr<CNode> pnode1(new CNode(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, pszDest, fInboundIn));
BOOST_CHECK(pnode1->fInbound == false);
BOOST_CHECK(pnode1->fFeeler == false);

fInboundIn = true;
CNode* pnode2 = new CNode(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, pszDest, fInboundIn);
std::unique_ptr<CNode> pnode2(new CNode(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, pszDest, fInboundIn));
BOOST_CHECK(pnode2->fInbound == true);
BOOST_CHECK(pnode2->fFeeler == false);

delete pnode1;
delete pnode2;
}

BOOST_AUTO_TEST_SUITE_END()
8 changes: 5 additions & 3 deletions src/test/scriptnum_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@

BOOST_FIXTURE_TEST_SUITE(scriptnum_tests, BasicTestingSetup)

static const long values[] = \
{ 0, 1, CHAR_MIN, CHAR_MAX, UCHAR_MAX, SHRT_MIN, USHRT_MAX, INT_MIN, INT_MAX, static_cast<long>UINT_MAX, LONG_MIN, LONG_MAX };
static const long offsets[] = { 1, 0x79, 0x80, 0x81, 0xFF, 0x7FFF, 0x8000, 0xFFFF, 0x10000};
/** A selection of numbers that do not trigger int64_t overflow
* when added/subtracted. */
static const int64_t values[] = { 0, 1, -2, 127, 128, -255, 256, (1LL << 15) - 1, -(1LL << 16), (1LL << 24) - 1, (1LL << 31), 1 - (1LL << 32), 1LL << 40 };

static const int64_t offsets[] = { 1, 0x79, 0x80, 0x81, 0xFF, 0x7FFF, 0x8000, 0xFFFF, 0x10000};

static bool verify(const CBigNum& bignum, const CScriptNum& scriptnum)
{
Expand Down
9 changes: 5 additions & 4 deletions src/wallet/test/wallet_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
// we repeat those tests this many times and only complain if all iterations of the test fail
#define RANDOM_REPEATS 5

std::vector<std::unique_ptr<CWalletTx>> wtxn;

typedef std::set<std::pair<const CWalletTx*,unsigned int> > CoinSet;

Expand All @@ -42,19 +43,19 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa
// so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe()
tx.vin.resize(1);
}
CWalletTx* wtx = new CWalletTx(pwalletMain, MakeTransactionRef(tx));
std::unique_ptr<CWalletTx> wtx(new CWalletTx(pwalletMain, MakeTransactionRef(std::move(tx))));
if (fIsFromMe) {
wtx->m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1);
}
COutput output(wtx, nInput, nAge, true, true);
COutput output(wtx.get(), nInput, nAge, true, true);
vCoins.push_back(output);
wtxn.emplace_back(std::move(wtx));
}

static void empty_wallet(void)
{
for (COutput output : vCoins)
delete output.tx;
vCoins.clear();
wtxn.clear();
}

static bool equal_sets(CoinSet a, CoinSet b)
Expand Down

0 comments on commit 99f478b

Please sign in to comment.