Skip to content

Commit b60c477

Browse files
author
MarcoFalke
committed
Merge bitcoin#23629: refactor tests to fix ubsan suppressions
faedb11 refactor tests to fix ubsan suppressions (MarcoFalke) Pull request description: The ubsan suppressions for test files have several issues: * They bloat the suppressions file, distracting from real bugs * They are file-wide, thus suppressing any other (newly introduced) issues in the same file * Some of them are causing compile issues with -Wimplicit-int-conversion Fix all issues by making the integer truncations or sign changes explicit. This is a refactor that shouldn't change the test binary unless compiled with sanitizers. ACKs for top commit: jonatack: ACK faedb11 per `git range-diff 1824644 fa9bab0 faedb11` and clang 13 debug build sanity check Tree-SHA512: a1a6aaf2cf52562a49220375deb10062b388c1ccb7ddcd9ef2a8e6b9873961c5b285704799589b7dd5579f3db8692ba72a9d622172b4e4d2423534cd550e8859
2 parents 1824644 + faedb11 commit b60c477

File tree

8 files changed

+25
-34
lines changed

8 files changed

+25
-34
lines changed

‎src/test/arith_uint256_tests.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,11 @@ static void shiftArrayRight(unsigned char* to, const unsigned char* from, unsign
129129
{
130130
unsigned int F = (T+bitsToShift/8);
131131
if (F < arrayLength)
132-
to[T] = from[F] >> (bitsToShift%8);
132+
to[T] = uint8_t(from[F] >> (bitsToShift % 8));
133133
else
134134
to[T] = 0;
135135
if (F + 1 < arrayLength)
136-
to[T] |= from[(F+1)] << (8-bitsToShift%8);
136+
to[T] |= uint8_t(from[(F + 1)] << (8 - bitsToShift % 8));
137137
}
138138
}
139139

