Skip to content

Commit

Permalink
Refactor processing of BranchRegions associated with an MCDCDecisionR…
Browse files Browse the repository at this point in the history 
…egion.

This fixes MC/DC issue #77871 in which
branches under ExpansionRegions were not being included in the creation of the
MC/DC record. The fix is a slight refactor in how branches associated with an
MCDCDecisionRegion are gathered such that ExpansionRegions can be scanned
recursively.
  • Loading branch information
@evodius96
evodius96 committed Jan 20, 2024
1 parent 30aa9fb commit bf66961
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 46 deletions.
111 changes: 65 additions & 46 deletions llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,27 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
return MaxBitmapID + (SizeInBits / CHAR_BIT);
}

static void
addMCDCBranches(unsigned FileID, const unsigned NumConds,
std::vector<CounterMappingRegion> &MCDCBranches,
const ArrayRef<CounterMappingRegion>::iterator &Begin,
const ArrayRef<CounterMappingRegion>::iterator &End) {
// Use the given iterator to scan to the end of the list of regions.
for (auto It = Begin; It != End; ++It)
if (It->FileID == FileID && MCDCBranches.size() < NumConds) {
if (It->Kind == CounterMappingRegion::MCDCBranchRegion)
// Gather BranchRegions associated within the given FileID until the
// NumConds limit is reached.
MCDCBranches.push_back(*It);
else if (It->Kind == CounterMappingRegion::ExpansionRegion) {
// If an ExpansionRegion is encountered, recur to check that any
// BranchRegions associated with the ExpansionRegion are included.
assert(It->ExpandedFileID > It->FileID);
addMCDCBranches(It->ExpandedFileID, NumConds, MCDCBranches, It, End);
}
}
}

Error CoverageMapping::loadFunctionRecord(
const CoverageMappingRecord &Record,
IndexedInstrProfReader &ProfileReader) {
Expand Down Expand Up @@ -636,20 +657,56 @@ Error CoverageMapping::loadFunctionRecord(
Record.MappingRegions[0].Count.isZero() && Counts[0] > 0)
return Error::success();

unsigned NumConds = 0;
const CounterMappingRegion *MCDCDecision;
std::vector<CounterMappingRegion> MCDCBranches;

FunctionRecord Function(OrigFuncName, Record.Filenames);
for (const auto &Region : Record.MappingRegions) {

const auto &RegionsBegin = Record.MappingRegions.begin();
const auto &RegionsEnd = Record.MappingRegions.end();
for (auto It = RegionsBegin; It != RegionsEnd; ++It) {
const auto &Region = *It;

// If an MCDCDecisionRegion is seen, track the BranchRegions that follow
// it according to Region.NumConditions.
if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion) {
assert(NumConds == 0);
MCDCDecision = &Region;
NumConds = Region.MCDCParams.NumConditions;
std::vector<CounterMappingRegion> MCDCBranches;
const unsigned NumConds = Region.MCDCParams.NumConditions;

// If a MCDCDecisionRegion was seen, use the current iterator to scan
// ahead to store the BranchRegions that correspond to it in a vector,
// according to the number of conditions recorded for the region (tracked
// by NumConds). Note that BranchRegions may be part of ExpansionRegions,
// which need to be followed recursively.
addMCDCBranches(It->FileID, NumConds, MCDCBranches, It, RegionsEnd);

// All of the corresponding BranchRegions ought to be accounted for.
assert(MCDCBranches.size() == NumConds);

// Evaluating the test vector bitmap for the decision region entails
// calculating precisely what bits are pertinent to this region alone.
// This is calculated based on the recorded offset into the global
// profile bitmap; the length is calculated based on the recorded
// number of conditions.
Expected<BitVector> ExecutedTestVectorBitmap =
Ctx.evaluateBitmap(&Region);
if (auto E = ExecutedTestVectorBitmap.takeError()) {
consumeError(std::move(E));
return Error::success();
}

// Since the bitmap identifies the executed test vectors for an MC/DC
// DecisionRegion, all of the information is now available to process.
// This is where the bulk of the MC/DC progressing takes place.
Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
Region, *ExecutedTestVectorBitmap, MCDCBranches);
if (auto E = Record.takeError()) {
consumeError(std::move(E));
return Error::success();
}

// Save the MC/DC Record so that it can be visualized later.
Function.pushMCDCRecord(*Record);
continue;
}

Expected<int64_t> ExecutionCount = Ctx.evaluate(Region.Count);
if (auto E = ExecutionCount.takeError()) {
consumeError(std::move(E));
Expand All @@ -661,44 +718,6 @@ Error CoverageMapping::loadFunctionRecord(
return Error::success();
}
Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount);

// If a MCDCDecisionRegion was seen, store the BranchRegions that
// correspond to it in a vector, according to the number of conditions
// recorded for the region (tracked by NumConds).
if (NumConds > 0 && Region.Kind == CounterMappingRegion::MCDCBranchRegion) {
MCDCBranches.push_back(Region);

// As we move through all of the MCDCBranchRegions that follow the
// MCDCDecisionRegion, decrement NumConds to make sure we account for
// them all before we calculate the bitmap of executed test vectors.
if (--NumConds == 0) {
// Evaluating the test vector bitmap for the decision region entails
// calculating precisely what bits are pertinent to this region alone.
// This is calculated based on the recorded offset into the global
// profile bitmap; the length is calculated based on the recorded
// number of conditions.
Expected<BitVector> ExecutedTestVectorBitmap =
Ctx.evaluateBitmap(MCDCDecision);
if (auto E = ExecutedTestVectorBitmap.takeError()) {
consumeError(std::move(E));
return Error::success();
}

// Since the bitmap identifies the executed test vectors for an MC/DC
// DecisionRegion, all of the information is now available to process.
// This is where the bulk of the MC/DC progressing takes place.
Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
*MCDCDecision, *ExecutedTestVectorBitmap, MCDCBranches);
if (auto E = Record.takeError()) {
consumeError(std::move(E));
return Error::success();
}

// Save the MC/DC Record so that it can be visualized later.
Function.pushMCDCRecord(*Record);
MCDCBranches.clear();
}
}
}

