Age | Commit message (Collapse) | Author |
|
Add ubsan suppressions for integer overflows in the getchaintxstats RPC.
getchainstatstx line "int nTxDiff = pindex->nChainTx - past_block.nChainTx" can
trigger ubsan integer overflows when assumeutxo snapshots are loaded, from
subtracting unsigned values and assigning the result to a signed int.
The overflow behavior probably exists in current code but is hard to trigger
because it would require calling getchainstatstx at the right time with
specific parameters as background blocks are being downloaded. But the overflow
behavior becomes easier to trigger in the upcoming commit removing fake
nChainTx values, so a suppression needs to be added before then for CI to pass.
getchainstatstx should probably be improved separately in another PR to not
need this suppression, and handle edge cases and missing nChainTx values more
carefully.
|
|
|
|
`libsecp256k1` code
cbea49c0d32badb975fbf22d44f8e25cc7972af7 build: Pass sanitize flags to instrument `libsecp256k1` code (Hennadii Stepanov)
Pull request description:
This PR is a revived https://github.com/bitcoin/bitcoin/pull/27991 with an addressed [comment](https://github.com/bitcoin/bitcoin/pull/27991#discussion_r1252148488).
Fixes https://github.com/bitcoin/bitcoin/issues/27990.
Might be tested as follows:
```
$ ./autogen.sh && ./configure --enable-fuzz --with-sanitizers=fuzzer CC=clang-13 CXX=clang++-13
$ make clean > /dev/null && make
$ objdump --disassemble=secp256k1_xonly_pubkey_serialize src/test/fuzz/fuzz | grep __sanitizer_cov
1953bd0:e8 bb c6 05 ff call 9b0290 <__sanitizer_cov_trace_const_cmp8>
1953d32:e8 69 c4 05 ff call 9b01a0 <__sanitizer_cov_trace_pc_indir>
1953d58:e8 43 c4 05 ff call 9b01a0 <__sanitizer_cov_trace_pc_indir>
1953d82:e8 19 c4 05 ff call 9b01a0 <__sanitizer_cov_trace_pc_indir>
```
ACKs for top commit:
fanquake:
ACK cbea49c0d32badb975fbf22d44f8e25cc7972af7
dergoegge:
reACK cbea49c0d32badb975fbf22d44f8e25cc7972af7
Tree-SHA512: 801994e75b711d20eaf0d675f378da07d693f4a7de026efd93860f5f1deabed855a83eca3561725263e4fe605fcc5f91eb73c021ec91c831864e6deb575e3885
|
|
Also a new UBSan suppression has been added.
|
|
|
|
|
|
Tested on aarch64 using the ASAN CI job. Currently unable to test on
x86_64 due to AppArmor & podman issues.
|
|
suppressions for `src/secp256k1` subtree
a7477744c5e1df56d3a1e9ab9fc400bfb0ef6ec3 Add UBSan `-fsanitize=integer` suppressions for `src/secp256k1` subtree (Hennadii Stepanov)
Pull request description:
Required for https://github.com/bitcoin/bitcoin/pull/27991 (see the [comment](https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1611472816)) and for the upcoming CMake-based build system.
ACKs for top commit:
MarcoFalke:
lgtm ACK a7477744c5e1df56d3a1e9ab9fc400bfb0ef6ec3
Tree-SHA512: 602fa3ad22d3b0f6981a51358677d2347c92c4c9f59626b497af10f7ba828ede37227d8ee717f089bf33bde5efe0854d53acc89bea46f0955e62b7f22c454d05
|
|
Now that the symbolizer is back in play, suppressions can once-again be
targeted to functions, rather than file-wide.
|
|
|
|
From https://github.com/llvm/llvm-project/blob/fa8401f9bfe81f4853bf9b67bff42a2cebffc10f/compiler-rt/include/fuzzer/FuzzedDataProvider.h
|
|
|
|
This is a refactor as long as no signed integer overflow appears. In
normal operation and absent bugs, signed integer overflow should never
happen in the touched code paths.
The main benefit of this refactor is to drop the file-wide ubsan
suppression unsigned-integer-overflow:txmempool.cpp.
For now, this only changes the internal private representation and the
publicly returned type remains uint64_t.
|
|
|
|
|
|
* The GCC suppression was fixed in gcc-11, which is available on all LTS
releases of Linux distros.
* The feerate suppression was likely fixed and does not trigger anymore.
If it was to trigger again, the underlying bug should be fixed instead
of suppressing it.
* The bench suppression does not trigger anymore.
Also, add comments to tsan suppressions on how to reproduce.
|
|
I am no-longer seeing this. Can anyone recreate the false-positive?
|
|
Xoroshiro128++ is a fast non-cryptographic random generator.
Reference implementation is available at https://prng.di.unimi.it/
Co-Authored-By: Pieter Wuille <pieter@wuille.net>
|
|
This reverts commit a3f5e541523a843e834df1858e16f89188fe19a2.
|
|
|
|
|
|
|
|
This change nukes the policy/fees->mempool circular dependency.
Easy to review using `diff --color-moved=dimmed-zebra`.
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/CChainState/Chainstate/g' $(git grep -l CChainState ':(exclude)doc/release-notes*')
-END VERIFY SCRIPT-
Co-authored-by: MacroFake <falke.marco@gmail.com>
|
|
|
|
Also remove uint8_t{} casts from values that are already of the same
type.
|
|
|
|
|
|
|
|
|
|
5d399f9f3df513a0400049238f5ef0ef2352d57e build: remove native B2 package (fanquake)
2037a3b6c1222d2802ff7c8463f2bb79ba8b57d8 build: header-only Boost (fanquake)
39e66e938fb688f5400ad94a1b317fcc2a87bc31 build: use header-only Boost unit test (fanquake)
Pull request description:
This PR converts our Boost usage to header only. We switch from using our last remaining Boost lib (unit test), to using it's header-only implementation (see https://www.boost.org/doc/libs/1_78_0/libs/test/doc/html/boost_test/adv_scenarios/single_header_customizations/multiple_translation_units.html).
Also related to #24291.
Guix build:
```bash
```
ACKs for top commit:
hebasto:
re-ACK 5d399f9f3df513a0400049238f5ef0ef2352d57e
MarcoFalke:
approach ACK 5d399f9f3df513a0400049238f5ef0ef2352d57e 📞
Tree-SHA512: e60835ee9c11aa941a64679616da2002d6cd86e464895372fafdd42ad6499d7eb1dde6f0013c60adaeb97bd191198430cb158a7a7417b38080dd7106b28e3ba5
|
|
|
|
|
|
|
|
|
|
|
|
fad84a25956ec081f22aebbda309d168a3dc0004 refactor: Fixup uint64_t-cast style in touched line (MarcoFalke)
fa041878de786f5be74ec74a06ec407c99ca8656 Fix implicit-integer-sign-change in bloom (MarcoFalke)
Pull request description:
Signed values don't really make sense when using `std::vector::operator[]`.
Fix that and remove the suppression.
ACKs for top commit:
PastaPastaPasta:
utACK fad84a25956ec081f22aebbda309d168a3dc0004
theStack:
Code-review ACK fad84a25956ec081f22aebbda309d168a3dc0004
Tree-SHA512: 7139dd9aa098c41e4af1b6e63dd80e71a92b0a98062d1676b01fe550ffa8e21a5f84a578afa7a536d70dad1b8a5017625e3a9e2dda6f864b452ec77b130ddf2a
|
|
fa2406a50a83184d101d1bb3f2b282ae280370ba zmq: Fix implicit-integer-sign-change (MarcoFalke)
Pull request description:
uint256::begin() returns unsigned data, so there is no reason to make it signed.
Fix that and remove the sanitizer suppression.
ACKs for top commit:
hebasto:
ACK fa2406a50a83184d101d1bb3f2b282ae280370ba
PastaPastaPasta:
utACK fa2406a50a83184d101d1bb3f2b282ae280370ba, I have reviewed the code and think it makes sense
Tree-SHA512: 150ebcf3fdc3e0f60b6fd8e5fe638737b01e8a0863296bd545fb5ed17d33ab23b2ff94204996aa7b4617650b7383bd86ed2d2bf46746b410feae449de179a2bd
|
|
faa630aa15bbda0f3b0cf3b6f31cf8fdaeb66975 test: Fix sanitizer suppresions in streams_tests (MarcoFalke)
Pull request description:
Two changes (that also make sense on their own) to remove the file-wide sanitizer suppression:
* `FindByte` no longer takes a `char`, but an `uint8_t`, after commit 196b4599201dbce3e0317e9b98753fa6a244b82d.
* The `key` vector of unsigned chars can be removed and inlined as initializer-list. This avoids a bunch of verbose code like `clear()` and `push_back` of `char`s.
ACKs for top commit:
PastaPastaPasta:
utACK faa630aa15bbda0f3b0cf3b6f31cf8fdaeb66975, I have reviewed the changes and agree it makes sense to merge
Tree-SHA512: 747b9d4676fad6d07f3955668639c93333625e69199ff4c499f01167de3875990d93db85e775a7f5b1b684575dceaec8aa000b4db15525fc47b699bac1c85e3d
|
|
|
|
|
|
|
|
fa832103aaa61e93b78ece9dd68c245a41afa6b6 Avoid integer sanitizer warnings in chain.o (MarcoFalke)
Pull request description:
The two changes make the code more self-documenting and also allow to remove 5 file-wide suppressions for the module
ACKs for top commit:
PastaPastaPasta:
utACK fa832103aaa61e93b78ece9dd68c245a41afa6b6
jonatack:
ACK fa832103aaa61e93b78ece9dd68c245a41afa6b6
Tree-SHA512: d32a06099c56eed9f69130a3209f989872acc593f849528acd7746ee6caa96688cc32de37e8e59ad5d25dcb8912e341f1a43e50642dadeff6ca7624d0873ad10
|
|
faa75fa19335e3e826efa4f2280609a2db34425d Avoid unsigned integer overflow in bitcoin-tx (MarcoFalke)
Pull request description:
While `npos` means "largest unsigned value" and adding `1` to it yields `0`, it may be clearer to just assign `0` to it and only increment otherwise.
This also allows to remove a file-wide suppression for `unsigned-integer-overflow`.
ACKs for top commit:
hebasto:
ACK faa75fa19335e3e826efa4f2280609a2db34425d, I have reviewed the code and it looks OK, I agree it can be merged.
theStack:
Code-review ACK faa75fa19335e3e826efa4f2280609a2db34425d
Tree-SHA512: c24436641e5d801341c948b812c7f711d5dff70efdf04af00fd3221f4b81d93f25608dddaa36230ba81ca7ab0d18bdd957095d4561e22621e4d69017934f0a16
|
|
|
|
|
|
|
|
|
|
|
|
fa99e108e778b5169b3de2ce557af68f1fe0ac0b Fix implicit-integer-sign-change in arith_uint256 (MarcoFalke)
Pull request description:
This refactor doesn't change behaviour, but clarifies that the numbers being dealt with aren't supposed to be negative. This helps when reading the code and allows to remove a sanitizer suppression for the whole file.
ACKs for top commit:
PastaPastaPasta:
utACK fa99e108e778b5169b3de2ce557af68f1fe0ac0b
shaavan:
ACK fa99e108e778b5169b3de2ce557af68f1fe0ac0b
Tree-SHA512: f227e2fd22021e39f0445ec041f4a299d13477c23cef0fc06c53fb3313cbe550cec329336224a7e8775d9045b8009423052b394e83d42a1e40772085dfcdd471
|