[clang] 896bceb - [MC/DC][Coverage] Add assertions into emitSourceRegions() (#89572)

[via cfe-commits]

Author: NAKAMURA Takumi
Date: 2024-05-23T13:57:12+09:00
New Revision: 896bceb9537ecd906668fdd2bcb0310b32174917

URL: https://github.com/llvm/llvm-project/commit/896bceb9537ecd906668fdd2bcb0310b32174917
DIFF: https://github.com/llvm/llvm-project/commit/896bceb9537ecd906668fdd2bcb0310b32174917.diff

LOG: [MC/DC][Coverage] Add assertions into emitSourceRegions() (#89572)

`emitSourceRegions()` has bugs to emit malformed MC/DC coverage
mappings. They were detected in `llvm-cov` as the crash.

Detect inconsistencies earlier in `clang` with assertions.

* mcdc-scratch-space.c covers #87000.

Added: 
    clang/test/CoverageMapping/mcdc-scratch-space.c

Modified: 
    clang/lib/CodeGen/CoverageMappingGen.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index f4de21bac4b46..993d7cc7e21fa 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -191,6 +191,10 @@ class SourceMappingRegion {
 
   bool isBranch() const { return FalseCount.has_value(); }
 
+  bool isMCDCBranch() const {
+    return std::holds_alternative<mcdc::BranchParameters>(MCDCParams);
+  }
+
   bool isMCDCDecision() const {
     return std::holds_alternative<mcdc::DecisionParameters>(MCDCParams);
   }
@@ -472,13 +476,19 @@ class CoverageMappingBuilder {
       // Ignore regions from system headers unless collecting coverage from
       // system headers is explicitly enabled.
       if (!SystemHeadersCoverage &&
-          SM.isInSystemHeader(SM.getSpellingLoc(LocStart)))
+          SM.isInSystemHeader(SM.getSpellingLoc(LocStart))) {
+        assert(!Region.isMCDCBranch() && !Region.isMCDCDecision() &&
+               "Don't suppress the condition in system headers");
         continue;
+      }
 
       auto CovFileID = getCoverageFileID(LocStart);
       // Ignore regions that don't have a file, such as builtin macros.
-      if (!CovFileID)
+      if (!CovFileID) {
+        assert(!Region.isMCDCBranch() && !Region.isMCDCDecision() &&
+               "Don't suppress the condition in non-file regions");
         continue;
+      }
 
       SourceLocation LocEnd = Region.getEndLoc();
       assert(SM.isWrittenInSameFile(LocStart, LocEnd) &&
@@ -488,8 +498,11 @@ class CoverageMappingBuilder {
       // This not only suppresses redundant regions, but sometimes prevents
       // creating regions with wrong counters if, for example, a statement's
       // body ends at the end of a nested macro.
-      if (Filter.count(std::make_pair(LocStart, LocEnd)))
+      if (Filter.count(std::make_pair(LocStart, LocEnd))) {
+        assert(!Region.isMCDCBranch() && !Region.isMCDCDecision() &&
+               "Don't suppress the condition");
         continue;
+      }
 
       // Find the spelling locations for the mapping region.
       SpellingRegion SR{SM, LocStart, LocEnd};

diff  --git a/clang/test/CoverageMapping/mcdc-scratch-space.c b/clang/test/CoverageMapping/mcdc-scratch-space.c
new file mode 100644
index 0000000000000..962d10653a028
--- /dev/null
+++ b/clang/test/CoverageMapping/mcdc-scratch-space.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c99 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s
+// XFAIL: *
+// REQUIRES: asserts
+
+int builtin_macro0(int a) {
+  return (__LINE__
+          && a);
+}
+
+int builtin_macro1(int a) {
+  return (a
+          || __LINE__);
+}
+
+#define PRE(x) pre_##x
+
+int pre0(int pre_a, int b_post) {
+  return (PRE(a)
+          && b_post);
+}
+
+#define POST(x) x##_post
+
+int post0(int pre_a, int b_post) {
+  return (pre_a
+          || POST(b));
+}