Skip to content

Commit d8b4604

Browse files
ManishearthManishearth
authored
Rollup merge of #73613 - oli-obk:const_prop_miscompile, r=wesleywiser
The const propagator cannot trace references. Thus we avoid propagation of a local the moment we encounter references to it. fixes #73609 cc @RalfJung r? @wesleywiser
2 parents 30fba22 + 5fa8b08 commit d8b4604

File tree

8 files changed

+204
-56
lines changed

8 files changed

+204
-56
lines changed

‎src/librustc_mir/transform/const_prop.rs

+38-16
Original file line numberDiff line numberDiff line change
@@ -574,8 +574,16 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
574574
}
575575

576576
// Do not try creating references (#67862)
577-
Rvalue::Ref(_, _, place_ref) => {
578-
trace!("skipping Ref({:?})", place_ref);
577+
Rvalue::AddressOf(_, place) | Rvalue::Ref(_, _, place) => {
578+
trace!("skipping AddressOf | Ref for {:?}", place);
579+
580+
// This may be creating mutable references or immutable references to cells.
581+
// If that happens, the pointed to value could be mutated via that reference.
582+
// Since we aren't tracking references, the const propagator loses track of what
583+
// value the local has right now.
584+
// Thus, all locals that have their reference taken
585+
// must not take part in propagation.
586+
Self::remove_const(&mut self.ecx, place.local);
579587

580588
return None;
581589
}
@@ -715,6 +723,9 @@ enum ConstPropMode {
715723
OnlyInsideOwnBlock,
716724
/// The `Local` can be propagated into but reads cannot be propagated.
717725
OnlyPropagateInto,
726+
/// The `Local` cannot be part of propagation at all. Any statement
727+
/// referencing it either for reading or writing will not get propagated.
728+
NoPropagation,
718729
}
719730

720731
struct CanConstProp {
@@ -780,7 +791,9 @@ impl<'tcx> Visitor<'tcx> for CanConstProp {
780791
// end of the block anyway, and inside the block we overwrite previous
781792
// states as applicable.
782793
ConstPropMode::OnlyInsideOwnBlock => {}
783-
other => {
794+
ConstPropMode::NoPropagation => {}
795+
ConstPropMode::OnlyPropagateInto => {}
796+
other @ ConstPropMode::FullConstProp => {
784797
trace!(
785798
"local {:?} can't be propagated because of multiple assignments",
786799
local,
@@ -812,7 +825,7 @@ impl<'tcx> Visitor<'tcx> for CanConstProp {
812825
| MutatingUse(MutatingUseContext::Borrow)
813826
| MutatingUse(MutatingUseContext::AddressOf) => {
814827
trace!("local {:?} can't be propagaged because it's used: {:?}", local, context);
815-
self.can_const_prop[local] = ConstPropMode::OnlyPropagateInto;
828+
self.can_const_prop[local] = ConstPropMode::NoPropagation;
816829
}
817830
}
818831
}
@@ -857,19 +870,22 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
857870
}
858871
}
859872
}
860-
if can_const_prop == ConstPropMode::OnlyInsideOwnBlock {
861-
trace!(
862-
"found local restricted to its block. Will remove it from const-prop after block is finished. Local: {:?}",
863-
place.local
864-
);
865-
self.locals_of_current_block.insert(place.local);
866-
}
867-
868-
if can_const_prop == ConstPropMode::OnlyPropagateInto {
869-
trace!("can't propagate into {:?}", place);
870-
if place.local != RETURN_PLACE {
871-
Self::remove_const(&mut self.ecx, place.local);
873+
match can_const_prop {
874+
ConstPropMode::OnlyInsideOwnBlock => {
875+
trace!(
876+
"found local restricted to its block. \
877+
Will remove it from const-prop after block is finished. Local: {:?}",
878+
place.local
879+
);
880+
self.locals_of_current_block.insert(place.local);
872881
}
882+
ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => {
883+
trace!("can't propagate into {:?}", place);
884+
if place.local != RETURN_PLACE {
885+
Self::remove_const(&mut self.ecx, place.local);
886+
}
887+
}
888+
ConstPropMode::FullConstProp => {}
873889
}
874890
} else {
875891
// Const prop failed, so erase the destination, ensuring that whatever happens
@@ -889,6 +905,12 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
889905
);
890906
Self::remove_const(&mut self.ecx, place.local);
891907
}
908+
} else {
909+
trace!(
910+
"cannot propagate into {:?}, because the type of the local is generic.",
911+
place,
912+
);
913+
Self::remove_const(&mut self.ecx, place.local);
892914
}
893915
} else {
894916
match statement.kind {

‎src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/32bit/rustc.main.ConstProp.diff

+2-16
Original file line numberDiff line numberDiff line change
@@ -46,22 +46,8 @@
4646
// mir::Constant
4747
// + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:23: 7:24
4848
// + literal: Const { ty: usize, val: Value(Scalar(0x00000003)) }
49-
- _7 = Len((*_1)); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
50-
- _8 = Lt(_6, _7); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
51-
+ _7 = const 3usize; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
52-
+ // ty::Const
53-
+ // + ty: usize
54-
+ // + val: Value(Scalar(0x00000003))
55-
+ // mir::Constant
56-
+ // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
57-
+ // + literal: Const { ty: usize, val: Value(Scalar(0x00000003)) }
58-
+ _8 = const false; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
59-
+ // ty::Const
60-
+ // + ty: bool
61-
+ // + val: Value(Scalar(0x00))
62-
+ // mir::Constant
63-
+ // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
64-
+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
49+
_7 = Len((*_1)); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
50+
_8 = Lt(_6, _7); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
6551
assert(move _8, "index out of bounds: the len is {} but the index is {}", move _7, _6) -> bb1; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
6652
}
6753

‎src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/64bit/rustc.main.ConstProp.diff

+2-16
Original file line numberDiff line numberDiff line change
@@ -46,22 +46,8 @@
4646
// mir::Constant
4747
// + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:23: 7:24
4848
// + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000003)) }
49-
- _7 = Len((*_1)); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
50-
- _8 = Lt(_6, _7); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
51-
+ _7 = const 3usize; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
52-
+ // ty::Const
53-
+ // + ty: usize
54-
+ // + val: Value(Scalar(0x0000000000000003))
55-
+ // mir::Constant
56-
+ // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
57-
+ // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000003)) }
58-
+ _8 = const false; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
59-
+ // ty::Const
60-
+ // + ty: bool
61-
+ // + val: Value(Scalar(0x00))
62-
+ // mir::Constant
63-
+ // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
64-
+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) }
49+
_7 = Len((*_1)); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
50+
_8 = Lt(_6, _7); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
6551
assert(move _8, "index out of bounds: the len is {} but the index is {}", move _7, _6) -> bb1; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
6652
}
6753

