Commit graph

134 commits

Author SHA1 Message Date
fanquake
217c0ce552
test: remove file-wide interpreter.cpp ubsan suppression 2024-03-02 15:26:58 -05:00
fanquake
e3b68b3b83
Merge bitcoin/bitcoin#28875: build: Pass sanitize flags to instrument libsecp256k1 code
cbea49c0d3 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 cbea49c0d3
  dergoegge:
    reACK cbea49c0d3

Tree-SHA512: 801994e75b711d20eaf0d675f378da07d693f4a7de026efd93860f5f1deabed855a83eca3561725263e4fe605fcc5f91eb73c021ec91c831864e6deb575e3885
2024-01-26 11:31:34 +00:00
Hennadii Stepanov
cbea49c0d3
build: Pass sanitize flags to instrument libsecp256k1 code
Also a new UBSan suppression has been added.
2024-01-19 10:08:41 +00:00
Fabian Jahr
6044628543
crypto, hash: replace custom rotl32 with std::rotl 2024-01-05 17:12:38 +01:00
MarcoFalke
fa9dc92c53
test: Add missing CBlockPolicyEstimator::processBlockTx suppression 2023-11-29 10:51:04 +01:00
fanquake
fd30e9688e
test: migrate to some per-symbol ubsan suppressions
Tested on aarch64 using the ASAN CI job. Currently unable to test on
x86_64 due to AppArmor & podman issues.
2023-11-14 15:52:55 +00:00
fanquake
fadad10126
Merge bitcoin/bitcoin#28131: test: Add UBSan -fsanitize=integer suppressions for src/secp256k1 subtree
a7477744c5 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 a7477744c5

Tree-SHA512: 602fa3ad22d3b0f6981a51358677d2347c92c4c9f59626b497af10f7ba828ede37227d8ee717f089bf33bde5efe0854d53acc89bea46f0955e62b7f22c454d05
2023-08-01 09:40:36 +01:00
fanquake
d0c6cc4abe
suppressions: note that 'type:ClassName::MethodName' should be used
Now that the symbolizer is back in play, suppressions can once-again be
targeted to functions, rather than file-wide.
2023-07-25 13:25:55 +01:00
Hennadii Stepanov
a7477744c5
Add UBSan -fsanitize=integer suppressions for src/secp256k1 subtree 2023-07-23 14:56:51 +01:00
MarcoFalke
fa367422ef
fuzz: Bump FuzzedDataProvider.h
From fa8401f9bf/compiler-rt/include/fuzzer/FuzzedDataProvider.h
2023-07-17 09:39:52 +02:00
MarcoFalke
fae55f989e
test: Add implicit-signed-integer-truncation:*/include/c++/ suppression 2023-06-23 12:03:51 +02:00
MarcoFalke
fa76f0d0ef
refactor: Make m_count_with_* in CTxMemPoolEntry int64_t, drop UBSAN supp
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.
2023-06-14 23:15:20 +02:00
Hennadii Stepanov
3ef756a5b5
Remove txmempool implicit-integer-sign-change sanitizer suppressions 2023-06-12 19:48:47 +01:00
Hennadii Stepanov
f03a708c11
doc, test: Document steps to reproduce TSan warning for libdb 2023-05-15 13:28:25 +01:00
MarcoFalke
fa15a9934e
test: Remove unused sanitizer suppressions
* 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.
2023-04-21 09:52:24 +02:00
fanquake
71b3e9b0ad
sanitizers: remove GetRNGState lsan suppression
I am no-longer seeing this. Can anyone recreate the false-positive?
2023-03-30 14:10:35 +01:00
Martin Leitner-Ankerl
5f05b27841 Add xoroshiro128++ PRNG
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>
2023-01-30 18:12:21 -05:00
MarcoFalke
faa835e7e5
Revert "test: Drop no longer needed race:epoll_ctl TSan suppression"
This reverts commit a3f5e54152.
2022-12-30 09:47:52 +01:00
Hennadii Stepanov
a3f5e54152
test: Drop no longer needed race:epoll_ctl TSan suppression 2022-12-27 18:33:34 +00:00
MacroFake
fadb714039
test: Remove unused sanitizer suppressions 2022-11-21 12:13:42 +01:00
Hennadii Stepanov
c8dc0e3eaa
refactor: Inline CTxMemPoolEntry class's functions 2022-11-16 20:17:05 +00:00
Hennadii Stepanov
75bbe594e5
refactor: Move CTxMemPoolEntry class to its own module
This change nukes the policy/fees->mempool circular dependency.

Easy to review using `diff --color-moved=dimmed-zebra`.
2022-11-16 20:16:07 +00:00
James O'Beirne
00eeb31c76 scripted-diff: rename CChainState -> Chainstate
-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>
2022-09-09 11:47:27 -04:00
MarcoFalke
fa07f84e31
Fix signed integer overflow in prioritisetransaction RPC 2022-06-22 09:32:09 +02:00
MarcoFalke
fa097d074b
addrman: Log too low compat value
Also remove uint8_t{} casts from values that are already of the same
type.
2022-02-25 14:16:32 +01:00
MarcoFalke
aaaa4dbab4
Avoid implicit-integer-sign-change in bech32.cpp 2022-02-25 09:43:54 +01:00
MarcoFalke
fae6b26758
test: Remove no longer needed suppressions 2022-02-25 09:41:05 +01:00
MarcoFalke
fa7e1471c0
test: Fix intermittent Tsan issue 2022-02-23 09:34:20 +01:00
Hennadii Stepanov
81738d2881
test: Remove suppression no longer needed with headers-only Boost.Test 2022-02-19 12:40:00 +02:00
fanquake
e0367e84b3
Merge bitcoin/bitcoin#24301: build: header-only Boost
5d399f9f3d build: remove native B2 package (fanquake)
2037a3b6c1 build: header-only Boost (fanquake)
39e66e938f 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 5d399f9f3d
  MarcoFalke:
    approach ACK 5d399f9f3d 📞

