Merging lcov reports containing records with 0 coverage is problematic due to wrong/inconsistent parsing of records #327
Comments
ziad-fernride
changed the title
ziad-fernride
changed the title
ziad-fernride
changed the title
|
I just realized that wrong line parsing happens also in the case of templates and inline functions even if the LH != 0. Simply records containing inline/templated have different begin and end line numbers and overall line count when all functions are hit than when some of them aren't. Which creates a problem when merging lcov reports from different platforms/test routes. It seems the issue is from the compilers themselves, but I was wondering if the lcov tooling can work around that ? |
|
Thanks for the report and the testcase. I'll see if I can figure it out. With respect to different start/end lines: the tool uses : as the primary lookup key - function name is rather secondary. I am not sure what happens if you have the "same" function with different start lines - but I rather suspect that they will be treated and reported as distinct functions. If you have the same start line and a different end line: then you will get a error; if you ignore that error, then it selects either the smallest or largest end line (can't remember which). If start/end line accuracy turns out to be a problem (which platform and compiler/version?) - then a (somewhat hacky) solution would be to provide a callback which would present you the file, start and end lines, and expect to receive back a 'fixed' start and end line. Your callback could do whatever it liked. |
The files attached are coming from a gcc compiler and llvm compiler paths. The main problem is that both compilers have wrong parsing when the record/(inline, templated)function is part of the test executable but not actually touched by the test. For example, comments are reported as untested lines in the case of LH:0 records. Which is part of what makes the bazel integration of coverage painful. Bazel generates a report for each test and then merges them into one. So once you merge a report with a tested record and a report with the wrongly parsed one, you end up with comments showing up uncovered. This also goes for curly brackets etc. The filters help eliminate the problems caused by the weird behaviour described above. But now trying to merge the different reports from different compilers that seem to screw up the parsing in different ways, or have inconsistent parsing of the same files bring a new angle where the filters are also confused. |
|
@henry2cox any thoughts on this issue ? Do you think there is any measure that can be taken to make the function determination more consistent across reports (platforms) ? |
|
Ah - sorry for the delay. The issue of inconsistent data and what the tool should do about it is more complicated. Right now, it does <something> when data is inconsistent and the inconsistency is ignored. Whether that action is "correct" or a good idea - is somewhat open to debate. YMMV. |
Signed-off-by: Henry Cox <[email protected]>
|
I think, fixed - please give a try and let me know. |
|
will do this weekend ! :) |
|
Sorry for the late response 🙏 Unfortunately this does not seem to fix it. As you can see I am on the correct version ... I use this to merge: report1.datreport2.datmerged.dat |
|
the lcov data file format changed recently (still reads - but does not generate - the old format); the new format explicitly stores function alias data rather than re-deriving it. (It also stores MC/DC data...but that is an unrelated item.) I think that the result is correct...4 total functions, one covered. $ ./bin/lcov --summary merged.dat --ignore inconsistent --branch
Reading tracefile x.info.
lcov: WARNING: (inconsistent) "project/common/trace/tracer_singleton.cpp":11: function '_ZN5project5trace15TracerSingleton3getEv' is not hit but line 13 is.
To skip consistency checks, see the 'check_data_consistency' section in man lcovrc(5).
(use "lcov --ignore-errors inconsistent,inconsistent ..." to suppress this warning)
lcov: WARNING: (inconsistent) "project/common/trace/tracer_singleton.cpp":17: function '_ZN5project5trace15TracerSingleton11set_backendENSt3__110unique_ptrINS0_17TracerBackendBaseENS2_14default_deleteIS4_EEEE' is not hit but line 19 is.
lcov: WARNING: (inconsistent) "project/common/trace/tracer_singleton.cpp":22: function '_ZN5project5trace15TracerSingleton11get_backendEv' is not hit but line 24 is.
Summary coverage rate:
source files: 1
lines.......: 62.5% (5 of 8 lines)
functions...: 25.0% (1 of 4 functions)
branches....: no data found
Message summary:
3 warning messages:
inconsistent: 3Update: I noticed a bug such that, when the 'inconsistent' message is ignored and the data 'fixed', that the fixed data is incomplete. This results in a strange looking HTML report. I think I fixed the problem but need to do more testing. |
|
Sorry, that was not the best example ... here is another example: report1.datreport2.datmerged.datAnd this is how the report/code looks: 
|
|
I'm not quite sure what we expect to see from the above data. It looks like the two input files are from different versions of source code. The input data looks inconsistent - so it might not be too surprising to see odd output data. For example:
lcov keys on file/line to determine uniqueness - so it thinks that these are different functions which happen to have the same name. Maybe it ought to check that - and give errors, at least in C/C++ code where function names are supposed to be unique. This creates weirdness in the 'function detail' window - for example, because the summary table at the top says that there are 7 functions/4 hit - but the 'detail' table shows only 5 entries (...because there are 2 pairs which have the same name but different location, but alias in the table). 'visit' order (..somewhat random) determines which one appears. This could be fixed (say, to disambiguate the duplicate names) - but that is rather a hack that I'm not eager to introduce because I don't think it is necessary - or even possible - to consistently display inconsistent data. The next question is: what is the source of this input data? Why is it inconsistent? Which platform(s) and compiler version(s)?
|
|
It is the same source code with the same version ... We have two platforms (qcc and clang) and depending on the target platform, the execution naturally takes a platform-dependent path in the code. The two reports are generated in the same PR run (the same commit), and we want to merge them to get the overall coverage of the code. |
So in my experience so far, merging reports is always a messy process, even in the case of the same platform the same source code the same version, since compilers seem to not bother parsing files correctly if the files are part of the test executable without being accessed by the test. Curly brackets, comments and all sort of stuff show up as uncovered lines. If you merge such a report with another report where the same file is 100% covered, the merged report will not show the same file to be covered 100%. If we don't apply filters and even in some cases exclude some patterns, we would not have a chance of having 100% coverage even if we have a 100% coverage and we only use 1 platform. This imperfection is part of the reasons we have filters to begin with, no ? 😆 The case of two platforms just adds one degree of imperfection to the mix, which is that different platforms parse function beginning and end differently. All filters are handling situations that should not happen but here we are 😆 and this is just one more case of that, no? |
|
Yeah - you aren't wrong. I'm not sure how many inconsistencies you see, or whether they are clustered in some subset of files. If they are clustered, then you might be able to hack things - say, to create two entries for those files ("foo_gcc.cpp" and "foo_llvm.cpp"), to manage them separately (and then write your 'criteria' callback to handle/merge them). I think that the real trick is to think about what you are trying to achieve - what the numbers actually mean to you - and then we can think of a way to make that happen. (That is more-or-less how we ended up with the set of tool features we have.) In the meantime: I have another small set of updates to push - mostly related to additional consistency checking and actions to 'fix' the issues. |
|
Yeah the use-case of merging reports that are inconsistent is legitimate imho. Merging reports from different platforms is the only way one can have a complete coverage view of the code in a mono-repo intended to run on several targets. Separating code based on platforms is not always possible since much of the code is shared among multiple platforms. What you said earlier is correct, it is safe to assume a function with the same signature is the same function, no ? Don't you think this a safe way of handling the inconsistency among different platforms ? One more observation, that can make the inconsistency handling even safer: The different between the platforms seems to be that one platform assumes the beginning of the function is the curly bracket, and one to be the function signature. So it is really a matter of +- 1 line. |
|
I guess my concern would be whether the tools misplace just the function decl/start lines - or also misplace something else that matters more. That's the problem with ignoring errors...all of them get ignored, even if you really wanted to look at certain ones. I suspect that the location may differ by more than 1 - say, if the signature extends across multiple lines, or if a bloody-minded user puts a comment in exactly the wrong place. Another way to track the multi-platform issue is to actually build distinct DBs, and generate a distinct differential reports for each. If you find new code that isn't covered in one, verify that it is covered in the other...now that (missed) point will appear in the 'UBC' category in the next build - and you know that you reviewed it and signed off earlier, so no need to investigate. |
Hi 👋
I think I found the root cause behind: #319
It seems that the parsing of files that with
LH:0(cpp files that are part of the test executable but never actually accessed by the test) is different/wrong compared to the parsing of files accessed by the test.An example I have is the following two following
.datfiles coming from two different test paths:coverage_report_1.dat
coverage_report_2.dat
Notice that the function beginning line and ending line are different in the reports ... Also lines found and so on.
Now once I try to merge the two, the merged lcov report will show two functions one hit and one not. Trying to filter functions will also fail to show the function as covered.
Would it make sense to have lcov drop records with
LH:0(unless all files haveLH:0, in which case, keep 1) since they add nothing to the coverage reporting anyways ?The text was updated successfully, but these errors were encountered: