fuzz coverage fails with clang? #341
Comments
|
What are the OS and the lcov version? |
|
Either Ubuntu 24.04 or 24.10 should reproduce:
|
|
The error for 24.04 is slightly different: |
|
Also, the exit code may not be passed up, which could be improved as well? |
|
Normal coverage also fails. What is the commit and the exact commands where it last worked? |
Working on it. |
Tested bitcoin#30454 @ 4105129 on Ubuntu 24.04 with the default However, on my machine, I had to add Additional |
|
Ah, so normal coverage may work with gcc. However, it would be nice if clang was supported as well (like it does with autotools). |
|
cc @vasild |
Tested bitcoin#30454 @ 4105129 on Ubuntu 24.10 with the default On Ubuntu 24.04 |
|
Did you pass |
I did. Why? |
|
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. |
|
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. #233 implements the Clang native workflow. * my time, of course; ymmv |
|
#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? #341 (comment) 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 #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 bitcoin#30454 is merged. |
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. |
|
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. |
|
@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? |
Last time I tested bitcoin#30075, which also uses
Of course, CMake won't fix other tools' bugs :)
A proper support for coverage report generation using clang should be a priority follow up, right? |
Ok, I'll try a bit more.
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 bitcoin#28772 , but personally I don't care too much, as long as I have a single, reasonable good way to get coverage reports. |
I tried commit 6f1d906, 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 |
|
Does anyone want to summarize the state of this, and migrate it to https://github.com/bitcoin/bitcoin/issues ? |
Consider this, using Clang's source-based code coverage feature:
# 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/bitcoindWorks 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 #233 to reduce the above commands to just a few |
Why do you need to add -fPIE here? CMake should be doing this automatically? |
|
Can you re-confirm please that you cannot generate a coverage report on Ubuntu 24.10 using clang 18.0 and lcov 2.1? |
|
Yes, the error remains. I see that you claim to be able to generate them in #341 (comment), but it would be good to have steps to reproduce from a fresh install of the distro:
|
|
From Professional CMake: A Practical Guide 19th Edition:
|
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:
Now, we use I am not sure if this is something we should address or if it is a deficiency in CMake (maybe it should use |
Hm, interesting. Just odd that it worked with autotools for years and for some reason cmake surfaced those errors. |
What is your environment?
Did you need to pass 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?
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). |
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
No, for autotools I used |
Do you mean this happens on lots of other systems? I tried your steps from above (swapped 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 -pieon two different systems (Ubuntu & Fedora), on two different architectures (x86_64 & aarch64), with Clang 18 (
Ok, so this is a regression we should track/fix, and possibly report upstream. I'll open an issue in the main repo. |
Addressed in bitcoin#30772. |
d9fcbfc build: Add `JOBS` variable support to `CoverageFuzz.cmake` script (Hennadii Stepanov) e7cf4a6 build: Add missed `-g` for "Coverage" build configuration (Hennadii Stepanov) fe2003a build: Add `COMMAND_ERROR_IS_FATAL` to every process in coverage scrips (Hennadii Stepanov) Pull request description: The first commit ensures early error catching. The second commit adds the `-g` flag that was missed during the migration from Autotools. This PR is intended to be tested with GCC compiler (as clang support is still under [scrutiny](hebasto#341)). Depending on the `lcov` version, additional flags `-DCMAKE_C_FLAGS="-fprofile-update=atomic" -DCMAKE_CXX_FLAGS="-fprofile-update=atomic"` may be required. ACKs for top commit: maflcko: review ACK d9fcbfc tdb3: cr re ACK d9fcbfc Tree-SHA512: 0998411dc1ccd60d7bd6b36f4e2881f699202c65dcc8c177b46380d0f255d291d9537f1dc6fb35478b632f3515d3484d8e7d2877126c57e3f02b21f90160f1eb
I get:
I used maflcko/DrahtBot@1b699e8
I can try to add easier steps to reproduce tomorrow.
The text was updated successfully, but these errors were encountered: