Commit Graph

42 Commits

Author SHA1 Message Date
fanquake
546e548755
Merge bitcoin/bitcoin#24153: test: remove unused sanitizer suppressions
fa3bdbd37be81b613e48a10aa77dfd3bcede61e1 test: remove unused sanitizer suppressions (MarcoFalke)

Pull request description:

  Looks like those are not needed (anymore)

ACKs for top commit:
  fanquake:
    ACK fa3bdbd37be81b613e48a10aa77dfd3bcede61e1

Tree-SHA512: 4bedb6363aba8ea7763291ee0cd074e6bfd77e691bb32999c3959393864dc396bacba1eced2b10d9d600b66e8b83b91f7bc6692331dbd113bbaa87e72d11e2e8
2024-09-14 20:24:02 +05:30
Kittywhiskers Van Gogh
6d690ede82
merge bitcoin#23970: Remove pointless and confusing shift in RelayAddress 2024-09-04 16:29:29 +00:00
Kittywhiskers Van Gogh
b75e83b298
merge bitcoin#24218: Fix implicit-integer-sign-change 2024-08-26 15:35:13 +00:00
fanquake
b34514191f
Merge bitcoin/bitcoin#21738: test: Use clang-12 for ASAN, Add missing suppression
fa00bb2c5ca64c7eb9e1846ffedc7829859812ca test: Add missing shift-base:nanobench.h suppression (MarcoFalke)
00004565ccdbaf6bf337e10a5f5ae463bd0ccf9a ci: Use clang-12 for asan task (MarcoFalke)

Pull request description:

ACKs for top commit:
  fanquake:
    ACK fa00bb2c5ca64c7eb9e1846ffedc7829859812ca

Tree-SHA512: fe7cd1ad9f3e73c09f7f84dfb0f276d0cda603c4d591b9338a0914bf1276b0247fd2faee7052f5962c3ae3280e7fa8b72f5b773b84c2a8882a89ed1f8c08256c
2024-07-27 13:04:24 +07:00
MarcoFalke
e5249fb307
Merge #21012: ci: Fuzz with integer sanitizer
faff3991a9be0ea7be31685fb46d94c212c5da34 ci: Fuzz with integer sanitizer (MarcoFalke)

Pull request description:

  Otherwise the suppressions file will go out of sync

ACKs for top commit:
  practicalswift:
    cr ACK faff3991a9be0ea7be31685fb46d94c212c5da34: patch looks correct

Tree-SHA512: 349216d071a2c5ccf24565fe0c52d7a570ec148d515d085616a284f1ab9992ce10ff82eb17962dddbcda765bbd3a9b15e8b25f34bdbed99fc36922d4161d307c
2024-06-20 12:19:21 +07:00
Kittywhiskers Van Gogh
baf8dd65cd
merge bitcoin#24190: Fix sanitizer suppresions in streams_tests 2024-02-28 13:37:34 -06:00
MarcoFalke
7cbf69dd56
Merge bitcoin/bitcoin#24319: refactor: Avoid unsigned integer overflow in core_write
fa6065661a86656a29e89ed1a3529cb7103f5394 refactor: Avoid unsigned integer overflow in core_write (MarcoFalke)

Pull request description:

  Also, I find the new code a bit easier to understand.

ACKs for top commit:
  shaavan:
    Code Review ACK fa6065661a86656a29e89ed1a3529cb7103f5394

Tree-SHA512: cd751e3b4dc97ef525eb8be8d0a49e9629389cb114df18d59a06e05388822af2939078e937f01494e6b317d601743b1a433ba47aa40c4dc602372d1f0fd0dc11
2024-02-28 13:16:39 -06:00
MarcoFalke
002db515dc
Merge bitcoin/bitcoin#24191: refactor: Make MessageBoxFlags enum underlying type unsigned
1111d33532516c16fb2e22660ac2745ce56ad6cd refactor: Make MessageBoxFlags enum underlying type unsigned (MarcoFalke)

Pull request description:

  All values in the enum are unsigned. Also, flags shouldn't be treated as signed types. So clarify the underlying type and remove a sanitizer suppression.

