Check coverage data consistency:

  - function should have non-zero hit count only if at least one
    contained line has a non-zero hit count - and vice versa.
  - for line with non-zero hit count, at least one branch expression
    should be evaluated - and vice versa.
Also add option to skip the consistency check (e.g., for legacy data).

Signed-off-by: Henry Cox <henry.cox@mediatek.com>

Author: henry2cox

bin/genhtml

@@ -95,6 +95,7 @@ use FindBin;
 use Time::HiRes;    # for profiling
 use Storable;
 use POSIX;
+use Data::Dumper;
 
 use lib "$FindBin::RealBin/../lib";
 use lcovutil qw (set_tool_name define_errors parse_ignore_errors
@@ -2570,24 +2571,6 @@ sub _categorizeLineCov
             next;
         }
         # LCOV_EXCL_STOP
-        next if defined($lineCovCurrent->value($line));
-
-        lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
-                      "line $filename:$line has branchcov but no linecov data");
-        # maybe should skip this branch if no line data - rather
-        #  than building fake line data.
-        my $hit        = 0;    # hit if any of the associated branches are hit
-        my $branchData = $branchCurrent->value($line);
-        BLOCKS: foreach my $block ($branchData->blocks()) {
-            my $b = $branchData->getBlock($block);
-            foreach my $expr (@$b) {
-                if ($expr->count() != 0) {
-                    $hit = 1;
-                    last BLOCKS;
-                }
-            }
-        }
-        $lineCovCurrent->append($line, $hit);
     }
 
     # it is sufficient to just walk the 'global' (merged) line
@@ -2597,9 +2580,15 @@ sub _categorizeLineCov
     #  coverages.)
     foreach my $line ($lineCovCurrent->keylist()) {
         my $type = $diffMap->type($filename, $diffMap->NEW, $line);
-        die("'current' line $filename:$line should not be marked 'delete'")
-            unless $type ne 'delete';
-
+        if ($type eq 'delete') {
+            # can happen in some inconsistent case, when there are certain
+            # out-of-range references in a file which contained diffs - and we
+            # ignored the error check
+            lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
+                "'current' line $filename:$line should not be marked 'delete'");
+            $lineCovCurrent->remove($line);
+            next;
+        }
         my $linedata;
         if (!exists($lineDataMap->{$line})) {
             $linedata = LineData->new($type);
@@ -2716,9 +2705,14 @@ sub _categorizeBranchCov
     #     Or because we did something bad and no longer reach what had been
     #     a high probability event?
     foreach my $line ($branchCurrent->keylist()) {
-        # should have been caught or fixed, above
-        die("line $filename:$line has branchcov but no linecov data")
-            unless exists($lineDataMap->{$line});
+        unless (exists($lineDataMap->{$line})) {
+            # unless ignored, should have been caught or fixed during
+            #  TraceInfo::_checkConsistency
+            lcovutil::ignorable_error($lcovutil::ERROR_INCONSISTENT_DATA,
+                "\"$filename\":$line line has branchcov but no linecov data (skipping)."
+            );
+            next;
+        }
         my $data = $lineDataMap->{$line};
 
         $branchCovLines{$line} = 1;
@@ -5123,7 +5117,6 @@ sub _synthesize
 
     return $self if ($last_line < 1 ||
                      $currentLast >= $last_line);
-
     my $why;
     if (0 == $currentLast) {
         # Synthesize source data from coverage trace to replace unreadable file
@@ -5148,7 +5141,6 @@ sub _synthesize
                                   $last_line . $suffix);
         $why = 'not long enough';
     }
-
     # Simulate gcov behavior
     my $notFound = "/* " . $self->path() . " $why */";
     my $synth    = "/* (content generated from line coverage data) */";
@@ -5488,6 +5480,8 @@ sub add_dependency
     lcovutil::debug(1,
                     "add depend $name' in -> " . $parent->[0]->[1]->[2] . "\n")
         unless exists($parent->[1]->{$name});
+    print("add depend $name -> " . $parent->[0]->[1]->[2] . "\n")
+        if $main::debugScheduler > 1;
     $parent->[1]->{$name} = 1;
 }
 
@@ -5505,10 +5499,24 @@ sub completed_dependency
     delete($pendingParent->[1]->{$name});
     print("completed $name -> $parent " . scalar(%{$pendingParent->[1]}) . "\n")
         if $main::debugScheduler > 1;
