Skip to content

Commit 57d5647

Browse files
evodius96evodius96
committed
Refactor processing of BranchRegions associated with an MCDCDecisionRegion.
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.
1 parent 424b9cf commit 57d5647

File tree

5 files changed

+239
-46
lines changed

5 files changed

+239
-46
lines changed

‎llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

Lines changed: 65 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,27 @@ static unsigned getMaxBitmapSize(const CounterMappingContext &Ctx,
582582
return MaxBitmapID + (SizeInBits / CHAR_BIT);
583583
}
584584

585+
static void
586+
addMCDCBranches(unsigned FileID, const unsigned NumConds,
587+
std::vector<const CounterMappingRegion *> &MCDCBranches,
588+
const ArrayRef<CounterMappingRegion>::iterator &Begin,
589+
const ArrayRef<CounterMappingRegion>::iterator &End) {
590+
// Use the given iterator to scan to the end of the list of regions.
591+
for (auto It = Begin; It != End; ++It)
592+
if (It->FileID == FileID && MCDCBranches.size() < NumConds) {
593+
if (It->Kind == CounterMappingRegion::MCDCBranchRegion)
594+
// Gather BranchRegions associated within the given FileID until the
595+
// NumConds limit is reached.
596+
MCDCBranches.push_back(&*It);
597+
else if (It->Kind == CounterMappingRegion::ExpansionRegion) {
598+
// If an ExpansionRegion is encountered, recur to check that any
599+
// BranchRegions associated with the ExpansionRegion are included.
600+
assert(It->ExpandedFileID > It->FileID);
601+
addMCDCBranches(It->ExpandedFileID, NumConds, MCDCBranches, It, End);
602+
}
603+
}
604+
}
605+
585606
Error CoverageMapping::loadFunctionRecord(
586607
const CoverageMappingRecord &Record,
587608
IndexedInstrProfReader &ProfileReader) {
@@ -638,20 +659,56 @@ Error CoverageMapping::loadFunctionRecord(
638659
Record.MappingRegions[0].Count.isZero() && Counts[0] > 0)
639660
return Error::success();
640661

641-
unsigned NumConds = 0;
642-
const CounterMappingRegion *MCDCDecision;
643-
std::vector<const CounterMappingRegion *> MCDCBranches;
644-
645662
FunctionRecord Function(OrigFuncName, Record.Filenames);
646-
for (const auto &Region : Record.MappingRegions) {
663+
664+
const auto &RegionsBegin = Record.MappingRegions.begin();
665+
const auto &RegionsEnd = Record.MappingRegions.end();
666+
for (auto It = RegionsBegin; It != RegionsEnd; ++It) {
667+
const auto &Region = *It;
668+
647669
// If an MCDCDecisionRegion is seen, track the BranchRegions that follow
648670
// it according to Region.NumConditions.
649671
if (Region.Kind == CounterMappingRegion::MCDCDecisionRegion) {
650-
assert(NumConds == 0);
651-
MCDCDecision = &Region;
652-
NumConds = Region.MCDCParams.NumConditions;
672+
std::vector<const CounterMappingRegion *> MCDCBranches;
673+
const unsigned NumConds = Region.MCDCParams.NumConditions;
674+
675+
// If a MCDCDecisionRegion was seen, use the current iterator to scan
676+
// ahead to store the BranchRegions that correspond to it in a vector,
677+
// according to the number of conditions recorded for the region (tracked
678+
// by NumConds). Note that BranchRegions may be part of ExpansionRegions,
679+
// which need to be followed recursively.
680+
addMCDCBranches(It->FileID, NumConds, MCDCBranches, It, RegionsEnd);
681+
682+
// All of the corresponding BranchRegions ought to be accounted for.
683+
assert(MCDCBranches.size() == NumConds);
684+
685+
// Evaluating the test vector bitmap for the decision region entails
686+
// calculating precisely what bits are pertinent to this region alone.
687+
// This is calculated based on the recorded offset into the global
688+
// profile bitmap; the length is calculated based on the recorded
689+
// number of conditions.
690+
Expected<BitVector> ExecutedTestVectorBitmap =
691+
Ctx.evaluateBitmap(&Region);
692+
if (auto E = ExecutedTestVectorBitmap.takeError()) {
693+
consumeError(std::move(E));
694+
return Error::success();
695+
}
696+
697+
// Since the bitmap identifies the executed test vectors for an MC/DC
698+
// DecisionRegion, all of the information is now available to process.
699+
// This is where the bulk of the MC/DC progressing takes place.
700+
Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
701+
Region, *ExecutedTestVectorBitmap, MCDCBranches);
702+
if (auto E = Record.takeError()) {
703+
consumeError(std::move(E));
704+
return Error::success();
705+
}
706+
707+
// Save the MC/DC Record so that it can be visualized later.
708+
Function.pushMCDCRecord(*Record);
653709
continue;
654710
}
711+
655712
Expected<int64_t> ExecutionCount = Ctx.evaluate(Region.Count);
656713
if (auto E = ExecutionCount.takeError()) {
657714
consumeError(std::move(E));
@@ -663,44 +720,6 @@ Error CoverageMapping::loadFunctionRecord(
663720
return Error::success();
664721
}
665722
Function.pushRegion(Region, *ExecutionCount, *AltExecutionCount);
666-
667-
// If a MCDCDecisionRegion was seen, store the BranchRegions that
668-
// correspond to it in a vector, according to the number of conditions
669-
// recorded for the region (tracked by NumConds).
670-
if (NumConds > 0 && Region.Kind == CounterMappingRegion::MCDCBranchRegion) {
671-
MCDCBranches.push_back(&Region);
672-
673-
// As we move through all of the MCDCBranchRegions that follow the
674-
// MCDCDecisionRegion, decrement NumConds to make sure we account for
675-
// them all before we calculate the bitmap of executed test vectors.
676-
if (--NumConds == 0) {
677-
// Evaluating the test vector bitmap for the decision region entails
678-
// calculating precisely what bits are pertinent to this region alone.
679-
// This is calculated based on the recorded offset into the global
680-
// profile bitmap; the length is calculated based on the recorded
681-
// number of conditions.
682-
Expected<BitVector> ExecutedTestVectorBitmap =
683-
Ctx.evaluateBitmap(MCDCDecision);
684-
if (auto E = ExecutedTestVectorBitmap.takeError()) {
685-
consumeError(std::move(E));
686-
return Error::success();
687-
}
688-
689-
// Since the bitmap identifies the executed test vectors for an MC/DC
690-
// DecisionRegion, all of the information is now available to process.
691-
// This is where the bulk of the MC/DC progressing takes place.
692-
Expected<MCDCRecord> Record = Ctx.evaluateMCDCRegion(
693-
*MCDCDecision, *ExecutedTestVectorBitmap, MCDCBranches);
694-
if (auto E = Record.takeError()) {
695-
consumeError(std::move(E));
696-
return Error::success();
697-
}
698-
699-
// Save the MC/DC Record so that it can be visualized later.
700-
Function.pushMCDCRecord(*Record);
701-
MCDCBranches.clear();
702-
}
703-
}
704723
}
705724

706725
// Don't create records for (filenames, function) pairs we've already seen.

‎llvm/test/tools/llvm-cov/Inputs/mcdc-macro.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#define C c
2+
#define D 1
3+
#define E (C != a) && (C > a)
4+
#define F E
5+
6+
void __attribute__((noinline)) func1(void) { return; }
7+
8+
void __attribute__((noinline)) func(int a, int b, int c) {
9+
if (a && D && E || b)
10+
func1();
11+
if (b && D)
12+
func1();
13+
if (a && (b && C) || (D && F))
14+
func1();
15+
}
16+
17+
int main() {
18+
func(2, 3, 3);
19+
return 0;
20+
}

‎llvm/test/tools/llvm-cov/Inputs/mcdc-macro.o

79.7 KB
Binary file not shown.

‎llvm/test/tools/llvm-cov/Inputs/mcdc-macro.proftext

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
main
2+
# Func Hash:
3+
24
4+
# Num Counters:
5+
1
6+
# Counter Values:
7+
1
8+
9+
foo
10+
# Func Hash:
11+
395201011017399473
12+
# Num Counters:
13+
22
14+
# Counter Values:
15+
1
16+
1
17+
0
18+
0
19+
1
20+
1
21+
1
22+
1
23+
1
24+
1
25+
1
26+
1
27+
1
28+
1
29+
0
30+
1
31+
1
32+
1
33+
0
34+
0
35+
0
36+
0
37+
# Num Bitmap Bytes:
38+
$13
39+
# Bitmap Byte Values:
40+
0x0
41+
0x0
42+
0x0
43+
0x20
44+
0x8
45+
0x0
46+
0x20
47+
0x0
48+
0x0
49+
0x0
50+
0x0
51+
0x0
52+
0x0
53+
54+
55+
bar
56+
# Func Hash:
57+
24
58+
# Num Counters:
59+
1
60+
# Counter Values:
61+
3
62+

‎llvm/test/tools/llvm-cov/mcdc-macro.test

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Test visualization of MC/DC constructs for branches in macro expansions.
2+
3+
// RUN: llvm-profdata merge %S/Inputs/mcdc-macro.proftext -o %t.profdata
4+
// 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
5+
6+
// CHECK: | | | Branch (2:11): [Folded - Ignored]
7+
// CHECK: | | | Branch (3:11): [True: 0, False: 0]
8+
// CHECK: | | | Branch (3:23): [True: 0, False: 0]
9+
// CHECK: | Branch (9:7): [True: 0, False: 0]
10+
// CHECK-NEXT: | Branch (9:22): [True: 0, False: 0]
11+
// CHECK-NEXT: ------------------
12+
// CHECK-NEXT: |---> MC/DC Decision Region (9:7) to (9:23)
13+
// CHECK-NEXT: |
14+
// CHECK-NEXT: | Number of Conditions: 5
15+
// CHECK-NEXT: | Condition C1 --> (9:7)
16+
// CHECK-NEXT: | Condition C2 --> (2:11)
17+
// CHECK-NEXT: | Condition C3 --> (3:11)
18+
// CHECK-NEXT: | Condition C4 --> (3:23)
19+
// CHECK-NEXT: | Condition C5 --> (9:22)
20+
// CHECK-NEXT: |
21+
// CHECK-NEXT: | Executed MC/DC Test Vectors:
22+
// CHECK-NEXT: |
23+
// CHECK-NEXT: | None.
24+
// CHECK-NEXT: |
25+
// CHECK-NEXT: | C1-Pair: not covered
26+
// CHECK-NEXT: | C2-Pair: constant folded
27+
// CHECK-NEXT: | C3-Pair: not covered
28+
// CHECK-NEXT: | C4-Pair: not covered
29+
// CHECK-NEXT: | C5-Pair: not covered
30+
// CHECK-NEXT: | MC/DC Coverage for Decision: 0.00%
31+
// CHECK-NEXT: |
32+
// CHECK-NEXT: ------------------
33+
34+
// CHECK: | | | Branch (2:11): [Folded - Ignored]
35+
// CHECK: | Branch (11:7): [True: 0, False: 0]
36+
// CHECK-NEXT: ------------------
37+
// CHECK-NEXT: |---> MC/DC Decision Region (11:7) to (11:13)
38+
// CHECK-NEXT: |
39+
// CHECK-NEXT: | Number of Conditions: 2
40+
// CHECK-NEXT: | Condition C1 --> (11:7)
41+
// CHECK-NEXT: | Condition C2 --> (2:11)
42+
// CHECK-NEXT: |
43+
// CHECK-NEXT: | Executed MC/DC Test Vectors:
44+
// CHECK-NEXT: |
45+
// CHECK-NEXT: | None.
46+
// CHECK-NEXT: |
47+
// CHECK-NEXT: | C1-Pair: not covered
48+
// CHECK-NEXT: | C2-Pair: constant folded
49+
// CHECK-NEXT: | MC/DC Coverage for Decision: 0.00%
50+
// CHECK-NEXT: |
51+
// CHECK-NEXT: ------------------
52+
53+
// CHECK: | | | Branch (1:11): [True: 0, False: 0]
54+
// CHECK: | | | Branch (2:11): [Folded - Ignored]
55+
// CHECK: | | | | | Branch (3:11): [True: 0, False: 0]
56+
// CHECK: | | | | | Branch (3:23): [True: 0, False: 0]
57+
// CHECK: | Branch (13:7): [True: 0, False: 0]
58+
// CHECK-NEXT: | Branch (13:13): [True: 0, False: 0]
59+
// CHECK-NEXT: ------------------
60+
// CHECK-NEXT: |---> MC/DC Decision Region (13:7) to (13:32)
61+
// CHECK-NEXT: |
62+
// CHECK-NEXT: | Number of Conditions: 6
63+
// CHECK-NEXT: | Condition C1 --> (13:7)
64+
// CHECK-NEXT: | Condition C2 --> (13:13)
65+
// CHECK-NEXT: | Condition C3 --> (1:11)
66+
// CHECK-NEXT: | Condition C4 --> (2:11)
67+
// CHECK-NEXT: | Condition C5 --> (3:11)
68+
// CHECK-NEXT: | Condition C6 --> (3:23)
69+
// CHECK-NEXT: |
70+
// CHECK-NEXT: | Executed MC/DC Test Vectors:
71+
// CHECK-NEXT: |
72+
// CHECK-NEXT: | None.
73+
// CHECK-NEXT: |
74+
// CHECK-NEXT: | C1-Pair: not covered
75+
// CHECK-NEXT: | C2-Pair: not covered
76+
// CHECK-NEXT: | C3-Pair: not covered
77+
// CHECK-NEXT: | C4-Pair: constant folded
78+
// CHECK-NEXT: | C5-Pair: not covered
79+
// CHECK-NEXT: | C6-Pair: not covered
80+
// CHECK-NEXT: | MC/DC Coverage for Decision: 0.00%
81+
// CHECK-NEXT: |
82+
// CHECK-NEXT: ------------------
83+
84+
Instructions for regenerating the test:
85+
86+
# cd %S/Inputs
87+
cp mcdc-macro.c /tmp
88+
89+
clang -fcoverage-mcdc -fprofile-instr-generate -fcoverage-compilation-dir=. \
90+
-fcoverage-mapping /tmp/mcdc-macro.c -o /tmp/mcdc-macro.o
91+
92+
mv /tmp/mcdc-macro.o %S/Inputs

0 commit comments

Comments
 (0)