‎src/test/mir-opt/const_prop_miscompile.rs

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#![feature(raw_ref_op)]
2+
3+
// EMIT_MIR rustc.foo.ConstProp.diff
4+
fn foo() {
5+
let mut u = (1,);
6+
*&mut u.0 = 5;
7+
let y = { u.0 } == 5;
8+
}
9+
10+
// EMIT_MIR rustc.bar.ConstProp.diff
11+
fn bar() {
12+
let mut v = (1,);
13+
unsafe {
14+
*&raw mut v.0 = 5;
15+
}
16+
let y = { v.0 } == 5;
17+
}
18+
19+
fn main() {
20+
foo();
21+
bar();
22+
}

‎src/test/mir-opt/const_prop_miscompile/rustc.bar.ConstProp.diff

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
- // MIR for `bar` before ConstProp
2+
+ // MIR for `bar` after ConstProp
3+
4+
fn bar() -> () {
5+
let mut _0: (); // return place in scope 0 at $DIR/const_prop_miscompile.rs:11:10: 11:10
6+
let mut _1: (i32,); // in scope 0 at $DIR/const_prop_miscompile.rs:12:9: 12:14
7+
let _2: (); // in scope 0 at $DIR/const_prop_miscompile.rs:13:5: 15:6
8+
let mut _3: *mut i32; // in scope 0 at $DIR/const_prop_miscompile.rs:14:10: 14:22
9+
let mut _5: i32; // in scope 0 at $DIR/const_prop_miscompile.rs:16:13: 16:20
10+
scope 1 {
11+
debug v => _1; // in scope 1 at $DIR/const_prop_miscompile.rs:12:9: 12:14
12+
let _4: bool; // in scope 1 at $DIR/const_prop_miscompile.rs:16:9: 16:10
13+
scope 2 {
14+
}
15+
scope 3 {
16+
debug y => _4; // in scope 3 at $DIR/const_prop_miscompile.rs:16:9: 16:10
17+
}
18+
}
19+
20+
bb0: {
21+
StorageLive(_1); // scope 0 at $DIR/const_prop_miscompile.rs:12:9: 12:14
22+
- _1 = (const 1i32,); // scope 0 at $DIR/const_prop_miscompile.rs:12:17: 12:21
23+
+ _1 = const (1i32,); // scope 0 at $DIR/const_prop_miscompile.rs:12:17: 12:21
24+
// ty::Const
25+
- // + ty: i32
26+
+ // + ty: (i32,)
27+
// + val: Value(Scalar(0x00000001))
28+
// mir::Constant
29+
- // + span: $DIR/const_prop_miscompile.rs:12:18: 12:19
30+
- // + literal: Const { ty: i32, val: Value(Scalar(0x00000001)) }
31+
+ // + span: $DIR/const_prop_miscompile.rs:12:17: 12:21
32+
+ // + literal: Const { ty: (i32,), val: Value(Scalar(0x00000001)) }
33+
StorageLive(_2); // scope 1 at $DIR/const_prop_miscompile.rs:13:5: 15:6
34+
StorageLive(_3); // scope 2 at $DIR/const_prop_miscompile.rs:14:10: 14:22
35+
_3 = &raw mut (_1.0: i32); // scope 2 at $DIR/const_prop_miscompile.rs:14:10: 14:22
36+
(*_3) = const 5i32; // scope 2 at $DIR/const_prop_miscompile.rs:14:9: 14:26
37+
// ty::Const
38+
// + ty: i32
39+
// + val: Value(Scalar(0x00000005))
40+
// mir::Constant
41+
// + span: $DIR/const_prop_miscompile.rs:14:25: 14:26
42+
// + literal: Const { ty: i32, val: Value(Scalar(0x00000005)) }
43+
StorageDead(_3); // scope 2 at $DIR/const_prop_miscompile.rs:14:26: 14:27
44+
_2 = const (); // scope 2 at $DIR/const_prop_miscompile.rs:13:5: 15:6
45+
// ty::Const
46+
// + ty: ()
47+
// + val: Value(Scalar(<ZST>))
48+
// mir::Constant
49+
// + span: $DIR/const_prop_miscompile.rs:13:5: 15:6
50+
// + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
51+
StorageDead(_2); // scope 1 at $DIR/const_prop_miscompile.rs:15:5: 15:6
52+
StorageLive(_4); // scope 1 at $DIR/const_prop_miscompile.rs:16:9: 16:10
53+
StorageLive(_5); // scope 1 at $DIR/const_prop_miscompile.rs:16:13: 16:20
54+
_5 = (_1.0: i32); // scope 1 at $DIR/const_prop_miscompile.rs:16:15: 16:18
55+
_4 = Eq(move _5, const 5i32); // scope 1 at $DIR/const_prop_miscompile.rs:16:13: 16:25
56+
// ty::Const
57+
// + ty: i32
58+
// + val: Value(Scalar(0x00000005))
59+
// mir::Constant
60+
// + span: $DIR/const_prop_miscompile.rs:16:24: 16:25
61+
// + literal: Const { ty: i32, val: Value(Scalar(0x00000005)) }
62+
StorageDead(_5); // scope 1 at $DIR/const_prop_miscompile.rs:16:24: 16:25
63+
_0 = const (); // scope 0 at $DIR/const_prop_miscompile.rs:11:10: 17:2
64+
// ty::Const
65+
// + ty: ()
66+
// + val: Value(Scalar(<ZST>))
67+
// mir::Constant
68+
// + span: $DIR/const_prop_miscompile.rs:11:10: 17:2
69+
// + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
70+
StorageDead(_4); // scope 1 at $DIR/const_prop_miscompile.rs:17:1: 17:2
71+
StorageDead(_1); // scope 0 at $DIR/const_prop_miscompile.rs:17:1: 17:2
72+
return; // scope 0 at $DIR/const_prop_miscompile.rs:17:2: 17:2
73+
}
74+
}
75+

