[PATCH] D76016: [ProfileData] Add option to ignore invalid regions

[Brian Gesiak via Phabricator via llvm-commits]

modocache created this revision.
modocache added reviewers: vsk, rnk, arphaman.
Herald added subscribers: dexonsmith, hiraditya.
Herald added a project: LLVM.

Clang's source location information reporting is always improving, but
llvm-cov and LLVM's profiling data reporters are "all-or-nothing": if
just a single region of code coverage data is invalid or malformed, the
entire coverage data file is scrapped, and llvm-cov reports "Failed to
load coverage: Malformed coverage data".

The C++ developers I support rely heavily on coverage data from their
unit tests, and this all-or-nothing approach to reporting has caused that
data to be unreliable. I fixed a recent incident of "Failed to load
coverage" by patching D65428 <https://reviews.llvm.org/D65428> into an older internal release of Clang
that we use, to fix location information for a single function that had
caused thousands of other functions to not be reported. More recently,
I've discovered that Clang generates invalid coverage data for C++20
coroutines, and this prevents coverage data from being reported for the
entire unit.

Before I can send up a fix for C++20 coroutines, I thought I'd provide a
stop-gap solution: allow developers to invoke
`llvm-cov -ignore-malformed-coverage-mapping-regions`, which will simply
drop mapping regions with invalid source location information. Using
this option would have allowed the developers I support to get coverage
data for most of their source code, even when Clang could not generate
valid information (such as prior to D65428 <https://reviews.llvm.org/D65428>, or prior to whatever I send
up for coroutines).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76016

Files:
  llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/test/tools/llvm-cov/warnings.h


Index: llvm/test/tools/llvm-cov/warnings.h
===================================================================
--- llvm/test/tools/llvm-cov/warnings.h
+++ llvm/test/tools/llvm-cov/warnings.h
@@ -14,3 +14,6 @@
 
 // RUN: not llvm-cov report %S/Inputs/malformedRegions.covmapping -instr-profile %S/Inputs/elf_binary_comdat.profdata 2>&1 | FileCheck %s -check-prefix=MALFORMED-REGION
 // MALFORMED-REGION: malformedRegions.covmapping: Failed to load coverage: Malformed coverage data
+// RUN: llvm-cov report -ignore-malformed-coverage-mapping-regions %S/Inputs/malformedRegions.covmapping -instr-profile %S/Inputs/elf_binary_comdat.profdata 2>&1 | FileCheck %s -check-prefix=IGNORE-MALFORMED-REGION
+// IGNORE-MALFORMED-REGION: warning: 1 functions have mismatched data
+// IGNORE-MALFORMED-REGION-NOT: Failed to load coverage
Index: llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
===================================================================
--- llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -45,6 +45,11 @@
 STATISTIC(CovMapNumRecords, "The # of coverage function records");
 STATISTIC(CovMapNumUsedRecords, "The # of used coverage function records");
 
+cl::opt<bool> coverage::IgnoreMalformedCoverageMappingRegions(
+    "ignore-malformed-coverage-mapping-regions",
+    cl::desc("Ignore malformed code coverage mapping regions"),
+    cl::init(false));
+
 void CoverageMappingIterator::increment() {
   if (ReadErr != coveragemap_error::success)
     return;
@@ -297,8 +302,16 @@
     auto CMR = CounterMappingRegion(C, InferredFileID, ExpandedFileID,
                                     LineStart, ColumnStart,
                                     LineStart + NumLines, ColumnEnd, Kind);
-    if (CMR.startLoc() > CMR.endLoc())
-      return make_error<CoverageMapError>(coveragemap_error::malformed);
+    if (CMR.startLoc() > CMR.endLoc()) {
+      LLVM_DEBUG(dbgs() << "Malformed coverage mapping region: "
+                        << CMR.startLoc().first << ":" << CMR.startLoc().second
+                        << " -> " << CMR.endLoc().first << ":"
+                        << CMR.endLoc().second << "\n");
+      if (IgnoreMalformedCoverageMappingRegions)
+        continue;
+      else
+        return make_error<CoverageMapError>(coveragemap_error::malformed);
+    }
     MappingRegions.push_back(CMR);
   }
   return Error::success();
Index: llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
===================================================================
--- llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
+++ llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
@@ -232,6 +232,13 @@
              BinaryCoverageReader::DecompressedData &Decompressed);
 };
 
+// Whether to ignore coverage mapping regions that are invalid, for example
+// regions for which the frontend has indicated the start location comes after
+// the end location. These are typically bugs in the frontend generating source
+// location information, but by enabling this option users can have coverage
+// information for regions such as these silently ignored.
+extern cl::opt<bool> IgnoreMalformedCoverageMappingRegions;
+
 } // end namespace coverage
 } // end namespace llvm
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76016.249699.patch
Type: text/x-patch
Size: 3302 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200311/923e4a11/attachment.bin>