[POC] cmake: Introduce LLVM’s Source-based Code Coverage reports #31394

1. 
   hebasto commented at 4:54 pm on November 29, 2024: member

   The gcov-based code coverage does not work with Clang (please refer to #31047 for more details).

   This PR employs the LLVM’s Source-based Code Coverage.

   Here are some examples of usage:

   0cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DBUILD_FOR_COVERAGE=ON
   1cmake --build build --target total_coverage
   2firefox build/test_bitcoin.coverage/index.html
   

   or

   0cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DBUILD_FOR_COVERAGE=ON
   1cmake --build build --target total_coverage
   2firefox build/total.coverage/index.html
   

   or

   0cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DBUILD_FOR_COVERAGE=ON -DBUILD_FOR_FUZZING=ON
   1cmake --build build --target fuzz_coverage
   2firefox build/fuzz.coverage/index.html
   

   ---

   As a proof of concept, the new targets still lack the ability to customize llvm-cov options.

   I haven’t yet assessed the quality of the resulting coverage reports. However, messages like this:

   0warning: 502 functions have mismatched data
   

   are quite concerning.

   ---

   Also see a discussion in https://github.com/hebasto/bitcoin/pull/233.

2. cmake: Introduce `BUILD_FOR_COVERAGE` build option

   The new `BUILD_FOR_COVERAGE` build option enables instrumentation for
   LLVM's Source-based Code Coverage reports.

   68108fa8b5
3. cmake: Add `test_bitcoin.coverage` target

   warning: 502 functions have mismatched data
   - test_bitcoin: 2
   - bitcoin-util: 329
   - bitcoin-tx: 171

   55014d1ae2
4. cmake: Add `total_coverage` target 30e25a7fb9
5. cmake: Add `fuzz_coverage` target e82cc06990
6. hebasto added the label Build system on Nov 29, 2024
7. hebasto added the label Tests on Nov 29, 2024
8. 
   DrahtBot commented at 4:54 pm on November 29, 2024: 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/31394.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #31884 (cmake: Make implicit libbitcoinkernel dependencies explicit by hebasto)
   • #31880 (cmake: Add optional source files to libraries directly by hebasto)
   • #31807 (kernel: Avoid duplicating symbols in the kernel and downstream users by theuni)
   • #31802 (Add bitcoin-{node,gui} to release binaries for IPC by Sjors)
   • #31741 (multiprocess: Add libmultiprocess git subtree by ryanofsky)
   • #31425 (RFC: Riscv bare metal CI job by TheCharlatan)
   • #31268 (cmake: add optional source files to bitcoin_crypto and crc32c directly by purpleKarrot)
   • #31161 (cmake: Set top-level target output locations by hebasto)
   • #30975 (ci: build multiprocess on most jobs by Sjors)

   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.

9. 
   dergoegge commented at 10:35 am on December 2, 2024: member

   Would we keep supporting generating coverage with gcc?
10. 
    theuni commented at 7:04 pm on December 3, 2024: member

    I haven’t played with coverage in CMake much, but this seems like a more reasonable approach than the build type.

    Would we keep supporting generating coverage with gcc?

    I don’t think we should keep both approaches. If we do want to continue to support gcc, IMO it should be migrated to this setting rather than keeping it as a build type.

    But.. there’s a lot of duplicated/hard-coded logic here. Can we not deduce how to filter/what to run based on target properties?

11. 
    maflcko commented at 8:06 am on December 13, 2024: member

    Would we keep supporting generating coverage with gcc?

    I don’t think we should keep both approaches. If we do want to continue to support gcc, IMO it should be migrated to this setting rather than keeping it as a build type.

    If gcc support is removed, it would be good to make sure https://github.com/corecheck/corecheck/blob/d15e92618a519e125758a6294e85ee9d6ce89265/workers/coverage-worker/entrypoint.sh#L50 and other places that currently rely on it are taken care of.

12. theuni added this to the milestone 29.0 on Feb 11, 2025
13. 
    sipa commented at 4:52 pm on February 13, 2025: member

    I suggest treating the broken GCC-based coverage as a 29.0 blocker, as it seems it’s been effectively unusable anyway.

    Adding an LLVM-based coverage mechanism then seems like an independent improvement.

14. 
    maflcko commented at 8:11 am on February 14, 2025: member

    I suggest treating the broken GCC-based coverage as a 29.0 blocker, as it seems it’s been effectively unusable anyway.

    I don’t see why this should be a 29.0 blocker at all, or why GCC is unusable, given that it is used, for example, see the previous comment above: #31394 (comment)

    Let’s continue discussion in the thread #31047 (comment)?

15. 
    vasild commented at 10:45 am on February 14, 2025: contributor

    Would we keep supporting generating coverage with gcc?

    I don’t think we should keep both approaches.

    Why not? GCC is the native compiler on Linux, a widely used OS for developing Bitcoin Core.

16. maflcko removed this from the milestone 29.0 on Feb 17, 2025
17. 
    maflcko commented at 3:39 pm on February 17, 2025: member

    I’ve removed the milestone here for now, because this looks like it is adding a new feature that never existed before, so shouldn’t be a blocker.
18. DrahtBot added the label Needs rebase on Feb 18, 2025
19. 
    DrahtBot commented at 1:31 pm on February 18, 2025: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.

Contributors
hebasto DrahtBot dergoegge theuni maflcko sipa vasild

Labels
Build system Tests Needs rebase