Skip to content

Commit 5f4afbb

Browse files
authored
Rollup merge of rust-lang#76659 - simonvandel:76432, r=oli-obk
SimplifyComparisonIntegral: fix miscompilation Fixes rust-lang#76432 Only insert StorageDeads if we actually removed one. Fixes an issue where we added StorageDead to a place with no StorageLive r? @oli-obk
2 parents 71cdf25 + e5447a2 commit 5f4afbb

File tree

3 files changed

+154
-21
lines changed

3 files changed

+154
-21
lines changed

compiler/rustc_mir/src/transform/simplify_comparison_integral.rs

+22-21
Original file line numberDiff line numberDiff line change
@@ -61,26 +61,6 @@ impl<'tcx> MirPass<'tcx> for SimplifyComparisonIntegral {
6161
_ => unreachable!(),
6262
}
6363

64-
let terminator = bb.terminator_mut();
65-
66-
// add StorageDead for the place switched on at the top of each target
67-
for bb_idx in new_targets.iter() {
68-
storage_deads_to_insert.push((
69-
*bb_idx,
70-
Statement {
71-
source_info: terminator.source_info,
72-
kind: StatementKind::StorageDead(opt.to_switch_on.local),
73-
},
74-
));
75-
}
76-
77-
terminator.kind = TerminatorKind::SwitchInt {
78-
discr: Operand::Move(opt.to_switch_on),
79-
switch_ty: opt.branch_value_ty,
80-
values: vec![new_value].into(),
81-
targets: new_targets,
82-
};
83-
8464
// delete comparison statement if it the value being switched on was moved, which means it can not be user later on
8565
if opt.can_remove_bin_op_stmt {
8666
bb.statements[opt.bin_op_stmt_idx].make_nop();
@@ -106,14 +86,35 @@ impl<'tcx> MirPass<'tcx> for SimplifyComparisonIntegral {
10686
}
10787
}
10888

89+
let terminator = bb.terminator();
90+
10991
// remove StorageDead (if it exists) being used in the assign of the comparison
11092
for (stmt_idx, stmt) in bb.statements.iter().enumerate() {
11193
if !matches!(stmt.kind, StatementKind::StorageDead(local) if local == opt.to_switch_on.local)
11294
{
11395
continue;
11496
}
115-
storage_deads_to_remove.push((stmt_idx, opt.bb_idx))
97+
storage_deads_to_remove.push((stmt_idx, opt.bb_idx));
98+
// if we have StorageDeads to remove then make sure to insert them at the top of each target
99+
for bb_idx in new_targets.iter() {
100+
storage_deads_to_insert.push((
101+
*bb_idx,
102+
Statement {
103+
source_info: terminator.source_info,
104+
kind: StatementKind::StorageDead(opt.to_switch_on.local),
105+
},
106+
));
107+
}
116108
}
109+
110+
let terminator = bb.terminator_mut();
111+
112+
terminator.kind = TerminatorKind::SwitchInt {
113+
discr: Operand::Move(opt.to_switch_on),
114+
switch_ty: opt.branch_value_ty,
115+
values: vec![new_value].into(),
116+
targets: new_targets,
117+
};
117118
}
118119

