Skip to content

Commit

Permalink
[MC/DC][Coverage] Add assertions into emitSourceRegions()
Browse files Browse the repository at this point in the history
`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-system-headers.cpp covers llvm#78920.
* mcdc-scratch-space.c covers llvm#87000.
  • Loading branch information
chapuni committed Apr 22, 2024
1 parent 36e2577 commit 13035b2
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 5 deletions.
29 changes: 24 additions & 5 deletions clang/lib/CodeGen/CoverageMappingGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,21 @@ class SourceMappingRegion {

bool isBranch() const { return FalseCount.has_value(); }

bool isMCDCBranch() const {
const auto *BranchParams = std::get_if<mcdc::BranchParameters>(&MCDCParams);
assert(BranchParams == nullptr || BranchParams->ID >= 0);
return (BranchParams != nullptr);
}

const auto &getMCDCBranchParams() const {
return mcdc::getParams<const mcdc::BranchParameters>(MCDCParams);
}

bool isMCDCDecision() const {
const auto *DecisionParams =
std::get_if<mcdc::DecisionParameters>(&MCDCParams);
assert(!DecisionParams || DecisionParams->NumConditions > 0);
return DecisionParams;
assert(DecisionParams == nullptr || DecisionParams->NumConditions > 0);
return (DecisionParams != nullptr);
}

const auto &getMCDCDecisionParams() const {
Expand Down Expand Up @@ -464,13 +474,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) &&
Expand All @@ -480,8 +496,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};
Expand Down
27 changes: 27 additions & 0 deletions clang/test/CoverageMapping/mcdc-scratch-space.c
Original file line number Diff line number Diff line change
@@ -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));
}
47 changes: 47 additions & 0 deletions clang/test/CoverageMapping/mcdc-system-headers.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// RUN: %clang_cc1 -std=c++11 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -fcoverage-mcdc -mllvm -system-headers-coverage -emit-llvm-only -o - %s | FileCheck %s

// Will crash w/o -system-headers-coverage
// RUN: not --crash %clang_cc1 -std=c++11 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -fcoverage-mcdc -emit-llvm-only -o - %s
// REQUIRES: asserts

#ifdef IS_SYSHEADER

#pragma clang system_header
#define CONST 42
#define EXPR1(x) (x)
#define EXPR2(x) ((x) * (x))

#else

#define IS_SYSHEADER
#include __FILE__

// CHECK: _Z5func0i:
int func0(int a) {
// CHECK: Decision,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:21 = M:0, C:2
// CHECK: Expansion,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:16 = #0 (Expanded file = 1)
return (CONST && a);
// CHECK: Branch,File 0, [[@LINE-1]]:20 -> [[@LINE-1]]:21 = #2, (#1 - #2) [2,0,0]
// CHECK: Branch,File 1, [[@LINE-15]]:15 -> [[@LINE-15]]:17 = 0, 0 [1,2,0]
}

// CHECK: _Z5func1ii:
int func1(int a, int b) {
// CHECK: Decision,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:21 = M:0, C:2
// CHECK: Branch,File 0, [[@LINE+1]]:11 -> [[@LINE+1]]:12 = (#0 - #1), #1 [1,0,2]
return (a || EXPR1(b));
// CHECK: Expansion,File 0, [[@LINE-1]]:16 -> [[@LINE-1]]:21 = #1 (Expanded file = 1)
// CHECK: Branch,File 1, [[@LINE-23]]:18 -> [[@LINE-23]]:21 = (#1 - #2), #2 [2,0,0]
}

// CHECK: _Z5func2ii:
int func2(int a, int b) {
// Decision,File 0, [[@LINE+3]]:11 -> [[@LINE+3]]:28 = M:0, C:2
// Expansion,File 0, [[@LINE+2]]:11 -> [[@LINE+2]]:16 = #0 (Expanded file = 1)
// Expansion,File 0, [[@LINE+1]]:23 -> [[@LINE+1]]:28 = #1 (Expanded file = 2)
return (EXPR2(a) && EXPR1(a));
// CHECK: Branch,File 1, [[@LINE-31]]:18 -> [[@LINE-31]]:29 = #1, (#0 - #1) [1,2,0]
// CHECK: Branch,File 2, [[@LINE-33]]:18 -> [[@LINE-33]]:21 = #2, (#1 - #2) [2,0,0]
}

#endif

0 comments on commit 13035b2

Please sign in to comment.