Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject borrows of projections in ConstProp. #110954

Merged
merged 2 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -714,13 +714,22 @@ impl CanConstProp {
}
}

impl Visitor<'_> for CanConstProp {
impl<'tcx> Visitor<'tcx> for CanConstProp {
fn visit_place(&mut self, place: &Place<'tcx>, mut context: PlaceContext, loc: Location) {
use rustc_middle::mir::visit::PlaceContext::*;

// Dereferencing just read the addess of `place.local`.
if place.projection.first() == Some(&PlaceElem::Deref) {
context = NonMutatingUse(NonMutatingUseContext::Copy);
}

self.visit_local(place.local, context, loc);
self.visit_projection(place.as_ref(), context, loc);
}

fn visit_local(&mut self, local: Local, context: PlaceContext, _: Location) {
use rustc_middle::mir::visit::PlaceContext::*;
match context {
// Projections are fine, because `&mut foo.x` will be caught by
// `MutatingUseContext::Borrow` elsewhere.
MutatingUse(MutatingUseContext::Projection)
// These are just stores, where the storing is not propagatable, but there may be later
// mutations of the same local via `Store`
| MutatingUse(MutatingUseContext::Call)
Expand Down Expand Up @@ -751,7 +760,6 @@ impl Visitor<'_> for CanConstProp {
NonMutatingUse(NonMutatingUseContext::Copy)
| NonMutatingUse(NonMutatingUseContext::Move)
| NonMutatingUse(NonMutatingUseContext::Inspect)
| NonMutatingUse(NonMutatingUseContext::Projection)
| NonMutatingUse(NonMutatingUseContext::PlaceMention)
| NonUse(_) => {}

Expand All @@ -771,6 +779,8 @@ impl Visitor<'_> for CanConstProp {
trace!("local {:?} can't be propagated because it's used: {:?}", local, context);
self.can_const_prop[local] = ConstPropMode::NoPropagation;
}
MutatingUse(MutatingUseContext::Projection)
| NonMutatingUse(NonMutatingUseContext::Projection) => bug!("visit_place should not pass {context:?} for {local:?}"),
}
}
}
Expand Down
46 changes: 46 additions & 0 deletions tests/mir-opt/const_prop/address_of_pair.fn0.ConstProp.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
- // MIR for `fn0` before ConstProp
+ // MIR for `fn0` after ConstProp

fn fn0() -> bool {
let mut _0: bool; // return place in scope 0 at $DIR/address_of_pair.rs:+0:17: +0:21
let mut _1: !; // in scope 0 at $DIR/address_of_pair.rs:+0:22: +9:2
let mut _2: (i32, bool); // in scope 0 at $DIR/address_of_pair.rs:+1:9: +1:17
let _4: (); // in scope 0 at $DIR/address_of_pair.rs:+4:5: +6:6
let mut _6: bool; // in scope 0 at $DIR/address_of_pair.rs:+7:16: +7:22
scope 1 {
debug pair => _2; // in scope 1 at $DIR/address_of_pair.rs:+1:9: +1:17
let _3: *mut bool; // in scope 1 at $DIR/address_of_pair.rs:+2:9: +2:12
scope 2 {
debug ptr => _3; // in scope 2 at $DIR/address_of_pair.rs:+2:9: +2:12
let _5: bool; // in scope 2 at $DIR/address_of_pair.rs:+7:9: +7:12
scope 3 {
}
scope 4 {
debug ret => _5; // in scope 4 at $DIR/address_of_pair.rs:+7:9: +7:12
}
}
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/address_of_pair.rs:+1:9: +1:17
_2 = (const 1_i32, const false); // scope 0 at $DIR/address_of_pair.rs:+1:20: +1:30
StorageLive(_3); // scope 1 at $DIR/address_of_pair.rs:+2:9: +2:12
_3 = &raw mut (_2.1: bool); // scope 1 at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
_2 = (const 1_i32, const false); // scope 2 at $DIR/address_of_pair.rs:+3:5: +3:22
StorageLive(_4); // scope 2 at $DIR/address_of_pair.rs:+4:5: +6:6
(*_3) = const true; // scope 3 at $DIR/address_of_pair.rs:+5:9: +5:20
_4 = const (); // scope 3 at $DIR/address_of_pair.rs:+4:5: +6:6
StorageDead(_4); // scope 2 at $DIR/address_of_pair.rs:+6:5: +6:6
StorageLive(_5); // scope 2 at $DIR/address_of_pair.rs:+7:9: +7:12
StorageLive(_6); // scope 2 at $DIR/address_of_pair.rs:+7:16: +7:22
_6 = (_2.1: bool); // scope 2 at $DIR/address_of_pair.rs:+7:16: +7:22
_5 = Not(move _6); // scope 2 at $DIR/address_of_pair.rs:+7:15: +7:22
StorageDead(_6); // scope 2 at $DIR/address_of_pair.rs:+7:21: +7:22
_0 = _5; // scope 4 at $DIR/address_of_pair.rs:+8:12: +8:15
StorageDead(_5); // scope 2 at $DIR/address_of_pair.rs:+9:1: +9:2
StorageDead(_3); // scope 1 at $DIR/address_of_pair.rs:+9:1: +9:2
StorageDead(_2); // scope 0 at $DIR/address_of_pair.rs:+9:1: +9:2
return; // scope 0 at $DIR/address_of_pair.rs:+9:2: +9:2
}
}