‎src/test/mir-opt/const_prop_miscompile/rustc.foo.ConstProp.diff

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
- // MIR for `foo` before ConstProp
2+
+ // MIR for `foo` after ConstProp
3+
4+
fn foo() -> () {
5+
let mut _0: (); // return place in scope 0 at $DIR/const_prop_miscompile.rs:4:10: 4:10
6+
let mut _1: (i32,); // in scope 0 at $DIR/const_prop_miscompile.rs:5:9: 5:14
7+
let mut _2: &mut i32; // in scope 0 at $DIR/const_prop_miscompile.rs:6:6: 6:14
8+
let mut _4: i32; // in scope 0 at $DIR/const_prop_miscompile.rs:7:13: 7:20
9+
scope 1 {
10+
debug u => _1; // in scope 1 at $DIR/const_prop_miscompile.rs:5:9: 5:14
11+
let _3: bool; // in scope 1 at $DIR/const_prop_miscompile.rs:7:9: 7:10
12+
scope 2 {
13+
debug y => _3; // in scope 2 at $DIR/const_prop_miscompile.rs:7:9: 7:10
14+
}
15+
}
16+
17+
bb0: {
18+
StorageLive(_1); // scope 0 at $DIR/const_prop_miscompile.rs:5:9: 5:14
19+
- _1 = (const 1i32,); // scope 0 at $DIR/const_prop_miscompile.rs:5:17: 5:21
20+
+ _1 = const (1i32,); // scope 0 at $DIR/const_prop_miscompile.rs:5:17: 5:21
21+
// ty::Const
22+
- // + ty: i32
23+
+ // + ty: (i32,)
24+
// + val: Value(Scalar(0x00000001))
25+
// mir::Constant
26+
- // + span: $DIR/const_prop_miscompile.rs:5:18: 5:19
27+
- // + literal: Const { ty: i32, val: Value(Scalar(0x00000001)) }
28+
+ // + span: $DIR/const_prop_miscompile.rs:5:17: 5:21
29+
+ // + literal: Const { ty: (i32,), val: Value(Scalar(0x00000001)) }
30+
StorageLive(_2); // scope 1 at $DIR/const_prop_miscompile.rs:6:6: 6:14
31+
_2 = &mut (_1.0: i32); // scope 1 at $DIR/const_prop_miscompile.rs:6:6: 6:14
32+
(*_2) = const 5i32; // scope 1 at $DIR/const_prop_miscompile.rs:6:5: 6:18
33+
// ty::Const
34+
// + ty: i32
35+
// + val: Value(Scalar(0x00000005))
36+
// mir::Constant
37+
// + span: $DIR/const_prop_miscompile.rs:6:17: 6:18
38+
// + literal: Const { ty: i32, val: Value(Scalar(0x00000005)) }
39+
StorageDead(_2); // scope 1 at $DIR/const_prop_miscompile.rs:6:18: 6:19
40+
StorageLive(_3); // scope 1 at $DIR/const_prop_miscompile.rs:7:9: 7:10
41+
StorageLive(_4); // scope 1 at $DIR/const_prop_miscompile.rs:7:13: 7:20
42+
_4 = (_1.0: i32); // scope 1 at $DIR/const_prop_miscompile.rs:7:15: 7:18
43+
_3 = Eq(move _4, const 5i32); // scope 1 at $DIR/const_prop_miscompile.rs:7:13: 7:25
44+
// ty::Const
45+
// + ty: i32
46+
// + val: Value(Scalar(0x00000005))
47+
// mir::Constant
48+
// + span: $DIR/const_prop_miscompile.rs:7:24: 7:25
49+
// + literal: Const { ty: i32, val: Value(Scalar(0x00000005)) }
50+
StorageDead(_4); // scope 1 at $DIR/const_prop_miscompile.rs:7:24: 7:25
51+
_0 = const (); // scope 0 at $DIR/const_prop_miscompile.rs:4:10: 8:2
52+
// ty::Const
53+
// + ty: ()
54+
// + val: Value(Scalar(<ZST>))
55+
// mir::Constant
56+
// + span: $DIR/const_prop_miscompile.rs:4:10: 8:2
57+
// + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
58+
StorageDead(_3); // scope 1 at $DIR/const_prop_miscompile.rs:8:1: 8:2
59+
StorageDead(_1); // scope 0 at $DIR/const_prop_miscompile.rs:8:1: 8:2
60+
return; // scope 0 at $DIR/const_prop_miscompile.rs:8:2: 8:2
61+
}
62+
}
63+

‎src/test/ui/mir/mir_detects_invalid_ops.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,6 @@ fn mod_by_zero() {
1919
fn oob_error_for_slices() {
2020
let a: *const [_] = &[1, 2, 3];
2121
unsafe {
22-
let _b = (*a)[3]; //~ ERROR this operation will panic at runtime [unconditional_panic]
22+
let _b = (*a)[3];
2323
}
2424
}

‎src/test/ui/mir/mir_detects_invalid_ops.stderr

+1-7
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,5 @@ error: this operation will panic at runtime
1212
LL | let _z = 1 % y;
1313
| ^^^^^ attempt to calculate the remainder with a divisor of zero
1414

15-
error: this operation will panic at runtime
16-
--> $DIR/mir_detects_invalid_ops.rs:22:18
17-
|
18-
LL | let _b = (*a)[3];
19-
| ^^^^^^^ index out of bounds: the len is 3 but the index is 3
20-
21-
error: aborting due to 3 previous errors
15+
error: aborting due to 2 previous errors
2216

0 commit comments

Comments
 (0)