@@ -144,9 +144,9 @@ static void shiftArrayLeft(unsigned char* to, const unsigned char* from, unsigne
144144
if (T >= bitsToShift/8)
145145
{
146146
unsigned int F = T-bitsToShift/8;
147-
to[T] = from[F] << (bitsToShift%8);
147+
to[T] = uint8_t(from[F] << (bitsToShift % 8));
148148
if (T >= bitsToShift/8+1)
149-
to[T] |= from[F-1] >> (8-bitsToShift%8);
149+
to[T] |= uint8_t(from[F - 1] >> (8 - bitsToShift % 8));
150150
}
151151
else {
152152
to[T] = 0;
@@ -202,7 +202,7 @@ BOOST_AUTO_TEST_CASE( unaryOperators ) // ! ~ -
202202
BOOST_CHECK(~ZeroL == MaxL);
203203

204204
unsigned char TmpArray[32];
205-
for (unsigned int i = 0; i < 32; ++i) { TmpArray[i] = ~R1Array[i]; }
205+
for (unsigned int i = 0; i < 32; ++i) { TmpArray[i] = uint8_t(~R1Array[i]); }
206206
BOOST_CHECK(arith_uint256V(std::vector<unsigned char>(TmpArray,TmpArray+32)) == (~R1L));
207207

208208
BOOST_CHECK(-ZeroL == ZeroL);
@@ -215,7 +215,7 @@ BOOST_AUTO_TEST_CASE( unaryOperators ) // ! ~ -
215215
// Check if doing _A_ _OP_ _B_ results in the same as applying _OP_ onto each
216216
// element of Aarray and Barray, and then converting the result into an arith_uint256.
217217
#define CHECKBITWISEOPERATOR(_A_,_B_,_OP_) \
218-
for (unsigned int i = 0; i < 32; ++i) { TmpArray[i] = _A_##Array[i] _OP_ _B_##Array[i]; } \
218+
for (unsigned int i = 0; i < 32; ++i) { TmpArray[i] = uint8_t(_A_##Array[i] _OP_ _B_##Array[i]); } \
219219
BOOST_CHECK(arith_uint256V(std::vector<unsigned char>(TmpArray,TmpArray+32)) == (_A_##L _OP_ _B_##L));
220220

221221
#define CHECKASSIGNMENTOPERATOR(_A_,_B_,_OP_) \

‎src/test/coins_tests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,11 +393,11 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test)
393393
// Update the expected result to know about the new output coins
394394
assert(tx.vout.size() == 1);
395395
const COutPoint outpoint(tx.GetHash(), 0);
396-
result[outpoint] = Coin(tx.vout[0], height, CTransaction(tx).IsCoinBase());
396+
result[outpoint] = Coin{tx.vout[0], int(height), CTransaction(tx).IsCoinBase()};
397397

398398
// Call UpdateCoins on the top cache
399399
CTxUndo undo;
400-
UpdateCoins(CTransaction(tx), *(stack.back()), undo, height);
400+
UpdateCoins(CTransaction(tx), *(stack.back()), undo, int(height));
401401

402402
// Update the utxo set for future spends
403403
utxoset.insert(outpoint);

‎src/test/pow_tests.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ BOOST_AUTO_TEST_CASE(CheckProofOfWork_test_overflow_target)
7373
{
7474
const auto consensus = CreateChainParams(*m_node.args, CBaseChainParams::MAIN)->GetConsensus();
7575
uint256 hash;
76-
unsigned int nBits = ~0x00800000;
76+
unsigned int nBits{~0x00800000U};
7777
hash.SetHex("0x1");
7878
BOOST_CHECK(!CheckProofOfWork(hash, nBits, consensus));
7979
}

‎src/test/prevector_tests.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ BOOST_AUTO_TEST_CASE(PrevectorTestInt)
220220
prevector_tester<8, int> test;
221221
for (int i = 0; i < 2048; i++) {
222222
if (InsecureRandBits(2) == 0) {
223-
test.insert(InsecureRandRange(test.size() + 1), InsecureRand32());
223+
test.insert(InsecureRandRange(test.size() + 1), int(InsecureRand32()));
224224
}
225225
if (test.size() > 0 && InsecureRandBits(2) == 1) {
226226
test.erase(InsecureRandRange(test.size()));
@@ -230,15 +230,15 @@ BOOST_AUTO_TEST_CASE(PrevectorTestInt)
230230
test.resize(new_size);
231231
}
232232
if (InsecureRandBits(3) == 3) {
233-
test.insert(InsecureRandRange(test.size() + 1), 1 + InsecureRandBool(), InsecureRand32());
233+
test.insert(InsecureRandRange(test.size() + 1), 1 + InsecureRandBool(), int(InsecureRand32()));
234234
}
235235
if (InsecureRandBits(3) == 4) {
236236
int del = std::min<int>(test.size(), 1 + (InsecureRandBool()));
237237
int beg = InsecureRandRange(test.size() + 1 - del);
238238
test.erase(beg, beg + del);
239239
}
240240
if (InsecureRandBits(4) == 5) {
241-
test.push_back(InsecureRand32());
241+
test.push_back(int(InsecureRand32()));
242242
}
243243
if (test.size() > 0 && InsecureRandBits(4) == 6) {
244244
test.pop_back();
@@ -247,7 +247,7 @@ BOOST_AUTO_TEST_CASE(PrevectorTestInt)
247247
int values[4];
248248
int num = 1 + (InsecureRandBits(2));
249249
for (int k = 0; k < num; k++) {
250-
values[k] = InsecureRand32();
250+
values[k] = int(InsecureRand32());
251251
}
252252
test.insert_range(InsecureRandRange(test.size() + 1), values, values + num);
253253
}
@@ -263,13 +263,13 @@ BOOST_AUTO_TEST_CASE(PrevectorTestInt)
263263
test.shrink_to_fit();
264264
}
265265
if (test.size() > 0) {
266-
test.update(InsecureRandRange(test.size()), InsecureRand32());
266+
test.update(InsecureRandRange(test.size()), int(InsecureRand32()));
267267
}
268268
if (InsecureRandBits(10) == 11) {
269269
test.clear();
270270
}
271271
if (InsecureRandBits(9) == 12) {
272-
test.assign(InsecureRandBits(5), InsecureRand32());
272+
test.assign(InsecureRandBits(5), int(InsecureRand32()));
273273
}
274274
if (InsecureRandBits(3) == 3) {
275275
test.swap();
@@ -283,8 +283,8 @@ BOOST_AUTO_TEST_CASE(PrevectorTestInt)
283283
if (InsecureRandBits(5) == 19) {
284284
unsigned int num = 1 + (InsecureRandBits(4));
285285
std::vector<int> values(num);
286-
for (auto &v : values) {
287-
v = InsecureRand32();
286+
for (int& v : values) {
287+
v = int(InsecureRand32());
288288
}
289289
test.resize_uninitialized(values);
290290
}

‎src/test/sighash_tests.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,9 @@ void static RandomScript(CScript &script) {
9191
script << oplist[InsecureRandRange(std::size(oplist))];
9292
}
9393

94-
void static RandomTransaction(CMutableTransaction &tx, bool fSingle) {
95-
tx.nVersion = InsecureRand32();
94+
void static RandomTransaction(CMutableTransaction& tx, bool fSingle)
95+
{
96+
tx.nVersion = int(InsecureRand32());
9697
tx.vin.clear();
9798
tx.vout.clear();
9899
tx.nLockTime = (InsecureRandBool()) ? InsecureRand32() : 0;
@@ -126,7 +127,7 @@ BOOST_AUTO_TEST_CASE(sighash_test)
126127
int nRandomTests = 50000;
127128
#endif
128129
for (int i=0; i<nRandomTests; i++) {
129-
int nHashType = InsecureRand32();
130+
int nHashType{int(InsecureRand32())};
130131
CMutableTransaction txTo;
131132
RandomTransaction(txTo, (nHashType & 0x1f) == SIGHASH_SINGLE);
132133
CScript scriptCode;

‎src/test/skiplist_tests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,8 @@ BOOST_AUTO_TEST_CASE(findearliestatleast_test)
114114
} else {
115115
// randomly choose something in the range [MTP, MTP*2]
116116
int64_t medianTimePast = vBlocksMain[i].GetMedianTimePast();
117-
int r = InsecureRandRange(medianTimePast);
118-
vBlocksMain[i].nTime = r + medianTimePast;
117+
int r{int(InsecureRandRange(medianTimePast))};
118+
vBlocksMain[i].nTime = uint32_t(r + medianTimePast);
119119
vBlocksMain[i].nTimeMax = std::max(vBlocksMain[i].nTime, vBlocksMain[i-1].nTimeMax);
120120
}
121121
}

‎src/test/transaction_tests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
220220
fValid = false;
221221
break;
222222
}
223-
COutPoint outpoint(uint256S(vinput[0].get_str()), vinput[1].get_int());
223+
COutPoint outpoint{uint256S(vinput[0].get_str()), uint32_t(vinput[1].get_int())};
224224
mapprevOutScriptPubKeys[outpoint] = ParseScript(vinput[2].get_str());
225225
if (vinput.size() >= 4)
226226
{
@@ -308,7 +308,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
308308
fValid = false;
309309
break;
310310
}
311-
COutPoint outpoint(uint256S(vinput[0].get_str()), vinput[1].get_int());
311+
COutPoint outpoint{uint256S(vinput[0].get_str()), uint32_t(vinput[1].get_int())};
312312
mapprevOutScriptPubKeys[outpoint] = ParseScript(vinput[2].get_str());
313313
if (vinput.size() >= 4)
314314
{

‎test/sanitizer_suppressions/ubsan‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,27 +77,17 @@ implicit-integer-sign-change:prevector.h
7777
implicit-integer-sign-change:script/bitcoinconsensus.cpp
7878
implicit-integer-sign-change:script/interpreter.cpp
7979
implicit-integer-sign-change:serialize.h
80-
implicit-integer-sign-change:test/arith_uint256_tests.cpp
81-
implicit-integer-sign-change:test/coins_tests.cpp
82-
implicit-integer-sign-change:test/pow_tests.cpp
83-
implicit-integer-sign-change:test/prevector_tests.cpp
84-
implicit-integer-sign-change:test/sighash_tests.cpp
85-
implicit-integer-sign-change:test/skiplist_tests.cpp
8680
implicit-integer-sign-change:test/streams_tests.cpp
87-
implicit-integer-sign-change:test/transaction_tests.cpp
8881
implicit-integer-sign-change:txmempool.cpp
8982
implicit-integer-sign-change:zmq/zmqpublishnotifier.cpp
9083
implicit-signed-integer-truncation,implicit-integer-sign-change:chain.h
91-
implicit-signed-integer-truncation,implicit-integer-sign-change:test/skiplist_tests.cpp
9284
implicit-signed-integer-truncation:addrman.cpp
9385
implicit-signed-integer-truncation:addrman.h
9486
implicit-signed-integer-truncation:chain.h
9587
implicit-signed-integer-truncation:crypto/
9688
implicit-signed-integer-truncation:node/miner.cpp
9789
implicit-signed-integer-truncation:net.cpp
9890
implicit-signed-integer-truncation:streams.h
99-
implicit-signed-integer-truncation:test/arith_uint256_tests.cpp
100-
implicit-signed-integer-truncation:test/skiplist_tests.cpp
10191
implicit-signed-integer-truncation:torcontrol.cpp
10292
implicit-unsigned-integer-truncation:crypto/
10393
shift-base:arith_uint256.cpp

0 commit comments

Comments
 (0)