search

hebasto / bitcoin

Bitcoin Core integration/staging tree
https://bitcoincore.org/en/download
MIT License
21 stars 5 forks source link

fuzz coverage fails with clang? #341

Closed maflcko closed 4 days ago

maflcko commented 2 months ago

I get:

/filter-lcov.py", line 19, in <module>
    with open(tracefile, 'r', encoding="utf8") as f:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: 'fuzz.info'
Combining tracefiles.
lcov: ERROR: (missing) 'fuzz_filtered.info' found from pattern 'fuzz_filtered.info' is not a readable file
    (use "lcov --ignore-errors missing ..." to bypass this error)
lcov: ERROR: (missing) 'baseline_filtered.info' found from pattern 'baseline_filtered.info' is not a readable file
    (use "lcov --ignore-errors missing ..." to bypass this error)
genhtml: ERROR: (missing) 'fuzz_coverage.info' found from pattern 'fuzz_coverage.info' is not a readable file
    (use "genhtml --ignore-errors missing ..." to bypass this error)

I used https://github.com/maflcko/DrahtBot/commit/1b699e8268970e552634029c4093c82327895df6

I can try to add easier steps to reproduce tomorrow.

hebasto commented 2 months ago

What are the OS and the lcov version?

maflcko commented 2 months ago

Either Ubuntu 24.04 or 24.10 should reproduce:

git checkout fa680ac6c016533ac512857647555c688c071b63 && mkdir -p empty_temp && cmake -B bld -DBUILD_FOR_FUZZING=ON -DSANITIZERS=fuzzer -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_BUILD_TYPE=Coverage && cmake --build ./bld -j$( nproc ) && cmake -DJOBS=$( nproc ) -DFUZZ_SEED_CORPUS_DIR=$PWD/empty_temp -DLCOV_OPTS='--rc branch_coverage=1 --ignore-errors mismatch,inconsistent' -P bld/CoverageFuzz.cmake

maflcko commented 2 months ago

The error for 24.04 is slightly different:


Processing src/util/CMakeFiles/bitcoin_util.dir/fs_helpers.cpp.gcda
Processing src/util/CMakeFiles/bitcoin_util.dir/strencodings.cpp.gcda
Processing src/util/CMakeFiles/bitcoin_util.dir/signalinterrupt.cpp.gcda
Processing src/util/CMakeFiles/bitcoin_util.dir/fs.cpp.gcda
Processing src/util/CMakeFiles/bitcoin_util.dir/exception.cpp.gcda
Processing src/util/CMakeFiles/bitcoin_util.dir/bip32.cpp.gcda
Processing src/util/CMakeFiles/bitcoin_util.dir/serfloat.cpp.gcda
Processing src/util/CMakeFiles/bitcoin_util.dir/time.cpp.gcda
Processing src/util/CMakeFiles/bitcoin_util.dir/feefrac.cpp.gcda
Processing src/util/CMakeFiles/bitcoin_util.dir/check.cpp.gcda
geninfo: ERROR: unable to open /b-c/bld/src/src/httpserver.cpp: No such file or directory
    (use "geninfo --ignore-errors source ..." to bypass this error)
Deleting all .da files in src and subdirectories
Done.
lcov: ERROR: trace file 'fuzz_filtered.info' is empty
    (use "lcov --ignore-errors empty ..." to bypass this error)
Combining tracefiles.
.. found 1 files to aggregate.
Merging fuzz_filtered.info..0 remaining
lcov: ERROR: trace file 'baseline_filtered.info' is empty
    (use "lcov --ignore-errors empty ..." to bypass this error)
genhtml: ERROR: cannot read file fuzz_coverage.info!
maflcko commented 2 months ago

Also, the exit code may not be passed up, which could be improved as well?


# echo $?
0
maflcko commented 2 months ago

Normal coverage also fails.

What is the commit and the exact commands where it last worked?

hebasto commented 2 months ago

