Skip to content

Commit bd64835

Browse files
committed
[coverage] MC/DC reports unrechable and uncoverable conditions
1 parent 1707521 commit bd64835

File tree

12 files changed

+218
-72
lines changed

12 files changed

+218
-72
lines changed

clang/docs/SourceBasedCodeCoverage.rst

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,17 @@ starts a new boolean expression that is separated from the other conditions by
522522
the operator ``func()``. When this is encountered, a warning will be generated
523523
and the boolean expression will not be instrumented.
524524

525+
Besides, MC/DC may report conditions with three states: ``uncoverable``, ``constant`` and ``unreachable``.
526+
``uncoverable`` means the condition could be evaluated but it cannot affect outcome of the decision.
527+
``constant`` means the condition is always evaluated to the same value.
528+
While ``unreachable`` means the condition is never evaluated.
529+
For instance, in ``a || true || b``, value of the decision is always ``true``.
530+
``a`` can not make the decision be ``false`` as it varies. And the second condition, ``true`` can not be evaluated to ``false``.
531+
While ``b`` is always short-circuited. Hence ``a`` is ``uncoverable``, ``true`` is ``constant`` and ``b`` is ``unreachable``.
532+
By default statistics of MCDC counts uncoverable and unreachable conditions but excludes constants. Users can pass option
533+
``--mcdc-exclude`` to control this behavior.
534+
If a decision is proved to no branch theoretically, it shows ``Folded`` rather than coverage percent.
535+
525536
Switch statements
526537
-----------------
527538

llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,13 @@ struct MCDCRecord {
396396
/// are effectively ignored.
397397
enum CondState { MCDC_DontCare = -1, MCDC_False = 0, MCDC_True = 1 };
398398

399+
enum CondResult {
400+
MCDC_Normal,
401+
MCDC_Constant,
402+
MCDC_Uncoverable,
403+
MCDC_Unreachable
404+
};
405+
399406
/// Emulate SmallVector<CondState> with a pair of BitVector.
400407
///
401408
/// True False DontCare (Impossible)
@@ -454,19 +461,21 @@ struct MCDCRecord {
454461
using TVPairMap = llvm::DenseMap<unsigned, TVRowPair>;
455462
using CondIDMap = llvm::DenseMap<unsigned, unsigned>;
456463
using LineColPairMap = llvm::DenseMap<unsigned, LineColPair>;
464+
using ResultVector = llvm::SmallVector<CondResult>;
457465

458466
private:
459467
CounterMappingRegion Region;
460468
TestVectors TV;
461469
std::optional<TVPairMap> IndependencePairs;
462470
BoolVector Folded;
471+
ResultVector CondResults;
463472
CondIDMap PosToID;
464473
LineColPairMap CondLoc;
465474

466475
public:
467476
MCDCRecord(const CounterMappingRegion &Region, TestVectors &&TV,
468-
BoolVector &&Folded, CondIDMap &&PosToID, LineColPairMap &&CondLoc)
469-
: Region(Region), TV(std::move(TV)), Folded(std::move(Folded)),
477+
BoolVector &&Folded,ResultVector &&CondResults, CondIDMap &&PosToID, LineColPairMap &&CondLoc)
478+
: Region(Region), TV(std::move(TV)), Folded(std::move(Folded)), CondResults(std::move(CondResults)),
470479
PosToID(std::move(PosToID)), CondLoc(std::move(CondLoc)) {
471480
findIndependencePairs();
472481
}
@@ -483,6 +492,12 @@ struct MCDCRecord {
483492
bool isCondFolded(unsigned Condition) const {
484493
return Folded[false][Condition] || Folded[true][Condition];
485494
}
495+
bool isCondConstant(unsigned Condition) const {
496+
return getCondResult(Condition) == CondResult::MCDC_Constant;
497+
}
498+
CondResult getCondResult(unsigned Condition) const {
499+
return CondResults[Condition];
500+
}
486501

487502
/// Return the evaluation of a condition (indicated by Condition) in an
488503
/// executed test vector (indicated by TestVectorIndex), which will be True,
@@ -523,20 +538,25 @@ struct MCDCRecord {
523538
return (*IndependencePairs)[PosToID[Condition]];
524539
}
525540

526-
float getPercentCovered() const {
527-
unsigned Folded = 0;
541+
/// Return if the decision is coverable and percent of covered conditions.
542+
/// Only coverable conditions are counted as denominator.
543+
std::pair<bool, float> getPercentCovered() const {
544+
unsigned Excluded = 0;
528545
unsigned Covered = 0;
529546
for (unsigned C = 0; C < getNumConditions(); C++) {
530-
if (isCondFolded(C))
531-
Folded++;
547+
if (isCondConstant(C))
548+
Excluded++;
532549
else if (isConditionIndependencePairCovered(C))
533550
Covered++;
534551
}
535552

536-
unsigned Total = getNumConditions() - Folded;
553+
unsigned Total = getNumConditions() - Excluded;
537554
if (Total == 0)
538-
return 0.0;
539-
return (static_cast<double>(Covered) / static_cast<double>(Total)) * 100.0;
555+
return {false, 0.0};
556+
return {
557+
true,
558+
(static_cast<double>(Covered) / static_cast<double>(Total)) * 100.0,
559+
};
540560
}
541561

542562
std::string getConditionHeaderString(unsigned Condition) {
@@ -571,7 +591,7 @@ struct MCDCRecord {
571591
// Add individual condition values to the string.
572592
OS << " " << TestVectorIndex + 1 << " { ";
573593
for (unsigned Condition = 0; Condition < NumConditions; Condition++) {
574-
if (isCondFolded(Condition))
594+
if (isCondConstant(Condition))
575595
OS << "C";
576596
else {
577597
switch (getTVCondition(TestVectorIndex, Condition)) {
@@ -607,14 +627,25 @@ struct MCDCRecord {
607627
std::ostringstream OS;
608628

609629
OS << " C" << Condition + 1 << "-Pair: ";
610-
if (isCondFolded(Condition)) {
630+
switch (getCondResult(Condition)) {
631+
case CondResult::MCDC_Normal:
632+
if (isConditionIndependencePairCovered(Condition)) {
633+
TVRowPair rows = getConditionIndependencePair(Condition);
634+
OS << "covered: (" << rows.first << ",";
635+
OS << rows.second << ")\n";
636+
} else
637+
OS << "not covered\n";
638+
break;
639+
case CondResult::MCDC_Constant:
611640
OS << "constant folded\n";
612-
} else if (isConditionIndependencePairCovered(Condition)) {
613-
TVRowPair rows = getConditionIndependencePair(Condition);
614-
OS << "covered: (" << rows.first << ",";
615-
OS << rows.second << ")\n";
616-
} else
617-
OS << "not covered\n";
641+
break;
642+
case CondResult::MCDC_Uncoverable:
643+
OS << "uncoverable\n";
644+
break;
645+
case CondResult::MCDC_Unreachable:
646+
OS << "unreachable\n";
647+
break;
648+
}
618649

619650
return OS.str();
620651
}

llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

Lines changed: 112 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,16 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
423423
/// Vector used to track whether a condition is constant folded.
424424
MCDCRecord::BoolVector Folded;
425425

426+
/// Vector used to track whether a condition is constant folded.
427+
MCDCRecord::ResultVector CondResults;
428+
426429
/// Mapping of calculated MC/DC Independence Pairs for each condition.
427430
MCDCRecord::TVPairMap IndependencePairs;
428431

432+
/// All possible Test Vectors for the boolean expression derived from
433+
/// binary decision diagran of the expression.
434+
MCDCRecord::TestVectors TestVectors;
435+
429436
/// Storage for ExecVectors
430437
/// ExecVectors is the alias of its 0th element.
431438
std::array<MCDCRecord::TestVectors, 2> ExecVectorsByCond;
@@ -449,6 +456,7 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
449456
Region(Region), DecisionParams(Region.getDecisionParams()),
450457
Branches(Branches), NumConditions(DecisionParams.NumConditions),
451458
Folded{{BitVector(NumConditions), BitVector(NumConditions)}},
459+
CondResults(NumConditions, MCDCRecord::CondResult::MCDC_Normal),
452460
IndependencePairs(NumConditions), ExecVectors(ExecVectorsByCond[false]),
453461
IsVersion11(IsVersion11) {}
454462

@@ -472,6 +480,7 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
472480

473481
assert(TVIdx < SavedNodes[ID].Width);
474482
assert(TVIdxs.insert(NextTVIdx).second && "Duplicate TVIdx");
483+
TestVectors.push_back({TV, MCDCCond});
475484

476485
if (!Bitmap[IsVersion11
477486
? DecisionParams.BitmapIdx * CHAR_BIT + TV.getIndex()
@@ -499,7 +508,6 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
499508
buildTestVector(TV, 0, 0);
500509
assert(TVIdxs.size() == unsigned(NumTestVectors) &&
501510
"TVIdxs wasn't fulfilled");
502-
503511
// Fill ExecVectors order by False items and True items.
504512
// ExecVectors is the alias of ExecVectorsByCond[false], so
505513
// Append ExecVectorsByCond[true] on it.
@@ -508,44 +516,128 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
508516
std::make_move_iterator(ExecVectorsT.end()));
509517
}
510518

519+
void findCoverablePairs(const MCDCRecord::CondIDMap &PosToID) {
520+
llvm::SmallVector<unsigned> FoldedCondPos;
521+
for (unsigned I = 0; I < CondResults.size(); ++I) {
522+
if (CondResults[I] == MCDCRecord::MCDC_Constant ||
523+
CondResults[I] == MCDCRecord::MCDC_Unreachable) {
524+
FoldedCondPos.push_back(I);
525+
}
526+
}
527+
if (FoldedCondPos.empty()) {
528+
return;
529+
}
530+
std::array<MCDCRecord::TestVectors, 2> PracticalTestVectorsByCond;
531+
for (const auto &TVWithCond : TestVectors) {
532+
const bool Practical =
533+
llvm::all_of(FoldedCondPos, [&](const unsigned &Pos) {
534+
const auto &[TV, Cond] = TVWithCond;
535+
const auto ID = PosToID.at(Pos);
536+
if (TV[ID] == MCDCRecord::MCDC_DontCare) {
537+
return true;
538+
}
539+
if (CondResults[Pos] == MCDCRecord::MCDC_Constant) {
540+
const auto ConstantValue = Branches[Pos]->Count.isZero()
541+
? MCDCRecord::MCDC_False
542+
: MCDCRecord::MCDC_True;
543+
if (TV[ID] == ConstantValue) {
544+
return true;
545+
}
546+
}
547+
return false;
548+
});
549+
550+
if (Practical) {
551+
PracticalTestVectorsByCond[TVWithCond.second].push_back(TVWithCond);
552+
}
553+
}
554+
555+
// If a condition:
556+
// - is uncoverable, all test vectors in exact one element of
557+
// `PracticalTestVectorsByCond` show it is `DontCare`;
558+
// - is unreachable, all test vectors in both elements of
559+
// `PracticalTestVectorsByCond` show it is `DontCare`;
560+
//
561+
// Otherwise, the condition is coverable as long as it has not been marked
562+
// as constant or unreachable before.
563+
for (unsigned Pos = 0; Pos < Branches.size(); ++Pos) {
564+
if (CondResults[Pos] != MCDCRecord::MCDC_Normal) {
565+
continue;
566+
}
567+
const auto ID = PosToID.at(Pos);
568+
unsigned InaccessibleCondCount =
569+
llvm::count_if(PracticalTestVectorsByCond,
570+
[=](const MCDCRecord::TestVectors &TestVectors) {
571+
for (const auto &[TV, Cond] : TestVectors) {
572+
if (TV[ID] != MCDCRecord::MCDC_DontCare) {
573+
return false;
574+
}
575+
}
576+
return true;
577+
});
578+
switch (InaccessibleCondCount) {
579+
case 1:
580+
CondResults[Pos] = MCDCRecord::CondResult::MCDC_Uncoverable;
581+
break;
582+
case 2:
583+
CondResults[Pos] = MCDCRecord::CondResult::MCDC_Unreachable;
584+
break;
585+
default:
586+
break;
587+
}
588+
}
589+
}
590+
511591
public:
512592
/// Process the MC/DC Record in order to produce a result for a boolean
513593
/// expression. This process includes tracking the conditions that comprise
514594
/// the decision region, calculating the list of all possible test vectors,
515595
/// marking the executed test vectors, and then finding an Independence Pair
516596
/// out of the executed test vectors for each condition in the boolean
517-
/// expression. A condition is tracked to ensure that its ID can be mapped to
518-
/// its ordinal position in the boolean expression. The condition's source
519-
/// location is also tracked, as well as whether it is constant folded (in
520-
/// which case it is excuded from the metric).
597+
/// expression. A condition is tracked to ensure that its ID can be mapped
598+
/// to its ordinal position in the boolean expression. The condition's
599+
/// source location is also tracked, as well as whether it is constant
600+
/// folded (in which case it is excuded from the metric).
521601
MCDCRecord processMCDCRecord() {
522602
MCDCRecord::CondIDMap PosToID;
523603
MCDCRecord::LineColPairMap CondLoc;
524604

525605
// Walk the Record's BranchRegions (representing Conditions) in order to:
526-
// - Hash the condition based on its corresponding ID. This will be used to
606+
// - Hash the condition based on its corresponding ID. This will be used
607+
// to
527608
// calculate the test vectors.
528609
// - Keep a map of the condition's ordinal position (1, 2, 3, 4) to its
529610
// actual ID. This will be used to visualize the conditions in the
530611
// correct order.
531612
// - Keep track of the condition source location. This will be used to
532613
// visualize where the condition is.
533-
// - Record whether the condition is constant folded so that we exclude it
614+
// - Record whether the condition is folded so that we exclude it
534615
// from being measured.
535616
for (auto [I, B] : enumerate(Branches)) {
536617
const auto &BranchParams = B->getBranchParams();
537618
PosToID[I] = BranchParams.ID;
538619
CondLoc[I] = B->startLoc();
539620
Folded[false][I] = B->FalseCount.isZero();
540621
Folded[true][I] = B->Count.isZero();
622+
if (B->Count.isZero() && B->FalseCount.isZero()) {
623+
CondResults[I] = MCDCRecord::CondResult::MCDC_Unreachable;
624+
} else if (B->Count.isZero() || B->FalseCount.isZero()) {
625+
CondResults[I] = MCDCRecord::CondResult::MCDC_Constant;
626+
}
627+
++I;
541628
}
542629

543630
// Using Profile Bitmap from runtime, mark the executed test vectors.
544631
findExecutedTestVectors();
545632

633+
// Identify all conditions making no difference on outcome of the decision.
634+
findCoverablePairs(PosToID);
635+
546636
// Record Test vectors, executed vectors, and independence pairs.
547-
return MCDCRecord(Region, std::move(ExecVectors), std::move(Folded),
548-
std::move(PosToID), std::move(CondLoc));
637+
return MCDCRecord(Region, std::move(ExecVectors),
638+
std::move(IndependencePairs), std::move(Folded),
639+
std::move(CondResults), std::move(PosToID),
640+
std::move(CondLoc));
549641
}
550642
};
551643

@@ -935,8 +1027,8 @@ Error CoverageMapping::loadFunctionRecord(
9351027
}
9361028

9371029
// Don't create records for (filenames, function) pairs we've already seen.
938-
auto FilenamesHash = hash_combine_range(Record.Filenames.begin(),
939-
Record.Filenames.end());
1030+
auto FilenamesHash =
1031+
hash_combine_range(Record.Filenames.begin(), Record.Filenames.end());
9401032
if (!RecordProvenance[FilenamesHash].insert(hash_value(OrigFuncName)).second)
9411033
return Error::success();
9421034

@@ -989,12 +1081,11 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
9891081

9901082
// If E is a no_data_found error, returns success. Otherwise returns E.
9911083
static Error handleMaybeNoDataFoundError(Error E) {
992-
return handleErrors(
993-
std::move(E), [](const CoverageMapError &CME) {
994-
if (CME.get() == coveragemap_error::no_data_found)
995-
return static_cast<Error>(Error::success());
996-
return make_error<CoverageMapError>(CME.get(), CME.getMessage());
997-
});
1084+
return handleErrors(std::move(E), [](const CoverageMapError &CME) {
1085+
if (CME.get() == coveragemap_error::no_data_found)
1086+
return static_cast<Error>(Error::success());
1087+
return make_error<CoverageMapError>(CME.get(), CME.getMessage());
1088+
});
9981089
}
9991090

10001091
Error CoverageMapping::loadFromFile(
@@ -1086,7 +1177,7 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
10861177
std::string Path = std::move(*PathOpt);
10871178
StringRef Arch = Arches.size() == 1 ? Arches.front() : StringRef();
10881179
if (Error E = loadFromFile(Path, Arch, CompilationDir, *ProfileReader,
1089-
*Coverage, DataFound))
1180+
*Coverage, DataFound))
10901181
return std::move(E);
10911182
} else if (CheckBinaryIDs) {
10921183
return createFileError(
@@ -1180,9 +1271,9 @@ class SegmentBuilder {
11801271
// emit closing segments in sorted order.
11811272
auto CompletedRegionsIt = ActiveRegions.begin() + FirstCompletedRegion;
11821273
std::stable_sort(CompletedRegionsIt, ActiveRegions.end(),
1183-
[](const CountedRegion *L, const CountedRegion *R) {
1184-
return L->endLoc() < R->endLoc();
1185-
});
1274+
[](const CountedRegion *L, const CountedRegion *R) {
1275+
return L->endLoc() < R->endLoc();
1276+
});
11861277

11871278
// Emit segments for all completed regions.
11881279
for (unsigned I = FirstCompletedRegion + 1, E = ActiveRegions.size(); I < E;
Binary file not shown.
88 Bytes
Binary file not shown.
80 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)