ACKs for top commit:
  hebasto:
    ACK 1111d33532516c16fb2e22660ac2745ce56ad6cd, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 48b16c4a0ace1a1e4d351d6eadadbb1bc42aef7fd82e24e3ea50c62f2c04a552ed21027158d09aa97e630c8c7d732cb150c38065333d7c2accbae46593b7ed9f
2024-02-28 13:16:39 -06:00
MarcoFalke
93027376bf
Merge bitcoin/bitcoin#24059: Fix implicit-integer-sign-change in arith_uint256
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
2024-02-28 13:16:39 -06:00
MarcoFalke
7e57600a22
Merge bitcoin/bitcoin#23992: fuzz: Limit fuzzed time to years 2000-2100
fa7238300c18938cdf627cacfc58d4b81602417f fuzz: Limit fuzzed time to years 2000-2100 (MarcoFalke)

Pull request description:

  It doesn't make sense to fuzz times in the past, as Bitcoin Core will refuse to start in the past.

  Fix that and also remove a sanitizer suppression, which would be hit in net_processing in `ProcessMessage`:

  ```cpp

               if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60)
                   addr.nTime = nNow - 5 * 24 * 60 * 60; // <-- Here
  ```

  This changes the format of fuzz inputs. Previously a time value was (de)serialized as 40 bytes, now it is 32 bytes.

ACKs for top commit:
  mzumsande:
    Code Review ACK fa7238300c18938cdf627cacfc58d4b81602417f

Tree-SHA512: ca6e7233beec2d9ef9fd481d8f1331942a4d2c8fe518b857629bebcc53a4f42ae123b994cf5d359384a0a8022098ff5a9c146600bc2593c6d88734e25bc240ad
2024-02-28 13:16:39 -06:00
fanquake
cd13274076
Merge bitcoin/bitcoin#23626: refactor: Fix implicit-signed-integer-truncation in cuckoocache.h
fa7da227daf8558be14f226c4366583fdc59ba10 refactor: Fix implicit-signed-integer-truncation in cuckoocache.h (MarcoFalke)

Pull request description:

  Using a file-wide suppression for this implicit truncation has several issues:

  * It is file-wide, thus suppressing any other (newly introduced) issues
  * The file doesn't compile with `-Wimplicit-int-conversion`

  Fix both issues by making the truncation explicit.

ACKs for top commit:
  fanquake:
    ACK fa7da227daf8558be14f226c4366583fdc59ba10

Tree-SHA512: bf2076ed94c4e80d0d29ff883080edc7a73144c73d6d3e872ec87966177ee3160f4760fc4c774aaa6fb591f4acee450a24b0f7c82291e0bef96582a6d134046e
2024-02-28 13:16:38 -06:00
fanquake
2a4558b4f7
Merge bitcoin/bitcoin#23553: test: Remove sanitizer suppression implicit-signed-integer-truncation:netaddress.cpp
fae5fec0fec851568a72724000193b2747c30414 test: Remove sanitizer suppression implicit-signed-integer-truncation:netaddress.cpp (MarcoFalke)

Pull request description:

  This reverts commit fa865287e5f35e0a376785834e966dd202d2959e.

  This was fixed in commit efd6f904c78769ad2e93c1f1de43014d284e7561.

ACKs for top commit:
  vasild:
    ACK fae5fec0fec851568a72724000193b2747c30414

Tree-SHA512: 3bebf1babd5c68cbb2506bcab9b8e9ffed8697213cf66190484748741f05c59b847a103be171360f7fd6ddb57dfd86ed34a123f72860b76e533ed46bb53a4852
2024-02-28 13:16:38 -06:00
MarcoFalke
75c877dbba
Merge bitcoin/bitcoin#22584: test: Add temporary sanitizer suppression implicit-signed-integer-truncation:netaddress.cpp
fa865287e5f35e0a376785834e966dd202d2959e test: Add temporary sanitizer suppression implicit-signed-integer-truncation:netaddress.cpp (MarcoFalke)