Normal coverage also fails.

What is the commit and the exact commands where it last worked?

Working on it.

hebasto commented 2 months ago

Normal coverage also fails.

What is the commit and the exact commands where it last worked?

Tested https://github.com/bitcoin/bitcoin/pull/30454 @ 41051290ab3b6c36312cec26a27f787cea9961b4 on Ubuntu 24.04 with the default gcc and lcov packages:

$ cmake -B build -DCMAKE_BUILD_TYPE=Coverage -DCMAKE_C_FLAGS="-fprofile-update=atomic" -DCMAKE_CXX_FLAGS="-fprofile-update=atomic"
$ cmake --build build
$ cmake -DLCOV_OPTS="--ignore-errors mismatch" -P build/Coverage.cmake
$ firefox ./build/test_bitcoin.coverage/index.html
$ firefox ./build/total.coverage/index.html

However, on my machine, I had to add --timeout-factor=4.

Additional -DCMAKE_C_FLAGS="-fprofile-update=atomic" became required after https://github.com/hebasto/bitcoin/pull/192.

maflcko commented 2 months ago

Ah, so normal coverage may work with gcc. However, it would be nice if clang was supported as well (like it does with autotools).

hebasto commented 2 months ago

cc @vasild

hebasto commented 2 months ago

Ah, so normal coverage may work with gcc. However, it would be nice if clang was supported as well (like it does with autotools).

Tested https://github.com/bitcoin/bitcoin/pull/30454 @ 41051290ab3b6c36312cec26a27f787cea9961b4 on Ubuntu 24.10 with the default clang and lcov packages:

$ cmake -B build -DCMAKE_BUILD_TYPE=Coverage -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
$ cmake --build build
$ cmake -DLCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch,inconsistent" -P build/Coverage.cmake

On Ubuntu 24.04 lcov still randomly fails.

maflcko commented 2 months ago

Did you pass -DJOBS=$( nproc )?

hebasto commented 2 months ago

Did you pass -DJOBS=$( nproc )?

I did. Why?

maflcko commented 2 months ago

Because I couldn't find it in your commands to reproduce the failure, and it seems to be required locally to reproduce it.

I'll try to create a full testing matrix to see what works and what needs fixing.

vasild commented 2 months ago

Clang has its own coverage workflow, which produces superior results. It also supports gcc/gcov/lcov like workflow but I think it is a waste of time* to try to fix that (if it is broken) to force Clang through the gcc way, provided it has a better native way.

https://github.com/hebasto/bitcoin/pull/233 implements the Clang native workflow.

* my time, of course; ymmv

maflcko commented 2 months ago

https://github.com/hebasto/bitcoin/issues/341#issuecomment-2310803071 claims that clang is working.

However, I can not get it to work with the same commands (there is no html generated). Can you double check or otherwise upload your html, or an excerpt from it?

https://github.com/hebasto/bitcoin/issues/341#issuecomment-2312165659 claims that the clang workflow is fixed in a pull request. However, the pull request looks outdated and probably needs rebase?

hebasto commented 2 months ago

#341 (comment) claims that clang is working.

However, I can not get it to work with the same commands (there is no html generated). Can you double check or otherwise upload your html, or an excerpt from it?

total_coverage.tar.gz

vasild commented 2 months ago

https://github.com/hebasto/bitcoin/issues/341#issuecomment-2312165659 claims that the clang workflow is fixed in a pull request. However, the pull request looks outdated and probably needs rebase?

s/fixed/implemented/, Clang workflow is implemented in https://github.com/hebasto/bitcoin/pull/233, but it was NACKed by @fanquake. Having the native Clang workflow is not a blocker for the CMake switch, so I stopped working on it. Maybe I will find some motivation to come back to it after https://github.com/bitcoin/bitcoin/pull/30454 is merged.

maflcko commented 2 months ago

