Skip to content

Commit af33631

Browse files
committed
[MC/DC] Introduce -fmcdc-single-conditions to include also single conditions
`-fmcdc-single-conditions` is `CC1Option` for now. This change discovers `isInstrumentedCondition(Cond)` on `DoStmt/ForStmt/IfStmt/WhleStmt/AbstractConditionalOperator` and add them into Decisions. An example of the report: ``` MC/DC Decision Region (mmm:nn) to (mmm:nn) Number of Conditions: 1 Condition C1 -->(mmm:nn) Executed MC/DC Test Vectors: C1 Result 1 { F = F } 2 { T = T } C1-Pair: covered: (1,2) MC/DC Coverage for Expression: 100.00% ``` The Decision is covered only if both `true` and `false` are covered. Fixes #95336
1 parent 176368a commit af33631

File tree

9 files changed

+190
-27
lines changed

9 files changed

+190
-27
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ Improvements to Coverage Mapping
118118

119119
- [MC/DC] Nested expressions are handled as individual MC/DC expressions.
120120

121+
- [MC/DC] Non-boolean expressions on conditions can be included with
122+
`-fmcdc-single-conditions`. (#GH95336)
123+
121124
Bug Fixes in This Version
122125
-------------------------
123126

clang/docs/SourceBasedCodeCoverage.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,10 @@ requires 8 test vectors.
510510
Expressions such as ``((a0 && b0) || (a1 && b1) || ...)`` can cause the
511511
number of test vectors to increase exponentially.
512512

513+
Clang handles only binary logical operators as MC/DC coverage. Single
514+
conditions without logcal operators on `do/for/while/if/?!` can be
515+
included with `-Xclang -fmcdc-single-conditions`.
516+
513517
Switch statements
514518
-----------------
515519

clang/include/clang/Basic/CodeGenOptions.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ CODEGENOPT(DumpCoverageMapping , 1, 0) ///< Dump the generated coverage mapping
236236
CODEGENOPT(MCDCCoverage , 1, 0) ///< Enable MC/DC code coverage criteria.
237237
VALUE_CODEGENOPT(MCDCMaxConds, 16, 32767) ///< MC/DC Maximum conditions.
238238
VALUE_CODEGENOPT(MCDCMaxTVs, 32, 0x7FFFFFFE) ///< MC/DC Maximum test vectors.
239+
VALUE_CODEGENOPT(MCDCSingleCond, 1, 0) ///< Enable MC/DC single conditions.
239240

240241
/// If -fpcc-struct-return or -freg-struct-return is specified.
241242
ENUM_CODEGENOPT(StructReturnConvention, StructReturnConventionKind, 2, SRCK_Default)

clang/include/clang/Driver/Options.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,6 +1742,10 @@ def fmcdc_max_test_vectors_EQ : Joined<["-"], "fmcdc-max-test-vectors=">,
17421742
Group<f_Group>, Visibility<[CC1Option]>,
17431743
HelpText<"Maximum number of test vectors in MC/DC coverage">,
17441744
MarshallingInfoInt<CodeGenOpts<"MCDCMaxTVs">, "0x7FFFFFFE">;
1745+
def fmcdc_single_conditions : Flag<["-"], "fmcdc-single-conditions">,
1746+
Group<f_Group>, Visibility<[CC1Option]>,
1747+
HelpText<"Include also single conditions as MC/DC coverage">,
1748+
MarshallingInfoFlag<CodeGenOpts<"MCDCSingleCond">>;
17451749
def fprofile_generate : Flag<["-"], "fprofile-generate">,
17461750
Group<f_Group>, Visibility<[ClangOption, CLOption]>,
17471751
HelpText<"Generate instrumented code to collect execution counts into default.profraw (overridden by LLVM_PROFILE_FILE env var)">;

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,20 +196,36 @@ RawAddress CodeGenFunction::CreateMemTempWithoutCast(QualType Ty,
196196
/// EvaluateExprAsBool - Perform the usual unary conversions on the specified
197197
/// expression and compare the result against zero, returning an Int1Ty value.
198198
llvm::Value *CodeGenFunction::EvaluateExprAsBool(const Expr *E) {
199+
auto DecisionExpr = stripCond(E);
200+
if (isMCDCDecisionExpr(DecisionExpr) && isInstrumentedCondition(DecisionExpr))
201+
maybeResetMCDCCondBitmap(DecisionExpr);
202+
else
203+
DecisionExpr = nullptr;
204+
199205
PGO.setCurrentStmt(E);
206+
llvm::Value *Result;
200207
if (const MemberPointerType *MPT = E->getType()->getAs<MemberPointerType>()) {
201208
llvm::Value *MemPtr = EmitScalarExpr(E);
202-
return CGM.getCXXABI().EmitMemberPointerIsNotNull(*this, MemPtr, MPT);
209+
Result = CGM.getCXXABI().EmitMemberPointerIsNotNull(*this, MemPtr, MPT);
210+
} else {
211+
QualType BoolTy = getContext().BoolTy;
212+
SourceLocation Loc = E->getExprLoc();
213+
CGFPOptionsRAII FPOptsRAII(*this, E);
214+
if (!E->getType()->isAnyComplexType())
215+
Result =
216+
EmitScalarConversion(EmitScalarExpr(E), E->getType(), BoolTy, Loc);
217+
else
218+
Result = EmitComplexToScalarConversion(EmitComplexExpr(E), E->getType(),
219+
BoolTy, Loc);
203220
}
204221

205-
QualType BoolTy = getContext().BoolTy;
206-
SourceLocation Loc = E->getExprLoc();
207-
CGFPOptionsRAII FPOptsRAII(*this, E);
208-
if (!E->getType()->isAnyComplexType())
209-
return EmitScalarConversion(EmitScalarExpr(E), E->getType(), BoolTy, Loc);
222+
if (DecisionExpr) {
223+
if (isMCDCBranchExpr(stripCond(E)))
224+
maybeUpdateMCDCCondBitmap(stripCond(E), Result);
225+
maybeUpdateMCDCTestVectorBitmap(DecisionExpr);
226+
}
210227

211-
return EmitComplexToScalarConversion(EmitComplexExpr(E), E->getType(), BoolTy,
212-
Loc);
228+
return Result;
213229
}
214230

215231
/// EmitIgnoredExpr - Emit code to compute the specified expression,

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,7 +1684,7 @@ class CodeGenFunction : public CodeGenTypeCache {
16841684

16851685
/// Zero-init the MCDC temp value.
16861686
void maybeResetMCDCCondBitmap(const Expr *E) {
1687-
if (isMCDCCoverageEnabled() && isBinaryLogicalOp(E)) {
1687+
if (isMCDCCoverageEnabled()) {
16881688
PGO.emitMCDCCondBitmapReset(Builder, E);
16891689
PGO.setCurrentStmt(E);
16901690
}
@@ -1693,7 +1693,7 @@ class CodeGenFunction : public CodeGenTypeCache {
16931693
/// Increment the profiler's counter for the given expression by \p StepV.
16941694
/// If \p StepV is null, the default increment is 1.
16951695
void maybeUpdateMCDCTestVectorBitmap(const Expr *E) {
1696-
if (isMCDCCoverageEnabled() && isBinaryLogicalOp(E)) {
1696+
if (isMCDCCoverageEnabled()) {
16971697
PGO.emitMCDCTestVectorBitmapUpdate(Builder, E, *this);
16981698
PGO.setCurrentStmt(E);
16991699
}

clang/lib/CodeGen/CodeGenPGO.cpp

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
168168
MCDC::State &MCDCState;
169169
/// Maximum number of supported MC/DC conditions in a boolean expression.
170170
unsigned MCDCMaxCond;
171+
/// Take single conditions into account.
172+
bool MCDCSingleCond;
171173
/// The profile version.
172174
uint64_t ProfileVersion;
173175
/// Diagnostics Engine used to report warnings.
@@ -176,10 +178,11 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
176178
MapRegionCounters(PGOHashVersion HashVersion, uint64_t ProfileVersion,
177179
llvm::DenseMap<const Stmt *, CounterPair> &CounterMap,
178180
MCDC::State &MCDCState, unsigned MCDCMaxCond,
179-
DiagnosticsEngine &Diag)
181+
bool MCDCSingleCond, DiagnosticsEngine &Diag)
180182
: NextCounter(0), Hash(HashVersion), CounterMap(CounterMap),
181183
MCDCState(MCDCState), MCDCMaxCond(MCDCMaxCond),
182-
ProfileVersion(ProfileVersion), Diag(Diag) {}
184+
MCDCSingleCond(MCDCSingleCond), ProfileVersion(ProfileVersion),
185+
Diag(Diag) {}
183186

184187
// Blocks and lambdas are handled as separate functions, so we need not
185188
// traverse them in the parent context.
@@ -240,12 +243,31 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
240243

241244
SmallVector<DecisionState, 1> DecisionStack;
242245

246+
llvm::DenseSet<const Expr *> StagingDecisions;
247+
248+
template <class T> bool pushInstrumentedCond(Stmt *S) {
249+
if (auto *St = dyn_cast<T>(S)) {
250+
if (auto *Cond = St->getCond();
251+
Cond && CodeGenFunction::isInstrumentedCondition(Cond)) {
252+
StagingDecisions.insert(CodeGenFunction::stripCond(Cond));
253+
return true;
254+
}
255+
}
256+
257+
return false;
258+
}
259+
243260
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
244261
bool dataTraverseStmtPre(Stmt *S) {
245262
/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
246263
if (MCDCMaxCond == 0)
247264
return true;
248265

266+
const auto *E = dyn_cast<Expr>(S);
267+
268+
if (StagingDecisions.contains(E))
269+
DecisionStack.emplace_back(E, true);
270+
249271
/// Mark "in splitting" when a leaf is met.
250272
if (!DecisionStack.empty()) {
251273
auto &StackTop = DecisionStack.back();
@@ -262,13 +284,20 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
262284
assert(!StackTop.Leaves.contains(S));
263285
}
264286

265-
if (const auto *E = dyn_cast<Expr>(S)) {
287+
if (E) {
266288
if (const auto *BinOp =
267289
dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E));
268290
BinOp && BinOp->isLogicalOp())
269291
DecisionStack.emplace_back(E);
270292
}
271293

294+
if (MCDCSingleCond) {
295+
pushInstrumentedCond<AbstractConditionalOperator>(S) ||
296+
pushInstrumentedCond<DoStmt>(S) || pushInstrumentedCond<IfStmt>(S) ||
297+
pushInstrumentedCond<ForStmt>(S) ||
298+
pushInstrumentedCond<WhileStmt>(S);
299+
}
300+
272301
return true;
273302
}
274303

@@ -1098,7 +1127,8 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) {
10981127
RegionCounterMap.reset(new llvm::DenseMap<const Stmt *, CounterPair>);
10991128
RegionMCDCState.reset(new MCDC::State);
11001129
MapRegionCounters Walker(HashVersion, ProfileVersion, *RegionCounterMap,
1101-
*RegionMCDCState, MCDCMaxConditions, CGM.getDiags());
1130+
*RegionMCDCState, MCDCMaxConditions,
1131+
CGM.getCodeGenOpts().MCDCSingleCond, CGM.getDiags());
11021132
if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
11031133
Walker.TraverseDecl(const_cast<FunctionDecl *>(FD));
11041134
else if (const ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(D))

clang/lib/CodeGen/CoverageMappingGen.cpp

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,9 +1164,7 @@ struct CounterCoverageMappingBuilder
11641164
/// result in the generation of a branch.
11651165
void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt,
11661166
const mcdc::ConditionIDs &Conds = {}) {
1167-
// Check for NULL conditions.
1168-
if (!C)
1169-
return;
1167+
assert(C && "Condition Expr shouldn't be null!");
11701168

11711169
// Ensure we are an instrumentable condition (i.e. no "&&" or "||"). Push
11721170
// region onto RegionStack but immediately pop it (which adds it to the
@@ -1630,6 +1628,9 @@ struct CounterCoverageMappingBuilder
16301628
}
16311629

16321630
void VisitWhileStmt(const WhileStmt *S) {
1631+
unsigned SourceRegionsSince = SourceRegions.size();
1632+
MCDCBuilder.checkDecisionRootOrPush(S->getCond());
1633+
16331634
extendRegion(S);
16341635

16351636
Counter ParentCount = getRegion().getCounter();
@@ -1676,10 +1677,15 @@ struct CounterCoverageMappingBuilder
16761677

16771678
// Create Branch Region around condition.
16781679
if (!llvm::EnableSingleByteCoverage)
1679-
createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
1680+
createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped,
1681+
MCDCBuilder.getCurCondIDs());
1682+
createOrCancelDecision(S->getCond(), SourceRegionsSince);
16801683
}
16811684

16821685
void VisitDoStmt(const DoStmt *S) {
1686+
unsigned SourceRegionsSince = SourceRegions.size();
1687+
MCDCBuilder.checkDecisionRootOrPush(S->getCond());
1688+
16831689
extendRegion(S);
16841690

16851691
Counter ParentCount = getRegion().getCounter();
@@ -1722,13 +1728,20 @@ struct CounterCoverageMappingBuilder
17221728

17231729
// Create Branch Region around condition.
17241730
if (!llvm::EnableSingleByteCoverage)
1725-
createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
1731+
createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped,
1732+
MCDCBuilder.getCurCondIDs());
1733+
createOrCancelDecision(S->getCond(), SourceRegionsSince);
17261734

17271735
if (BodyHasTerminateStmt)
17281736
HasTerminateStmt = true;
17291737
}
17301738

17311739
void VisitForStmt(const ForStmt *S) {
1740+
const Expr *Cond = S->getCond();
1741+
unsigned SourceRegionsSince = SourceRegions.size();
1742+
if (Cond)
1743+
MCDCBuilder.checkDecisionRootOrPush(Cond);
1744+
17321745
extendRegion(S);
17331746
if (S->getInit())
17341747
Visit(S->getInit());
@@ -1775,7 +1788,7 @@ struct CounterCoverageMappingBuilder
17751788
assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount ||
17761789
llvm::EnableSingleByteCoverage);
17771790

1778-
if (const Expr *Cond = S->getCond()) {
1791+
if (Cond) {
17791792
propagateCounts(CondCount, Cond);
17801793
adjustForOutOfOrderTraversal(getEnd(S));
17811794
}
@@ -1797,8 +1810,11 @@ struct CounterCoverageMappingBuilder
17971810
}
17981811

17991812
// Create Branch Region around condition.
1800-
if (!llvm::EnableSingleByteCoverage)
1801-
createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
1813+
if (!llvm::EnableSingleByteCoverage && Cond) {
1814+
createBranchRegion(Cond, BodyCount, BranchCount.Skipped,
1815+
MCDCBuilder.getCurCondIDs());
1816+
createOrCancelDecision(Cond, SourceRegionsSince);
1817+
}
18021818
}
18031819

18041820
void VisitCXXForRangeStmt(const CXXForRangeStmt *S) {
@@ -2069,6 +2085,9 @@ struct CounterCoverageMappingBuilder
20692085
else if (S->isConstexpr())
20702086
return coverIfConstexpr(S);
20712087

2088+
unsigned SourceRegionsSince = SourceRegions.size();
2089+
MCDCBuilder.checkDecisionRootOrPush(S->getCond());
2090+
20722091
extendRegion(S);
20732092
if (S->getInit())
20742093
Visit(S->getInit());
@@ -2127,7 +2146,9 @@ struct CounterCoverageMappingBuilder
21272146

21282147
if (!llvm::EnableSingleByteCoverage)
21292148
// Create Branch Region around condition.
2130-
createBranchRegion(S->getCond(), ThenCount, ElseCount);
2149+
createBranchRegion(S->getCond(), ThenCount, ElseCount,
2150+
MCDCBuilder.getCurCondIDs());
2151+
createOrCancelDecision(S->getCond(), SourceRegionsSince);
21312152
}
21322153

21332154
void VisitCXXTryStmt(const CXXTryStmt *S) {
@@ -2150,6 +2171,9 @@ struct CounterCoverageMappingBuilder
21502171
}
21512172

21522173
void VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
2174+
unsigned SourceRegionsSince = SourceRegions.size();
2175+
MCDCBuilder.checkDecisionRootOrPush(E->getCond());
2176+
21532177
extendRegion(E);
21542178

21552179
Counter ParentCount = getRegion().getCounter();
@@ -2189,7 +2213,9 @@ struct CounterCoverageMappingBuilder
21892213

21902214
// Create Branch Region around condition.
21912215
if (!llvm::EnableSingleByteCoverage)
2192-
createBranchRegion(E->getCond(), TrueCount, FalseCount);
2216+
createBranchRegion(E->getCond(), TrueCount, FalseCount,
2217+
MCDCBuilder.getCurCondIDs());
2218+
createOrCancelDecision(E->getCond(), SourceRegionsSince);
21932219
}
21942220

21952221
inline unsigned findMCDCBranchesInSourceRegion(

0 commit comments

Comments
 (0)