Skip to content

Commit 10e2be0

Browse files
committed
[coverage] MC/DC reports unrechable and uncoverable conditions
1 parent 755d8f2 commit 10e2be0

File tree

12 files changed

+219
-80
lines changed

12 files changed

+219
-80
lines changed

clang/docs/SourceBasedCodeCoverage.rst

+11
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,17 @@ starts a new boolean expression that is separated from the other conditions by
517517
the operator ``func()``. When this is encountered, a warning will be generated
518518
and the boolean expression will not be instrumented.
519519

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

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

+49-20
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,13 @@ struct MCDCRecord {
385385
/// are effectively ignored.
386386
enum CondState { MCDC_DontCare = -1, MCDC_False = 0, MCDC_True = 1 };
387387

388+
enum CondResult {
389+
MCDC_Normal,
390+
MCDC_Constant,
391+
MCDC_Uncoverable,
392+
MCDC_Unreachable
393+
};
394+
388395
/// Emulate SmallVector<CondState> with a pair of BitVector.
389396
///
390397
/// True False DontCare (Impossible)
@@ -443,30 +450,36 @@ struct MCDCRecord {
443450
using TVPairMap = llvm::DenseMap<unsigned, TVRowPair>;
444451
using CondIDMap = llvm::DenseMap<unsigned, unsigned>;
445452
using LineColPairMap = llvm::DenseMap<unsigned, LineColPair>;
453+
using ResultVector = llvm::SmallVector<CondResult>;
446454

447455
private:
448456
CounterMappingRegion Region;
449457
TestVectors TV;
450458
TVPairMap IndependencePairs;
451-
BoolVector Folded;
459+
ResultVector CondResults;
452460
CondIDMap PosToID;
453461
LineColPairMap CondLoc;
454462

455463
public:
456464
MCDCRecord(const CounterMappingRegion &Region, TestVectors &&TV,
457-
TVPairMap &&IndependencePairs, BoolVector &&Folded,
465+
TVPairMap &&IndependencePairs, ResultVector &&CondResults,
458466
CondIDMap &&PosToID, LineColPairMap &&CondLoc)
459467
: Region(Region), TV(std::move(TV)),
460468
IndependencePairs(std::move(IndependencePairs)),
461-
Folded(std::move(Folded)), PosToID(std::move(PosToID)),
462-
CondLoc(std::move(CondLoc)){};
469+
CondResults(std::move(CondResults)), PosToID(std::move(PosToID)),
470+
CondLoc(std::move(CondLoc)) {};
463471

464472
CounterMappingRegion getDecisionRegion() const { return Region; }
465473
unsigned getNumConditions() const {
466474
return Region.getDecisionParams().NumConditions;
467475
}
468476
unsigned getNumTestVectors() const { return TV.size(); }
469-
bool isCondFolded(unsigned Condition) const { return Folded[Condition]; }
477+
bool isCondConstant(unsigned Condition) const {
478+
return getCondResult(Condition) == CondResult::MCDC_Constant;
479+
}
480+
CondResult getCondResult(unsigned Condition) const {
481+
return CondResults[Condition];
482+
}
470483

471484
/// Return the evaluation of a condition (indicated by Condition) in an
472485
/// executed test vector (indicated by TestVectorIndex), which will be True,
@@ -506,20 +519,25 @@ struct MCDCRecord {
506519
return IndependencePairs[PosToID[Condition]];
507520
}
508521

509-
float getPercentCovered() const {
510-
unsigned Folded = 0;
522+
/// Return if the decision is coverable and percent of covered conditions.
523+
/// Only coverable conditions are counted as denominator.
524+
std::pair<bool, float> getPercentCovered() const {
525+
unsigned Excluded = 0;
511526
unsigned Covered = 0;
512527
for (unsigned C = 0; C < getNumConditions(); C++) {
513-
if (isCondFolded(C))
514-
Folded++;
528+
if (isCondConstant(C))
529+
Excluded++;
515530
else if (isConditionIndependencePairCovered(C))
516531
Covered++;
517532
}
518533

519-
unsigned Total = getNumConditions() - Folded;
534+
unsigned Total = getNumConditions() - Excluded;
520535
if (Total == 0)
521-
return 0.0;
522-
return (static_cast<double>(Covered) / static_cast<double>(Total)) * 100.0;
536+
return {false, 0.0};
537+
return {
538+
true,
539+
(static_cast<double>(Covered) / static_cast<double>(Total)) * 100.0,
540+
};
523541
}
524542

525543
std::string getConditionHeaderString(unsigned Condition) {
@@ -554,7 +572,7 @@ struct MCDCRecord {
554572
// Add individual condition values to the string.
555573
OS << " " << TestVectorIndex + 1 << " { ";
556574
for (unsigned Condition = 0; Condition < NumConditions; Condition++) {
557-
if (isCondFolded(Condition))
575+
if (isCondConstant(Condition))
558576
OS << "C";
559577
else {
560578
switch (getTVCondition(TestVectorIndex, Condition)) {
@@ -590,14 +608,25 @@ struct MCDCRecord {
590608
std::ostringstream OS;
591609

592610
OS << " C" << Condition + 1 << "-Pair: ";
593-
if (isCondFolded(Condition)) {
611+
switch (getCondResult(Condition)) {
612+
case CondResult::MCDC_Normal:
613+
if (isConditionIndependencePairCovered(Condition)) {
614+
TVRowPair rows = getConditionIndependencePair(Condition);
615+
OS << "covered: (" << rows.first << ",";
616+
OS << rows.second << ")\n";
617+
} else
618+
OS << "not covered\n";
619+
break;
620+
case CondResult::MCDC_Constant:
594621
OS << "constant folded\n";
595-
} else if (isConditionIndependencePairCovered(Condition)) {
596-
TVRowPair rows = getConditionIndependencePair(Condition);
597-
OS << "covered: (" << rows.first << ",";
598-
OS << rows.second << ")\n";
599-
} else
600-
OS << "not covered\n";
622+
break;
623+
case CondResult::MCDC_Uncoverable:
624+
OS << "uncoverable\n";
625+
break;
626+
case CondResult::MCDC_Unreachable:
627+
OS << "unreachable\n";
628+
break;
629+
}
601630

602631
return OS.str();
603632
}

llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

+112-26
Original file line numberDiff line numberDiff line change
@@ -362,11 +362,15 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
362362
unsigned NumConditions;
363363

364364
/// Vector used to track whether a condition is constant folded.
365-
MCDCRecord::BoolVector Folded;
365+
MCDCRecord::ResultVector CondResults;
366366

367367
/// Mapping of calculated MC/DC Independence Pairs for each condition.
368368
MCDCRecord::TVPairMap IndependencePairs;
369369

370+
/// All possible Test Vectors for the boolean expression derived from
371+
/// binary decision diagran of the expression.
372+
MCDCRecord::TestVectors TestVectors;
373+
370374
/// Storage for ExecVectors
371375
/// ExecVectors is the alias of its 0th element.
372376
std::array<MCDCRecord::TestVectors, 2> ExecVectorsByCond;
@@ -392,8 +396,9 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
392396
: NextIDsBuilder(Branches), TVIdxBuilder(this->NextIDs), Bitmap(Bitmap),
393397
Region(Region), DecisionParams(Region.getDecisionParams()),
394398
Branches(Branches), NumConditions(DecisionParams.NumConditions),
395-
Folded(NumConditions, false), IndependencePairs(NumConditions),
396-
ExecVectors(ExecVectorsByCond[false]), IsVersion11(IsVersion11) {}
399+
CondResults(NumConditions, MCDCRecord::CondResult::MCDC_Normal),
400+
IndependencePairs(NumConditions), ExecVectors(ExecVectorsByCond[false]),
401+
IsVersion11(IsVersion11) {}
397402

398403
private:
399404
// Walk the binary decision diagram and try assigning both false and true to
@@ -415,6 +420,7 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
415420

416421
assert(TVIdx < SavedNodes[ID].Width);
417422
assert(TVIdxs.insert(NextTVIdx).second && "Duplicate TVIdx");
423+
TestVectors.push_back({TV, MCDCCond});
418424

419425
if (!Bitmap[IsVersion11
420426
? DecisionParams.BitmapIdx * CHAR_BIT + TV.getIndex()
@@ -442,7 +448,6 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
442448
buildTestVector(TV, 0, 0);
443449
assert(TVIdxs.size() == unsigned(NumTestVectors) &&
444450
"TVIdxs wasn't fulfilled");
445-
446451
// Fill ExecVectors order by False items and True items.
447452
// ExecVectors is the alias of ExecVectorsByCond[false], so
448453
// Append ExecVectorsByCond[true] on it.
@@ -474,48 +479,130 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
474479
}
475480
}
476481

482+
void findCoverablePairs(const MCDCRecord::CondIDMap &PosToID) {
483+
llvm::SmallVector<unsigned> FoldedCondPos;
484+
for (unsigned I = 0; I < CondResults.size(); ++I) {
485+
if (CondResults[I] == MCDCRecord::MCDC_Constant ||
486+
CondResults[I] == MCDCRecord::MCDC_Unreachable) {
487+
FoldedCondPos.push_back(I);
488+
}
489+
}
490+
if (FoldedCondPos.empty()) {
491+
return;
492+
}
493+
std::array<MCDCRecord::TestVectors, 2> PracticalTestVectorsByCond;
494+
for (const auto &TVWithCond : TestVectors) {
495+
const bool Practical =
496+
llvm::all_of(FoldedCondPos, [&](const unsigned &Pos) {
497+
const auto &[TV, Cond] = TVWithCond;
498+
const auto ID = PosToID.at(Pos);
499+
if (TV[ID] == MCDCRecord::MCDC_DontCare) {
500+
return true;
501+
}
502+
if (CondResults[Pos] == MCDCRecord::MCDC_Constant) {
503+
const auto ConstantValue = Branches[Pos]->Count.isZero()
504+
? MCDCRecord::MCDC_False
505+
: MCDCRecord::MCDC_True;
506+
if (TV[ID] == ConstantValue) {
507+
return true;
508+
}
509+
}
510+
return false;
511+
});
512+
513+
if (Practical) {
514+
PracticalTestVectorsByCond[TVWithCond.second].push_back(TVWithCond);
515+
}
516+
}
517+
518+
// If a condition:
519+
// - is uncoverable, all test vectors in exact one element of
520+
// `PracticalTestVectorsByCond` show it is `DontCare`;
521+
// - is unreachable, all test vectors in both elements of
522+
// `PracticalTestVectorsByCond` show it is `DontCare`;
523+
//
524+
// Otherwise, the condition is coverable as long as it has not been marked
525+
// as constant or unreachable before.
526+
for (unsigned Pos = 0; Pos < Branches.size(); ++Pos) {
527+
if (CondResults[Pos] != MCDCRecord::MCDC_Normal) {
528+
continue;
529+
}
530+
const auto ID = PosToID.at(Pos);
531+
unsigned InaccessibleCondCount =
532+
llvm::count_if(PracticalTestVectorsByCond,
533+
[=](const MCDCRecord::TestVectors &TestVectors) {
534+
for (const auto &[TV, Cond] : TestVectors) {
535+
if (TV[ID] != MCDCRecord::MCDC_DontCare) {
536+
return false;
537+
}
538+
}
539+
return true;
540+
});
541+
switch (InaccessibleCondCount) {
542+
case 1:
543+
CondResults[Pos] = MCDCRecord::CondResult::MCDC_Uncoverable;
544+
break;
545+
case 2:
546+
CondResults[Pos] = MCDCRecord::CondResult::MCDC_Unreachable;
547+
break;
548+
default:
549+
break;
550+
}
551+
}
552+
}
553+
477554
public:
478555
/// Process the MC/DC Record in order to produce a result for a boolean
479556
/// expression. This process includes tracking the conditions that comprise
480557
/// the decision region, calculating the list of all possible test vectors,
481558
/// marking the executed test vectors, and then finding an Independence Pair
482559
/// out of the executed test vectors for each condition in the boolean
483-
/// expression. A condition is tracked to ensure that its ID can be mapped to
484-
/// its ordinal position in the boolean expression. The condition's source
485-
/// location is also tracked, as well as whether it is constant folded (in
486-
/// which case it is excuded from the metric).
560+
/// expression. A condition is tracked to ensure that its ID can be mapped
561+
/// to its ordinal position in the boolean expression. The condition's
562+
/// source location is also tracked, as well as whether it is constant
563+
/// folded (in which case it is excuded from the metric).
487564
MCDCRecord processMCDCRecord() {
488565
unsigned I = 0;
489566
MCDCRecord::CondIDMap PosToID;
490567
MCDCRecord::LineColPairMap CondLoc;
491568

492569
// Walk the Record's BranchRegions (representing Conditions) in order to:
493-
// - Hash the condition based on its corresponding ID. This will be used to
570+
// - Hash the condition based on its corresponding ID. This will be used
571+
// to
494572
// calculate the test vectors.
495573
// - Keep a map of the condition's ordinal position (1, 2, 3, 4) to its
496574
// actual ID. This will be used to visualize the conditions in the
497575
// correct order.
498576
// - Keep track of the condition source location. This will be used to
499577
// visualize where the condition is.
500-
// - Record whether the condition is constant folded so that we exclude it
578+
// - Record whether the condition is folded so that we exclude it
501579
// from being measured.
502580
for (const auto *B : Branches) {
503581
const auto &BranchParams = B->getBranchParams();
504582
PosToID[I] = BranchParams.ID;
505583
CondLoc[I] = B->startLoc();
506-
Folded[I++] = (B->Count.isZero() || B->FalseCount.isZero());
584+
if (B->Count.isZero() && B->FalseCount.isZero()) {
585+
CondResults[I] = MCDCRecord::CondResult::MCDC_Unreachable;
586+
} else if (B->Count.isZero() || B->FalseCount.isZero()) {
587+
CondResults[I] = MCDCRecord::CondResult::MCDC_Constant;
588+
}
589+
++I;
507590
}
508591

509592
// Using Profile Bitmap from runtime, mark the executed test vectors.
510593
findExecutedTestVectors();
511594

512-
// Compare executed test vectors against each other to find an independence
513-
// pairs for each condition. This processing takes the most time.
595+
// Compare executed test vectors against each other to find an
596+
// independence pairs for each condition. This processing takes the most
597+
// time.
514598
findIndependencePairs();
515599

600+
// Identify all conditions making no difference on outcome of the decision.
601+
findCoverablePairs(PosToID);
602+
516603
// Record Test vectors, executed vectors, and independence pairs.
517604
return MCDCRecord(Region, std::move(ExecVectors),
518-
std::move(IndependencePairs), std::move(Folded),
605+
std::move(IndependencePairs), std::move(CondResults),
519606
std::move(PosToID), std::move(CondLoc));
520607
}
521608
};
@@ -907,8 +994,8 @@ Error CoverageMapping::loadFunctionRecord(
907994
}
908995

909996
// Don't create records for (filenames, function) pairs we've already seen.
910-
auto FilenamesHash = hash_combine_range(Record.Filenames.begin(),
911-
Record.Filenames.end());
997+
auto FilenamesHash =
998+
hash_combine_range(Record.Filenames.begin(), Record.Filenames.end());
912999
if (!RecordProvenance[FilenamesHash].insert(hash_value(OrigFuncName)).second)
9131000
return Error::success();
9141001

@@ -958,12 +1045,11 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
9581045

9591046
// If E is a no_data_found error, returns success. Otherwise returns E.
9601047
static Error handleMaybeNoDataFoundError(Error E) {
961-
return handleErrors(
962-
std::move(E), [](const CoverageMapError &CME) {
963-
if (CME.get() == coveragemap_error::no_data_found)
964-
return static_cast<Error>(Error::success());
965-
return make_error<CoverageMapError>(CME.get(), CME.getMessage());
966-
});
1048+
return handleErrors(std::move(E), [](const CoverageMapError &CME) {
1049+
if (CME.get() == coveragemap_error::no_data_found)
1050+
return static_cast<Error>(Error::success());
1051+
return make_error<CoverageMapError>(CME.get(), CME.getMessage());
1052+
});
9671053
}
9681054

9691055
Error CoverageMapping::loadFromFile(
@@ -1055,7 +1141,7 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
10551141
std::string Path = std::move(*PathOpt);
10561142
StringRef Arch = Arches.size() == 1 ? Arches.front() : StringRef();
10571143
if (Error E = loadFromFile(Path, Arch, CompilationDir, *ProfileReader,
1058-
*Coverage, DataFound))
1144+
*Coverage, DataFound))
10591145
return std::move(E);
10601146
} else if (CheckBinaryIDs) {
10611147
return createFileError(
@@ -1149,9 +1235,9 @@ class SegmentBuilder {
11491235
// emit closing segments in sorted order.
11501236
auto CompletedRegionsIt = ActiveRegions.begin() + FirstCompletedRegion;
11511237
std::stable_sort(CompletedRegionsIt, ActiveRegions.end(),
1152-
[](const CountedRegion *L, const CountedRegion *R) {
1153-
return L->endLoc() < R->endLoc();
1154-
});
1238+
[](const CountedRegion *L, const CountedRegion *R) {
1239+
return L->endLoc() < R->endLoc();
1240+
});
11551241

11561242
// Emit segments for all completed regions.
11571243
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)