// Don't create records for (filenames, function) pairs we've already seen.
Expand Down
20 changes: 20 additions & 0 deletions llvm/test/tools/llvm-cov/Inputs/mcdc-macro.c
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#define C c
#define D 1
#define E (C != a) && (C > a)
#define F E

void __attribute__((noinline)) func1(void) { return; }

void __attribute__((noinline)) func(int a, int b, int c) {
if (a && D && E || b)
func1();
if (b && D)
func1();
if (a && (b && C) || (D && F))
func1();
}

int main() {
func(2, 3, 3);
return 0;
}
Binary file added llvm/test/tools/llvm-cov/Inputs/mcdc-macro.o
View file
Binary file not shown.
62 changes: 62 additions & 0 deletions llvm/test/tools/llvm-cov/Inputs/mcdc-macro.proftext
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
main
# Func Hash:
24
# Num Counters:
1
# Counter Values:
1

foo
# Func Hash:
395201011017399473
# Num Counters:
22
# Counter Values:
1
1
0
0
1
1
1
1
1
1
1
1
1
1
0
1
1
1
0
0
0
0
# Num Bitmap Bytes:
$13
# Bitmap Byte Values:
0x0
0x0
0x0
0x20
0x8
0x0
0x20
0x0
0x0
0x0
0x0
0x0
0x0


bar
# Func Hash:
24
# Num Counters:
1
# Counter Values:
3

92 changes: 92 additions & 0 deletions llvm/test/tools/llvm-cov/mcdc-macro.test
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Test visualization of MC/DC constructs for branches in macro expansions.

// RUN: llvm-profdata merge %S/Inputs/mcdc-macro.proftext -o %t.profdata
// RUN: llvm-cov show --show-expansions --show-branches=count --show-mcdc %S/Inputs/mcdc-macro.o -instr-profile %t.profdata -path-equivalence=/tmp,%S/Inputs %S/Inputs/mcdc-macro.c | FileCheck %s