17 changes: 17 additions & 0 deletions tests/mir-opt/const_prop/address_of_pair.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// unit-test: ConstProp

// EMIT_MIR address_of_pair.fn0.ConstProp.diff
pub fn fn0() -> bool {
let mut pair = (1, false);
let ptr = core::ptr::addr_of_mut!(pair.1);
pair = (1, false);
unsafe {
*ptr = true;
}
let ret = !pair.1;
return ret;
}

pub fn main() {
println!("{}", fn0());
}
3 changes: 1 addition & 2 deletions tests/mir-opt/const_prop_miscompile.bar.ConstProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@

bb0: {
StorageLive(_1); // scope 0 at $DIR/const_prop_miscompile.rs:+1:9: +1:14
- _1 = (const 1_i32,); // scope 0 at $DIR/const_prop_miscompile.rs:+1:17: +1:21
+ _1 = const (1_i32,); // scope 0 at $DIR/const_prop_miscompile.rs:+1:17: +1:21
_1 = (const 1_i32,); // scope 0 at $DIR/const_prop_miscompile.rs:+1:17: +1:21
StorageLive(_2); // scope 1 at $DIR/const_prop_miscompile.rs:+2:5: +4:6
StorageLive(_3); // scope 2 at $DIR/const_prop_miscompile.rs:+3:10: +3:22
_3 = &raw mut (_1.0: i32); // scope 2 at $DIR/const_prop_miscompile.rs:+3:10: +3:22
Expand Down
3 changes: 1 addition & 2 deletions tests/mir-opt/const_prop_miscompile.foo.ConstProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@

bb0: {
StorageLive(_1); // scope 0 at $DIR/const_prop_miscompile.rs:+1:9: +1:14
- _1 = (const 1_i32,); // scope 0 at $DIR/const_prop_miscompile.rs:+1:17: +1:21
+ _1 = const (1_i32,); // scope 0 at $DIR/const_prop_miscompile.rs:+1:17: +1:21
_1 = (const 1_i32,); // scope 0 at $DIR/const_prop_miscompile.rs:+1:17: +1:21
StorageLive(_2); // scope 1 at $DIR/const_prop_miscompile.rs:+2:6: +2:14
_2 = &mut (_1.0: i32); // scope 1 at $DIR/const_prop_miscompile.rs:+2:6: +2:14
(*_2) = const 5_i32; // scope 1 at $DIR/const_prop_miscompile.rs:+2:5: +2:18
Expand Down