[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 509098.
zequanwu added a comment.

- Handle invalid start location.
- Add a test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147073

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/invalid_location.cpp


Index: clang/test/CoverageMapping/invalid_location.cpp
===================================================================
--- /dev/null
+++ clang/test/CoverageMapping/invalid_location.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -std=c++20 -triple %itanium_abi_triple %s | FileCheck %s
+
+namespace std { template <typename> class initializer_list {}; }
+
+template <typename> struct T {
+  T(std::initializer_list<int>, int = int());
+  bool b;
+};
+
+template <typename> struct S1 {
+  static void foo() {
+    class C;
+    0 ? T<C>{} : T<C>{};
+    0 ? 1 : 2;
+  }
+};
+
+void bar() {
+  S1<int>::foo();
+}
+
+
+// CHECK:      _ZN2S1IiE3fooEv:
+// CHECK-NEXT:   File 0, 11:21 -> 15:4 = #0
+// CHECK-NEXT:   File 0, 13:5 -> 13:6 = #0
+// CHECK-NEXT:   Branch,File 0, 13:5 -> 13:6 = 0, 0
+// CHECK-NEXT:   Gap,File 0, 13:8 -> 13:9 = #1
+// FIXME: It's supposed to have two different counters for true and false
+//        branches at line 13 as it has for line 14, shown below. It's missing
+//        now because 'T<C>{}' doesn't have a valid end location for now.
+// CHECK-NETX:   File 0, 13:9 -> 13:14 = #3
+// CHECK-NETX:   File 0, 13:18 -> 13:23 = (#0 - #3)
+
+// CHECK-NEXT:   File 0, 14:5 -> 14:6 = #0
+// CHECK-NEXT:   Branch,File 0, 14:5 -> 14:6 = 0, 0
+// CHECK-NEXT:   Gap,File 0, 14:8 -> 14:9 = #2
+// CHECK-NEXT:   File 0, 14:9 -> 14:10 = #2
+// CHECK-NEXT:   File 0, 14:13 -> 14:14 = (#0 - #2)
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===================================================================
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -602,6 +602,13 @@
       MostRecentLocation = *StartLoc;
     }
 
+    // If the either location is invalid, set it to std::nullopt to avoid
+    // letting users of RegionStack think that region has a valid start/end
+    // location.
+    if (StartLoc && StartLoc->isInvalid())
+      StartLoc = std::nullopt;
+    if (EndLoc && EndLoc->isInvalid())
+      EndLoc = std::nullopt;
     RegionStack.emplace_back(Count, FalseCount, StartLoc, EndLoc);
 
     return RegionStack.size() - 1;
@@ -624,7 +631,8 @@
     assert(RegionStack.size() >= ParentIndex && "parent not in stack");
     while (RegionStack.size() > ParentIndex) {
       SourceMappingRegion &Region = RegionStack.back();
-      if (Region.hasStartLoc()) {
+      if (Region.hasStartLoc() &&
+          (Region.hasEndLoc() || RegionStack[ParentIndex].hasEndLoc())) {
         SourceLocation StartLoc = Region.getBeginLoc();
         SourceLocation EndLoc = Region.hasEndLoc()
                                     ? Region.getEndLoc()
@@ -691,7 +699,7 @@
         assert(SM.isWrittenInSameFile(Region.getBeginLoc(), EndLoc));
         assert(SpellingRegion(SM, Region).isInSourceOrder());
         SourceRegions.push_back(Region);
-        }
+      }
       RegionStack.pop_back();
     }
   }
@@ -716,7 +724,8 @@
 
     // The statement may be spanned by an expansion. Make sure we handle a file
     // exit out of this expansion before moving to the next statement.
-    if (SM.isBeforeInTranslationUnit(StartLoc, S->getBeginLoc()))
+    if (StartLoc.isValid() &&
+        SM.isBeforeInTranslationUnit(StartLoc, S->getBeginLoc()))
       MostRecentLocation = EndLoc;
 
     return ExitCount;
@@ -891,6 +900,8 @@
   /// Find a valid gap range between \p AfterLoc and \p BeforeLoc.
   std::optional<SourceRange> findGapAreaBetween(SourceLocation AfterLoc,
                                                 SourceLocation BeforeLoc) {
+    if (AfterLoc.isInvalid() || BeforeLoc.isInvalid())
+      return std::nullopt;
     // If AfterLoc is in function-like macro, use the right parenthesis
     // location.
     if (AfterLoc.isMacroID()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147073.509098.patch
Type: text/x-patch
Size: 3848 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230328/55d3d809/attachment.bin>