// CHECK: | | | Branch (2:11): [Folded - Ignored]
// CHECK: | | | Branch (3:11): [True: 0, False: 0]
// CHECK: | | | Branch (3:23): [True: 0, False: 0]
// CHECK: | Branch (9:7): [True: 0, False: 0]
// CHECK-NEXT: | Branch (9:22): [True: 0, False: 0]
// CHECK-NEXT: ------------------
// CHECK-NEXT: |---> MC/DC Decision Region (9:7) to (9:23)
// CHECK-NEXT: |
// CHECK-NEXT: | Number of Conditions: 5
// CHECK-NEXT: | Condition C1 --> (9:7)
// CHECK-NEXT: | Condition C2 --> (2:11)
// CHECK-NEXT: | Condition C3 --> (3:11)
// CHECK-NEXT: | Condition C4 --> (3:23)
// CHECK-NEXT: | Condition C5 --> (9:22)
// CHECK-NEXT: |
// CHECK-NEXT: | Executed MC/DC Test Vectors:
// CHECK-NEXT: |
// CHECK-NEXT: | None.
// CHECK-NEXT: |
// CHECK-NEXT: | C1-Pair: not covered
// CHECK-NEXT: | C2-Pair: constant folded
// CHECK-NEXT: | C3-Pair: not covered
// CHECK-NEXT: | C4-Pair: not covered
// CHECK-NEXT: | C5-Pair: not covered
// CHECK-NEXT: | MC/DC Coverage for Decision: 0.00%
// CHECK-NEXT: |
// CHECK-NEXT: ------------------

// CHECK: | | | Branch (2:11): [Folded - Ignored]
// CHECK: | Branch (11:7): [True: 0, False: 0]
// CHECK-NEXT: ------------------
// CHECK-NEXT: |---> MC/DC Decision Region (11:7) to (11:13)
// CHECK-NEXT: |
// CHECK-NEXT: | Number of Conditions: 2
// CHECK-NEXT: | Condition C1 --> (11:7)
// CHECK-NEXT: | Condition C2 --> (2:11)
// CHECK-NEXT: |
// CHECK-NEXT: | Executed MC/DC Test Vectors:
// CHECK-NEXT: |
// CHECK-NEXT: | None.
// CHECK-NEXT: |
// CHECK-NEXT: | C1-Pair: not covered
// CHECK-NEXT: | C2-Pair: constant folded
// CHECK-NEXT: | MC/DC Coverage for Decision: 0.00%
// CHECK-NEXT: |
// CHECK-NEXT: ------------------

// CHECK: | | | Branch (1:11): [True: 0, False: 0]
// CHECK: | | | Branch (2:11): [Folded - Ignored]
// CHECK: | | | | | Branch (3:11): [True: 0, False: 0]
// CHECK: | | | | | Branch (3:23): [True: 0, False: 0]
// CHECK: | Branch (13:7): [True: 0, False: 0]
// CHECK-NEXT: | Branch (13:13): [True: 0, False: 0]
// CHECK-NEXT: ------------------
// CHECK-NEXT: |---> MC/DC Decision Region (13:7) to (13:32)
// CHECK-NEXT: |
// CHECK-NEXT: | Number of Conditions: 6
// CHECK-NEXT: | Condition C1 --> (13:7)
// CHECK-NEXT: | Condition C2 --> (13:13)
// CHECK-NEXT: | Condition C3 --> (1:11)
// CHECK-NEXT: | Condition C4 --> (2:11)
// CHECK-NEXT: | Condition C5 --> (3:11)
// CHECK-NEXT: | Condition C6 --> (3:23)
// CHECK-NEXT: |
// CHECK-NEXT: | Executed MC/DC Test Vectors:
// CHECK-NEXT: |
// CHECK-NEXT: | None.
// CHECK-NEXT: |
// CHECK-NEXT: | C1-Pair: not covered
// CHECK-NEXT: | C2-Pair: not covered
// CHECK-NEXT: | C3-Pair: not covered
// CHECK-NEXT: | C4-Pair: constant folded
// CHECK-NEXT: | C5-Pair: not covered
// CHECK-NEXT: | C6-Pair: not covered
// CHECK-NEXT: | MC/DC Coverage for Decision: 0.00%
// CHECK-NEXT: |
// CHECK-NEXT: ------------------

Instructions for regenerating the test:

# cd %S/Inputs
cp mcdc-macro.c /tmp

clang -fcoverage-mcdc -fprofile-instr-generate -fcoverage-compilation-dir=. \
-fcoverage-mapping /tmp/mcdc-macro.c -o /tmp/mcdc-macro.o

mv /tmp/mcdc-macro.o %S/Inputs

0 comments on commit bf66961

Please sign in to comment.