fuzz: More accurate coverage reports #30156

pull dergoegge wants to merge 1 commits into bitcoin:master from dergoegge:2024-05-cov-reset-counters changing 1 files +22 −0
  1. dergoegge commented at 10:18 AM on May 23, 2024: member

    Our coverage reports include coverage of initialization code, which can be misleading when trying to evaluate the coverage a fuzz harness achieves through fuzzing alone.

    This PR proposes to make fuzz coverage reports more accurate by resetting coverage counters after initialization code has been run. This makes it easier to evaluate which code was actually reached through fuzzing (e.g. to spot fuzz blockers).

  2. DrahtBot commented at 10:18 AM on May 23, 2024: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK maflcko, brunoerg
    Concept ACK TheCharlatan, hebasto, kevkevinpal, instagibbs

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

  3. DrahtBot added the label Tests on May 23, 2024
  4. TheCharlatan commented at 10:23 AM on May 23, 2024: contributor

    Concept ACK

  5. in src/test/fuzz/fuzz.cpp:214 in dc8c30bbd1 outdated
     209 |  #if defined(PROVIDE_FUZZ_MAIN_FUNCTION)
     210 |  int main(int argc, char** argv)
     211 |  {
     212 |      initialize();
     213 | +
     214 | +    ResetCoverageCounters();
    


    maflcko commented at 10:52 AM on May 23, 2024:

    Why not call this at the end of initialize inside the function body?


    dergoegge commented at 11:12 AM on May 23, 2024:

    No reason, done!

  6. dergoegge force-pushed on May 23, 2024
  7. hebasto commented at 12:32 PM on May 23, 2024: member

    Concept ACK on improving coverage.

  8. brunoerg commented at 12:59 PM on May 23, 2024: contributor

    Concept ACK

  9. DrahtBot added the label CI failed on May 23, 2024
  10. DrahtBot commented at 1:44 PM on May 23, 2024: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is 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.

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

    <sub>Debug: https://github.com/bitcoin/bitcoin/runs/25325963327</sub>

  11. kevkevinpal commented at 1:44 PM on May 23, 2024: contributor

    Concept ACK 52506a0

    Made a clean build of this change using

    sudo make clean && ./autogen.sh &&  CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined && sudo make -j"$(($(nproc)+1))" install
    

    Running fuzz tests twice

    (master: f15778536ad421f9805f0c005f0eb7b84dda1b4e)

    FUZZ=process_message src/test/fuzz/fuzz -runs=1 qa-assets/fuzz_seed_corpus/process_message
    INFO: Running with entropic power schedule (0xFF, 100).
    INFO: Seed: 4218378445
    INFO: Loaded 1 modules   (425045 inline 8-bit counters): 425045 [0x558dbe652ce0, 0x558dbe6ba935),
    INFO: Loaded 1 PC tables (425045 PCs): 425045 [0x558dbe6ba938,0x558dbed36e88),
    INFO:     2759 files found in qa-assets/fuzz_seed_corpus/process_message
    INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 814557 bytes
    INFO: seed corpus: files: 2759 min: 1b max: 814557b total: 14744612b rss: 218Mb
    [#1024](/bitcoin-bitcoin/1024/)   pulse  cov: 11518 ft: 23753 corp: 583/80Kb exec/s: 341 rss: 454Mb
    [#2048](/bitcoin-bitcoin/2048/)   pulse  cov: 13725 ft: 35158 corp: 1025/223Kb exec/s: 256 rss: 454Mb
    [#3698](/bitcoin-bitcoin/3698/)   INITED cov: 14297 ft: 52528 corp: 1782/8638Kb exec/s: 160 rss: 581Mb
    [#3698](/bitcoin-bitcoin/3698/)   DONE   cov: 14297 ft: 52528 corp: 1782/8638Kb lim: 630378 exec/s: 160 rss: 581Mb
    Done 3698 runs in 23 second(s)
    
    FUZZ=process_message src/test/fuzz/fuzz -runs=1 qa-assets/fuzz_seed_corpus/process_message
    INFO: Running with entropic power schedule (0xFF, 100).
    INFO: Seed: 555650094
    INFO: Loaded 1 modules   (425045 inline 8-bit counters): 425045 [0x557c8bfd0ce0, 0x557c8c038935),
    INFO: Loaded 1 PC tables (425045 PCs): 425045 [0x557c8c038938,0x557c8c6b4e88),
    INFO:     2759 files found in qa-assets/fuzz_seed_corpus/process_message
    INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 814557 bytes
    INFO: seed corpus: files: 2759 min: 1b max: 814557b total: 14744612b rss: 219Mb
    [#3696](/bitcoin-bitcoin/3696/)   INITED cov: 14264 ft: 52459 corp: 1784/8568Kb exec/s: 160 rss: 583Mb
    [#3696](/bitcoin-bitcoin/3696/)   DONE   cov: 14264 ft: 52459 corp: 1784/8568Kb lim: 630378 exec/s: 160 rss: 583Mb
    Done 3696 runs in 23 second(s)
    

    (dergoegge:2024-05-cov-reset-counters: 52506a0)

    FUZZ=process_message src/test/fuzz/fuzz -runs=1 qa-assets/fuzz_seed_corpus/process_message
    INFO: Running with entropic power schedule (0xFF, 100).
    INFO: Seed: 1724170080
    INFO: Loaded 1 modules   (424990 inline 8-bit counters): 424990 [0x55773dc1cc20, 0x55773dc8483e),
    INFO: Loaded 1 PC tables (424990 PCs): 424990 [0x55773dc84840,0x55773e300a20),
    INFO:     2759 files found in qa-assets/fuzz_seed_corpus/process_message
    INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 814557 bytes
    INFO: seed corpus: files: 2759 min: 1b max: 814557b total: 14744612b rss: 219Mb
    [#1024](/bitcoin-bitcoin/1024/)   pulse  cov: 11499 ft: 23802 corp: 589/81Kb exec/s: 341 rss: 455Mb
    [#2048](/bitcoin-bitcoin/2048/)   pulse  cov: 13711 ft: 35251 corp: 1034/226Kb exec/s: 256 rss: 455Mb
    [#3701](/bitcoin-bitcoin/3701/)   INITED cov: 14277 ft: 52623 corp: 1793/8700Kb exec/s: 160 rss: 582Mb
    [#3701](/bitcoin-bitcoin/3701/)   DONE   cov: 14277 ft: 52623 corp: 1793/8700Kb lim: 630378 exec/s: 160 rss: 582Mb
    Done 3701 runs in 23 second(s)
    
    FUZZ=process_message src/test/fuzz/fuzz -runs=1 qa-assets/fuzz_seed_corpus/process_message
    INFO: Running with entropic power schedule (0xFF, 100).
    INFO: Seed: 3858639904
    INFO: Loaded 1 modules   (424990 inline 8-bit counters): 424990 [0x562169278c20, 0x5621692e083e),
    INFO: Loaded 1 PC tables (424990 PCs): 424990 [0x5621692e0840,0x56216995ca20),
    INFO:     2759 files found in qa-assets/fuzz_seed_corpus/process_message
    INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 814557 bytes
    INFO: seed corpus: files: 2759 min: 1b max: 814557b total: 14744612b rss: 219Mb
    [#1024](/bitcoin-bitcoin/1024/)   pulse  cov: 11480 ft: 23795 corp: 578/80Kb exec/s: 341 rss: 456Mb
    [#2048](/bitcoin-bitcoin/2048/)   pulse  cov: 13698 ft: 35272 corp: 1017/222Kb exec/s: 256 rss: 456Mb
    [#3700](/bitcoin-bitcoin/3700/)   INITED cov: 14266 ft: 52596 corp: 1769/8685Kb exec/s: 160 rss: 582Mb
    [#3700](/bitcoin-bitcoin/3700/)   DONE   cov: 14266 ft: 52596 corp: 1769/8685Kb lim: 630378 exec/s: 160 rss: 582Mb
    Done 3700 runs in 23 second(s)
    
  12. instagibbs commented at 2:14 PM on May 23, 2024: member

    concept ACK

  13. [fuzz] Avoid collecting initialization coverage 949abebea0
  14. dergoegge force-pushed on May 23, 2024
  15. in src/test/fuzz/fuzz.cpp:83 in 949abebea0
      78 | @@ -79,6 +79,26 @@ void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target,
      79 |  static std::string_view g_fuzz_target;
      80 |  static const TypeTestOneInput* g_test_one_input{nullptr};
      81 |  
      82 | +
      83 | +#if defined(__clang__) && defined(__linux__)
    


    dergoegge commented at 5:01 PM on May 23, 2024:

    I don't know how to make this work outside of these params. I don't want to use a macro, I think this should always be enabled if possible.


    maflcko commented at 7:16 AM on May 24, 2024:

    Can you share the compile error, or which config ran into an issue?


    dergoegge commented at 8:40 AM on May 24, 2024:

    https://cirrus-ci.com/task/6736987506343936:

    Undefined symbols for architecture x86_64:
      "___llvm_profile_reset_counters", referenced from:
          initialize() in libtest_fuzz.a(libtest_fuzz_a-fuzz.o)
      "___gcov_reset", referenced from:
          initialize() in libtest_fuzz.a(libtest_fuzz_a-fuzz.o)
    ld: symbol(s) not found for architecture x86_64
    

    https://cirrus-ci.com/task/6596250017988608:

    /usr/bin/ld: libtest_fuzz.a(libtest_fuzz_a-fuzz.o): in function `ResetCoverageCounters()':
    ./src/test/fuzz/fuzz.cpp:90: undefined reference to `__llvm_profile_reset_counters'
    /usr/bin/ld: ./src/test/fuzz/fuzz.cpp:94: undefined reference to `__gcov_reset'
    /usr/bin/ld: ./src/test/fuzz/fuzz.cpp:90: undefined reference to `__llvm_profile_reset_counters'
    /usr/bin/ld: ./src/test/fuzz/fuzz.cpp:94: undefined reference to `__gcov_reset'
    collect2: error: ld returned 1 exit status
    

    https://cirrus-ci.com/task/5470350111145984:

    test/fuzz/fuzz.cpp:84:1: error: attributes are not permitted in this position [-Werror=attributes]
       84 | __attribute__((weak)) extern "C" void __llvm_profile_reset_counters(void);
          | ^~~~~~~~~~~~~
    test/fuzz/fuzz.cpp:84:33: note: attributes may be inserted here
       84 | __attribute__((weak)) extern "C" void __llvm_profile_reset_counters(void);
          |                                 ^
    test/fuzz/fuzz.cpp:85:1: error: attributes are not permitted in this position [-Werror=attributes]
       85 | __attribute__((weak)) extern "C" void __gcov_reset(void);
          | ^~~~~~~~~~~~~
    test/fuzz/fuzz.cpp:85:33: note: attributes may be inserted here
       85 | __attribute__((weak)) extern "C" void __gcov_reset(void);
          |                                 ^
    test/fuzz/fuzz.cpp: In function ‘void ResetCoverageCounters()’:
    test/fuzz/fuzz.cpp:89:9: error: the address of ‘void __llvm_profile_reset_counters()’ will never be NULL [-Werror=address]
       89 |     if (__llvm_profile_reset_counters) {
          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    test/fuzz/fuzz.cpp:84:39: note: ‘void __llvm_profile_reset_counters()’ declared here
       84 | __attribute__((weak)) extern "C" void __llvm_profile_reset_counters(void);
          |                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    test/fuzz/fuzz.cpp:93:9: error: the address of ‘void __gcov_reset()’ will never be NULL [-Werror=address]
       93 |     if (__gcov_reset) {
          |         ^~~~~~~~~~~~
    test/fuzz/fuzz.cpp:85:39: note: ‘void __gcov_reset()’ declared here
       85 | __attribute__((weak)) extern "C" void __gcov_reset(void);
          |                                       ^~~~~~~~~~~~
    cc1plus: all warnings being treated as errors
    

    fanquake commented at 8:34 AM on May 29, 2024:

    Might followup here.


    hodlinator commented at 9:49 PM on March 15, 2025:

    (Worked on in #32059).

  16. DrahtBot removed the label CI failed on May 24, 2024
  17. dergoegge commented at 8:41 AM on May 24, 2024: member

    https://drahtbot.space/host_reports/DrahtBot/reports/coverage_fuzz/monotree/949abebea0059edd/cc5858cb70d9ce14/fuzz.coverage/index.html

    The difference only really becomes visible for individual coverage reports, e.g. for process_messages.

  18. maflcko commented at 8:59 AM on May 24, 2024: member

    utACK 949abebea0059edd929b653b4b475a5880fc0a3e

  19. DrahtBot requested review from TheCharlatan on May 24, 2024
  20. DrahtBot requested review from hebasto on May 24, 2024
  21. DrahtBot requested review from brunoerg on May 24, 2024
  22. DrahtBot requested review from instagibbs on May 24, 2024
  23. dergoegge commented at 10:20 AM on May 24, 2024: member

    As an example for the minisketch harness, files reported as reached by the fuzzer:

    master:

    src/coins.h
    src/common/args.cpp
    src/common/system.cpp
    src/compat/byteswap.h
    src/compat/endian.h
    src/crypto/chacha20.cpp
    src/crypto/chacha20.h
    src/crypto/common.h
    src/crypto/sha256.cpp
    src/crypto/sha512.cpp
    src/crypto/sha512.h
    src/crypto/siphash.cpp
    src/cuckoocache.h
    src/hash.cpp
    src/hash.h
    src/logging.cpp
    src/logging.h
    src/minisketch/include/minisketch.h
    src/minisketch/src/fields/generic_4bytes.cpp
    src/minisketch/src/fields/generic_common_impl.h
    src/minisketch/src/int_utils.h
    src/minisketch/src/lintrans.h
    src/minisketch/src/minisketch.cpp
    src/minisketch/src/sketch.h
    src/minisketch/src/sketch_impl.h
    src/net.cpp
    src/netaddress.cpp
    src/netbase.h
    src/policy/feerate.h
    src/prevector.h
    src/primitives/transaction.h
    src/pubkey.cpp
    src/random.cpp
    src/randomenv.cpp
    src/rpc/client.cpp
    src/rpc/server.cpp
    src/rpc/server.h
    src/rpc/util.cpp
    src/rpc/util.h
    src/script/miniscript.cpp
    src/script/miniscript.h
    src/script/script.h
    src/script/sigcache.cpp
    src/script/sign.cpp
    src/script/solver.cpp
    src/secp256k1/src/hash_impl.h
    src/secp256k1/src/secp256k1.c
    src/secp256k1/src/selftest.h
    src/secp256k1/src/util.h
    src/serialize.h
    src/span.h
    src/support/allocators/secure.h
    src/support/cleanse.cpp
    src/support/lockedpool.cpp
    src/support/lockedpool.h
    src/sync.h
    src/test/fuzz/FuzzedDataProvider.h
    src/test/fuzz/fuzz.cpp
    src/test/fuzz/fuzz.h
    src/test/fuzz/minisketch.cpp
    src/test/fuzz/script_assets_test_minimizer.cpp
    src/test/fuzz/txrequest.cpp
    src/test/fuzz/util.h
    src/test/util/script.h
    src/test/util/setup_common.cpp
    src/uint256.h
    src/univalue/include/univalue.h
    src/univalue/lib/univalue.cpp
    src/univalue/lib/univalue_read.cpp
    src/util/check.h
    src/util/fs.cpp
    src/util/fs.h
    src/util/spanparsing.h
    src/util/strencodings.cpp
    src/util/strencodings.h
    src/util/string.h
    src/util/threadinterrupt.cpp
    src/util/time.cpp
    src/util/time.h
    src/util/vector.h
    

    pull:

    src/minisketch/include/minisketch.h
    src/minisketch/src/fields/generic_4bytes.cpp
    src/minisketch/src/fields/generic_common_impl.h
    src/minisketch/src/int_utils.h
    src/minisketch/src/lintrans.h
    src/minisketch/src/minisketch.cpp
    src/minisketch/src/sketch.h
    src/minisketch/src/sketch_impl.h
    src/span.h
    src/test/fuzz/FuzzedDataProvider.h
    src/test/fuzz/fuzz.cpp
    src/test/fuzz/minisketch.cpp
    src/test/fuzz/util.h
    src/util/check.h
    src/util/fs.cpp
    src/util/fs.h
    src/util/time.h
    
  24. brunoerg approved
  25. brunoerg commented at 12:20 PM on May 24, 2024: contributor

    nice, utACK 949abebea0059edd929b653b4b475a5880fc0a3e

  26. dergoegge commented at 5:22 PM on May 28, 2024: member

    rfm?

  27. fanquake merged this on May 29, 2024
  28. fanquake closed this on May 29, 2024


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-28 21:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me