-
+    # LCOV_EXCL_START
+    if ($main::debugScheduler > 1) {
+        # print parent dependencies
+        while (my ($k, $d) = each(%{$pendingParent->[1]})) {
+            die("unexpected parent dependency $k ARRAY: " . $d->[0] . ' ' .
+                join('/', @{$d->[2]}))
+                if ('ARRAY' eq ref($d) || 1 != $d);
+            print("  $k\n");
+        }
+    }
+    # LCOV_EXCL_STOP
     if (!%{$pendingParent->[1]}) {
         # no more dependencies - schedule this one
-        push(@{$self->[WORKLIST]}, $pendingParent->[0]);
+        my $p = $pendingParent->[0];
+        print("completed dependencies - schedule " . $p->[0] . ' ' .
+              join('/', @{$p->[2]}) . "\n")
+            if $main::debugScheduler > 1;
+        push(@{$self->[WORKLIST]}, $p);
         delete($pending->{$parent});
     }
 }
@@ -5914,7 +5922,25 @@ sub compute
                 next WORK;
             }
         }
-        die("unexpected empty worklist??") unless (@$worklist || @$joblist);
+        #LCOV_EXCL_START
+        unless (@$worklist || @$joblist) {
+            foreach my $e (keys %$pending) {
+                # something went wrong - print some debug data
+                lcovutil::info("me: $e: depends\n");
+                my $p = $pending->{$e};
+                while (my ($k, $d) = each(%{$p->[1]})) {
+                    lcovutil::info("  $k\n");
+                    die("unexpected data for depend $k: $d") unless $d == 1;
+                }
+                lcovutil::info("   parent: " . $p->[0]->[0] . ' ' .
+                               join('/', @{$p->[0]->[2]}) . "\n");
+            }
+            # can get here if child process hit a 'die' (as opposed to an error)
+            # and "--keep-going" was specified.
+            die("unexpected empty worklist??");
+        }
+        #LCOV_EXCL_STOP
+
         # schedule a few items from the worklist if we can..
         $segmentID = $self->_segment_worklist($segmentID);
 

bin/perl2lcov

@@ -79,11 +79,11 @@ For example:
 
     # write Perl line, branch, condition, and subroutine coverage data to
     #  'myPerlDB' in the current directory
-  \$ perl -MDevel::Cover=-db,./myPerlDB,-coverage,statement,branch,condition,subroutine,-silent,1 myScript.pl
+  \$ perl -MDevel::Cover=-db,.\/myPerlDB,-coverage,statement,branch,condition,subroutine,-silent,1 myScript.pl
     # OR: write all the coverage types that Perl knows about to 'myPerlDB2' -
-    #   note that perl2lcov will ignore types it does not understand/does not use
-    #   (pod, time, and path)
-  \$ perl -MDevel::Cover=-db,./myPerlDB2,-silent,1 myScript.pl
+    #   note that perl2lcov will ignore types it does not understand/does
+    #   not use (pod, time, and path)
+  \$ perl -MDevel::Cover=-db,.\/myPerlDB2,-silent,1 myScript.pl
     # run 'cover' from the Devel::Cover installation - to extract runtime
     #   data into a usable form.  This will also generate an HTML report
     #   in 'myCoverDB'
@@ -93,6 +93,28 @@ For example:
     # and generate a genhtml-format coverage report:
   \$ genhtml -o html_report perldata.info ...
 
+Note that the data generateed by Devel::Cover is not always internally
+consistent.  For example:
+
+  - some which are never called, do not appear in the coverage data.
+
+  - sometimes, a line will appear to be executed (non-zero hit count) but
+    none of its contained branch expressions have been evaluated.
+    (If the line was executed, then at least one branch condition must have
+    been evaluated.
+
+This can cause the various tools in the lcov package to generate errors of
+type 'inconsistent'.
+In that case, you can:
+
+  - skip consistency checks entirely:  see the 'skip_consistency_checks' section
+    in man lovrc(5)
+
+  - ignore the error:  see the '--ignore-error' section in man genhtml(1)
+
+  - exclude the offending code: see the '--exclude', '--filter', and
+    '--omit-lines' sections in man genhtml(1).
+
 END_OF_USAGE
 }
 
@@ -110,7 +132,7 @@ sub findPackage
         if ($line < $v->[0]) {
             $max = $mid - 1;
         } elsif ($line > $v->[0]) {
-            $best = $v->[1];
+            $best = $v;
             $min  = $mid + 1;
         } else {
             # line number matched...which ought not to happen because
@@ -119,7 +141,7 @@ sub findPackage
             # That won't be the line containing "package ..." - unless the
             # user wrote the whole thing on one line.  Not clever.  Deserves
             # to lose, if something in here breaks.
-            return $v->[1];
+            return $v;
         }
     }
     return $best;
@@ -175,6 +197,9 @@ foreach my $db (@ARGV) {
         # use statement coverage to mark un-evaluated branches
         my ($stmts, $branches, $conditions, $subroutines);
         my @packageExtents;
+        # Devel::Cover doesn't instrument all the functions in every file -
+        # so need a workaround to find better extents for some of them
+        my @functionExtents;
 
         foreach my $criteria ($f->items) {
             # some types we don't use
@@ -188,11 +213,13 @@ foreach my $db (@ARGV) {
                 $subroutines = $c;
                 if (-f $file) {
                     open(GREP, '-|', 'grep', '--line-number', '-E',
-                         '^\s*package ', $file) or
+                         '^\s*(package|sub) ', $file) or
                         die("unable to grep $file: $!");
                     while (<GREP>) {
-                        if (/^(\d+):\s*package\s+(\S+?);/) {
+                        if (/^(\d+):\s*package\s+(\S+)\s*;/) {
                             push(@packageExtents, [$1, $2 . '::']);
+                        } elsif (/^(\d+):\s*sub\s+([^\s(]+)/) {
+                            push(@functionExtents, [$1, $2]);
                         } else {
                             die("unexpected grep output '$_'");
                         }
@@ -248,7 +275,7 @@ foreach my $db (@ARGV) {
                     if ($name !~ /(BEGIN|__ANON__)/) {
                         my $p = findPackage(\@packageExtents, $line);
                         if (defined($p)) {
-                            $name = $p . $name;
+                            $name = $p->[1] . $name;
                         }
                         $functionMap->define_function($name, $line);
                         $functionMap->add_count($name, $count);
@@ -347,6 +374,58 @@ foreach my $db (@ARGV) {
         $fileData->sum()->union($lineMap);
         $fileData->sumbr()->union($branchMap);
         $fileData->func()->union($functionMap);
+
+        # have to do this manually due to some Perl quirks -
+        # in particular, there may be code outside of the subroutine we are
+        # walking...and we want to correct the end line
+        TraceFile::_deriveFunctionEndLines($fileData);
+        my $lineData = $fileData->sum();
+        my $funcData = $fileData->testfnc();
+
+        foreach my $func ($fileData->func()->valuelist()) {
+            # where is the nearest 'package' after my start line?
+            my $first = $func->line();
+            my $end   = $func->end_line();
+            next unless defined($end);
+            # find package or function enclosing my end line..
+            my $last = $end;
+            foreach my $ext (\@packageExtents, \@functionExtents) {
+                while (1) {
+                    my $p = findPackage($ext, $last);
+                    if (defined($p) && $p->[0] > $first) {
+                        $last = $p->[0] - 1;
+                        lcovutil::info(1,
+                                       $func->name() .
+                                           ": found update end line $last in " .
+                                           $p->[1] . "\n");
+                        # iterate in case there is another package above the first one
+                    } else {
+                        last;
+                    }
+                }
+            }
+            next unless $last < $end;
+
+            # what is the last executable line before the 'package' or 'sub' decl?
+            while ($last > $first) {
+                if (defined($lineData->value($last))) {
+                    last;
+                }
+                --$last;
+            }
+            lcovutil::info(1,
+                           "resetting " . $func->name() .
+                               " end line to $last (from $end)\n");
+            $func->set_end_line($last);
+
+            foreach my $tn ($funcData->keylist()) {
+                my $d = $funcData->value($tn);
+                my $f = $d->findKey($first);
+                $f->set_end_line($last);
+            }
+
+        }    #foreach function
+
     }    # foreach file
 }    #foreach cover db
 

bin/py2lcov

@@ -56,6 +56,17 @@ def main():
     usageString="""py2lcov: Translate Python coverage data to LCOV .info format.
 Please also see https://coverage.readthedocs.io
 
+Note that the '--no-functions' argument may result in subtly inconsistent coverage
+data if a 'no-functions' coverage DB is merged with one which contains derived
+function data because the 'def myFunc(...)' line will acquire a 'hit' count
+of 1 because the python interpreter considers the 'def' to have been executed
+when the line is interpreted (i.e., when the function is defined).
+This will generate an 'inconsistent' error if the function is not executed in
+your tests becasue the (derived) function will have a zero hit count but the
+first line of the function has a non-zero count.
+Best practice is to either always specify '--no-functions' or never specify
+'--no-functions'.
+
 Note that xml2lcov does not implement the full suite of LCOV features
 (e.g., filtering, substitutions, etc.).
 Please generate the translated LCOV format file and then read the data

bin/xml2lcovutil.py

@@ -121,7 +121,7 @@ def close(self):
         self._outf.close()
 
         if self._args.version and None == self._versionScript:
-            cmd = "%(lcov)s -a %(info)s -o %(info)s --version-script '%(vers)s' %(checksum)s--rc compute_file_version=1" % {
+            cmd = "%(lcov)s -a %(info)s -o %(info)s --version-script '%(vers)s' %(checksum)s--rc compute_file_version=1 --branch-coverage --ignore inconsistent" % {
                 'lcov': os.path.join(os.path.split(sys.argv[0])[0], 'lcov'),
                 'checksum': "--checksum " if self._args.checksum else '',
                 'info': self._args.output,