[Coverage] Prevent detection of false instantiations in case of macro expansion.

The root of the problem was that findMainViewFileID(File, Function)
could return some ID for any given file, even though that file
was not the main file for that function.

This patch ensures that the result of this function is conformed
with the result of findMainViewFileID(Function).

Differential Revision: http://reviews.llvm.org/D18787

llvm-svn: 266436

lib/ProfileData/CoverageMapping.cpp

@@ -375,21 +375,7 @@ static SmallBitVector gatherFileIDs(StringRef SourceFile,
   return FilenameEquivalence;
 }
 
-static Optional<unsigned> findMainViewFileID(StringRef SourceFile,
-                                             const FunctionRecord &Function) {
-  SmallBitVector IsNotExpandedFile(Function.Filenames.size(), true);
-  SmallBitVector FilenameEquivalence = gatherFileIDs(SourceFile, Function);
-  for (const auto &CR : Function.CountedRegions)
-    if (CR.Kind == CounterMappingRegion::ExpansionRegion &&
-        FilenameEquivalence[CR.FileID])
-      IsNotExpandedFile[CR.ExpandedFileID] = false;
-  IsNotExpandedFile &= FilenameEquivalence;
-  int I = IsNotExpandedFile.find_first();
-  if (I == -1)
-    return None;
-  return I;
-}
-
+/// Return the ID of the file where the definition of the function is located.
 static Optional<unsigned> findMainViewFileID(const FunctionRecord &Function) {
   SmallBitVector IsNotExpandedFile(Function.Filenames.size(), true);
   for (const auto &CR : Function.CountedRegions)
@@ -401,6 +387,16 @@ static Optional<unsigned> findMainViewFileID(const FunctionRecord &Function) {
   return I;
 }
 
+/// Check if SourceFile is the file that contains the definition of
+/// the Function. Return the ID of the file in that case or None otherwise.
+static Optional<unsigned> findMainViewFileID(StringRef SourceFile,
+                                             const FunctionRecord &Function) {
+  Optional<unsigned> I = findMainViewFileID(Function);
+  if (I && SourceFile == Function.Filenames[*I])
+    return I;
+  return None;
+}
+
 /// Sort a nested sequence of regions from a single file.
 template <class It> static void sortNestedRegions(It First, It Last) {
   std::sort(First, Last,
@@ -422,13 +418,11 @@ CoverageData CoverageMapping::getCoverageForFile(StringRef Filename) {
 
   for (const auto &Function : Functions) {
     auto MainFileID = findMainViewFileID(Filename, Function);
-    if (!MainFileID)
-      continue;
     auto FileIDs = gatherFileIDs(Filename, Function);
     for (const auto &CR : Function.CountedRegions)
       if (FileIDs.test(CR.FileID)) {
         Regions.push_back(CR);
-        if (isExpansion(CR, *MainFileID))
+        if (MainFileID && isExpansion(CR, *MainFileID))
           FileCoverage.Expansions.emplace_back(CR, Function);
       }
   }

test/tools/llvm-cov/Inputs/prevent_false_instantiations.cpp

@@ -0,0 +1,15 @@
+#include "prevent_false_instantiations.h"
+
+void func1() {
+  DO_SOMETHING();
+}
+
+void func2() {
+  DO_SOMETHING();
+}
+
+int main() {
+  func1();
+  func2();
+  return 0;
+}

test/tools/llvm-cov/Inputs/prevent_false_instantiations.proftext

@@ -0,0 +1,26 @@
+_Z5func1v
+# Func Hash:
+3
+# Num Counters:
+2
+# Counter Values:
+1
+0
+
+_Z5func2v
+# Func Hash:
+3
+# Num Counters:
+2
+# Counter Values:
+1
+0
+
+main
+# Func Hash:
+0
+# Num Counters:
+1
+# Counter Values:
+1
+

test/tools/llvm-cov/lit.local.cfg

@@ -1 +1 @@
-config.suffixes = ['.test', '.m', '.cpp', '.c']
+config.suffixes = ['.test', '.m', '.cpp', '.c', '.h']

test/tools/llvm-cov/prevent_false_instantiations.h

@@ -0,0 +1,10 @@
+// Checks that function instantiations don't go to a wrong file.
+
+// CHECK-NOT: {{_Z5func[1,2]v}}
+
+// RUN: llvm-profdata merge %S/Inputs/prevent_false_instantiations.proftext -o %t.profdata
+// RUN: llvm-cov show %S/Inputs/prevent_false_instantiations.covmapping -instr-profile %t.profdata -filename-equivalence %s | FileCheck %s
+
+#define DO_SOMETHING() \
+  do {                 \
+  } while (0)

unittests/ProfileData/CoverageMappingTest.cpp

@@ -454,6 +454,44 @@ TEST_P(MaybeSparseCoverageMappingTest, strip_unknown_filename_prefix) {
   ASSERT_EQ("func", Names[0]);
 }
 
+TEST_P(MaybeSparseCoverageMappingTest, dont_detect_false_instantiations) {
+  InstrProfRecord Record1("foo", 0x1234, {10});
+  InstrProfRecord Record2("bar", 0x2345, {20});
+  ProfileWriter.addRecord(std::move(Record1));
+  ProfileWriter.addRecord(std::move(Record2));
+
+  startFunction("foo", 0x1234);
+  addCMR(Counter::getCounter(0), "expanded", 1, 1, 1, 10);
+  addExpansionCMR("main", "expanded", 4, 1, 4, 5);
+
+  startFunction("bar", 0x2345);
+  addCMR(Counter::getCounter(0), "expanded", 1, 1, 1, 10);
+  addExpansionCMR("main", "expanded", 9, 1, 9, 5);
+
+  loadCoverageMapping();
+
+  std::vector<const FunctionRecord *> Instantiations =
+      LoadedCoverage->getInstantiations("expanded");
+  ASSERT_TRUE(Instantiations.empty());
+}
+
+TEST_P(MaybeSparseCoverageMappingTest, load_coverage_for_expanded_file) {
+  InstrProfRecord Record("func", 0x1234, {10});
+  ProfileWriter.addRecord(std::move(Record));
+
+  startFunction("func", 0x1234);
+  addCMR(Counter::getCounter(0), "expanded", 1, 1, 1, 10);
+  addExpansionCMR("main", "expanded", 4, 1, 4, 5);
+
+  loadCoverageMapping();
+
+  CoverageData Data = LoadedCoverage->getCoverageForFile("expanded");
+  std::vector<CoverageSegment> Segments(Data.begin(), Data.end());
+  ASSERT_EQ(2U, Segments.size());
+  EXPECT_EQ(CoverageSegment(1, 1, 10, true), Segments[0]);
+  EXPECT_EQ(CoverageSegment(1, 10, false), Segments[1]);
+}
+
 INSTANTIATE_TEST_CASE_P(MaybeSparse, MaybeSparseCoverageMappingTest,
                         ::testing::Bool());