Introduce `g_fuzzing` global for fuzzing checks #31093

dergoegge commented at 11:32 AM on October 15, 2024: member

This PR introduces a global g_fuzzing that indicates if we are fuzzing.

If g_fuzzing is true then:

Assume checks are enabled
Special fuzzing paths are taken (e.g. pow check is reduced to one bit)

Closes #30950 #31057

DrahtBot commented at 11:32 AM on October 15, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	brunoerg, marcofleon, maflcko

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

dergoegge commented at 11:32 AM on October 15, 2024: member

cc @maflcko @ryanofsky

DrahtBot added the label CI failed on Oct 15, 2024

DrahtBot commented at 12:19 PM on October 15, 2024: contributor

🚧 At least one of the CI tasks failed. <sub>Debug: https://github.com/bitcoin/bitcoin/runs/31551484411</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

dergoegge force-pushed on Oct 15, 2024

maflcko commented at 12:21 PM on October 15, 2024: member

Looks like this fails, because it can't be used via Assume in a consteval context? However, I had difficulties anyway using Assert/Assume in those contexts, so I wonder if they shouldn't just use assert directly?

in src/util/check.h:56 in 80ee9f092d outdated

  52 | @@ -51,6 +53,7 @@ constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] con
  53 |              assertion_fail(file, line, func, assertion);
  54 |          }
  55 |      }
  56 | +    if (g_fuzzing && !val) abort();

maflcko commented at 12:24 PM on October 15, 2024:

Any reason to use a different branch here? Removing the constexpr above and adding ||g_fuzzing to the if should achieve the same? There shouldn't be a downside, because the g_fuzzing will always be checked anyway?

dergoegge commented at 1:13 PM on October 15, 2024:

Will change on next push

maflcko commented at 3:27 PM on October 23, 2024:

Are you still working on this?

dergoegge commented at 10:00 AM on October 24, 2024:

Done!

maflcko commented at 10:16 AM on October 24, 2024:

I don't see it. I still see else if (g_fuzzing && !val) abort();. Thus, I don't see any ||g_fuzzing. Also, the std::abort wasn't removed nor the "duplicate" val check.

maflcko commented at 10:17 AM on October 24, 2024:

If you want to keep them for some reason, it would be good to explain, and then also add the missing includes.

dergoegge commented at 10:31 AM on October 24, 2024:

I misunderstood what you meant, slow start today😴 should be fixed now!

dergoegge force-pushed on Oct 15, 2024

dergoegge commented at 10:04 AM on October 16, 2024: member

The CI failure is a little puzzling:

[13:26:14.486] Run bitset with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-runs=1', PosixPath('/ci_container_base/ci/scratch/qa-assets/fuzz_corpora/bitset')]INFO: Running with entropic power schedule (0xFF, 100).
[13:26:14.486] INFO: Seed: 1575764882
[13:26:14.486] INFO: Loaded 1 modules   (622699 inline 8-bit counters): 622699 [0x557b75e3b468, 0x557b75ed34d3), 
[13:26:14.486] INFO: Loaded 1 PC tables (622699 PCs): 622699 [0x557b75ed34d8,0x557b76853b88), 
[13:26:14.486] INFO:     1372 files found in /ci_container_base/ci/scratch/qa-assets/fuzz_corpora/bitset
[13:26:14.486] INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1048576 bytes
[13:26:14.486] INFO: seed corpus: files: 1372 min: 1b max: 4194338b total: 10778447b rss: 110Mb
[13:26:14.486] /ci_container_base/src/util/bitset.h:369:34: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'value_type' (aka 'unsigned short') changed the value to 65535 (16-bit, unsigned)
[13:26:14.486]     [#0](/bitcoin-bitcoin/0/) 0x557b72af9a65 in bitset_detail::MultiIntBitSet<unsigned short, 2u>::Fill(unsigned int) ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/./src/util/bitset.h:369:34
[13:26:14.486]     [#1](/bitcoin-bitcoin/1/) 0x557b72a604a3 in void (anonymous namespace)::TestType<bitset_detail::MultiIntBitSet<unsigned short, 2u>>(std::span<unsigned char const, 18446744073709551615ul>) ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/./src/test/fuzz/bitset.cpp:182:30
[13:26:14.486]     [#2](/bitcoin-bitcoin/2/) 0x557b72a4763c in bitset_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/./src/test/fuzz/bitset.cpp:291:9
[13:26:14.486]     [#3](/bitcoin-bitcoin/3/) 0x557b731e929d in std::function<void (std::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::span<unsigned char const, 18446744073709551615ul>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
[13:26:14.486]     [#4](/bitcoin-bitcoin/4/) 0x557b731e929d in LLVMFuzzerTestOneInput ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/util/./src/test/fuzz/fuzz.cpp:211:5
[13:26:14.486]     [#5](/bitcoin-bitcoin/5/) 0x557b728a24f4 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c494f4) (BuildId: 1cb0f5769fd52f5e14ffeabc7e79e3bb0eee7ba7)
[13:26:14.486]     [#6](/bitcoin-bitcoin/6/) 0x557b728a1be9 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c48be9) (BuildId: 1cb0f5769fd52f5e14ffeabc7e79e3bb0eee7ba7)
[13:26:14.486]     [#7](/bitcoin-bitcoin/7/) 0x557b728a3806 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c4a806) (BuildId: 1cb0f5769fd52f5e14ffeabc7e79e3bb0eee7ba7)
[13:26:14.486]     [#8](/bitcoin-bitcoin/8/) 0x557b728a3d17 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c4ad17) (BuildId: 1cb0f5769fd52f5e14ffeabc7e79e3bb0eee7ba7)
[13:26:14.486]     [#9](/bitcoin-bitcoin/9/) 0x557b7289120f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c3820f) (BuildId: 1cb0f5769fd52f5e14ffeabc7e79e3bb0eee7ba7)
[13:26:14.486]     [#10](/bitcoin-bitcoin/10/) 0x557b728bb896 in main (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c62896) (BuildId: 1cb0f5769fd52f5e14ffeabc7e79e3bb0eee7ba7)
[13:26:14.486]     [#11](/bitcoin-bitcoin/11/) 0x7ff169e111c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
[13:26:14.486]     [#12](/bitcoin-bitcoin/12/) 0x7ff169e1128a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
[13:26:14.486]     [#13](/bitcoin-bitcoin/13/) 0x557b728861f4 in _start (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1c2d1f4) (BuildId: 1cb0f5769fd52f5e14ffeabc7e79e3bb0eee7ba7)
[13:26:14.486] 
[13:26:14.486] SUMMARY: UndefinedBehaviorSanitizer: implicit-signed-integer-truncation /ci_container_base/src/util/bitset.h:369:34 
[13:26:14.486] MS: 0 ; base unit: 0000000000000000000000000000000000000000
[13:26:14.486] 0x9,0x25,0x3a,0x9b,0xff,
[13:26:14.486] \011%:\233\377
[13:26:14.486] artifact_prefix='./'; Test unit written to ./crash-29d7e95e22f18f0e6312373978433d7d97b91a95
[13:26:14.486] Base64: CSU6m/8=

in src/pow.cpp:142 in e312d7596c outdated

 138 | @@ -138,11 +139,8 @@ bool PermittedDifficultyTransition(const Consensus::Params& params, int64_t heig
 139 |  // the most signficant bit of the last byte of the hash is set.
 140 |  bool CheckProofOfWork(uint256 hash, unsigned int nBits, const Consensus::Params& params)
 141 |  {
 142 | -#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
 143 | -    return (hash.data()[31] & 0x80) == 0;
 144 | -#else
 145 | +    if(g_fuzzing) return (hash.data()[31] & 0x80) == 0;

maflcko commented at 10:57 AM on October 16, 2024:

clang-format for new code :sweat_smile:

dergoegge force-pushed on Oct 16, 2024

dergoegge force-pushed on Oct 17, 2024

DrahtBot removed the label CI failed on Oct 17, 2024

laanwj added the label Tests on Oct 20, 2024

brunoerg commented at 10:44 AM on October 23, 2024: contributor

Concept ACK

dergoegge force-pushed on Oct 24, 2024

dergoegge commented at 1:46 PM on October 24, 2024: member

Looks like switching from Assume to assert in BitSet has a noticeable impact on the Linearize* bench tests: https://corecheck.dev/bitcoin/bitcoin/pulls/31093

maflcko commented at 2:33 PM on October 24, 2024: member

You can just turn the Assume to assert at compile-time, eagerly, if possible (decided by the compiler) at no downside, at the benefit that it will be evaluated and checked at compile-time:

<details><summary>possible diff</summary>

diff --git a/src/util/bitset.h b/src/util/bitset.h
index 7db6f2424f..6f9e808c37 100644
--- a/src/util/bitset.h
+++ b/src/util/bitset.h
@@ -101,7 +101,7 @@ class IntBitSet
         /** Progress to the next 1 bit (only if != IteratorEnd). */
         constexpr Iterator& operator++() noexcept
         {
-            assert(m_val != 0);
+            Assume(m_val != 0);
             m_val &= m_val - I{1U};
             if (m_val != 0) m_pos = std::countr_zero(m_val);
             return *this;
@@ -109,7 +109,7 @@ class IntBitSet
         /** Get the current bit position (only if != IteratorEnd). */
         constexpr unsigned operator*() const noexcept
         {
-            assert(m_val != 0);
+            Assume(m_val != 0);
             return m_pos;
         }
     };
@@ -136,39 +136,39 @@ public:
     /** Construct a bitset with the singleton i. */
     static constexpr IntBitSet Singleton(unsigned i) noexcept
     {
-        assert(i < MAX_SIZE);
+        Assume(i < MAX_SIZE);
         return IntBitSet(I(1U) << i);
     }
     /** Construct a bitset with bits 0..count-1 (inclusive) set to 1. */
     static constexpr IntBitSet Fill(unsigned count) noexcept
     {
         IntBitSet ret;
-        assert(count <= MAX_SIZE);
+        Assume(count <= MAX_SIZE);
         if (count) ret.m_val = I(~I{0}) >> (MAX_SIZE - count);
         return ret;
     }
     /** Set a bit to 1. */
     constexpr void Set(unsigned pos) noexcept
     {
-        assert(pos < MAX_SIZE);
+        Assume(pos < MAX_SIZE);
         m_val |= I{1U} << pos;
     }
     /** Set a bit to the specified value. */
     constexpr void Set(unsigned pos, bool val) noexcept
     {
-        assert(pos < MAX_SIZE);
+        Assume(pos < MAX_SIZE);
         m_val = (m_val & ~I(I{1U} << pos)) | (I(val) << pos);
     }
     /** Set a bit to 0. */
     constexpr void Reset(unsigned pos) noexcept
     {
-        assert(pos < MAX_SIZE);
+        Assume(pos < MAX_SIZE);
         m_val &= ~I(I{1U} << pos);
     }
     /** Retrieve a bit at the given position. */
     constexpr bool operator[](unsigned pos) const noexcept
     {
-        assert(pos < MAX_SIZE);
+        Assume(pos < MAX_SIZE);
         return (m_val >> pos) & 1U;
     }
     /** Compute the number of 1 bits in the bitset. */
@@ -186,13 +186,13 @@ public:
     /** Find the first element (requires Any()). */
     constexpr unsigned First() const noexcept
     {
-        assert(m_val != 0);
+        Assume(m_val != 0);
         return std::countr_zero(m_val);
     }
     /** Find the last element (requires Any()). */
     constexpr unsigned Last() const noexcept
     {
-        assert(m_val != 0);
+        Assume(m_val != 0);
         return std::bit_width(m_val) - 1;
     }
     /** Set this object's bits to be the binary AND between respective bits from this and a. */
@@ -281,7 +281,7 @@ class MultiIntBitSet
         /** Progress to the next 1 bit (only if != IteratorEnd). */
         constexpr Iterator& operator++() noexcept
         {
-            assert(m_idx < N);
+            Assume(m_idx < N);
             m_val &= m_val - I{1U};
             if (m_val == 0) {
                 while (true) {
@@ -301,7 +301,7 @@ class MultiIntBitSet
         /** Get the current bit position (only if != IteratorEnd). */
         constexpr unsigned operator*() const noexcept
         {
-            assert(m_idx < N);
+            Assume(m_idx < N);
             return m_pos;
         }
     };
@@ -316,13 +316,13 @@ public:
     /** Set a bit to 1. */
     void constexpr Set(unsigned pos) noexcept
     {
-        assert(pos < MAX_SIZE);
+        Assume(pos < MAX_SIZE);
         m_val[pos / LIMB_BITS] |= I{1U} << (pos % LIMB_BITS);
     }
     /** Set a bit to the specified value. */
     void constexpr Set(unsigned pos, bool val) noexcept
     {
-        assert(pos < MAX_SIZE);
+        Assume(pos < MAX_SIZE);
         m_val[pos / LIMB_BITS] = (m_val[pos / LIMB_BITS] & ~I(I{1U} << (pos % LIMB_BITS))) |
                                  (I{val} << (pos % LIMB_BITS));
     }
@@ -341,19 +341,19 @@ public:
     /** Set a bit to 0. */
     void constexpr Reset(unsigned pos) noexcept
     {
-        assert(pos < MAX_SIZE);
+        Assume(pos < MAX_SIZE);
         m_val[pos / LIMB_BITS] &= ~I(I{1U} << (pos % LIMB_BITS));
     }
     /** Retrieve a bit at the given position. */
     bool constexpr operator[](unsigned pos) const noexcept
     {
-        assert(pos < MAX_SIZE);
+        Assume(pos < MAX_SIZE);
         return (m_val[pos / LIMB_BITS] >> (pos % LIMB_BITS)) & 1U;
     }
     /** Construct a bitset with the singleton pos. */
     static constexpr MultiIntBitSet Singleton(unsigned pos) noexcept
     {
-        assert(pos < MAX_SIZE);
+        Assume(pos < MAX_SIZE);
         MultiIntBitSet ret;
         ret.m_val[pos / LIMB_BITS] = I{1U} << (pos % LIMB_BITS);
         return ret;
@@ -361,12 +361,12 @@ public:
     /** Construct a bitset with bits 0..count-1 (inclusive) set to 1. */
     static constexpr MultiIntBitSet Fill(unsigned count) noexcept
     {
-        assert(count <= MAX_SIZE);
+        Assume(count <= MAX_SIZE);
         MultiIntBitSet ret;
         if (count) {
             unsigned i = 0;
             while (count > LIMB_BITS) {
-                ret.m_val[i++] = I(~I{0});
+                ret.m_val[i++] = ~I{0};
                 count -= LIMB_BITS;
             }
             ret.m_val[i] = I(~I{0}) >> (LIMB_BITS - count);
@@ -402,7 +402,7 @@ public:
         unsigned p = 0;
         while (m_val[p] == 0) {
             ++p;
-            assert(p < N);
+            Assume(p < N);
         }
         return std::countr_zero(m_val[p]) + p * LIMB_BITS;
     }
@@ -411,7 +411,7 @@ public:
     {
         unsigned p = N - 1;
         while (m_val[p] == 0) {
-            assert(p > 0);
+            Assume(p > 0);
             --p;
         }
         return std::bit_width(m_val[p]) - 1 + p * LIMB_BITS;
diff --git a/src/util/check.h b/src/util/check.h
index 8d3c6ca64d..121038df4b 100644
--- a/src/util/check.h
+++ b/src/util/check.h
@@ -42,9 +42,9 @@ void assertion_fail(std::string_view file, int line, std::string_view func, std:
 
 /** Helper for Assert()/Assume() */
 template <bool IS_ASSERT, typename T>
-T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion)
+constexpr T&& inline_assertion_check(LIFETIMEBOUND T&& val, [[maybe_unused]] const char* file, [[maybe_unused]] int line, [[maybe_unused]] const char* func, [[maybe_unused]] const char* assertion)
 {
-    if (IS_ASSERT || g_fuzzing
+    if (IS_ASSERT || std::is_constant_evaluated()||g_fuzzing
 #ifdef ABORT_ON_FAILED_ASSUME
                   || true
 #endif

</details>

dergoegge commented at 2:36 PM on October 24, 2024: member

I'll try reverting the bitset changes, I remember that it failed to compile without but let's see...

maflcko commented at 2:39 PM on October 24, 2024: member

You can push the diff as a dummy commit for now, if you want. I'll create a separate pull for it, because the Assume change at compile time possibly makes sense on its own. Then you can rebase on that and squash everything down to one commit again.

dergoegge marked this as a draft on Oct 24, 2024

dergoegge force-pushed on Oct 24, 2024

DrahtBot added the label CI failed on Oct 24, 2024

DrahtBot commented at 3:29 PM on October 24, 2024: contributor

🚧 At least one of the CI tasks failed. <sub>Debug: https://github.com/bitcoin/bitcoin/runs/32015960361</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label CI failed on Oct 24, 2024

maflcko approved

maflcko commented at 6:19 PM on October 24, 2024: member

lgtm, after squash

dergoegge commented at 9:42 AM on October 25, 2024: member

Waiting for #31150 to go in

fanquake referenced this in commit b95adf057a on Oct 25, 2024

Introduce `g_fuzzing` global for fuzzing checks 9f243cd7fa

dergoegge force-pushed on Oct 25, 2024

dergoegge marked this as ready for review on Oct 25, 2024

brunoerg approved

brunoerg commented at 1:14 PM on October 25, 2024: contributor

crACK 9f243cd7fa6654e3b71ba6bff82cceed547c5d53

dergoegge requested review from maflcko on Oct 25, 2024

marcofleon commented at 2:48 PM on October 25, 2024: contributor

Tested ACK 9f243cd7fa6654e3b71ba6bff82cceed547c5d53

Reviewed the code, which changes the fuzzing check from compile time to run time. This should work for our purposes and is a reasonable solution to #30950. It's better than #31028, which I'll close if this gets merged.

Built with BUILD_FUZZ_BINARY (without BUILD_FOR_FUZZING) and ran the p2p_headers_presync target on a corpus that I have. There was no timeout.

FUZZ=p2p_headers_presync ./fuzzbuildgfuzz/src/test/fuzz/fuzz ../fuzzcorpus/p2p_headers_presync/
p2p_headers_presync: succeeded against 781 files in 1s.

maflcko commented at 9:08 AM on October 28, 2024: member

review ACK 9f243cd7fa6654e3b71ba6bff82cceed547c5d53 🗜

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 9f243cd7fa6654e3b71ba6bff82cceed547c5d53 🗜
08lGpbkgVhcFKipondTngBBc44P3JvdyCzjDwpY6jkQE9U7u7UDhHjPBiQ9v07Mi8l28j7TUMxPWsMJWT+ElCw==

</details>

fanquake merged this on Oct 28, 2024

fanquake closed this on Oct 28, 2024

sipa commented at 2:00 AM on October 31, 2024: member

This means that no Assume can be compiled out in production builds anymore, as they all involve a g_fuzzing check?

See #31178.

TheCharlatan referenced this in commit a73b2bd0f0 on Nov 14, 2024

bug-castercv502 referenced this in commit fdcc066ca0 on Sep 28, 2025

bitcoin locked this on Oct 31, 2025