119120
for (idx, bb_idx) in storage_deads_to_remove {

src/test/mir-opt/issue_76432.rs

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Check that we do not insert StorageDead at each target if StorageDead was never seen
2+
3+
// EMIT_MIR issue_76432.test.SimplifyComparisonIntegral.diff
4+
use std::fmt::Debug;
5+
6+
fn test<T: Copy + Debug + PartialEq>(x: T) {
7+
let v: &[T] = &[x, x, x];
8+
match v {
9+
[ref v1, ref v2, ref v3] => [v1 as *const _, v2 as *const _, v3 as *const _],
10+
_ => unreachable!(),
11+
};
12+
}
13+
14+
fn main() {
15+
test(0u32);
16+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
- // MIR for `test` before SimplifyComparisonIntegral
2+
+ // MIR for `test` after SimplifyComparisonIntegral
3+
4+
fn test(_1: T) -> () {
5+
debug x => _1; // in scope 0 at $DIR/issue_76432.rs:6:38: 6:39
6+
let mut _0: (); // return place in scope 0 at $DIR/issue_76432.rs:6:44: 6:44
7+
let _2: &[T]; // in scope 0 at $DIR/issue_76432.rs:7:9: 7:10
8+
let mut _3: &[T; 3]; // in scope 0 at $DIR/issue_76432.rs:7:19: 7:29
9+
let _4: &[T; 3]; // in scope 0 at $DIR/issue_76432.rs:7:19: 7:29
10+
let _5: [T; 3]; // in scope 0 at $DIR/issue_76432.rs:7:20: 7:29
11+
let mut _6: T; // in scope 0 at $DIR/issue_76432.rs:7:21: 7:22
12+
let mut _7: T; // in scope 0 at $DIR/issue_76432.rs:7:24: 7:25
13+
let mut _8: T; // in scope 0 at $DIR/issue_76432.rs:7:27: 7:28
14+
let _9: [*const T; 3]; // in scope 0 at $DIR/issue_76432.rs:8:5: 11:6
15+
let mut _10: usize; // in scope 0 at $DIR/issue_76432.rs:9:9: 9:33
16+
let mut _11: usize; // in scope 0 at $DIR/issue_76432.rs:9:9: 9:33
17+
let mut _12: bool; // in scope 0 at $DIR/issue_76432.rs:9:9: 9:33
18+
let mut _16: *const T; // in scope 0 at $DIR/issue_76432.rs:9:38: 9:52
19+
let mut _17: *const T; // in scope 0 at $DIR/issue_76432.rs:9:38: 9:52
20+
let mut _18: *const T; // in scope 0 at $DIR/issue_76432.rs:9:54: 9:68
21+
let mut _19: *const T; // in scope 0 at $DIR/issue_76432.rs:9:54: 9:68
22+
let mut _20: *const T; // in scope 0 at $DIR/issue_76432.rs:9:70: 9:84
23+
let mut _21: *const T; // in scope 0 at $DIR/issue_76432.rs:9:70: 9:84
24+
let mut _22: !; // in scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL
25+
scope 1 {
26+
debug v => _2; // in scope 1 at $DIR/issue_76432.rs:7:9: 7:10
27+
let _13: &T; // in scope 1 at $DIR/issue_76432.rs:9:10: 9:16
28+
let _14: &T; // in scope 1 at $DIR/issue_76432.rs:9:18: 9:24
29+
let _15: &T; // in scope 1 at $DIR/issue_76432.rs:9:26: 9:32
30+
scope 2 {
31+
debug v1 => _13; // in scope 2 at $DIR/issue_76432.rs:9:10: 9:16
32+
debug v2 => _14; // in scope 2 at $DIR/issue_76432.rs:9:18: 9:24
33+
debug v3 => _15; // in scope 2 at $DIR/issue_76432.rs:9:26: 9:32
34+
}
35+
}
36+
37+
bb0: {
38+
StorageLive(_2); // scope 0 at $DIR/issue_76432.rs:7:9: 7:10
39+
StorageLive(_3); // scope 0 at $DIR/issue_76432.rs:7:19: 7:29
40+
StorageLive(_4); // scope 0 at $DIR/issue_76432.rs:7:19: 7:29
41+
StorageLive(_5); // scope 0 at $DIR/issue_76432.rs:7:20: 7:29
42+
StorageLive(_6); // scope 0 at $DIR/issue_76432.rs:7:21: 7:22
43+
_6 = _1; // scope 0 at $DIR/issue_76432.rs:7:21: 7:22
44+
StorageLive(_7); // scope 0 at $DIR/issue_76432.rs:7:24: 7:25
45+
_7 = _1; // scope 0 at $DIR/issue_76432.rs:7:24: 7:25
46+
StorageLive(_8); // scope 0 at $DIR/issue_76432.rs:7:27: 7:28
47+
_8 = _1; // scope 0 at $DIR/issue_76432.rs:7:27: 7:28
48+
_5 = [move _6, move _7, move _8]; // scope 0 at $DIR/issue_76432.rs:7:20: 7:29
49+
StorageDead(_8); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29
50+
StorageDead(_7); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29
51+
StorageDead(_6); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29
52+
_4 = &_5; // scope 0 at $DIR/issue_76432.rs:7:19: 7:29
53+
_3 = _4; // scope 0 at $DIR/issue_76432.rs:7:19: 7:29
54+
_2 = move _3 as &[T] (Pointer(Unsize)); // scope 0 at $DIR/issue_76432.rs:7:19: 7:29
55+
StorageDead(_3); // scope 0 at $DIR/issue_76432.rs:7:28: 7:29
56+
StorageDead(_4); // scope 0 at $DIR/issue_76432.rs:7:29: 7:30
57+
StorageLive(_9); // scope 1 at $DIR/issue_76432.rs:8:5: 11:6
58+
_10 = Len((*_2)); // scope 1 at $DIR/issue_76432.rs:9:9: 9:33
59+
_11 = const 3_usize; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33
60+
- _12 = Eq(move _10, const 3_usize); // scope 1 at $DIR/issue_76432.rs:9:9: 9:33
61+
- switchInt(move _12) -> [false: bb1, otherwise: bb2]; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33
62+
+ nop; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33
63+
+ switchInt(move _10) -> [3_usize: bb2, otherwise: bb1]; // scope 1 at $DIR/issue_76432.rs:9:9: 9:33
64+
}
65+
66+
bb1: {
67+
StorageLive(_22); // scope 1 at $SRC_DIR/std/src/macros.rs:LL:COL
68+
begin_panic::<&str>(const "internal error: entered unreachable code"); // scope 1 at $SRC_DIR/std/src/macros.rs:LL:COL
69+
// mir::Constant
70+
// + span: $SRC_DIR/std/src/macros.rs:LL:COL
71+
// + literal: Const { ty: fn(&str) -> ! {std::rt::begin_panic::<&str>}, val: Value(Scalar(<ZST>)) }
72+
// ty::Const
73+
// + ty: &str
74+
// + val: Value(Slice { data: Allocation { bytes: [105, 110, 116, 101, 114, 110, 97, 108, 32, 101, 114, 114, 111, 114, 58, 32, 101, 110, 116, 101, 114, 101, 100, 32, 117, 110, 114, 101, 97, 99, 104, 97, 98, 108, 101, 32, 99, 111, 100, 101], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [1099511627775], len: Size { raw: 40 } }, size: Size { raw: 40 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 40 })
75+
// mir::Constant
76+
// + span: $SRC_DIR/core/src/macros/mod.rs:LL:COL
77+
// + literal: Const { ty: &str, val: Value(Slice { data: Allocation { bytes: [105, 110, 116, 101, 114, 110, 97, 108, 32, 101, 114, 114, 111, 114, 58, 32, 101, 110, 116, 101, 114, 101, 100, 32, 117, 110, 114, 101, 97, 99, 104, 97, 98, 108, 101, 32, 99, 111, 100, 101], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [1099511627775], len: Size { raw: 40 } }, size: Size { raw: 40 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 40 }) }
78+
}
79+
80+
bb2: {
81+
StorageLive(_13); // scope 1 at $DIR/issue_76432.rs:9:10: 9:16
82+
_13 = &(*_2)[0 of 3]; // scope 1 at $DIR/issue_76432.rs:9:10: 9:16
83+
StorageLive(_14); // scope 1 at $DIR/issue_76432.rs:9:18: 9:24
84+
_14 = &(*_2)[1 of 3]; // scope 1 at $DIR/issue_76432.rs:9:18: 9:24
85+
StorageLive(_15); // scope 1 at $DIR/issue_76432.rs:9:26: 9:32
86+
_15 = &(*_2)[2 of 3]; // scope 1 at $DIR/issue_76432.rs:9:26: 9:32
87+
StorageLive(_16); // scope 2 at $DIR/issue_76432.rs:9:38: 9:52
88+
StorageLive(_17); // scope 2 at $DIR/issue_76432.rs:9:38: 9:52
89+
_17 = &raw const (*_13); // scope 2 at $DIR/issue_76432.rs:9:38: 9:40
90+
_16 = _17; // scope 2 at $DIR/issue_76432.rs:9:38: 9:52
91+
StorageLive(_18); // scope 2 at $DIR/issue_76432.rs:9:54: 9:68
92+
StorageLive(_19); // scope 2 at $DIR/issue_76432.rs:9:54: 9:68
93+
_19 = &raw const (*_14); // scope 2 at $DIR/issue_76432.rs:9:54: 9:56
94+
_18 = _19; // scope 2 at $DIR/issue_76432.rs:9:54: 9:68
95+
StorageLive(_20); // scope 2 at $DIR/issue_76432.rs:9:70: 9:84
96+
StorageLive(_21); // scope 2 at $DIR/issue_76432.rs:9:70: 9:84
97+
_21 = &raw const (*_15); // scope 2 at $DIR/issue_76432.rs:9:70: 9:72
98+
_20 = _21; // scope 2 at $DIR/issue_76432.rs:9:70: 9:84
99+
_9 = [move _16, move _18, move _20]; // scope 2 at $DIR/issue_76432.rs:9:37: 9:85
100+
StorageDead(_21); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85
101+
StorageDead(_20); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85
102+
StorageDead(_19); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85
103+
StorageDead(_18); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85
104+
StorageDead(_17); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85
105+
StorageDead(_16); // scope 2 at $DIR/issue_76432.rs:9:84: 9:85
106+
StorageDead(_15); // scope 1 at $DIR/issue_76432.rs:9:84: 9:85
107+
StorageDead(_14); // scope 1 at $DIR/issue_76432.rs:9:84: 9:85
108+
StorageDead(_13); // scope 1 at $DIR/issue_76432.rs:9:84: 9:85
109+
StorageDead(_9); // scope 1 at $DIR/issue_76432.rs:11:6: 11:7
110+
_0 = const (); // scope 0 at $DIR/issue_76432.rs:6:44: 12:2
111+
StorageDead(_5); // scope 0 at $DIR/issue_76432.rs:12:1: 12:2
112+
StorageDead(_2); // scope 0 at $DIR/issue_76432.rs:12:1: 12:2
113+
return; // scope 0 at $DIR/issue_76432.rs:12:2: 12:2
114+
}
115+
}
116+

0 commit comments

Comments
 (0)