[Coverage] Fix PR39258: support coverage regions that start deeper than they end

[orivej]

Summary

popRegions used to assume that the start location of a region can't be nested deeper than the end location, which is not always true.

[orivej]

This looks better to me. (There is no difference in check-clang test results.)

[orivej]

Fix typo

[vsk]

Thanks, LGTM.

test/CoverageMapping/macros.c

60

Please use CHECK-LABEL here.

[orivej]

Thanks! Could you merge this (after a while to let others review)?

test/CoverageMapping/macros.c

60

CHECK-LABEL would accept unchecked output between the previous CHECK-NEXT and func6. It seems that the intention of this test is to check all lines of output, so I used CHECK-NEXT as in other tests in this file.

[orivej]

Use relative line offset.

[orivej]

Remove unrelated change.

[vsk]

Friendly ping -- @orivej were you still looking for more feedback? If not, do you still need someone to land this patch on your behalf?

[orivej]

@vsk, I think this is ready, but I don't have the merge rights. Could you commit it and D53231 for me?

lib/CodeGen/CoverageMappingGen.cpp

void completeTopLevelDeferredRegion(Counter Count,
 
     SourceMappingRegion DR = RegionStack.back();
     DR.setStartLoc(Start);
     DR.setDeferred(false);
     DeferredRegion = DR;
     completeDeferred(Count, DeferredEndLoc);
   }
 
+  size_t locationDepth(SourceLocation Loc) {
+    size_t Depth = 0;
+    while (Loc.isValid()) {
+      Loc = getIncludeOrExpansionLoc(Loc);
+      Depth++;
+    }
+    return Depth;
+  }
+
   /// Pop regions from the stack into the function's list of regions.
   ///
   /// Adds all regions from \c ParentIndex to the top of the stack to the
   /// function's \c SourceRegions.
   void popRegions(size_t ParentIndex) {
     assert(RegionStack.size() >= ParentIndex && "parent not in stack");
     bool ParentOfDeferredRegion = false;
     while (RegionStack.size() > ParentIndex) {
       SourceMappingRegion &Region = RegionStack.back();
       if (Region.hasStartLoc()) {
         SourceLocation StartLoc = Region.getBeginLoc();
         SourceLocation EndLoc = Region.hasEndLoc()
                                     ? Region.getEndLoc()
                                     : RegionStack[ParentIndex].getEndLoc();
+        size_t StartDepth = locationDepth(StartLoc);
+        size_t EndDepth = locationDepth(EndLoc);
         while (!SM.isWrittenInSameFile(StartLoc, EndLoc)) {
+          bool UnnestStart = StartDepth >= EndDepth;
+          bool UnnestEnd = EndDepth >= StartDepth;
+          if (UnnestEnd) {
             // The region ends in a nested file or macro expansion. Create a
             // separate region for each expansion.
             SourceLocation NestedLoc = getStartOfFileOrMacro(EndLoc);
             assert(SM.isWrittenInSameFile(NestedLoc, EndLoc));
 
             if (!isRegionAlreadyAdded(NestedLoc, EndLoc))
               SourceRegions.emplace_back(Region.getCounter(), NestedLoc, EndLoc);
 
             EndLoc = getPreciseTokenLocEnd(getIncludeOrExpansionLoc(EndLoc));
             if (EndLoc.isInvalid())
               llvm::report_fatal_error("File exit not handled before popRegions");
+            EndDepth--;
           }
+          if (UnnestStart) {
+            // The region begins in a nested file or macro expansion. Create a
+            // separate region for each expansion.
+            SourceLocation NestedLoc = getEndOfFileOrMacro(StartLoc);
+            assert(SM.isWrittenInSameFile(StartLoc, NestedLoc));
+
+            if (!isRegionAlreadyAdded(StartLoc, NestedLoc))
+              SourceRegions.emplace_back(Region.getCounter(), StartLoc, NestedLoc);
+
+            StartLoc = getIncludeOrExpansionLoc(StartLoc);
+            if (StartLoc.isInvalid())
+              llvm::report_fatal_error("File exit not handled before popRegions");
+            StartDepth--;
+          }
+        }
+        Region.setStartLoc(StartLoc);
         Region.setEndLoc(EndLoc);
 
         MostRecentLocation = EndLoc;
         // If this region happens to span an entire expansion, we need to make
         // sure we don't overlap the parent region with it.
         if (StartLoc == getStartOfFileOrMacro(StartLoc) &&
             EndLoc == getEndOfFileOrMacro(EndLoc))
           MostRecentLocation = getIncludeOrExpansionLoc(EndLoc);

test/CoverageMapping/macros.c

 // RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name macros.c %s | FileCheck %s
 
 #define MACRO return; bar()
 #define MACRO_2 bar()
 #define MACRO_1 return; MACRO_2
 #define MACRO_3 MACRO_2
+#define GOTO goto
 
 void bar() {}
 
 // CHECK: func
 void func() {  // CHECK-NEXT: File 0, [[@LINE]]:13 -> [[@LINE+5]]:2 = #0
   int i = 0;
   // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:8 = #0
   MACRO;       // CHECK-NEXT: File 0, [[@LINE]]:8 -> [[@LINE+2]]:2 = 0

 void func5() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+4]]:2 = #0
   int i = 0;
   if (i > 5) // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:12 = #0
     MACRO_3; // CHECK-NEXT: Expansion,File 0, [[@LINE]]:5 -> [[@LINE]]:12 = #1
 }
 // CHECK-NEXT: Expansion,File 1, 6:17 -> 6:24 = #1
 // CHECK-NEXT: File 2, 4:17 -> 4:22 = #1
 
+// CHECK-NEXT: func6
+void func6(unsigned count) { // CHECK-NEXT: File 0, [[@LINE]]:28 -> [[@LINE+4]]:2 = #0
+begin:                       // CHECK-NEXT: File 0, [[@LINE]]:1 -> [[@LINE+3]]:2 = #1
+    if (count--)             // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:16 = #1
+        GOTO begin;          // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:19 = #2
+}
+// CHECK-NEXT: Expansion,File 0, [[@LINE-2]]:9 -> [[@LINE-2]]:13 = #2
+// CHECK-NEXT: File 1, 7:14 -> 7:18 = #2
+
 int main(int argc, const char *argv[]) {
   func();
   func2();
   func3();
   func4();
 }