Pull request description:

  This is required to unbreak the fuzzers while a fix is being worked on.

  https://cirrus-ci.com/task/4787303177519104?logs=ci#L3020

  ```
  netaddress.cpp:1190:18: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'uint8_t' (aka 'unsigned char') changed the value to 255 (8-bit, unsigned)

ACKs for top commit:
  practicalswift:
    cr ACK fa865287e5f35e0a376785834e966dd202d2959e
  tryphe:
    untested ACK fa865287e5f35e0a376785834e966dd202d2959e
  lsilva01:
    ACK fa865287e5

Tree-SHA512: 4a54ec68c014c7a4c9ab268c3a04321db5eb9b2857646b41406d8d4908a3d349848b4549e80aea6afd9a0c3639522a48fe578527139519b12439eae9f0c4c46c
2024-02-28 13:16:38 -06:00
MarcoFalke
c342ce95b8
Merge bitcoin/bitcoin#22146: Reject invalid coin height and output index when loading assumeutxo
fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4 Reject invalid coin height and output index when loading assumeutxo (MarcoFalke)

Pull request description:

  It should be impossible to have a coin at a height higher than the height of the snapshot block, so reject those early to avoid integer wraparounds and hash collisions later on.

  Same for the outpoint index.

  Both issues were found by fuzzing:

  * The height issue by OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34793
  * The outpoint issue by my fuzz server: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34793#c2

ACKs for top commit:
  practicalswift:
    cr ACK fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4: patch looks correct
  jamesob:
    crACK fa9ebedec3
  theStack:
    Code review ACK fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4
  benthecarman:
    crACK fa9ebedec3f982bb5bb459ea33d74c94d9b5cec4

Tree-SHA512: dae7caee4b3862b23ebdf2acb7edec4baf75b0dbf1409b370b1a73aa6b632b317ebfac596dcbaf4edfb1301b513f45465ea75328962460f35e2af0d7e547c9ac
2024-02-28 13:16:38 -06:00
MarcoFalke
0557c32264
Merge bitcoin/bitcoin#22202: test: Add temporary coinstats suppressions
faca40ec68a25180f90a5b9ef017f931354d5bc6 test: Add temporary coinstats suppressions (MarcoFalke)

Pull request description:

  Needed for my fuzzer to continue to run

ACKs for top commit:
  practicalswift:
    cr ACK faca40ec68a25180f90a5b9ef017f931354d5bc6: suppression looks necessary (temporarily)

Tree-SHA512: 5bdff9a24a60546cfe31e775fa2aa5e238aefda2ed2604bef18c82b1b80c51ca3cbe058d6c7988fa75305258b70076036a3e430b9b7de13a111309fa7a66745b
2024-02-28 13:16:37 -06:00
MarcoFalke
3450fa5fe4
Merge bitcoin/bitcoin#21846: fuzz: Add -fsanitize=integer suppression needed for RPC fuzzer (generateblock)
575792e6ffe23c8236a1f8431f6be445e448809b fuzz: Add -fsanitize=integer suppression needed for RPC fuzzer (practicalswift)

Pull request description:

  Add `-fsanitize=integer` suppression needed for RPC fuzzer (`generateblock`).

  Context: https://github.com/bitcoin-core/qa-assets/pull/59/checks?check_run_id=2494624259

  ```
  miner.cpp:130:21: runtime error: implicit conversion from type 'int64_t' (aka 'long') of value 244763573890 (64-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4245405314 (32-bit, unsigned)
      #0 0x56143974eaf3 in BlockAssembler::CreateNewBlock(CScript const&) miner.cpp:130:21
      #1 0x56143993690d in generateblock()::$_4::operator()(RPCHelpMan const&, JSONRPCRequest const&) const rpc/mining.cpp:370:127
  ```

ACKs for top commit:
  practicalswift:
    > review ACK [575792e](575792e6ff), but this function shouldn't be called by the rpc fuzzer, at least not without sanitizing num_blocks
  MarcoFalke:
    review ACK 575792e6ffe23c8236a1f8431f6be445e448809b

Tree-SHA512: c2133d1064bf17df0e7749ef4a0f7664b5c8082040491a1035d39f0c6e5d96997b347ef2354411e285c7f1f973e34515f1c3c88eb3de60fab64ca4d2adf6dd74
2024-02-28 13:16:35 -06:00
Kittywhiskers Van Gogh
eab94ac07b
merge bitcoin#23859: Add missing suppressions for crypto_diff_fuzz_chacha20.cpp 2024-02-27 10:06:20 -06:00
Kittywhiskers Van Gogh
0e8d4a1a95
partial bitcoin#21798: Create a block template in tx_pool targets
excludes:
- fa03d0acd (except ubsan suppression entry)
2024-02-27 10:06:19 -06:00
Kittywhiskers Van Gogh
ad71db2dcc
merge bitcoin#21604: Document why no symbol names can be used for suppressions 2024-02-27 10:06:19 -06:00
Kittywhiskers Van Gogh
c116d8405a
merge bitcoin#21599: Replace file level integer overflow suppression with function level suppression 2024-02-27 10:06:19 -06:00
Kittywhiskers Van Gogh
8a0dc8cfa1
merge bitcoin#21586: Add missing suppression for signed-integer-overflow:txmempool.cpp 2024-02-27 10:06:19 -06:00
Kittywhiskers Van Gogh
53cf0d5cea
merge bitcoin#21000: Add UBSan suppressions needed for fuzz tests to not warn under -fsanitize=integer 2024-02-27 10:06:18 -06:00
MarcoFalke
592c4e30e7
Merge bitcoin/bitcoin#23795: refactor: Remove implicit-integer-sign-change suppressions in validation
fadd73037e266edb844f0972e82e9213171ef214 refactor: Remove implicit-integer-sign-change suppressions in validation.cpp (MarcoFalke)

Pull request description:

  A file-wide suppression is problematic because it will wave through future violations, potentially bugs.

  Fix that by using per-statement casts.

ACKs for top commit:
  shaavan:
    ACK fadd73037e266edb844f0972e82e9213171ef214
  theStack:
    Code-review ACK fadd73037e266edb844f0972e82e9213171ef214

Tree-SHA512: a8a05613be35382b92d7970f958a4e8f4332432056eaa9d72f6719495134b93aaaeea692899d9035654d0e0cf56bcd759671eeeacfd0535582c0ea048ab58a56
2023-12-26 22:26:20 -06:00
fanquake
5a4406ef98 Merge bitcoin/bitcoin#26153: Reduce wasted pseudorandom bytes in ChaCha20 + various improvements
511aa4f1c7508f15cab8d7e58007900ad6fd3d5d Add unit test for ChaCha20's new caching (Pieter Wuille)
fb243d25f754da8f01793b41e2d225b917f3e5d7 Improve test vectors for ChaCha20 (Pieter Wuille)
93aee8bbdad808b7009279b67470d496cc26b936 Inline ChaCha20 32-byte specific constants (Pieter Wuille)
62ec713961ade7b58e90c905395558a41e8a59f0 Only support 32-byte keys in ChaCha20{,Aligned} (Pieter Wuille)
f21994a02e1cc46d41995581b54222abc655be93 Use ChaCha20Aligned in MuHash3072 code (Pieter Wuille)
5d16f757639e2cc6e81db6e07bc1d5dd74abca6c Use ChaCha20 caching in FastRandomContext (Pieter Wuille)
38eaece67b1bc37b2f502348c5d7537480a34346 Add fuzz test for testing that ChaCha20 works as a stream (Pieter Wuille)
5f05b27841af0bed1b6e7de5f46ffe33e5919e4d Add xoroshiro128++ PRNG (Martin Leitner-Ankerl)
12ff72476ac0dbf8add736ad3fb5fad2eeab156c Make unrestricted ChaCha20 cipher not waste keystream bytes (Pieter Wuille)
6babf402130a8f3ef3058594750aeaa50b8f5044 Rename ChaCha20::Seek -> Seek64 to clarify multiple of 64 (Pieter Wuille)
e37bcaa0a6dbb334ab6e817efcb609ccee6edc39 Split ChaCha20 into aligned/unaligned variants (Pieter Wuille)

Pull request description:

  This is an alternative to #25354 (by my benchmarking, somewhat faster), subsumes #25712, and adds additional test vectors.

  It separates the multiple-of-64-bytes-only "core" logic (which becomes simpler) from a layer around which performs caching/slicing to support arbitrary byte amounts. Both have their uses (in particular, the MuHash3072 code can benefit from multiple-of-64-bytes assumptions), plus the separation results in more readable code. Also, since FastRandomContext effectively had its own (more naive) caching on top of ChaCha20, that can be dropped in favor of ChaCha20's new built-in caching.

  I thought about rebasing #25712 on top of this, but the changes before are fairly extensive, so redid it instead.

ACKs for top commit:
  ajtowns:
    ut reACK 511aa4f1c7508f15cab8d7e58007900ad6fd3d5d
  dhruv:
    tACK crACK 511aa4f1c7

Tree-SHA512: 3aa80971322a93e780c75a8d35bd39da3a9ea570fbae4491eaf0c45242f5f670a24a592c50ad870d5fd09b9f88ec06e274e8aa3cefd9561d623c63f7198cf2c7
2023-11-19 10:20:12 -06:00
Kittywhiskers Van Gogh
8c742e9d15 merge bitcoin#23756: Fix implicit integer sign changes in strencodings 2023-09-04 20:50:27 -05:00
Wladimir J. van der Laan
069624dd62 Merge #20844: test: Add sanitizer suppressions for AMD EPYC CPUs
fa6c114ae604571435e8c4d25906a8b6d5b9984c test: Add sanitizer suppressions for AMD EPYC CPUs (MarcoFalke)

Pull request description:

  Currently the ci system only runs on intel cpus (and some arm devices), but it won't run on CPUs `Using the 'shani(1way,2way)' SHA256 implementation` (excerpt from debug log).

  For reference, google cloud CPUs (which is what Cirrus CI uses) print `Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation`

  The traceback I got:

  ```
  crypto/sha256_shani.cpp:87:18: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_t' (aka 'unsigned long')
      #0 0x55c0000e95ec in sha256_shani::Transform(unsigned int*, unsigned char const*, unsigned long) /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256_shani.cpp:87:18
      #1 0x55bfffb926f8 in (anonymous namespace)::SelfTest() /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256.cpp:517:9
      #2 0x55bfffb906ed in SHA256AutoDetect[abi:cxx11]() /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256.cpp:626:5
      #3 0x55bfff87ab97 in BasicTestingSetup::BasicTestingSetup(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<char const*, std::allocator<char const*> > const&) /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/util/setup_common.cpp:104:5
      #4 0x55bffe885877 in main /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/qt/test/test_main.cpp:52:27
      #5 0x7f20c3bf60b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
      #6 0x55bffe7a5f6d in _start (/root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/qt/test/test_bitcoin-qt+0x1d00f6d)

  SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow crypto/sha256_shani.cpp:87:18 in

ACKs for top commit:
  laanwj:
    Anyhow ACK fa6c114ae604571435e8c4d25906a8b6d5b9984c

Tree-SHA512: 968a1d28eedec58c337b1323862f583cb1bcd78c5f03396940b9ab53ded12f8c6652877909aba05ee5586532137418fd817ff979bd7bef6e07856094f9d7f9b1
2023-08-29 21:40:46 -05:00
Konstantin Akimov
028281ae15 fix: add supression for integer conversion in crc32 library 2023-06-07 01:50:18 -05:00
MarcoFalke
e38953f0cb Merge #20379: tests: Remove no longer needed UBSan suppression (float divide-by-zero in validation.cpp)
0ccb3addf68067200892963521a92713c4667a63 tests: Remove no longer needed UBSan suppression (float-divide-by-zero in validation.cpp) (practicalswift)

Pull request description:

  Remove no longer needed UBSan suppression.

  The float divide-by-zero in `validation.cpp` was fixed by instagibbs in ec30a79f1c430cc7fbda37e5d747b0b31b262fa5 (#15283).

ACKs for top commit:
  MarcoFalke:
    ACK 0ccb3addf68067200892963521a92713c4667a63

Tree-SHA512: 89a4f4b7371fa5725d9f801cee7ebbd17523f66017c9acfa813657dcb8d837f42209eff44ce9e5d48296a630bab9599d75f10024a0c7da7defb228f4eae3392a
2023-04-16 23:40:59 +03:00
MarcoFalke
f8d32e7e1b Merge #20344: wallet: fix scanning progress calculation for single block range
5e146022daa4336de94447e5b8e5418296286927 wallet: fix scanning progress calculation for single block range (Sebastian Falbesoner)

Pull request description:

  If the blockchain is rescanned for a single block (i.e. start and stop hashes are equal, and with that also the estimated start/stop verification progress values) the progress calculation could lead to a NaN value caused by a division by zero (0.0/0.0), resulting in an invalid JSON result for the `getwalletinfo` RPC.  This PR fixes this behaviour by setting the progress to zero in that special case. Fixes #20297.

  The behaviour can easily be reproduced by continuously running single block rescans in an endless loop, e.g. via
  ```bash
  #!/bin/bash
  while true
  do
      bitcoin-cli rescanblockchain $(bitcoin-cli getblockcount)
  done
  ```

  and at the same time perform some `getwalletinfo` RPCs.

  On the master branch, this leads to frequent invalid responses (tested on mainchain):
  ```
  $ bitcoin-cli getwalletinfo
  error: couldn't parse reply from server
  $ curl --user `cat ~/.bitcoin/.cookie` --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getwalletinfo", "params": []}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  {"result":{"walletname":"","walletversion":169900,"format":"bdb","balance":0.00000000,"unconfirmed_balance":0.00000000,"immature_balance":0.00000000,"txcount":0,"keypoololdest":1603677276,"keypoolsize":1000,"hdseedid":"3196e33ecb47c7130e6ca60f2f895f9259860dca","keypoolsize_hd_internal":1000,"paytxfee":0.00000000,"private_keys_enabled":true,"avoid_reuse":false,"scanning":{"duration":0,"progress":},"descriptors":false},"error":null,"id":"curltest"}
  ```
  (note that missing value for "progress" in the JSON result).

  On the PR branch, the behaviour doesn't occur anymore.

ACKs for top commit:
  MarcoFalke:
    review ACK 5e146022daa4336de94447e5b8e5418296286927
  promag:
    Core review ACK 5e146022daa4336de94447e5b8e5418296286927.

Tree-SHA512: f0e6aad5a6cd08b36c5fe820fff0ef26663229b39169a4dbe757f3c795a41cf5c69c9dc90efe7515675ae1059307f8971123781a0514d10704123a6f28b125ab
2023-04-16 23:40:59 +03:00
W. J. van der Laan
6ed514647f
Merge bitcoin/bitcoin#21905: net: initialize nMessageSize to uint32_t max
9c891b64ffd14bc8216dbd5eb60816043af265b6 net: initialize nMessageSize to max uint32_t instead of -1 (eugene)

Pull request description:

  nMessageSize is uint32_t and is set to -1. This will warn with `-fsanitize=implicit-integer-sign-change` when V1TransportDeserializer calls into the ctor.  This pull initializes nMessageSize to `numeric_limits<uint32_t>::max()` instead and removes the ubsan suppression.

ACKs for top commit:
  laanwj:
    Code review ACK 9c891b64ffd14bc8216dbd5eb60816043af265b6
  promag:
    Code review ACK 9c891b64ffd14bc8216dbd5eb60816043af265b6.

Tree-SHA512: f05173d9553a01d207a5a7f8ff113d9e11354c50b494a67d44d3931c151581599a9da4e28f40edd113f4698ea9115e6092b2a5b7329c841426726772076c1493
2023-04-14 23:34:14 -05:00
fanquake
106ca023a0 Merge #19630: Cleanup fee estimation code
a3abeec33a6ae903e514c7a7b6f587b7c17288a0 policy/fees: remove a floating-point division by zero (Antoine Poinsot)
c36869bbf6a38626833b4aea53be024c48ede475 policy/fees: unify some duplicated for loops (Antoine Poinsot)
569d92a4d2924a1f6d50775980b591552f6372e7 policy/fees: small readability improvements (Antoine Poinsot)
5b8cb35621891b681f9b49a9de5f6d8da4ccdecc policy/fee: remove requireGreater parameter in EstimateMedianVal() (Antoine Poinsot)
dba8196b447b6a85be66890db70928100e867d8b policy/fees: correct decay explanation comments (Antoine Poinsot)

Pull request description:

  This (*does not* change behaviour and) cleans up a bit of unused code in `CBlockPolicyEstimator` and friends, and slightly improves readability of the rest (comment correction etc.). The last commit is a small reformatting one which I could not resist but am happy to remove at will.

ACKs for top commit:
  jnewbery:
    utACK a3abeec33a6ae903e514c7a7b6f587b7c17288a0
  MarcoFalke:
    ACK a3abeec33a6ae903e514c7a7b6f587b7c17288a0 💹
  ariard:
    Code Review ACK a3abeec.

Tree-SHA512: b7620bcd23a2ffa8f7ed859467868fc0f6488279e3ee634f6d408872cb866ad086a037e8ace76599a05b7e9c07768adf5016b0ae782d153196b9c030db4c34a5
2023-01-01 20:16:57 -06:00
Kittywhiskers Van Gogh
9e9e170bd6 merge bitcoin#20736: Replace boost::variant with std::variant for RPCArg.m_fallback 2022-10-20 16:08:45 -05:00
MarcoFalke
7d687f916d Partial Merge #17517: ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le
fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 ci: Remove unparseable lines from supp file for old xenial clang tsan (MarcoFalke)
fa1bfc476c9208a4c412c8ca74d05f52bb47766f ci: ubsan report_error_type=1 and add suppressions (MarcoFalke)
fa69cef13e5aab8264339eb3d50a9e89d59efd87 test: Print stderr when subprocess fails (MarcoFalke)
2222c305866a77065ab5be24c1c252bae252bb59 test: Use char instead of unsigned char (MarcoFalke)
faa8023ce9a47b282e1fac3ca8b3a7bb0042935a ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le (MarcoFalke)

Pull request description:

  Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running the ci system on ppc64le

ACKs for top commit:
  practicalswift:
    ACK fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 assuming Travis is happy -- diff looks correct :)

Tree-SHA512: f4f26232d3a0ef38da245869340f723d279a3db9823befbc735fb5a00024dae041c7306d7ae55d2488e6f86aa96cdea155b007aefb561fba505141e8dbc717dc
2022-10-17 15:41:14 -05:00
Kittywhiskers Van Gogh
887b4324d3 trivial: revert dashification of source and header filenames 2022-08-09 14:16:29 +05:30
fanquake
1ce15a1ac6
Merge #20430: sanitizers: Add suppression for unsigned-integer-overflow in libstdc++
0f020cdf0a8c3c88499c96522470e2e5e79f27fa sanitizers: Add suppression for unsigned-integer-overflow in libstdc++ basic_string.tcc (Jonas Schnelli)

Pull request description:

  Reported here: https://bitcoinbuilds.org/logs/e35cd579-0f0f-47e4-b49a-4ceba8ff9830.log
  Issue: https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/basic_string.tcc#L1271

ACKs for top commit:
  MarcoFalke:
    cr ACK 0f020cdf0a8c3c88499c96522470e2e5e79f27fa
  practicalswift:
    cr ACK 0f020cdf0a8c3c88499c96522470e2e5e79f27fa

Tree-SHA512: e304259a1eed878263bd715b4d16c57f8974264c23ccd6799f85e8141b2eb0b5c6468a6452ffbc7334f57c1957b6e43bb248760b3c0718d93f092d57764d0a8f
2022-06-18 22:14:04 -07:00
MarcoFalke
abee458dee Merge #17209: tests: Remove no longer needed UBSan suppressions (issues fixed). Add documentation.
0616138a0797cf68ad869906c36cf0767e20b313 tests: Remove no longer needed UBSan suppressions (issues fixed). Add documentation. (practicalswift)

Pull request description:

  Remove no longer needed UBSan suppressions (issues fixed). Add documentation.

  This PR is the CI-only subset of #17208 (which touches code).

  From a fuzzing perspective it would be really nice to be able to run UBSan with as few suppressions as possible :)

