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).
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
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.
209 #if defined(PROVIDE_FUZZ_MAIN_FUNCTION)
210 int main(int argc, char** argv)
211 {
212 initialize();
213+
214+ ResetCoverageCounters();
initialize inside the function body?
🚧 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.
Concept ACK 52506a0
Made a clean build of this change using
0sudo make clean && ./autogen.sh && CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined && sudo make -j"$(($(nproc)+1))" install
(master: f15778536ad421f9805f0c005f0eb7b84dda1b4e)
0FUZZ=process_message src/test/fuzz/fuzz -runs=1 qa-assets/fuzz_seed_corpus/process_message
1INFO: Running with entropic power schedule (0xFF, 100).
2INFO: Seed: 4218378445
3INFO: Loaded 1 modules (425045 inline 8-bit counters): 425045 [0x558dbe652ce0, 0x558dbe6ba935),
4INFO: Loaded 1 PC tables (425045 PCs): 425045 [0x558dbe6ba938,0x558dbed36e88),
5INFO: 2759 files found in qa-assets/fuzz_seed_corpus/process_message
6INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 814557 bytes
7INFO: seed corpus: files: 2759 min: 1b max: 814557b total: 14744612b rss: 218Mb
8[#1024](/bitcoin-bitcoin/1024/) pulse cov: 11518 ft: 23753 corp: 583/80Kb exec/s: 341 rss: 454Mb
9[#2048](/bitcoin-bitcoin/2048/) pulse cov: 13725 ft: 35158 corp: 1025/223Kb exec/s: 256 rss: 454Mb
10[#3698](/bitcoin-bitcoin/3698/) INITED cov: 14297 ft: 52528 corp: 1782/8638Kb exec/s: 160 rss: 581Mb
11[#3698](/bitcoin-bitcoin/3698/) DONE cov: 14297 ft: 52528 corp: 1782/8638Kb lim: 630378 exec/s: 160 rss: 581Mb
12Done 3698 runs in 23 second(s)
13
14FUZZ=process_message src/test/fuzz/fuzz -runs=1 qa-assets/fuzz_seed_corpus/process_message
15INFO: Running with entropic power schedule (0xFF, 100).
16INFO: Seed: 555650094
17INFO: Loaded 1 modules (425045 inline 8-bit counters): 425045 [0x557c8bfd0ce0, 0x557c8c038935),
18INFO: Loaded 1 PC tables (425045 PCs): 425045 [0x557c8c038938,0x557c8c6b4e88),
19INFO: 2759 files found in qa-assets/fuzz_seed_corpus/process_message
20INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 814557 bytes
21INFO: seed corpus: files: 2759 min: 1b max: 814557b total: 14744612b rss: 219Mb
22[#3696](/bitcoin-bitcoin/3696/) INITED cov: 14264 ft: 52459 corp: 1784/8568Kb exec/s: 160 rss: 583Mb
23[#3696](/bitcoin-bitcoin/3696/) DONE cov: 14264 ft: 52459 corp: 1784/8568Kb lim: 630378 exec/s: 160 rss: 583Mb
24Done 3696 runs in 23 second(s)
(dergoegge:2024-05-cov-reset-counters: 52506a0)
0FUZZ=process_message src/test/fuzz/fuzz -runs=1 qa-assets/fuzz_seed_corpus/process_message
1INFO: Running with entropic power schedule (0xFF, 100).
2INFO: Seed: 1724170080
3INFO: Loaded 1 modules (424990 inline 8-bit counters): 424990 [0x55773dc1cc20, 0x55773dc8483e),
4INFO: Loaded 1 PC tables (424990 PCs): 424990 [0x55773dc84840,0x55773e300a20),
5INFO: 2759 files found in qa-assets/fuzz_seed_corpus/process_message
6INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 814557 bytes
7INFO: seed corpus: files: 2759 min: 1b max: 814557b total: 14744612b rss: 219Mb
8[#1024](/bitcoin-bitcoin/1024/) pulse cov: 11499 ft: 23802 corp: 589/81Kb exec/s: 341 rss: 455Mb
9[#2048](/bitcoin-bitcoin/2048/) pulse cov: 13711 ft: 35251 corp: 1034/226Kb exec/s: 256 rss: 455Mb
10[#3701](/bitcoin-bitcoin/3701/) INITED cov: 14277 ft: 52623 corp: 1793/8700Kb exec/s: 160 rss: 582Mb
11[#3701](/bitcoin-bitcoin/3701/) DONE cov: 14277 ft: 52623 corp: 1793/8700Kb lim: 630378 exec/s: 160 rss: 582Mb
12Done 3701 runs in 23 second(s)
13
14FUZZ=process_message src/test/fuzz/fuzz -runs=1 qa-assets/fuzz_seed_corpus/process_message
15INFO: Running with entropic power schedule (0xFF, 100).
16INFO: Seed: 3858639904
17INFO: Loaded 1 modules (424990 inline 8-bit counters): 424990 [0x562169278c20, 0x5621692e083e),
18INFO: Loaded 1 PC tables (424990 PCs): 424990 [0x5621692e0840,0x56216995ca20),
19INFO: 2759 files found in qa-assets/fuzz_seed_corpus/process_message
20INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 814557 bytes
21INFO: seed corpus: files: 2759 min: 1b max: 814557b total: 14744612b rss: 219Mb
22[#1024](/bitcoin-bitcoin/1024/) pulse cov: 11480 ft: 23795 corp: 578/80Kb exec/s: 341 rss: 456Mb
23[#2048](/bitcoin-bitcoin/2048/) pulse cov: 13698 ft: 35272 corp: 1017/222Kb exec/s: 256 rss: 456Mb
24[#3700](/bitcoin-bitcoin/3700/) INITED cov: 14266 ft: 52596 corp: 1769/8685Kb exec/s: 160 rss: 582Mb
25[#3700](/bitcoin-bitcoin/3700/) DONE cov: 14266 ft: 52596 corp: 1769/8685Kb lim: 630378 exec/s: 160 rss: 582Mb
26Done 3700 runs in 23 second(s)
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__)
https://cirrus-ci.com/task/6736987506343936:
0Undefined symbols for architecture x86_64:
1 "___llvm_profile_reset_counters", referenced from:
2 initialize() in libtest_fuzz.a(libtest_fuzz_a-fuzz.o)
3 "___gcov_reset", referenced from:
4 initialize() in libtest_fuzz.a(libtest_fuzz_a-fuzz.o)
5ld: symbol(s) not found for architecture x86_64
https://cirrus-ci.com/task/6596250017988608:
0/usr/bin/ld: libtest_fuzz.a(libtest_fuzz_a-fuzz.o): in function `ResetCoverageCounters()':
1./src/test/fuzz/fuzz.cpp:90: undefined reference to `__llvm_profile_reset_counters'
2/usr/bin/ld: ./src/test/fuzz/fuzz.cpp:94: undefined reference to `__gcov_reset'
3/usr/bin/ld: ./src/test/fuzz/fuzz.cpp:90: undefined reference to `__llvm_profile_reset_counters'
4/usr/bin/ld: ./src/test/fuzz/fuzz.cpp:94: undefined reference to `__gcov_reset'
5collect2: error: ld returned 1 exit status
https://cirrus-ci.com/task/5470350111145984:
0test/fuzz/fuzz.cpp:84:1: error: attributes are not permitted in this position [-Werror=attributes]
1 84 | __attribute__((weak)) extern "C" void __llvm_profile_reset_counters(void);
2 | ^~~~~~~~~~~~~
3test/fuzz/fuzz.cpp:84:33: note: attributes may be inserted here
4 84 | __attribute__((weak)) extern "C" void __llvm_profile_reset_counters(void);
5 | ^
6test/fuzz/fuzz.cpp:85:1: error: attributes are not permitted in this position [-Werror=attributes]
7 85 | __attribute__((weak)) extern "C" void __gcov_reset(void);
8 | ^~~~~~~~~~~~~
9test/fuzz/fuzz.cpp:85:33: note: attributes may be inserted here
10 85 | __attribute__((weak)) extern "C" void __gcov_reset(void);
11 | ^
12test/fuzz/fuzz.cpp: In function ‘void ResetCoverageCounters()’:
13test/fuzz/fuzz.cpp:89:9: error: the address of ‘void __llvm_profile_reset_counters()’ will never be NULL [-Werror=address]
14 89 | if (__llvm_profile_reset_counters) {
15 | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
16test/fuzz/fuzz.cpp:84:39: note: ‘void __llvm_profile_reset_counters()’ declared here
17 84 | __attribute__((weak)) extern "C" void __llvm_profile_reset_counters(void);
18 | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
19test/fuzz/fuzz.cpp:93:9: error: the address of ‘void __gcov_reset()’ will never be NULL [-Werror=address]
20 93 | if (__gcov_reset) {
21 | ^~~~~~~~~~~~
22test/fuzz/fuzz.cpp:85:39: note: ‘void __gcov_reset()’ declared here
23 85 | __attribute__((weak)) extern "C" void __gcov_reset(void);
24 | ^~~~~~~~~~~~
25cc1plus: all warnings being treated as errors
The difference only really becomes visible for individual coverage reports, e.g. for process_messages.
process_messages
As an example for the minisketch harness, files reported as reached by the fuzzer:
master:
0src/coins.h
1src/common/args.cpp
2src/common/system.cpp
3src/compat/byteswap.h
4src/compat/endian.h
5src/crypto/chacha20.cpp
6src/crypto/chacha20.h
7src/crypto/common.h
8src/crypto/sha256.cpp
9src/crypto/sha512.cpp
10src/crypto/sha512.h
11src/crypto/siphash.cpp
12src/cuckoocache.h
13src/hash.cpp
14src/hash.h
15src/logging.cpp
16src/logging.h
17src/minisketch/include/minisketch.h
18src/minisketch/src/fields/generic_4bytes.cpp
19src/minisketch/src/fields/generic_common_impl.h
20src/minisketch/src/int_utils.h
21src/minisketch/src/lintrans.h
22src/minisketch/src/minisketch.cpp
23src/minisketch/src/sketch.h
24src/minisketch/src/sketch_impl.h
25src/net.cpp
26src/netaddress.cpp
27src/netbase.h
28src/policy/feerate.h
29src/prevector.h
30src/primitives/transaction.h
31src/pubkey.cpp
32src/random.cpp
33src/randomenv.cpp
34src/rpc/client.cpp
35src/rpc/server.cpp
36src/rpc/server.h
37src/rpc/util.cpp
38src/rpc/util.h
39src/script/miniscript.cpp
40src/script/miniscript.h
41src/script/script.h
42src/script/sigcache.cpp
43src/script/sign.cpp
44src/script/solver.cpp
45src/secp256k1/src/hash_impl.h
46src/secp256k1/src/secp256k1.c
47src/secp256k1/src/selftest.h
48src/secp256k1/src/util.h
49src/serialize.h
50src/span.h
51src/support/allocators/secure.h
52src/support/cleanse.cpp
53src/support/lockedpool.cpp
54src/support/lockedpool.h
55src/sync.h
56src/test/fuzz/FuzzedDataProvider.h
57src/test/fuzz/fuzz.cpp
58src/test/fuzz/fuzz.h
59src/test/fuzz/minisketch.cpp
60src/test/fuzz/script_assets_test_minimizer.cpp
61src/test/fuzz/txrequest.cpp
62src/test/fuzz/util.h
63src/test/util/script.h
64src/test/util/setup_common.cpp
65src/uint256.h
66src/univalue/include/univalue.h
67src/univalue/lib/univalue.cpp
68src/univalue/lib/univalue_read.cpp
69src/util/check.h
70src/util/fs.cpp
71src/util/fs.h
72src/util/spanparsing.h
73src/util/strencodings.cpp
74src/util/strencodings.h
75src/util/string.h
76src/util/threadinterrupt.cpp
77src/util/time.cpp
78src/util/time.h
79src/util/vector.h
pull:
0src/minisketch/include/minisketch.h
1src/minisketch/src/fields/generic_4bytes.cpp
2src/minisketch/src/fields/generic_common_impl.h
3src/minisketch/src/int_utils.h
4src/minisketch/src/lintrans.h
5src/minisketch/src/minisketch.cpp
6src/minisketch/src/sketch.h
7src/minisketch/src/sketch_impl.h
8src/span.h
9src/test/fuzz/FuzzedDataProvider.h
10src/test/fuzz/fuzz.cpp
11src/test/fuzz/minisketch.cpp
12src/test/fuzz/util.h
13src/util/check.h
14src/util/fs.cpp
15src/util/fs.h
16src/util/time.h