path mismatch between diff-file and baseline/current info/dat file

[ziad-fernride]

Hi 😄 I am this issue where I get this error:

genhtml: WARNING: (mismatch) source file 'my/file/path.h' (in both baseline and current .info files) has same basename as 'diff' entry '/home/ziad/my/file/path.h' - but a different path.  Possible pathname mismatch?  Perhaps see "--elide-path-mismatch" and/or "--substitute" options in 'man genhtml'

The thing is, I git diff with --no-prefix and the diff.txt file has no mention of /home/ziad ... how do I avoid this issue ? And how can I use the substitute option here ? my regex expression attempts failed 😓

Thanks 🙏

[henry2cox]

You can simply ignore the error and see what happens: genhtml --ignore mismatch ... That will very likely lead to mis-categorization, though - because it appears that you do have some changes in at least some instrumented source files ("my/file/path.h", in particular).

If you are feeling lucky (and you are pretty sure that file basenames in your project are unique) - then you can use the genhtml --elide-path-mismatch ... flag to tell the tool to use best effort matching. (This is risky/not recommended/may cause very strange errors - but often does exactly what you want.)

You can also take a look at your .info and .diff files to see what the paths are in them - the tool wants to see exact string matches - ("./foo.h' and 'foo.h' are NOT the same). If something doesn't match...then --substitute is your friend (this is equivalent to sed, with the same regexps). If your environment and configuration is more complex - then you may need a --resolve-script callback.

The tests have a few examples of substitutions and callbacks (but are obviously being applied to toys - so may not be quite what you need).

You didn't say which version of lcov you are using (but this part of the implementation has been fairly stable for a long time - so using TOT may or may not help you much).

As always: a small example which illustrates the issue is helpful.

Henry

[ziad-fernride]

Hey Henry, thanks for responding !

The version is genhtml: LCOV version release_candidate-2024.07.14.0.100-g36d81dffc9 I checked the diff file, and my/file/path.h is mentioned exactly like that in the diff.txt ... No mention of the prefix ... yet the tool seems to add it itself ? 🤔 If this does not ring a bell, I will try to provide a sharable sample code

Thanks again for your help ! 😊

[ziad-fernride]

I now tried --substitute "s/\/home\/ziad\///" and the tool now is interpreting the path as b/my/file/path.h in the diff.txt file ... as if the tool is insisting on adding a prefix somehow

[henry2cox]

Your version looks like it was cloned from TOT, probably quite recently.

The tool calls rel2abs as it reads the diff file - then applies substitutions, etc. off of that. (I don't remember the details of 'why' - but it is likely too late to change/would break too many projects.) Your substitution path looks like it should work - but git diff is also shooting you in the foot slightly, by prepending the file pathname with a/ or b/ in the diff output. You can either account for that in your substitution regexp, or you could use the gitdiff utility from the 'scripts' directory (mentioned below) to compute the diff (it munges the git paths appropriately).

From a clarity perspective: perl understands delimiters, so --substitute s@/home/ziad/@@ might be easier to read.

If not enough clues: yeah. Probably need testcase.

[henry2cox]

Forgot to mention that there is a 'gitdiff' script in the release which formats the data in the way that genhtml likes to see it. (It merely wraps 'git diff', of course.)

[henry2cox]

I suspect that your example will be happy if you edit the DiffMap::_read_udiff method in '.../bin/genhtml to remove the rel2abs calls (just assign $1 directly). (You will still need to remove the 'ab' prefix inserted by git, of course.) I think that the original reason to convert to absolute paths was because 'lcov --capture' didn't support relative paths - and so the relative path in the diff result would always mismatch with the absolute paths in the .info file. That issue was fixed some time ago - so I suspect that the hack is no longer required. (Will be some time before I test this with various projects to be sure of the impact.)

[shabanzd]

I see, cool tips, thanks! 🤩 I managed to make it work at the end with git diff --no-prefix, substitution and some ignores. The differential reports look a lot better but there are still many false positives including curly brackets (despite using the line and trivial filters) making gating unreliable 😕

Is the use of directives my only choice left ? I have seen a github issue where you mentioned that these can be on compiler level, but I still can't see why the filter works so well with some parts of the code but not others. Will take a closer look tomorrow on your PRs to try and better understand the filtering mechanism.

Some of the false positives I have seen so far are even code lines that appear to be newly covered, although they were always covered (these are less harmful for gating)

[henry2cox]

The various filters are regexp based - so, if your code doesn't match what the filter was looking for: you lose. It may or may not be possible (or easy) to improve them. However, our use is such that we need filters to be conservative: better to leave something in that ought to have been removed than to remove something that later causes an expensive bug.

I think that your observation about braces relates to function closing brace - but you might want to post a few examples (possibly to a different bug report, possibly to a discussion - as it isn't so related to the original issue). Not sure what you mean about false positives on newly covered lines.

Are you using GCC, LLVM, or some other tool? There are different coverage data bugs (features?) depending on compiler, version, and use model (gcov vs. profraw, for example). Some of the bogus coverage artifacts we see come and go from one build to the next.

With respect to gating: I think that you mean what the tool and the paper call "[code] coverage criteria": some set of coverage-related checks which must pass in order for you to declare a "green" build. In our case: we use the criteria mentioned in the paper (no new uncovered code, no losses). There are cases where we need to add exclusions or to sign off on something - typically for trace or debug code (actual scenario tends to be complicated). 100% branch coverage is not possible, in general - even with heroic effort (we can talk about why) - so signoff is almost always required. Part of the idea is that 'signed off' code becomes UBC in subsequent builds - and we know that we looked at it, were happy, and signed off on it. No need for manual review. Then - if something changes in future (i.e., we hit it) - then it will become GBC (which may trigger review) and will be picked up by our ratchet such that we will again catch it should it be subsequently lost (...possibly requiring sign off again).

Your criteria callback can be arbitrarily complicated - and can use tool internal APIs if necessary (i.e., to walk the source and coverage structures). Using internal APIs is not necessarily a great idea in general, though - as the API can and does change. (We have regression tests for such things.)

[ziad-fernride]

I am sorry I was unclear ... let me explain the situation a bit further ... The false positives I mentioned are not in the report itself, they are in the differential report.

Example: I am running the following command:

        genhtml "report_${{ env.HEAD_SHA }}.dat"
        --baseline-file "report_$(git merge-base HEAD "origin/${{ github.base_ref }}").dat"
        --substitute "s@$(pwd)/@@"
        --ignore-errors empty,unmapped,range,source,unsupported
        --synthesize-missing
        --show-proportion
        --filter line,function,trivial
        --verbose
        --diff-file diff.txt
        --elide-path-mismatch
        --criteria-script third_party/lcov/criteria
        --output-dir "$(coverage_report_out_dir)"

and I get this:

The criteria is the one in your examples, so I get: Coverage criteria failed: UNC + LBC + UIC != 0: 0 + 0 + 6

Two of the UICs are shown in the snapshot above ... cool, now let's see the reports of the baseline lcov report and the new lcov report:

 genhtml \
    "${dat_report}"
    --output-directory "$coverage_report_out_dir"
    --prefix="$(pwd)"
    --ignore-errors unmapped,category
    --legend
    --branch-coverage 
    --filter function,line,trivial 
    --quiet 
    --synthesize-missing

We get:

old report

new report

You can see that even with respect to the tool itself, the diff result are wrong ... I have more examples, like an { after an if statement, that was always uncovered but shows up as a UIC.

It feels like the diff is not applied ... I generate the diff.txt like this:

git diff --unified --no-prefix "$(git merge-base HEAD "origin/${{ github.base_ref }}")"
        "${{ env.HEAD_SHA }}" > diff.txt

[henry2cox]

... still many false positives including curly brackets ...

Thought came to me while out with dog.... Not a recommended approach for some obvious reasons - but you could use the --omit-lines feature to (blindly) exclude coverpoints on lines which contain only a close brace: lcov ... --omit-lines '^\s*}\s*$' (you can admit EOL comments, etc. if you prefer).

If you were to do this - then I suggest to check a differential report before/after filtering - to see if it hit anything that you care about.

[henry2cox]

You can see that even with respect to the tool itself, the diff result are wrong ... I have more examples, like an { after an if statement, that was always uncovered but shows up as a UIC.

It feels like the diff is not applied ... I generate the diff.txt like this:

Looks like a bug or incorrect data. Not what I observe in various projects. I fear that I need a testcase.

(At some effort: you should be able to manufacture such a testcase by extracting only the relevant section of the .info and diff files. I think, no source required.)

One more thing to be aware of:

• if you apply filters in genhtml, then you almost certainly should also be using a --version-script callback - so that the tool knows that it has matching source code, before it tries to apply source-based filtering.
• if you are generating a differential report: then genhtml will try to filter your baseline data as well...and this might be invalid because your source code has moved on. If you are generating a vanilla report: then you are presumably looking at your current code - so no problem.

It is almost always a better idea to filter at capture or aggregate time - not at report generation.

[ziad-fernride]

Yeah I also don't like that I am doing the filtering at report generation time ... but we use bazel and get the lcov .dat report ready ... I briefly tried to apply filters to the .dat report I get from bazel and generate a filtered one, but my quick attempt failed ... would that work in theory ?

[henry2cox]

Filtering at report generation time is safe enough if you enable version checking. However, even then can cause misleading results because lcov will simply not filter (skip over) files whose version mismatches. That can cause apparent losses or holes in source that you expected to be filtered out.

This is largely fixable if your source is version controlled (i.e., not generated code) - but I never bothered to implement it because none of our projects would use it. Also, because it would be pretty hard for a mere mortal to understand or debug when something goes wrong. Tool already has enough complicated features acting under the covers.

[henry2cox]

... would that work in theory

If I understood the question: yes. Should work.

Where is your baseline data coming from? For most of our projects, the baseline is from the previous green build or from the previous release (or from some project-defined significant time). If stored in some DB - then that data pretty much needs to have been filtered before it was stored - for the reasons mentioned above. (Otherwise: you need to check out and possibly build that version again...)

Note that it is still a Good Idea (tm) to version-stamp your data - especially if you have a nontrivial project and multiple sites. Version mismatches can cause all manner of very strange results which can really confuse people. (Insert war story here.)

[ziad-fernride]

so the scenario I am describing is:

• Every build generates a .dat lcov report.
• The .dat lcov report is then filtered and saved in a DB (commit hash is the key for example).
• Next build downloads the filtered lcov report of its base commit run, and uses it as a baseline lcov report.
• The build generates a new lcov report.
• The new lcov report is also filtered.
• Now generate a differential report between baseline and new lcov reports

[ziad-fernride]

The filtering half worked 😆

 lcov -o new_report_filtered.dat -a new_report.dat --filter function,trivial,line
Combining tracefiles.
.. found 1 files to aggregate.
Reading tracefile new_report.dat.
Apply filtering..
Finished filter file processing
Writing data to new_report_filtered.dat
Summary coverage rate:
  source files: 263
  lines.......: 92.8% (11134 of 12004 lines)
  functions...: 95.2% (1713 of 1799 functions)
Filter suppressions 'blank':
    126 instances
    126 coverpoints
Filter suppressions 'function':
    1799 instances
    17106 coverpoints
Filter suppressions 'trivial':
    20 instances
    48 coverpoints
Filter suppressions 'brace':
    1971 instances
    1971 coverpoints
Message summary:
  no messages were reported

Summary above looks good ! Notice :

functions...: 95.2% (1713 of 1799 functions)

Now I do:

lcov --summary new_report_filtered.dat
Reading tracefile new_report_filtered.dat.
Summary coverage rate:
  source files: 262
  lines.......: 92.8% (11134 of 12004 lines)
  functions...: 57.7% (9875 of 17106 functions)
Message summary:
  no messages were reported

Functions filters don't seem to be permanently there ... while line and trivial filters seem to be persistent

[henry2cox]

so the scenario I am describing is:

Yes - that is pretty much what we do - except that the next job uses the previous success (not necessarily the previous build) for its baseline. Passing coverage criteria is part of the "success" criteria - which is how ratcheting works.

[ziad-fernride]

Yes - that is pretty much what we do - except that the next job uses the previous success (not necessarily the previous build)

Yes 👍 , I am here assuming one can't base their branch on a failing commit.

[henry2cox]

Functions filters don't seem to be permanently there ... while line and trivial filters seem to be persistent

Yeah - my fault. Unfortunate terminology - but too late to change, I fear.

"function" filter is not actually a filter - just a grouping mechanism, and really has effect only (or primarily) in genhtml. It isn't saved into the .info file.

I should have called it "--group function" rather than "--filter function" :-( But - at the time - I wasn't really thinking ahead as much as I should have been.

[henry2cox]

Yes 👍 , I am here assuming one can't base their branch on a failing commit.

My experience is that developers are very creative :-)

[henry2cox]

Totally unrelated...but I see a rear_radar_objects variable in your code snippet. If you happen to work at (or work on) Garmin or similar bike radar units...I have some FW bugs and/or misfeatures that I would very much like to see fixed. (My coordinates are not hard to deduce.)

[shabanzd]

If you happen to work at (or work on) Garmin or similar bike radar units

Haha, no but I own a garmin and have some stuff to report as well ! 😂

[henry2cox]

Thanks for closing...but it turns out that I have some fixes/enhancements related to this report :-) Will push them after testing, qualification, etc. The commit log for those fixes references this issue - so github will annotate appropriately.