[PATCH] D66763: Ignore object files that lack coverage information. Before this change, if multiple binary files were presented, all of them must have been instrumented or the load would fail with coverage_map_error::no_data_found.

[Dean Sturtevant via Phabricator via llvm-commits]

deansturtevant updated this revision to Diff 217421.
deansturtevant added a comment.

Responded to review comments. Simplified the code and the tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66763/new/

https://reviews.llvm.org/D66763

Files:
  llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
  llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
  llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/instrumented
  llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/instrumented.cc
  llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/instrumented.profdata
  llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/instrumented.raw
  llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/not_instrumented
  llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/not_instrumented.cc
  llvm/test/tools/llvm-cov/multiple-objects-not-all-instrumented.test


Index: llvm/test/tools/llvm-cov/multiple-objects-not-all-instrumented.test
===================================================================
--- /dev/null
+++ llvm/test/tools/llvm-cov/multiple-objects-not-all-instrumented.test
@@ -0,0 +1,12 @@
+RUN: llvm-cov export --format=lcov --instr-profile=%S/Inputs/multiple_objects_not_all_instrumented/instrumented.profdata \
+RUN:   -object %S/Inputs/multiple_objects_not_all_instrumented/not_instrumented \
+RUN:   -object %S/Inputs/multiple_objects_not_all_instrumented/instrumented | FileCheck -check-prefix=FN %s
+
+FN:1,_Z2f1v
+
+Instructions for regenerating the test:
+
+clang -std=c++11 not_instrumented.cc -o not_instrumented
+clang -std=c++11 -mllvm -enable-name-compression=false -fprofile-instr-generate -fcoverage-mapping instrumented.cc -o instrumented
+LLVM_PROFILE_FILE="instrumented.raw" ./instrumented
+llvm-profdata merge instrumented.raw -o instrumented.profdata
Index: llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/not_instrumented.cc
===================================================================
--- /dev/null
+++ llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/not_instrumented.cc
@@ -0,0 +1 @@
+int main(int argc, char** argv) {}
Index: llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/instrumented.cc
===================================================================
--- /dev/null
+++ llvm/test/tools/llvm-cov/Inputs/multiple_objects_not_all_instrumented/instrumented.cc
@@ -0,0 +1,5 @@
+void f1() {}
+
+int main(int argc, char** argv) {
+  f1();
+}
Index: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
===================================================================
--- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -270,6 +270,18 @@
   return std::move(Coverage);
 }
 
+// If E is a no_data_found error, returns success. Otherwise returns E.
+static Error handleMaybeNoDataFoundError(Error E) {
+  return handleErrors(
+      std::move(E), [](const CoverageMapError &CME) {
+        if (CME.get() == coveragemap_error::no_data_found) {
+          return static_cast<Error>(Error::success());
+        } else {
+          return make_error<CoverageMapError>(CME.get());
+        }
+      });
+}
+
 Expected<std::unique_ptr<CoverageMapping>>
 CoverageMapping::load(ArrayRef<StringRef> ObjectFilenames,
                       StringRef ProfileFilename, ArrayRef<StringRef> Arches) {
@@ -289,12 +301,22 @@
         CovMappingBufOrErr.get()->getMemBufferRef();
     auto CoverageReadersOrErr =
         BinaryCoverageReader::create(CovMappingBufRef, Arch, Buffers);
-    if (Error E = CoverageReadersOrErr.takeError())
-      return std::move(E);
+    if (Error E = CoverageReadersOrErr.takeError()) {
+      E = handleMaybeNoDataFoundError(std::move(E));
+      if (E) {
+        return std::move(E);
+      } else {  // E == success (originally a no_data_found error).
+        continue;
+      }
+    }
     for (auto &Reader : CoverageReadersOrErr.get())
       Readers.push_back(std::move(Reader));
     Buffers.push_back(std::move(CovMappingBufOrErr.get()));
   }
+  // If no readers were created, either no objects were provided or none of them
+  // had coverage data. Return an error in the latter case.
+  if (Readers.empty() && !ObjectFilenames.empty())
+    return make_error<CoverageMapError>(coveragemap_error::no_data_found);
   return load(Readers, *ProfileReader);
 }
 
Index: llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
===================================================================
--- llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -527,6 +527,7 @@
 
   /// Load the coverage mapping from the given object files and profile. If
   /// \p Arches is non-empty, it must specify an architecture for each object.
+  /// Ignores non-instrumented object files unless all are not instrumented.
   static Expected<std::unique_ptr<CoverageMapping>>
   load(ArrayRef<StringRef> ObjectFilenames, StringRef ProfileFilename,
        ArrayRef<StringRef> Arches = None);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66763.217421.patch
Type: text/x-patch
Size: 4178 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190827/2ac1ac53/attachment.bin>