[Coverage] Fix an assertion failure if the definition of an unused function spans multiple files.

[ikudrin]

Summary

We have an assertion failure if, for example, the definition of an unused inline function starts
in one macro and ends in another. This patch fixes the issue by finding the common ancestor
of the start and end locations of that function's body and changing the locations accordingly.

[vsk]

Thanks, this looks good overall. I just have a few minor comments.

lib/CodeGen/CoverageMappingGen.cpp

329 ↗

(On Diff #59648)

SM.getFileID() can be expensive if there's a miss in the SourceManager's cache. Seeing as End isn't updated in this loop, it might be profitable/cleaner to maintain dedicated StartFileID and EndFileID variables. Wdyt?

331 ↗

(On Diff #59648)

Please add a string here, e.g "Declaration start loc not nested within a known region".

336 ↗

(On Diff #59648)

^ ditto.

[ikudrin]

• Use StartFileID and EndFileID variables to eliminate redundant calls to SM.getFileID().
• Add comment strings to asserts.

[vsk]

LGTM.

[chapuni]

FYI :)

--- a/clang/test/CoverageMapping/unused_function.cpp
+++ b/clang/test/CoverageMapping/unused_function.cpp
@@ -3,34 +3,34 @@
 #define START_SCOPE {
 #define END_SCOPE }

-// CHECK: _Z2f0v:
+// CHECK: {{_Z2f0v|\?f0@@YAXXZ}}:
 // CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:20 = 0
 inline void f0() {}

-// CHECK: _Z2f1v:
+// CHECK: {{_Z2f1v|\?f1@@YAXXZ}}:
 // CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:31 = 0
 inline void f1() START_SCOPE }

-// CHECK: _Z2f2v:
+// CHECK: {{_Z2f2v|\?f2@@YAXXZ}}:
 // CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:29 = 0
 inline void f2() { END_SCOPE

-// CHECK: _Z2f3v:
+// CHECK: {{_Z2f3v|\?f3@@YAXXZ}}:
 // CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:39 = 0
 inline void f3() START_SCOPE END_SCOPE

-// CHECK: _Z2f4v:
+// CHECK: {{_Z2f4v|\?f4@@YAXXZ}}:
 // CHECK-NEXT: File 0, [[@LINE+2]]:10 -> [[@LINE+3]]:2 = 0
 inline void f4()
 #include "Inputs/starts_a_scope_only"
 }

-// CHECK: _Z2f5v:
+// CHECK: {{_Z2f5v|\?f5@@YAXXZ}}:
 // CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+2]]:36 = 0
 inline void f5() {
 #include "Inputs/ends_a_scope_only"

-// CHECK: _Z2f6v:
+// CHECK: {{_Z2f6v|\?f6@@YAXXZ}}:
 // CHECK-NEXT: File 0, [[@LINE+2]]:10 -> [[@LINE+3]]:36 = 0
 inline void f6()
 #include "Inputs/starts_a_scope_only"

cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp

return Loc.isMacroID() ? SM.getImmediateExpansionRange(Loc).first
                            : SM.getIncludeLoc(SM.getFileID(Loc));
   }
 
   /// \brief Return true if \c Loc is a location in a built-in macro.
   bool isInBuiltin(SourceLocation Loc) {
     return strcmp(SM.getBufferName(SM.getSpellingLoc(Loc)), "<built-in>") == 0;
   }
 
+  /// \brief Check whether \c Loc is included or expanded from \c Parent.
+  bool isNestedIn(SourceLocation Loc, FileID Parent) {
+    do {
+      Loc = getIncludeOrExpansionLoc(Loc);
+      if (Loc.isInvalid())
+        return false;
+    } while (!SM.isInFileID(Loc, Parent));
+    return true;
+  }
+
   /// \brief Get the start of \c S ignoring macro arguments and builtin macros.
   SourceLocation getStart(const Stmt *S) {
     SourceLocation Loc = S->getLocStart();
     while (SM.isMacroArgExpansion(Loc) || isInBuiltin(Loc))
       Loc = SM.getImmediateExpansionRange(Loc).first;
     return Loc;
   }
 
struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder {
   EmptyCoverageMappingBuilder(CoverageMappingModuleGen &CVM, SourceManager &SM,
                               const LangOptions &LangOpts)
       : CoverageMappingBuilder(CVM, SM, LangOpts) {}
 
   void VisitDecl(const Decl *D) {
     if (!D->hasBody())
       return;
     auto Body = D->getBody();
-    SourceRegions.emplace_back(Counter(), getStart(Body), getEnd(Body));
+    SourceLocation Start = getStart(Body);
+    SourceLocation End = getEnd(Body);
+    if (!SM.isWrittenInSameFile(Start, End)) {
+      // Walk up to find the common ancestor.
+      // Correct the locations accordingly.
+      FileID StartFileID = SM.getFileID(Start);
+      FileID EndFileID = SM.getFileID(End);
+      while (StartFileID != EndFileID && !isNestedIn(End, StartFileID)) {
+        Start = getIncludeOrExpansionLoc(Start);
+        assert(Start.isValid() &&
+               "Declaration start location not nested within a known region");
+        StartFileID = SM.getFileID(Start);
+      }
+      while (StartFileID != EndFileID) {
+        End = getPreciseTokenLocEnd(getIncludeOrExpansionLoc(End));
+        assert(End.isValid() &&
+               "Declaration end location not nested within a known region");
+        EndFileID = SM.getFileID(End);
+      }
+    }
+    SourceRegions.emplace_back(Counter(), Start, End);
   }
 
   /// \brief Write the mapping data to the output stream
   void write(llvm::raw_ostream &OS) {
     SmallVector<unsigned, 16> FileIDMapping;
     gatherFileIDs(FileIDMapping);
     emitSourceRegions();
 
void adjustForOutOfOrderTraversal(SourceLocation EndLoc) {
     // macros. Avoid adding duplicate regions in such case.
     if (getRegion().hasEndLoc() &&
         MostRecentLocation == getEndOfFileOrMacro(MostRecentLocation) &&
         isRegionAlreadyAdded(getStartOfFileOrMacro(MostRecentLocation),
                              MostRecentLocation))
       MostRecentLocation = getIncludeOrExpansionLoc(MostRecentLocation);
   }
 
-  /// \brief Check whether \c Loc is included or expanded from \c Parent.
-  bool isNestedIn(SourceLocation Loc, FileID Parent) {
-    do {
-      Loc = getIncludeOrExpansionLoc(Loc);
-      if (Loc.isInvalid())
-        return false;
-    } while (!SM.isInFileID(Loc, Parent));
-    return true;
-  }
-
   /// \brief Adjust regions and state when \c NewLoc exits a file.
   ///
   /// If moving from our most recently tracked location to \c NewLoc exits any
   /// files, this adjusts our current region stack and creates the file regions
   /// for the exited file.
   void handleFileExit(SourceLocation NewLoc) {
     if (NewLoc.isInvalid() ||
         SM.isWrittenInSameFile(MostRecentLocation, NewLoc))

cfe/trunk/test/CoverageMapping/Inputs/ends_a_scope_only

+}

cfe/trunk/test/CoverageMapping/Inputs/starts_a_scope_only

+{

cfe/trunk/test/CoverageMapping/unused_function.cpp

+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s | FileCheck %s
+
+#define START_SCOPE {
+#define END_SCOPE }
+
+// CHECK: _Z2f0v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:20 = 0
+inline void f0() {}
+
+// CHECK: _Z2f1v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:31 = 0
+inline void f1() START_SCOPE }
+
+// CHECK: _Z2f2v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:29 = 0
+inline void f2() { END_SCOPE
+
+// CHECK: _Z2f3v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+1]]:39 = 0
+inline void f3() START_SCOPE END_SCOPE
+
+// CHECK: _Z2f4v:
+// CHECK-NEXT: File 0, [[@LINE+2]]:10 -> [[@LINE+3]]:2 = 0
+inline void f4()
+#include "Inputs/starts_a_scope_only"
+}
+
+// CHECK: _Z2f5v:
+// CHECK-NEXT: File 0, [[@LINE+1]]:18 -> [[@LINE+2]]:36 = 0
+inline void f5() {
+#include "Inputs/ends_a_scope_only"
+
+// CHECK: _Z2f6v:
+// CHECK-NEXT: File 0, [[@LINE+2]]:10 -> [[@LINE+3]]:36 = 0
+inline void f6()
+#include "Inputs/starts_a_scope_only"
+#include "Inputs/ends_a_scope_only"