build: RFC Coverage build type #31047

issue fanquake openend this issue on October 7, 2024
  1. fanquake commented at 11:01 am on October 7, 2024: member

    Porting this issue from: https://github.com/hebasto/bitcoin/issues/341.

    It’s not clear if using our Coverage build type works with Clang, or not. In the linked thread there are simultaneous claims that it “works”, but also that it does not work. It seems from the discussion that even using GCC with the Coverage build type is flaky.

    There’s been suggestions to add a Coverage mode for Clang: https://github.com/hebasto/bitcoin/pull/233, however adding a second way of doing things, when the current way may not work properly, and (likely?) currently isn’t being used by anyone, doesn’t seem like a good approach, and adds even more complication to the build system. Note that devs are already using Clang for coverage regardless of if it currently exists in the build system (cc @dergoegge, @marcofleon, @vasild).

    If using Clangs native coverage mode is what is preferred/is “better”, then it’d seem better for us to replace the current, more GCC focussed implementation, with one that is geared towards LLVM/Clang, if the idea is to provide something that works out-of-the-box, and is generally used by the developers working on the project.

  2. fanquake added the label Brainstorming on Oct 7, 2024
  3. fanquake added the label Build system on Oct 7, 2024
  4. dergoegge commented at 11:28 am on October 7, 2024: member

    I haven’t tried after the transition to CMake but I always had trouble with the build in coverage build mode (e.g. make cov). Usually there was problem with path prefixes or a lcov version mismatch. The coverage reports themselves were always hard to read and I could not find any documentation on how to read them, for example:

    Screenshot from 2024-10-07 12-18-00

    How is branch coverage supposed to be interpreted from this?

    I’ve switched all my coverage reports to using LLVM’s native coverage tooling: https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#id3. I don’t use our build in command to produce coverage reports but I think it’s a useful thing to have, so devs can easily produce their own coverage reports. I can recommend switching to LLVM’s native coverage reporting, as that has been far less of a struggle in my experience.

  5. vasild commented at 11:32 am on October 7, 2024: contributor

    For me it seems best to use each compiler’s native workflow. That is: gcov+lcov if using GCC and the source based coverage if using Clang.

    Would be nice if that is integrated into the build system so it boils down to just cmake --i-want-coverage, but if not, then some good instructions in doc/ would be fine as well.

  6. fanquake commented at 11:36 am on October 7, 2024: member

    For me it seems best to use each compiler’s native workflow.

    Sure, anyone can still use whatever they like. However our build system doesn’t need to (and shouldn’t have to) handle all possible combinations of compiler + tooling. Providing a single convenience build type that is the same as what is used by the majority of the developers seems fine, however I don’t think we need to natively support anything outside of that, given it’s unlikely to be tested/used, or work properly, and just futher complicates our build system.

  7. vasild commented at 1:12 pm on October 7, 2024: contributor

    our build system doesn’t need to (and shouldn’t have to) handle all possible combinations of compiler + tooling

    It is just two:

    1. gcc compiler + gcov/lcov and
    2. clang compiler + llvm-cov (source based).

    (I think it is a waste of time and we should ignore a third option where we try to use the gcov/lcov workflow with clang).

    Providing a single convenience build type that is the same as what is used by the majority of the developers seems fine

    I do not object this, but then if only 1. or only 2. is supported then the coverage build should produce an error if the compiler is the other one.

  8. fanquake commented at 1:17 pm on October 7, 2024: member

    It is just two:

    It’s not though, because gcc compiler + gcov/lcov already encompasses more than 1 thing. i.e supporting different versions of gcov/lcov across releases.

  9. marcofleon commented at 5:40 pm on October 7, 2024: contributor
    I don’t have a strong preference for which one is supported by the build system. I happened to learn about llvm-cov before coming upon the coverage section in the docs, so that’s what I’ve been using on both macOS and Linux. It’s been working well for me so far.
  10. fanquake commented at 8:55 am on October 10, 2024: member
    cc @maflcko. Do you have an opinion here?
  11. maflcko commented at 11:54 am on October 10, 2024: member
    I liked the summary stats per folder produced by the current approach, but if others like other stuff better, I don’t mind.
  12. dergoegge commented at 12:54 pm on October 10, 2024: member

    I liked the summary stats per folder produced by the current approach, but if others like other stuff better, I don’t mind.

    The LLVM approach also supports that:

    Screenshot from 2024-10-10 13-52-58

    edit: this is possible with the -show-directory-coverage option for llvm-cov show.

  13. vasild commented at 3:43 pm on October 10, 2024: contributor

    I liked the summary stats per folder produced by the current approach, but if others like other stuff better, I don’t mind.

    Maybe I misunderstand, but somehow the above implies that we have to commit to one and give up on the other method. But one can create gcc or clang coverages regardless of what is integrated into the cmake build system. That integration would be just a convenience and we could have no integration, or just gcc (the current master), or just clang or both gcc and clang.

  14. maflcko commented at 4:27 pm on October 10, 2024: member
    Yeah. I guess I don’t really mind either way. As long as there is at least one documented, tested and working solution.
  15. maflcko commented at 11:51 am on October 28, 2024: member
    I switched https://maflcko.github.io/b-c-cov/ to GCC, just to get something working again. But the config is open source and pull requests with changes are welcome: https://github.com/maflcko/b-c-cov
  16. fanquake commented at 11:58 am on November 19, 2024: member

    Apparently code coverage on oss-fuzz has been broken since CMake. See https://issues.oss-fuzz.com/issues/379122777. Full logs (https://oss-fuzz-build-logs.storage.googleapis.com/log-6b12c5ff-d6c4-441c-a10f-2dffff96e82c.txt):

     0Step [#7](/bitcoin-bitcoin/7/): error: /workspace/out/libfuzzer-coverage-x86_64/src/bitcoin-core/build_fuzz/src/wallet/wallet/walletdb.h: No such file or directory
     1Step [#7](/bitcoin-bitcoin/7/): warning: The file '/src/bitcoin-core/build_fuzz/src/wallet/wallet/walletdb.h' isn't covered.
     2Step [#7](/bitcoin-bitcoin/7/): error: /workspace/out/libfuzzer-coverage-x86_64/src/bitcoin-core/build_fuzz/src/wallet/wallet/rpc/wallet.cpp: No such file or directory
     3Step [#7](/bitcoin-bitcoin/7/): warning: The file '/src/bitcoin-core/build_fuzz/src/wallet/wallet/rpc/wallet.cpp' isn't covered.
     4Step [#7](/bitcoin-bitcoin/7/): error: /workspace/out/libfuzzer-coverage-x86_64/src/bitcoin-core/build_fuzz/src/wallet/util/task_runner.h: No such file or directory
     5Step [#7](/bitcoin-bitcoin/7/): warning: The file '/src/bitcoin-core/build_fuzz/src/wallet/util/task_runner.h' isn't covered.
     6Step [#7](/bitcoin-bitcoin/7/): warning: 60 functions have mismatched data
     7Step [#7](/bitcoin-bitcoin/7/): [2024-11-17 06:58:59,574 DEBUG] Finished generating per-file code coverage summary.
     8Step [#7](/bitcoin-bitcoin/7/): [2024-11-17 06:58:59,574 DEBUG] Generating file view html index file as: "/workspace/out/libfuzzer-coverage-x86_64/report/linux/file_view_index.html".
     9Step [#7](/bitcoin-bitcoin/7/): Traceback (most recent call last):
    10Step [#7](/bitcoin-bitcoin/7/):   File "/opt/code_coverage/coverage_utils.py", line 829, in <module>
    11Step [#7](/bitcoin-bitcoin/7/):     sys.exit(Main())
    12Step [#7](/bitcoin-bitcoin/7/):   File "/opt/code_coverage/coverage_utils.py", line 823, in Main
    13Step [#7](/bitcoin-bitcoin/7/):     return _CmdPostProcess(args)
    14Step [#7](/bitcoin-bitcoin/7/):   File "/opt/code_coverage/coverage_utils.py", line 780, in _CmdPostProcess
    15Step [#7](/bitcoin-bitcoin/7/):     processor.PrepareHtmlReport()
    16Step [#7](/bitcoin-bitcoin/7/):   File "/opt/code_coverage/coverage_utils.py", line 577, in PrepareHtmlReport
    17Step [#7](/bitcoin-bitcoin/7/):     self.GenerateFileViewHtmlIndexFile(per_file_coverage_summary,
    18Step [#7](/bitcoin-bitcoin/7/):   File "/opt/code_coverage/coverage_utils.py", line 450, in GenerateFileViewHtmlIndexFile
    19Step [#7](/bitcoin-bitcoin/7/):     self.GetCoverageHtmlReportPathForFile(file_path),
    20Step [#7](/bitcoin-bitcoin/7/):   File "/opt/code_coverage/coverage_utils.py", line 422, in GetCoverageHtmlReportPathForFile
    21Step [#7](/bitcoin-bitcoin/7/):     assert os.path.isfile(
    22Step [#7](/bitcoin-bitcoin/7/): AssertionError: "/src/bitcoin-core/build_fuzz/crc32c/include/crc32c/crc32c.h" is not a file.
    23Step [#7](/bitcoin-bitcoin/7/): ********************************************************************************
    24Step [#7](/bitcoin-bitcoin/7/): Code coverage report generation failed.
    25Step [#7](/bitcoin-bitcoin/7/): To reproduce, run:
    26Step [#7](/bitcoin-bitcoin/7/): python infra/helper.py build_image bitcoin-core
    27Step [#7](/bitcoin-bitcoin/7/): python infra/helper.py build_fuzzers --sanitizer coverage bitcoin-core
    28Step [#7](/bitcoin-bitcoin/7/): python infra/helper.py coverage bitcoin-core
    29Step [#7](/bitcoin-bitcoin/7/): ********************************************************************************
    30Finished Step [#7](/bitcoin-bitcoin/7/)
    31ERROR
    
  17. fanquake added this to the milestone 29.0 on Nov 25, 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: 2024-12-21 15:12 UTC

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