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
  1. Prabhat1308 commented at 6:58 pm on April 2, 2025: contributor
    Followup from comment 1 31933 and comment 2 31933 , have added the instructions to generate coverage report for fuzz tests.
  2. 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.

  3. DrahtBot renamed this:
    doc: Add fuzz based coverage report generation
    doc: Add fuzz based coverage report generation
    on Apr 2, 2025
  4. DrahtBot added the label Docs on Apr 2, 2025
  5. instagibbs commented at 7:00 pm on April 2, 2025: member
    I just went through figuring this out myself, will review
  6. 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 like build_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-lcov

    But if others agree with this , I will change it and might have to retouch the llvm/clang based coverage report generation too then .

  7. 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.
  8. 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 the runs=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 the libfuzzer and now using the test_runner. I have opened #32209 to fix the path issue while generating profraw using the runner .
  9. maflcko approved
  10. instagibbs commented at 7:22 pm on April 2, 2025: member
    works for me at least
  11. mabu44 commented at 9:13 pm on April 2, 2025: none
    Interesting. 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?
  12. Prabhat1308 force-pushed on Apr 3, 2025
  13. Prabhat1308 commented at 6:58 am on April 3, 2025: contributor

    Interesting. 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 ?

  14. maflcko commented at 7:32 am on April 3, 2025: member

    Interesting. 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.

  15. Prabhat1308 force-pushed on Apr 3, 2025
  16. in 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.
  17. maflcko approved
  18. maflcko commented at 7:56 am on April 3, 2025: member
    lgtm
  19. in 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)
  20. mabu44 commented at 4:17 pm on April 3, 2025: none

    I 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.

  21. Prabhat1308 commented at 4:19 pm on April 3, 2025: contributor

    I 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.

  22. Prabhat1308 force-pushed on Apr 3, 2025
  23. fanquake referenced this in commit 65dcbec756 on Apr 4, 2025
  24. mabu44 commented at 4:32 pm on April 4, 2025: none
    Tested ACK 085bd71721a6749d342dd62e886a4f8e505dc5fd Tested on this branch merged with master because of the dependency from #32209
  25. kevkevinpal commented at 1:26 pm on April 5, 2025: contributor

    Tested ACK 5dcbec

    I tested these commands on master, running the fuzz tests against (qa-assets) and inspecting the coverage reports

  26. Prabhat1308 force-pushed on Apr 7, 2025
  27. DrahtBot added the label CI failed on Apr 7, 2025
  28. DrahtBot 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.

  29. Add fuzz test coverage report generation
    Signed-off-by: Prabhat Verma <prabhatverma329@gmail.com>
    7677fde4c7
  30. Prabhat1308 force-pushed on Apr 7, 2025
  31. maflcko commented at 10:34 am on April 7, 2025: member
    lgtm ACK 7677fde4c788bc3051edfe81b3b564388b2d5da1
  32. DrahtBot requested review from mabu44 on Apr 7, 2025
  33. DrahtBot removed the label CI failed on Apr 7, 2025
  34. mabu44 commented at 6:54 pm on April 7, 2025: none
    Tested 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.
  35. ryanofsky assigned ryanofsky on Apr 8, 2025
  36. ryanofsky merged this on Apr 8, 2025
  37. ryanofsky closed this on Apr 8, 2025

  38. 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