Top commit has no ACKs.

Tree-SHA512: a926ab3e80e12a805af110fbff470cdc61ef4db536919a5b8896ea8b70f761114a52d9b1c0f48b11c1d48338351bf2e003e01ce60c613612f26ba298dcc29cd9
2022-05-17 01:20:21 -04:00
UdjinM6
89f606a927
trivial: add some missing dashifications (#4772) 2022-04-19 09:09:42 +03:00
Kittywhiskers Van Gogh
aab4207f74 ci: ensure that ubsan suppression list is passed, add bls.h to it 2022-03-17 19:23:55 +05:30
Wladimir J. van der Laan
56a75722c9 Merge #17708: prevector: avoid misaligned member accesses
5f26855f109af53a336d5f98ed0ae584e7a31f84 test: Remove ubsan alignment suppressions (Wladimir J. van der Laan)
9d933ef9191417b4b7d29eaa3c3a571f814acc8e prevector: avoid misaligned member accesses (Anthony Towns)

Pull request description:

  Ensure prevector data is appropriately aligned. Earlier discussion in #17530.

  **Edit laanwj**: In contrast to #17530, it does this without increase in size of any of the coin cache data structures (x86_64, clang)

  | Struct        | (size,align) before           | (size,align) after  |
  | ------------- | ------------- | ------- |
  | Coin | 48, 8        |     48, 8   |
  | CCoinsCacheEntry | 56, 8    |   56, 8  |
  | CScript | 32, 1       |      32, 8  |

ACKs for top commit:
  laanwj:
    ACK 5f26855f109af53a336d5f98ed0ae584e7a31f84
  practicalswift:
    ACK 5f26855f109af53a336d5f98ed0ae584e7a31f84
  jonatack:
    ACK 5f26855f109af53a336d5f98ed0ae584e7a31f84

Tree-SHA512: 98d112d6856f683d5b212410b73f3071d2994f1efb046a2418a35890aa1cf1aa7c96a960fc2e963fa15241e861093c1ea41951cf5b4b5431f88345eb1dd0a98a
2021-09-23 17:30:17 -03:00
MarcoFalke
aefeef25b8 Merge #14357: streams: Fix broken streams_vector_reader test. Remove unused seek(size_t).
4f4993fe2a Remove UBSan suppression (practicalswift)
958e1a307e streams: Remove unused seek(size_t) (practicalswift)

Pull request description:

  Fix broken `streams_vector_reader` test. Remove unused `seek(size_t)`.

  Before this change the test `streams_vector_reader` triggered an unintended unsigned integer wraparound. It tried so seek using a negative value in `reader.seek(-6)`.

  Changes in this PR:
  * Fix broken `VectorReader::seek(size_t)` test case
  * Remove unused `seek(size_t)`

Tree-SHA512: 6c6affd680626363eef9e496748f2f86a522325abab9d6b13161f41125cdc29ceb36c2c1509c90b8ff108d606df7629e55e094cc2b6253b05a892b81ce176b71
2021-09-07 12:26:47 -04:00
Wladimir J. van der Laan
7ac0213b1e Merge #14242: Avoid triggering undefined behaviour (std::memset(nullptr, 0, 0)) if an invalid string is passed to DecodeSecret(...)
d855e4cac8303ad4e34ac31cfa7634286589ce99 Avoid triggering undefined behaviour (std::memset(nullptr, 0, 0)) if an invalid string is passed to DecodeSecret(...) (practicalswift)

Pull request description:

  Avoid triggering undefined behaviour (`std::memset(nullptr, 0, 0)`) if an invalid string is passed to `DecodeSecret(...)`.

  Background reading: [memcpy (and friends) with NULL pointers](https://www.imperialviolet.org/2016/06/26/nonnull.html)

  Steps to reproduce:

  ```
  ./configure --with-sanitizers=undefined && make check && ./test/functional/test_runner.py
  ```

Tree-SHA512: b8325ced4f724d9c03065e0747af56b1f297a90d9fb09a24d46c3231a90dce3df6299f2c41f863b5cec18eaeded7b46ee4b93d9a52adc2541eb4c44d2c0965d9
2021-08-18 22:38:11 -03:00
MarcoFalke
8e118581b2 Merge #14764: travis: Run thread sanitizer on unit tests
fa7d36b8e7 test: Move UBSAN suppressions to test/sanitizer_suppressions/ubsan (MarcoFalke)
fa36d4e456 travis: --disable-hardening for xenial thread sanitizer (MarcoFalke)
89bf196c88 travis: Run thread sanitizer (MarcoFalke)

Pull request description:

  On unit tests only for now. Disabled for the gui unit tests and all functional tests.

Tree-SHA512: 56f7d3b44e7cb68c76a2dc5abd85658955b1c2188932e988667c5a1cbcdd6be995d37bb949d62c6eb08a4aebfc43ff0370b7da1719d4e4f322a3495c1941a5e0
2021-08-18 13:56:42 -05:00