Skip to content

Commit 382f31a

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of rust-lang#102045 - RalfJung:const-prop-regression-fix, r=oli-obk
fix ConstProp handling of written_only_inside_own_block_locals Fixes a regression introduced by rust-lang#100239, which adds an early return and thus skips some code in `visit_terminator` that must be run for soundness. Fixes rust-lang#101973
2 parents 26727dc + 7373788 commit 382f31a

File tree

3 files changed

+140
-20
lines changed

3 files changed

+140
-20
lines changed

‎compiler/rustc_mir_transform/src/const_prop.rs

+20-20
Original file line numberDiff line numberDiff line change
@@ -1066,32 +1066,32 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
10661066
let source_info = terminator.source_info;
10671067
self.source_info = Some(source_info);
10681068
self.super_terminator(terminator, location);
1069+
// Do NOT early return in this function, it does some crucial fixup of the state at the end!
10691070
match &mut terminator.kind {
10701071
TerminatorKind::Assert { expected, ref mut cond, .. } => {
10711072
if let Some(ref value) = self.eval_operand(&cond) {
10721073
trace!("assertion on {:?} should be {:?}", value, expected);
10731074
let expected = Scalar::from_bool(*expected);
1074-
let Ok(value_const) = self.ecx.read_scalar(&value) else {
1075-
// FIXME should be used use_ecx rather than a local match... but we have
1076-
// quite a few of these read_scalar/read_immediate that need fixing.
1077-
return
1078-
};
1079-
if expected != value_const {
1080-
// Poison all places this operand references so that further code
1081-
// doesn't use the invalid value
1082-
match cond {
1083-
Operand::Move(ref place) | Operand::Copy(ref place) => {
1084-
Self::remove_const(&mut self.ecx, place.local);
1075+
// FIXME should be used use_ecx rather than a local match... but we have
1076+
// quite a few of these read_scalar/read_immediate that need fixing.
1077+
if let Ok(value_const) = self.ecx.read_scalar(&value) {
1078+
if expected != value_const {
1079+
// Poison all places this operand references so that further code
1080+
// doesn't use the invalid value
1081+
match cond {
1082+
Operand::Move(ref place) | Operand::Copy(ref place) => {
1083+
Self::remove_const(&mut self.ecx, place.local);
1084+
}
1085+
Operand::Constant(_) => {}
1086+
}
1087+
} else {
1088+
if self.should_const_prop(value) {
1089+
*cond = self.operand_from_scalar(
1090+
value_const,
1091+
self.tcx.types.bool,
1092+
source_info.span,
1093+
);
10851094
}
1086-
Operand::Constant(_) => {}
1087-
}
1088-
} else {
1089-
if self.should_const_prop(value) {
1090-
*cond = self.operand_from_scalar(
1091-
value_const,
1092-
self.tcx.types.bool,
1093-
source_info.span,
1094-
);
10951095
}
10961096
}
10971097
}

‎src/test/mir-opt/issue-101973.rs

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// compile-flags: -O -C debug-assertions=on
2+
// This needs inlining followed by ConstProp to reproduce, so we cannot use "unit-test".
3+
4+
#[inline]
5+
pub fn imm8(x: u32) -> u32 {
6+
let mut out = 0u32;
7+
out |= (x >> 0) & 0xff;
8+
out
9+
}
10+
11+
// EMIT_MIR issue_101973.inner.ConstProp.diff
12+
#[inline(never)]
13+
pub fn inner(fields: u32) -> i64 {
14+
imm8(fields).rotate_right(((fields >> 8) & 0xf) << 1) as i32 as i64
15+
}
16+
17+
fn main() {
18+
let val = inner(0xe32cf20f);
19+
assert_eq!(val as u64, 0xfffffffff0000000);
20+
}

‎src/test/mir-opt/issue_101973.inner.ConstProp.diff

+100
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
- // MIR for `inner` before ConstProp
2+
+ // MIR for `inner` after ConstProp
3+
4+
fn inner(_1: u32) -> i64 {
5+
debug fields => _1; // in scope 0 at $DIR/issue-101973.rs:+0:14: +0:20
6+
let mut _0: i64; // return place in scope 0 at $DIR/issue-101973.rs:+0:30: +0:33
7+
let mut _2: i32; // in scope 0 at $DIR/issue-101973.rs:+1:5: +1:65
8+
let mut _3: u32; // in scope 0 at $DIR/issue-101973.rs:+1:5: +1:58
9+
let mut _4: u32; // in scope 0 at $DIR/issue-101973.rs:+1:5: +1:17
10+
let mut _5: u32; // in scope 0 at $DIR/issue-101973.rs:+1:10: +1:16
11+
let mut _6: u32; // in scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
12+
let mut _7: u32; // in scope 0 at $DIR/issue-101973.rs:+1:31: +1:52
13+
let mut _8: u32; // in scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
14+
let mut _9: u32; // in scope 0 at $DIR/issue-101973.rs:+1:33: +1:39
15+
let mut _10: (u32, bool); // in scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
16+
let mut _11: (u32, bool); // in scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
17+
scope 1 (inlined imm8) { // at $DIR/issue-101973.rs:14:5: 14:17
18+
debug x => _5; // in scope 1 at $DIR/issue-101973.rs:5:13: 5:14
19+
let mut _12: u32; // in scope 1 at $DIR/issue-101973.rs:7:12: 7:27
20+
let mut _13: u32; // in scope 1 at $DIR/issue-101973.rs:7:12: 7:20
21+
let mut _14: u32; // in scope 1 at $DIR/issue-101973.rs:7:13: 7:14
22+
let mut _15: (u32, bool); // in scope 1 at $DIR/issue-101973.rs:7:12: 7:20
23+
scope 2 {
24+
debug out => _4; // in scope 2 at $DIR/issue-101973.rs:6:9: 6:16
25+
}
26+
}
27+
scope 3 (inlined core::num::<impl u32>::rotate_right) { // at $DIR/issue-101973.rs:14:5: 14:58
28+
debug self => _4; // in scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
29+
debug n => _6; // in scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
30+
let mut _16: u32; // in scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
31+
let mut _17: u32; // in scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
32+
}
33+
34+
bb0: {
35+
StorageLive(_2); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:65
36+
StorageLive(_3); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:58
37+
StorageLive(_4); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:17
38+
StorageLive(_5); // scope 0 at $DIR/issue-101973.rs:+1:10: +1:16
39+
_5 = _1; // scope 0 at $DIR/issue-101973.rs:+1:10: +1:16
40+
_4 = const 0_u32; // scope 1 at $DIR/issue-101973.rs:6:19: 6:23
41+
StorageLive(_12); // scope 2 at $DIR/issue-101973.rs:7:12: 7:27
42+
StorageLive(_13); // scope 2 at $DIR/issue-101973.rs:7:12: 7:20
43+
StorageLive(_14); // scope 2 at $DIR/issue-101973.rs:7:13: 7:14
44+
_14 = _5; // scope 2 at $DIR/issue-101973.rs:7:13: 7:14
45+
_15 = CheckedShr(_14, const 0_i32); // scope 2 at $DIR/issue-101973.rs:7:12: 7:20
46+
assert(!move (_15.1: bool), "attempt to shift right by `{}`, which would overflow", const 0_i32) -> bb3; // scope 2 at $DIR/issue-101973.rs:7:12: 7:20
47+
}
48+
49+
bb1: {
50+
_8 = move (_10.0: u32); // scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
51+
StorageDead(_9); // scope 0 at $DIR/issue-101973.rs:+1:44: +1:45
52+
_7 = BitAnd(move _8, const 15_u32); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:52
53+
StorageDead(_8); // scope 0 at $DIR/issue-101973.rs:+1:51: +1:52
54+
_11 = CheckedShl(_7, const 1_i32); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
55+
assert(!move (_11.1: bool), "attempt to shift left by `{}`, which would overflow", const 1_i32) -> bb2; // scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
56+
}
57+
58+
bb2: {
59+
_6 = move (_11.0: u32); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
60+
StorageDead(_7); // scope 0 at $DIR/issue-101973.rs:+1:56: +1:57
61+
StorageLive(_16); // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
62+
_16 = _4; // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
63+
StorageLive(_17); // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
64+
_17 = _6; // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
65+
_3 = rotate_right::<u32>(move _16, move _17) -> bb4; // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
66+
// mir::Constant
67+
// + span: $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
68+
// + literal: Const { ty: extern "rust-intrinsic" fn(u32, u32) -> u32 {rotate_right::<u32>}, val: Value(<ZST>) }
69+
}
70+
71+
bb3: {
72+
_13 = move (_15.0: u32); // scope 2 at $DIR/issue-101973.rs:7:12: 7:20
73+
StorageDead(_14); // scope 2 at $DIR/issue-101973.rs:7:19: 7:20
74+
_12 = BitAnd(move _13, const 255_u32); // scope 2 at $DIR/issue-101973.rs:7:12: 7:27
75+
StorageDead(_13); // scope 2 at $DIR/issue-101973.rs:7:26: 7:27
76+
_4 = BitOr(_4, move _12); // scope 2 at $DIR/issue-101973.rs:7:5: 7:27
77+
StorageDead(_12); // scope 2 at $DIR/issue-101973.rs:7:26: 7:27
78+
StorageDead(_5); // scope 0 at $DIR/issue-101973.rs:+1:16: +1:17
79+
StorageLive(_6); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
80+
StorageLive(_7); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:52
81+
StorageLive(_8); // scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
82+
StorageLive(_9); // scope 0 at $DIR/issue-101973.rs:+1:33: +1:39
83+
_9 = _1; // scope 0 at $DIR/issue-101973.rs:+1:33: +1:39
84+
_10 = CheckedShr(_9, const 8_i32); // scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
85+
assert(!move (_10.1: bool), "attempt to shift right by `{}`, which would overflow", const 8_i32) -> bb1; // scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
86+
}
87+
88+
bb4: {
89+
StorageDead(_17); // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
90+
StorageDead(_16); // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
91+
StorageDead(_6); // scope 0 at $DIR/issue-101973.rs:+1:57: +1:58
92+
StorageDead(_4); // scope 0 at $DIR/issue-101973.rs:+1:57: +1:58
93+
_2 = move _3 as i32 (Misc); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:65
94+
StorageDead(_3); // scope 0 at $DIR/issue-101973.rs:+1:64: +1:65
95+
_0 = move _2 as i64 (Misc); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:72
96+
StorageDead(_2); // scope 0 at $DIR/issue-101973.rs:+1:71: +1:72
97+
return; // scope 0 at $DIR/issue-101973.rs:+2:2: +2:2
98+
}
99+
}
100+

0 commit comments

Comments
 (0)