doc: Add fuzz based coverage report generation #32206
pull Prabhat1308 wants to merge 1 commits into bitcoin:master from Prabhat1308:pv/31933_followup changing 1 files +41 −0-
Prabhat1308 commented at 6:58 pm on April 2, 2025: contributorFollowup from comment 1 31933 and comment 2 31933 , have added the instructions to generate coverage report for fuzz tests.
-
DrahtBot commented at 6:58 pm on April 2, 2025: contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage & Benchmarks
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32206.
Reviews
See the guideline for information on the review process.
Type Reviewers ACK maflcko, mabu44 Stale ACK kevkevinpal If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #31668 (Added rescan option for import descriptors by saikiran57)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
-
DrahtBot renamed this:
doc: Add fuzz based coverage report generation
doc: Add fuzz based coverage report generation
on Apr 2, 2025 -
DrahtBot added the label Docs on Apr 2, 2025
-
instagibbs commented at 7:00 pm on April 2, 2025: memberI just went through figuring this out myself, will review
-
in doc/developer-notes.md:581 in 861309946c outdated
574@@ -575,6 +575,54 @@ llvm-cov show \ 575 576 The generated coverage report can be accessed at `build/coverage_report/index.html`. 577 578+#### Compiling for Fuzz Coverage 579+ 580+```shell 581+cmake -B build \
instagibbs commented at 7:07 pm on April 2, 2025:how do you feel about naming the dir something more usage specific likebuild_fuzz_cov
since devs will likely be using build for other things?
Prabhat1308 commented at 7:02 am on April 3, 2025:I have been following the docs since
build
was used before with lcov coverage . https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-lcovBut if others agree with this , I will change it and might have to retouch the llvm/clang based coverage report generation too then .
in doc/developer-notes.md:587 in 861309946c outdated
582+ -DCMAKE_C_COMPILER="clang" \ 583+ -DCMAKE_CXX_COMPILER="clang++" \ 584+ -DCMAKE_C_FLAGS="-fprofile-instr-generate -fcoverage-mapping" \ 585+ -DCMAKE_CXX_FLAGS="-fprofile-instr-generate -fcoverage-mapping" \ 586+ -DBUILD_FOR_FUZZING=ON \ 587+ -DSANITIZERS=fuzzer
maflcko commented at 7:09 pm on April 2, 2025:Could drop libFuzzer, as it isn’t needed. Also libFuzzer may have a bug, according to #32158 (review), which could cause issues here.
Prabhat1308 commented at 6:50 am on April 3, 2025:dropped libFuzzer
maflcko commented at 7:33 am on April 3, 2025:dropped libFuzzer
I can still see it?
Prabhat1308 commented at 7:43 am on April 3, 2025:should not be there now.in doc/developer-notes.md:595 in 861309946c outdated
590+ 591+Run fuzz test with the fuzz target 592+```shell 593+mkdir -p build/raw_profile_data 594+# Run fuzz test with the target 595+LLVM_PROFILE_FILE="$(pwd)/build/raw_profile_data/txorphan.profraw" FUZZ=txorphan ./build/bin/fuzz ../qa-assets/fuzz_corpora/txorphan -runs=1
maflcko commented at 7:10 pm on April 2, 2025:if you drop libfuzzer, would also have to drop theruns=1
here, or use the python runner./test/fuzz/test_runner.py ... txorphan
Prabhat1308 commented at 6:50 am on April 3, 2025:I have dropped thelibfuzzer
and now using thetest_runner
. I have opened #32209 to fix the path issue while generating profraw using the runner .maflcko approvedinstagibbs commented at 7:22 pm on April 2, 2025: memberworks for me at leastmabu44 commented at 9:13 pm on April 2, 2025: noneInteresting. It works for me. Concept ACK 861309946c3a8bb4c0793129f0f307ed908f1a76 After running the commands, I have some “slow-unit-xxxxx” files in the project root directory that are tracked by git. Can those be created in the build directory?Prabhat1308 force-pushed on Apr 3, 2025Prabhat1308 commented at 6:58 am on April 3, 2025: contributorInteresting. It works for me. Concept ACK 8613099 After running the commands, I have some “slow-unit-xxxxx” files in the project root directory that are tracked by git. Can those be created in the build directory?
I don’t seem to have these files . Which system are you running on ?
maflcko commented at 7:32 am on April 3, 2025: memberInteresting. It works for me. Concept ACK 8613099 After running the commands, I have some “slow-unit-xxxxx” files in the project root directory that are tracked by git. Can those be created in the build directory?
I don’t seem to have these files . Which system are you running on ?
They are created by libFuzzer. See the
-artifact_prefix
option by libFuzzer.My recommendation would be to just not use libFuzzer here when it is not needed and given the non-determinism and other issues it introduces.
Prabhat1308 force-pushed on Apr 3, 2025in doc/developer-notes.md:597 in 52172bd58e outdated
592+```shell 593+mkdir -p build/raw_profile_data 594+# Run fuzz test with the target 595+LLVM_PROFILE_FILE="$(pwd)/build/raw_profile_data/txorphan.profraw" ./build/test/fuzz/test_runner.py ../qa-assets/fuzz_corpora txorphan 596+llvm-profdata merge build/raw_profile_data/txorphan.profraw -o build/coverage.profdata 597+```
maflcko commented at 7:56 am on April 3, 2025:I guess this can be removed, except for
0# Run one fuzz target 1LLVM_PROFILE_FILE="$(pwd)/build/raw_profile_data/%m_%p.profraw" ./build/test/fuzz/test_runner.py ./qa-assets/fuzz_corpora txorphan
which could be moved into the section below, which could then just say “For one or more fuzz targets”?
Prabhat1308 commented at 8:41 am on April 7, 2025:Done.maflcko approvedmaflcko commented at 7:56 am on April 3, 2025: memberlgtmin doc/developer-notes.md:603 in 52172bd58e outdated
598+ 599+For multiple fuzz targets 600+ 601+```shell 602+# Run multiple fuzz targets 603+LLVM_PROFILE_FILE="$(pwd)/build/raw_profile_data/%m_%p.profraw" ./build/test/fuzz/test_runner.py ./qa-assets/fuzz_corpora
mabu44 commented at 4:06 pm on April 3, 2025:This should be ../qa-assets (with two dots)mabu44 commented at 4:17 pm on April 3, 2025: noneI don’t seem to have these files. Which system are you running on?
I am running on Debian. With the new version I cannot get it working. The LLVM_PROFILE_FILE variable seem to be ignored. A “default.profraw” file is generated in the root directory of the source code. I don’t get any files in raw_profile_data.
Prabhat1308 commented at 4:19 pm on April 3, 2025: contributorI don’t seem to have these files. Which system are you running on?
I am running on Debian. With the new version I cannot get it working. The LLVM_PROFILE_FILE variable seem to be ignored. A “default.profraw” file is generated in the root directory of the source code. I don’t get any files in raw_profile_data.
This PR depends on #32209 to solve this issue.
Prabhat1308 force-pushed on Apr 3, 2025fanquake referenced this in commit 65dcbec756 on Apr 4, 2025kevkevinpal commented at 1:26 pm on April 5, 2025: contributorTested ACK 5dcbec
I tested these commands on master, running the fuzz tests against (qa-assets) and inspecting the coverage reports
Prabhat1308 force-pushed on Apr 7, 2025DrahtBot added the label CI failed on Apr 7, 2025DrahtBot commented at 9:39 am on April 7, 2025: contributor🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/40085914234
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.
Add fuzz test coverage report generation
Signed-off-by: Prabhat Verma <prabhatverma329@gmail.com>
Prabhat1308 force-pushed on Apr 7, 2025maflcko commented at 10:34 am on April 7, 2025: memberlgtm ACK 7677fde4c788bc3051edfe81b3b564388b2d5da1DrahtBot requested review from mabu44 on Apr 7, 2025DrahtBot removed the label CI failed on Apr 7, 2025mabu44 commented at 6:54 pm on April 7, 2025: noneTested ACK 7677fde4c788bc3051edfe81b3b564388b2d5da1 Tested on this branch merged with master on Debian 12 (using llvm 16). Executed the provided commands (adding the -16 suffix to llvm executable names) and inspected the resulting coverage report.ryanofsky assigned ryanofsky on Apr 8, 2025ryanofsky merged this on Apr 8, 2025ryanofsky closed this on Apr 8, 2025
Prabhat1308 deleted the branch on Apr 9, 2025
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: 2025-04-16 15:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me