Skip to content

Commit eb963ff

Browse files
richkadelrichkadel
committed
Fixes reported bugs in Rust Coverage
Fixes: #79569 Fixes: #79566 Fixes: #79565 For the first issue (#79569), I got hit a `debug_assert!()` before encountering the reported error message (because I have `debug = true` enabled in my config.toml). The assertion showed me that some `SwitchInt`s can have more than one target pointing to the same `BasicBlock`. I had thought that was invalid, but since it seems to be possible, I'm allowing this now. I added a new test for this. ---- In the last two cases above, both tests (intentionally) fail to compile, but the `InstrumentCoverage` pass is invoked anyway. The MIR starts with an `Unreachable` `BasicBlock`, which I hadn't encountered before. (I had assumed the `InstrumentCoverage` pass would only be invoked with MIRs from successful compilations.) I don't have test infrastructure set up to test coverage on files that fail to compile, so I didn't add a new test.
1 parent 9eb3a7c commit eb963ff

File tree

8 files changed

+553
-18
lines changed

8 files changed

+553
-18
lines changed

‎compiler/rustc_mir/src/transform/coverage/graph.rs

+16-12
Original file line numberDiff line numberDiff line change
@@ -32,24 +32,28 @@ impl CoverageGraph {
3232

3333
// Pre-transform MIR `BasicBlock` successors and predecessors into the BasicCoverageBlock
3434
// equivalents. Note that since the BasicCoverageBlock graph has been fully simplified, the
35-
// each predecessor of a BCB leader_bb should be in a unique BCB, and each successor of a
36-
// BCB last_bb should be in its own unique BCB. Therefore, collecting the BCBs using
37-
// `bb_to_bcb` should work without requiring a deduplication step.
35+
// each predecessor of a BCB leader_bb should be in a unique BCB. It is possible for a
36+
// `SwitchInt` to have multiple targets to the same destination `BasicBlock`, so
37+
// de-duplication is required. This is done without reordering the successors.
3838

39+
let bcbs_len = bcbs.len();
40+
let mut seen = IndexVec::from_elem_n(false, bcbs_len);
3941
let successors = IndexVec::from_fn_n(
4042
|bcb| {
43+
for b in seen.iter_mut() {
44+
*b = false;
45+
}
4146
let bcb_data = &bcbs[bcb];
42-
let bcb_successors =
47+
let mut bcb_successors = Vec::new();
48+
for successor in
4349
bcb_filtered_successors(&mir_body, &bcb_data.terminator(mir_body).kind)
4450
.filter_map(|&successor_bb| bb_to_bcb[successor_bb])
45-
.collect::<Vec<_>>();
46-
debug_assert!({
47-
let mut sorted = bcb_successors.clone();
48-
sorted.sort_unstable();
49-
let initial_len = sorted.len();
50-
sorted.dedup();
51-
sorted.len() == initial_len
52-
});
51+
{
52+
if !seen[successor] {
53+
seen[successor] = true;
54+
bcb_successors.push(successor);
55+
}
56+
}
5357
bcb_successors
5458
},
5559
bcbs.len(),

‎compiler/rustc_mir/src/transform/coverage/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage {
7878
return;
7979
}
8080

81+
match mir_body.basic_blocks()[mir::START_BLOCK].terminator().kind {
82+
TerminatorKind::Unreachable => {
83+
trace!("InstrumentCoverage skipped for unreachable `START_BLOCK`");
84+
return;
85+
}
86+
_ => {}
87+
}
88+
8189
trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
8290
Instrumentor::new(&self.name(), tcx, mir_body).inject_counters();
8391
trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());

‎src/test/run-make-fulldeps/coverage-reports/expected_export_coverage.match_or_pattern.json

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
{
2+
"data": [
3+
{
4+
"files": [
5+
{
6+
"filename": "../coverage/match_or_pattern.rs",
7+
"summary": {
8+
"functions": {
9+
"count": 1,
10+
"covered": 1,
11+
"percent": 100
12+
},
13+
"instantiations": {
14+
"count": 1,
15+
"covered": 1,
16+
"percent": 100
17+
},
18+
"lines": {
19+
"count": 37,
20+
"covered": 33,
21+
"percent": 89.1891891891892
22+
},
23+
"regions": {
24+
"count": 25,
25+
"covered": 17,
26+
"notcovered": 8,
27+
"percent": 68
28+
}
29+
}
30+
}
31+
],
32+
"totals": {
33+
"functions": {
34+
"count": 1,
35+
"covered": 1,
36+
"percent": 100
37+
},
38+
"instantiations": {
39+
"count": 1,
40+
"covered": 1,
41+
"percent": 100
42+
},
43+
"lines": {
44+
"count": 37,
45+
"covered": 33,
46+
"percent": 89.1891891891892
47+
},
48+
"regions": {
49+
"count": 25,
50+
"covered": 17,
51+
"notcovered": 8,
52+
"percent": 68
53+
}
54+
}
55+
}
56+
],
57+
"type": "llvm.coverage.json.export",
58+
"version": "2.0.1"
59+
}

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.match_or_pattern.txt

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
1| |#![feature(or_patterns)]
2+
2| |
3+
3| 1|fn main() {
4+
4| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure
5+
5| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from
6+
6| 1| // dependent conditions.
7+
7| 1| let is_true = std::env::args().len() == 1;
8+
8| 1|
9+
9| 1| let mut a: u8 = 0;
10+
10| 1| let mut b: u8 = 0;
11+
11| 1| if is_true {
12+
12| 1| a = 2;
13+
13| 1| b = 0;
14+
14| 1| }
15+
^0
16+
15| 1| match (a, b) {
17+
16| | // Or patterns generate MIR `SwitchInt` with multiple targets to the same `BasicBlock`.
18+
17| | // This test confirms a fix for Issue #79569.
19+
18| 0| (0 | 1, 2 | 3) => {}
20+
19| 1| _ => {}
21+
20| | }
22+
21| 1| if is_true {
23+
22| 1| a = 0;
24+
23| 1| b = 0;
25+
24| 1| }
26+
^0
27+
25| 1| match (a, b) {
28+
26| 0| (0 | 1, 2 | 3) => {}
29+
27| 1| _ => {}
30+
28| | }
31+
29| 1| if is_true {
32+
30| 1| a = 2;
33+
31| 1| b = 2;
34+
32| 1| }
35+
^0
36+
33| 1| match (a, b) {
37+
34| 0| (0 | 1, 2 | 3) => {}
38+
35| 1| _ => {}
39+
36| | }
40+
37| 1| if is_true {
41+
38| 1| a = 0;
42+
39| 1| b = 2;
43+
40| 1| }
44+
^0
45+
41| 1| match (a, b) {
46+
42| 1| (0 | 1, 2 | 3) => {}
47+
43| 0| _ => {}
48+
44| | }
49+
45| 1|}
50+

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage_counters.async.txt

+6-6
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,8 @@ Counter in file 0 79:14 -> 79:16, 0
2828
Counter in file 0 81:1 -> 81:2, 0
2929
Counter in file 0 91:25 -> 91:34, 0
3030
Counter in file 0 5:1 -> 5:25, #1
31-
Counter in file 0 5:25 -> 6:14, #1
32-
Counter in file 0 7:9 -> 7:10, #2
33-
Counter in file 0 9:9 -> 9:10, (#1 - #2)
34-
Counter in file 0 11:1 -> 11:2, (#2 + (#1 - #2))
3531
Counter in file 0 21:1 -> 21:23, #1
32+
Counter in file 0 17:20 -> 17:21, #1
3633
Counter in file 0 67:5 -> 67:23, #1
3734
Counter in file 0 38:1 -> 38:19, #1
3835
Counter in file 0 38:19 -> 42:12, #1
@@ -46,14 +43,18 @@ Counter in file 0 44:27 -> 44:32, #8
4643
Counter in file 0 44:36 -> 44:38, (#6 + 0)
4744
Counter in file 0 45:14 -> 45:16, #7
4845
Counter in file 0 47:1 -> 47:2, (#5 + (#6 + #7))
46+
Counter in file 0 13:20 -> 13:21, #1
4947
Counter in file 0 29:1 -> 29:22, #1
5048
Counter in file 0 93:1 -> 101:2, #1
5149
Counter in file 0 91:1 -> 91:25, #1
50+
Counter in file 0 5:25 -> 6:14, #1
51+
Counter in file 0 7:9 -> 7:10, #2
52+
Counter in file 0 9:9 -> 9:10, (#1 - #2)
53+
Counter in file 0 11:1 -> 11:2, (#2 + (#1 - #2))
5254
Counter in file 0 51:5 -> 52:18, #1
5355
Counter in file 0 53:13 -> 53:14, #2
5456
Counter in file 0 63:13 -> 63:14, (#1 - #2)
5557
Counter in file 0 65:5 -> 65:6, (#2 + (#1 - #2))
56-
Counter in file 0 17:20 -> 17:21, #1
5758
Counter in file 0 49:1 -> 68:12, #1
5859
Counter in file 0 69:9 -> 69:10, #2
5960
Counter in file 0 69:14 -> 69:27, (#1 + 0)
@@ -70,7 +71,6 @@ Counter in file 0 87:14 -> 87:16, #3
7071
Counter in file 0 89:1 -> 89:2, (#3 + (#2 + (#1 - (#3 + #2))))
7172
Counter in file 0 17:1 -> 17:20, #1
7273
Counter in file 0 66:5 -> 66:23, #1
73-
Counter in file 0 13:20 -> 13:21, #1
7474
Counter in file 0 17:9 -> 17:10, #1
7575
Counter in file 0 17:9 -> 17:10, #1
7676
Counter in file 0 117:17 -> 117:19, #1

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage_counters.match_or_pattern.txt

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
Counter in file 0 3:1 -> 11:15, #1
2+
Counter in file 0 11:16 -> 14:6, #2
3+
Counter in file 0 14:6 -> 14:7, (#1 - #2)
4+
Counter in file 0 15:11 -> 15:17, (#2 + (#1 - #2))
5+
Counter in file 0 18:27 -> 18:29, #5
6+
Counter in file 0 19:14 -> 19:16, (#3 + #4)
7+
Counter in file 0 21:8 -> 21:15, ((#3 + #4) + #5)
8+
Counter in file 0 21:16 -> 24:6, #6
9+
Counter in file 0 24:6 -> 24:7, (((#3 + #4) + #5) - #6)
10+
Counter in file 0 25:11 -> 25:17, (#6 + (((#3 + #4) + #5) - #6))
11+
Counter in file 0 26:27 -> 26:29, #9
12+
Counter in file 0 27:14 -> 27:16, (#7 + #8)
13+
Counter in file 0 29:8 -> 29:15, ((#7 + #8) + #9)
14+
Counter in file 0 29:16 -> 32:6, #10
15+
Counter in file 0 32:6 -> 32:7, (((#7 + #8) + #9) - #10)
16+
Counter in file 0 33:11 -> 33:17, (#10 + (((#7 + #8) + #9) - #10))
17+
Counter in file 0 34:27 -> 34:29, #13
18+
Counter in file 0 35:14 -> 35:16, (#11 + #12)
19+
Counter in file 0 37:8 -> 37:15, ((#11 + #12) + #13)
20+
Counter in file 0 37:16 -> 40:6, #14
21+
Counter in file 0 40:6 -> 40:7, (((#11 + #12) + #13) - #14)
22+
Counter in file 0 41:11 -> 41:17, (#14 + (((#11 + #12) + #13) - #14))
23+
Counter in file 0 42:27 -> 42:29, #17
24+
Counter in file 0 43:14 -> 43:16, (#15 + #16)
25+
Counter in file 0 45:1 -> 45:2, ((#15 + #16) + #17)
26+
Emitting segments for file: ../coverage/match_or_pattern.rs
27+
Combined regions:
28+
3:1 -> 11:15 (count=1)
29+
11:16 -> 14:6 (count=1)
30+
14:6 -> 14:7 (count=0)
31+
15:11 -> 15:17 (count=1)
32+
18:27 -> 18:29 (count=0)
33+
19:14 -> 19:16 (count=1)
34+
21:8 -> 21:15 (count=1)
35+
21:16 -> 24:6 (count=1)
36+
24:6 -> 24:7 (count=0)
37+
25:11 -> 25:17 (count=1)
38+
26:27 -> 26:29 (count=0)
39+
27:14 -> 27:16 (count=1)
40+
29:8 -> 29:15 (count=1)
41+
29:16 -> 32:6 (count=1)
42+
32:6 -> 32:7 (count=0)
43+
33:11 -> 33:17 (count=1)
44+
34:27 -> 34:29 (count=0)
45+
35:14 -> 35:16 (count=1)
46+
37:8 -> 37:15 (count=1)
47+
37:16 -> 40:6 (count=1)
48+
40:6 -> 40:7 (count=0)
49+
41:11 -> 41:17 (count=1)
50+
42:27 -> 42:29 (count=1)
51+
43:14 -> 43:16 (count=0)
52+
45:1 -> 45:2 (count=1)
53+
Segment at 3:1 (count = 1), RegionEntry
54+
Segment at 11:15 (count = 0), Skipped
55+
Segment at 11:16 (count = 1), RegionEntry
56+
Segment at 14:6 (count = 0), RegionEntry
57+
Segment at 14:7 (count = 0), Skipped
58+
Segment at 15:11 (count = 1), RegionEntry
59+
Segment at 15:17 (count = 0), Skipped
60+
Segment at 18:27 (count = 0), RegionEntry
61+
Segment at 18:29 (count = 0), Skipped
62+
Segment at 19:14 (count = 1), RegionEntry
63+
Segment at 19:16 (count = 0), Skipped
64+
Segment at 21:8 (count = 1), RegionEntry
65+
Segment at 21:15 (count = 0), Skipped
66+
Segment at 21:16 (count = 1), RegionEntry
67+
Segment at 24:6 (count = 0), RegionEntry
68+
Segment at 24:7 (count = 0), Skipped
69+
Segment at 25:11 (count = 1), RegionEntry
70+
Segment at 25:17 (count = 0), Skipped
71+
Segment at 26:27 (count = 0), RegionEntry
72+
Segment at 26:29 (count = 0), Skipped
73+
Segment at 27:14 (count = 1), RegionEntry
74+
Segment at 27:16 (count = 0), Skipped
75+
Segment at 29:8 (count = 1), RegionEntry
76+
Segment at 29:15 (count = 0), Skipped
77+
Segment at 29:16 (count = 1), RegionEntry
78+
Segment at 32:6 (count = 0), RegionEntry
79+
Segment at 32:7 (count = 0), Skipped
80+
Segment at 33:11 (count = 1), RegionEntry
81+
Segment at 33:17 (count = 0), Skipped
82+
Segment at 34:27 (count = 0), RegionEntry
83+
Segment at 34:29 (count = 0), Skipped
84+
Segment at 35:14 (count = 1), RegionEntry
85+
Segment at 35:16 (count = 0), Skipped
86+
Segment at 37:8 (count = 1), RegionEntry
87+
Segment at 37:15 (count = 0), Skipped
88+
Segment at 37:16 (count = 1), RegionEntry
89+
Segment at 40:6 (count = 0), RegionEntry
90+
Segment at 40:7 (count = 0), Skipped
91+
Segment at 41:11 (count = 1), RegionEntry
92+
Segment at 41:17 (count = 0), Skipped
93+
Segment at 42:27 (count = 1), RegionEntry
94+
Segment at 42:29 (count = 0), Skipped
95+
Segment at 43:14 (count = 0), RegionEntry
96+
Segment at 43:16 (count = 0), Skipped
97+
Segment at 45:1 (count = 1), RegionEntry
98+
Segment at 45:2 (count = 0), Skipped

0 commit comments

Comments
 (0)