#341 (comment) claims that clang is working. However, I can not get it to work with the same commands (there is no html generated). Can you double check or otherwise upload your html, or an excerpt from it?

total_coverage.tar.gz

Interesting. I guess it is flaky and this happened to pass for some reason.

I'll try to switch everything to G++, for now, as a workaround.

maflcko commented 2 months ago

I tried to use GCC (on the autotools branch), but it had many problems:

I can try GCC on the cmake branch, but I'd be surprised if cmake magically fixed those issues.

This means that after cmake is merged as-is, it likely won't be possible to generate a useful coverage report.

vasild commented 2 months ago

I didn't try GCC, but Clang works quite smooth with autotools or cmake (as per #233), a downside is that it is not integrated into either build system so it requires manual fiddling with compiler flags.

vasild commented 2 months ago

@maflcko, from what you say it follows that you can't generate a useful coverage report with GCC on the autotools branch as well, right?

hebasto commented 2 months ago

I tried to use GCC (on the autotools branch), but it had many problems:

Last time I tested https://github.com/bitcoin/bitcoin/pull/30075, which also uses __gcov_reset, it worked for me.

I can try GCC on the cmake branch, but I'd be surprised if cmake magically fixed those issues.

Of course, CMake won't fix other tools' bugs :)

This means that after cmake is merged as-is, it likely won't be possible to generate a useful coverage report.

A proper support for coverage report generation using clang should be a priority follow up, right?

maflcko commented 2 months ago

Last time I tested bitcoin#30075, which also uses __gcov_reset, it worked for me.

Ok, I'll try a bit more.

A proper support for coverage report generation using clang should be a priority follow up, right?

Yeah, I'd say some kind of solution should happen before the autotools removal.

In theory a complete re-work can be considered, maybe along https://github.com/bitcoin/bitcoin/pull/28772 , but personally I don't care too much, as long as I have a single, reasonable good way to get coverage reports.

maflcko commented 2 months ago

Last time I tested bitcoin#30075, which also uses __gcov_reset, it worked for me.

Ok, I'll try a bit more.

I tried commit 6f1d9064381d834b, which is one before the pull request and it did not work for me: https://drahtbot.space/host_reports/DrahtBot/reports/coverage_fuzz/monotree/6f1d9064381d834b/65ac5a6b0b3f11ad/fuzz.coverage/src/test/fuzz/banman.cpp.gcov.html

fanquake commented 2 months ago

Does anyone want to summarize the state of this, and migrate it to https://github.com/bitcoin/bitcoin/issues ?

vasild commented 2 months ago

reasonable good way to get coverage reports

Consider this, using Clang's source-based code coverage feature:

It’s called “source-based” because it operates on AST and preprocessor information directly. This allows it to generate very precise coverage data.

# Compile

cmake \
    -G "Ninja" \
    -DCMAKE_BUILD_TYPE=Debug \
    -DAPPEND_CXXFLAGS="-fprofile-instr-generate -fcoverage-mapping -fPIE" \
    -DAPPEND_LDFLAGS="-fprofile-instr-generate -fcoverage-mapping" \
    -S /tmp/source -B /tmp/build

cmake --build /tmp/build -j32

# Run tests

mkdir /tmp/coverage
export LLVM_PROFILE_FILE=/tmp/coverage/%m_%p.profraw

/tmp/build/src/test/test_bitcoin
/tmp/build/test/functional/test_runner.py

# Collect coverage and create a HTML report