Tree-SHA512: e60835ee9c11aa941a64679616da2002d6cd86e464895372fafdd42ad6499d7eb1dde6f0013c60adaeb97bd191198430cb158a7a7417b38080dd7106b28e3ba5
2022-02-14 10:04:17 +00:00
fanquake
39e66e938f
build: use header-only Boost unit test 2022-02-13 20:59:02 +00:00
MarcoFalke
fa6065661a
refactor: Avoid unsigned integer overflow in core_write 2022-02-11 17:21:44 +01:00
MarcoFalke
fa2807ef19
test: Remove unused integer sanitizer suppressions 2022-02-09 21:26:25 +01:00
MarcoFalke
fac62056b5
Fix integer sanitizer suppressions in validation.cpp 2022-02-07 15:20:36 +01:00
MarcoFalke
fad81548fa
test: Avoid testing negative block heights 2022-02-02 15:32:06 +01:00
MarcoFalke
219d728fcb
Merge bitcoin/bitcoin#24219: Fix implicit-integer-sign-change in bloom
fad84a2595 refactor: Fixup uint64_t-cast style in touched line (MarcoFalke)
fa041878de 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 fad84a2595
  theStack:
    Code-review ACK fad84a2595

Tree-SHA512: 7139dd9aa098c41e4af1b6e63dd80e71a92b0a98062d1676b01fe550ffa8e21a5f84a578afa7a536d70dad1b8a5017625e3a9e2dda6f864b452ec77b130ddf2a
2022-02-02 15:00:22 +01:00
MarcoFalke
36f8e99d24
Merge bitcoin/bitcoin#24218: zmq: Fix implicit-integer-sign-change
fa2406a50a 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 fa2406a50a
  PastaPastaPasta:
    utACK fa2406a50a, I have reviewed the code and think it makes sense

Tree-SHA512: 150ebcf3fdc3e0f60b6fd8e5fe638737b01e8a0863296bd545fb5ed17d33ab23b2ff94204996aa7b4617650b7383bd86ed2d2bf46746b410feae449de179a2bd
2022-02-01 10:18:53 +01:00
MarcoFalke
fcac16fff8
Merge bitcoin/bitcoin#24190: test: Fix sanitizer suppresions in streams_tests
faa630aa15 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 196b459920.
  * 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 faa630aa15, I have reviewed the changes and agree it makes sense to merge

Tree-SHA512: 747b9d4676fad6d07f3955668639c93333625e69199ff4c499f01167de3875990d93db85e775a7f5b1b684575dceaec8aa000b4db15525fc47b699bac1c85e3d
2022-02-01 09:42:34 +01:00
MarcoFalke
fa041878de
Fix implicit-integer-sign-change in bloom 2022-01-31 17:23:54 +01:00
MarcoFalke
fa2406a50a
zmq: Fix implicit-integer-sign-change 2022-01-31 16:53:12 +01:00
MarcoFalke
1111d33532
refactor: Make MessageBoxFlags enum underlying type unsigned 2022-01-31 09:27:12 +01:00
MarcoFalke
b25a752dfd
Merge bitcoin/bitcoin#24146: Avoid integer sanitizer warnings in chain.o
fa832103aa 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 fa832103aa
  jonatack:
    ACK fa832103aa

Tree-SHA512: d32a06099c56eed9f69130a3209f989872acc593f849528acd7746ee6caa96688cc32de37e8e59ad5d25dcb8912e341f1a43e50642dadeff6ca7624d0873ad10
2022-01-31 09:23:54 +01:00
MarcoFalke
1245c62fef
Merge bitcoin/bitcoin#24139: Avoid unsigned integer overflow in bitcoin-tx
faa75fa193 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 faa75fa193, I have reviewed the code and it looks OK, I agree it can be merged.
  theStack:
    Code-review ACK faa75fa193

Tree-SHA512: c24436641e5d801341c948b812c7f711d5dff70efdf04af00fd3221f4b81d93f25608dddaa36230ba81ca7ab0d18bdd957095d4561e22621e4d69017934f0a16
2022-01-28 15:26:24 +01:00
MarcoFalke
faa630aa15
test: Fix sanitizer suppresions in streams_tests 2022-01-28 10:23:42 +01:00
MarcoFalke
fa3bdbd37b
test: remove unused sanitizer suppressions 2022-01-25 16:15:53 +01:00
MarcoFalke
fa832103aa
Avoid integer sanitizer warnings in chain.o 2022-01-25 10:49:46 +01:00
MarcoFalke
faa75fa193
Avoid unsigned integer overflow in bitcoin-tx 2022-01-24 16:07:34 +01:00
MarcoFalke
faedb111d2
refactor tests to fix ubsan suppressions 2022-01-20 15:25:23 +01:00
MarcoFalke
92a71f65be
Merge bitcoin/bitcoin#24059: Fix implicit-integer-sign-change in arith_uint256
fa99e108e7 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 fa99e108e7
  shaavan:
    ACK fa99e108e7

Tree-SHA512: f227e2fd22021e39f0445ec041f4a299d13477c23cef0fc06c53fb3313cbe550cec329336224a7e8775d9045b8009423052b394e83d42a1e40772085dfcdd471
2022-01-17 08:48:23 +01:00
MarcoFalke
fa99e108e7
Fix implicit-integer-sign-change in arith_uint256 2022-01-13 15:56:59 +01:00