llvm-profdata-devel merge /tmp/coverage/*.profraw --output=/tmp/coverage/all.profdata
llvm-cov-devel show -format=html -output-dir=/tmp/coverage/result \
    -Xdemangler=/usr/local/llvm-devel/bin/llvm-cxxfilt -instr-profile=/tmp/coverage/all.profdata \
    -object=/tmp/build/src/test/test_bitcoin \
    -object=/tmp/build/src/bitcoind

Works like a charm and creates better reports than GCC's (e.g. it expands templates, which GCC coverage does not, at least last time I played with it). I tried to integrate this in the build system in https://github.com/hebasto/bitcoin/pull/233 to reduce the above commands to just a few cmake invocations, maybe there are better ways to integrate it, but even if not integrated it is not too difficult to run the above commands.

fanquake commented 2 months ago

-DAPPEND_CXXFLAGS="-fprofile-instr-generate -fcoverage-mapping -fPIE"

Why do you need to add -fPIE here? CMake should be doing this automatically?

hebasto commented 2 months ago

@maflcko

Can you re-confirm please that you cannot generate a coverage report on Ubuntu 24.10 using clang 18.0 and lcov 2.1?

maflcko commented 2 months ago

Yes, the error remains. I see that you claim to be able to generate them in https://github.com/hebasto/bitcoin/issues/341#issuecomment-2310803071, but it would be good to have steps to reproduce from a fresh install of the distro:

export DEBIAN_FRONTEND=noninteractive && apt update && apt install llvm lcov git vim ccache clang build-essential cmake pkg-config python3 libevent-dev libboost-dev cmake -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./b-c && cd b-c && cmake -B ./bld-cmake -DBUILD_FOR_FUZZING=ON -DSANITIZERS=fuzzer -DCMAKE_BUILD_TYPE=Coverage -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DENABLE_WALLET=OFF && cmake --build ./bld-cmake --parallel $( nproc ) && mkdir empty_dir && cmake -DJOBS=$( nproc ) -DFUZZ_SEED_CORPUS_DIR=$PWD/empty_dir -DLCOV_OPTS='--rc branch_coverage=1 --ignore-errors mismatch,inconsistent' -P ./bld-cmake/CoverageFuzz.cmake


versionbits                              #2 DONE   cov: 30 ft: 31 corp: 1/1b lim: 4 exec/s: 0 rss: 64Mb
Capturing coverage data from src
geninfo cmd: '/usr/bin/geninfo src --output-filename fuzz.info --test-name fuzz-tests --gcov-tool /b-c/bld-cmake/cov_tool_wrapper.sh --ignore-errors mismatch --ignore-errors inconsistent --rc branch_coverage=1 --branch-coverage'
Found LLVM gcov version 18.1.8, which emulates gcov version 4.2.0
Using intermediate gcov format
Recording 'internal' directories:
    /b-c/bld-cmake/src
Writing temporary data to /tmp/geninfo_datQ3gO
Scanning src for .gcda files ...
geninfo: WARNING: (format) invalid characters removed from testname
    (use "geninfo --ignore-errors format,format ..." to suppress this warning)
Found 336 data files in src
using: chunkSize: 1, nchunks:336, intervalLength:16
geninfo: WARNING: (gcov) GCOV did not produce any data for /b-c/bld-cmake/src/secp256k1/src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult.c.gcda
    (use "geninfo --ignore-errors gcov,gcov ..." to suppress this warning)
geninfo: WARNING: (gcov) GCOV did not produce any data for /b-c/bld-cmake/src/secp256k1/src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult_gen.c.gcda
geninfo: WARNING: (gcov) GCOV did not produce any data for /b-c/bld-cmake/src/CMakeFiles/bitcoin_node.dir/policy/settings.cpp.gcda
Finished processing 336 GCDA files
Apply filtering..
Message summary:
  1 error message:
    source: 1
  4 warning messages:
    format: 1
    gcov: 3
geninfo: ERROR: (source) unable to open /b-c/bld-cmake/src/src/index/blockfilterindex.cpp: No such file or directory
    (use "geninfo --ignore-errors source ..." to bypass this error)
Deleting all .da files in src and subdirectories
Done.
Message summary:
  no messages were reported
Traceback (most recent call last):
  File "/b-c/bld-cmake/./filter-lcov.py", line 19, in <module>
    with open(tracefile, 'r', encoding="utf8") as f:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: 'fuzz.info'
lcov: ERROR: (missing) 'fuzz_filtered.info' found from pattern 'fuzz_filtered.info' is not a readable file
    (use "lcov --ignore-errors missing ..." to bypass this error)
Combining tracefiles.
lcov: ERROR: (missing) 'baseline_filtered.info' found from pattern 'baseline_filtered.info' is not a readable file
    (use "lcov --ignore-errors missing ..." to bypass this error)
genhtml: ERROR: (missing) 'fuzz_coverage.info' found from pattern 'fuzz_coverage.info' is not a readable file
    (use "genhtml --ignore-errors missing ..." to bypass this error)
hebasto commented 2 months ago

From Professional CMake: A Practical Guide 19th Edition:

In practice, using gcov-based coverage data with clang can be challenging. Due to the way paths are written to the coverage files, and the way those paths are handled between gcovr and llvm-cov, the processing will often fail.

vasild commented 2 months ago

-DAPPEND_CXXFLAGS="-fprofile-instr-generate -fcoverage-mapping -fPIE"

Why do you need to add -fPIE here? CMake should be doing this automatically?

Aha! Sharp eyes, I was wondering if somebody is going to note this. It is a bit off-topic wrt the coverage, but I will elaborate since you ask. So, at least in my environment, compiling without pie and linking with pie sometimes works and sometimes does not work:

  1. for the most simple program main() return 0, compile without pie, link with -fPIE -pie works
  2. for a more involved program that has a class inside: linking fails with ld: error: relocation R_X86_64_32S cannot be used against local symbol; recompile with -fPIC and the solution is to compile with -fPIE
  3. even the most simple prog fails to link with the same error if -fprofile-instr-generate -fcoverage-mapping is used for compiling and linking

Now, we use include(CheckPIESupported) to ask CMake to check for PIE for us. What it does is to create some simple program, compile it without pie and try to link with -fPIE -pie. That succeeds normally (1. from above). However if I add -fprofile-instr-generate -fcoverage-mapping to compile and link flags, then CMake's check for PIE fails due to 3.

I am not sure if this is something we should address or if it is a deficiency in CMake (maybe it should use -fPIE when compiling the test program which it will try to link with -fPIE -pie?). In anyway, a simple solution is to force -fPIE to the compile flags like I did above.

maflcko commented 2 months ago

In practice, using gcov-based coverage data with clang can be challenging. Due to the way paths are written to the coverage files, and the way those paths are handled between gcovr and llvm-cov, the processing will often fail.

Hm, interesting. Just odd that it worked with autotools for years and for some reason cmake surfaced those errors.

fanquake commented 2 months ago

So, at least in my environment,

What is your environment?

I am not sure if this is something we should address or if it is a deficiency in CMake

Did you need to pass -fPIE previously with autotools? If no, then this is a regression that we should fix regardless of if it's a bug in CMake.

Outside of that, it sounds like it could be a bug in CMake, which we should report upstream, or, is it's PIE check only meant to work sometimes?

Hm, interesting. Just odd that it worked with autotools for years and for some reason cmake surfaced those errors.

Yea, seems odd that something wouldn't work with CMake. Especially since the book quoted seems to claim that the issue is contained between "gcov-based coverage data with clang " (i.e even if there may be issues between the tools, this isn't some fundamental CMake limitation).

vasild commented 2 months ago

What is your environment?

FreeBSD with Clang 20. It is easy to test how widespread this is:

echo 'int main(int, char**) { return 0; }' > a.cc
c++ -fprofile-instr-generate -fcoverage-mapping -c a.cc -o a.o
c++ -fprofile-instr-generate -fcoverage-mapping a.o -o a -fPIE -pie
ld: error: relocation R_X86_64_32S cannot be used against local symbol; recompile with -fPIC
>>> defined in a.o
>>> referenced by a.cc
>>>               a.o:(main)

# then remove -fprofile-instr-generate -fcoverage-mapping from the above commands and see that it works

Did you need to pass -fPIE previously with autotools?

No, for autotools I used F="-fprofile-instr-generate -fcoverage-mapping" CXXFLAGS=${F} LDFLAGS=${F} autogen configure make.

fanquake commented 2 months ago

It is easy to test how widespread this is:

Do you mean this happens on lots of other systems? I tried your steps from above (swapped c++ for clang++):

echo 'int main(int, char**) { return 0; }' > a.cc
clang++ -fprofile-instr-generate -fcoverage-mapping -c a.cc -o a.o
clang++ -fprofile-instr-generate -fcoverage-mapping a.o -o a -fPIE -pie

on two different systems (Ubuntu & Fedora), on two different architectures (x86_64 & aarch64), with Clang 18 (Ubuntu clang version 18.1.3 (1ubuntu1), clang version 18.1.8 (Fedora 18.1.8-3.fc41)) and it compiled both times.

No, for autotools I used

Ok, so this is a regression we should track/fix, and possibly report upstream. I'll open an issue in the main repo.

hebasto commented 2 months ago

Also, the exit code may not be passed up, which could be improved as well?

# echo $?
0

Addressed in https://github.com/bitcoin/bitcoin/pull/30772.

fanquake commented 1 month ago

I retested these steps (using trixie), and it still seems broken? Will port this issue to bitcoin/bitcoin:

geninfo: WARNING: (gcov) GCOV did not produce any data for /b-c/bld-cmake/src/secp256k1/src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult.c.gcno
geninfo: WARNING: (gcov) GCOV did not produce any data for /b-c/bld-cmake/src/util/CMakeFiles/bitcoin_util.dir/__/sync.cpp.gcno
Finished processing 351 GCNO files
Apply filtering..
Message summary:
  1 error message:
    source: 1
  7 warning messages:
    gcov: 6
    inconsistent: 1
geninfo: WARNING: (inconsistent) "/b-c/bld-cmake/src/test/fuzz/src/test/fuzz/deserialize.cpp":0:  function __cxx_global_var_init found on line but no corresponding 'line' coverage data point.  Cannot derive function end line.  See lcovrc man entry for 'derive_function_end_line'.
    (use "geninfo --ignore-errors inconsistent,inconsistent ..." to suppress this warning)
geninfo: ERROR: (source) unable to open /b-c/bld-cmake/src/test/fuzz/src/test/fuzz/deserialize.cpp: No such file or directory
    (use "geninfo --ignore-errors source ..." to bypass this error)
CMake Error at bld-cmake/CoverageInclude.cmake:45 (execute_process):
  execute_process failed command indexes:

    1: "Child return code: 1"

Call Stack (most recent call first):
  bld-cmake/CoverageFuzz.cmake:5 (include)
maflcko commented 1 month ago

I think I'll try GCC again. Was https://github.com/bitcoin/bitcoin/pull/30772 meant to fix the missing coverage (https://github.com/hebasto/bitcoin/issues/341#issuecomment-2314339110)?

fanquake commented 3 weeks ago

https://github.com/bitcoin/bitcoin/issues/31047

maflcko commented 3 weeks ago

I think I'll try GCC again. Was bitcoin#30772 meant to fix the missing coverage (#341 (comment))?

Yeah, looks like it is working now for some reason: https://drahtbot.space/host_reports/DrahtBot/reports/coverage_fuzz/monotree/e569eb8d91739bf1/84cea7068728bc2c/fuzz.coverage/src/test/fuzz/buffered_file.cpp.gcov.html

However, the gcov_reset is still ignored, it seems: https://drahtbot.space/host_reports/DrahtBot/reports/coverage_fuzz/monotree/e569eb8d91739bf1/84cea7068728bc2c/fuzz.coverage/src/test/fuzz/addrman.cpp.gcov.html