Skip to content

Commit

Permalink
Rollup merge of rust-lang#111441 - cjgillot:issue-111422, r=JakobDegen
Browse files Browse the repository at this point in the history 
Verify copies of mutable pointers in 2 stages in ReferencePropagation

Fixes rust-lang#111422

In the first stage, we mark the copies as reborrows, to be checked later.
In the second stage, we walk the reborrow chains to verify that all stages are fully replacable.

The replacement itself mirrors the check, and iterates through the reborrow chain.

r? `````@RalfJung`````
cc `````@JakobDegen`````
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored May 11, 2023
2 parents 2f87268 + 9fb1c73 commit 3aec76e
Show file tree
Hide file tree
Showing 5 changed files with 230 additions and 34 deletions.
90 changes: 60 additions & 30 deletions compiler/rustc_mir_transform/src/ref_prop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let ssa = SsaLocals::new(body);

let mut replacer = compute_replacement(tcx, body, &ssa);
debug!(?replacer.targets, ?replacer.allowed_replacements, ?replacer.storage_to_remove);
debug!(?replacer.targets);
debug!(?replacer.allowed_replacements);
debug!(?replacer.storage_to_remove);

replacer.visit_body_preserves_cfg(body);

Expand Down Expand Up @@ -190,8 +192,11 @@ fn compute_replacement<'tcx>(
continue;
}

// Whether the current local is subject to the uniqueness rule.
let needs_unique = ty.is_mutable_ptr();

// If this a mutable reference that we cannot fully replace, mark it as unknown.
if ty.is_mutable_ptr() && !fully_replacable_locals.contains(local) {
if needs_unique && !fully_replacable_locals.contains(local) {
debug!("not fully replaceable");
continue;
}
Expand All @@ -203,32 +208,33 @@ fn compute_replacement<'tcx>(
// have been visited before.
Rvalue::Use(Operand::Copy(place) | Operand::Move(place))
| Rvalue::CopyForDeref(place) => {
if let Some(rhs) = place.as_local() {
if let Some(rhs) = place.as_local() && ssa.is_ssa(rhs) {
let target = targets[rhs];
if matches!(target, Value::Pointer(..)) {
// Only see through immutable reference and pointers, as we do not know yet if
// mutable references are fully replaced.
if !needs_unique && matches!(target, Value::Pointer(..)) {
targets[local] = target;
} else if ssa.is_ssa(rhs) {
let refmut = body.local_decls[rhs].ty.is_mutable_ptr();
targets[local] = Value::Pointer(tcx.mk_place_deref(rhs.into()), refmut);
} else {
targets[local] = Value::Pointer(tcx.mk_place_deref(rhs.into()), needs_unique);
}
}
}
Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) => {
let mut place = *place;
// Try to see through `place` in order to collapse reborrow chains.
if place.projection.first() == Some(&PlaceElem::Deref)
&& let Value::Pointer(target, refmut) = targets[place.local]
&& let Value::Pointer(target, inner_needs_unique) = targets[place.local]
// Only see through immutable reference and pointers, as we do not know yet if
// mutable references are fully replaced.
&& !refmut
&& !inner_needs_unique
// Only collapse chain if the pointee is definitely live.
&& can_perform_opt(target, location)
{
place = target.project_deeper(&place.projection[1..], tcx);
}
assert_ne!(place.local, local);
if is_constant_place(place) {
targets[local] = Value::Pointer(place, ty.is_mutable_ptr());
targets[local] = Value::Pointer(place, needs_unique);
}
}
// We do not know what to do, so keep as not-a-pointer.
Expand Down Expand Up @@ -276,16 +282,35 @@ fn compute_replacement<'tcx>(
return;
}

if let Value::Pointer(target, refmut) = self.targets[place.local]
&& place.projection.first() == Some(&PlaceElem::Deref)
{
let perform_opt = (self.can_perform_opt)(target, loc);
if perform_opt {
self.allowed_replacements.insert((target.local, loc));
} else if refmut {
// This mutable reference is not fully replacable, so drop it.
self.targets[place.local] = Value::Unknown;
if place.projection.first() != Some(&PlaceElem::Deref) {
// This is not a dereference, nothing to do.
return;
}

let mut place = place.as_ref();
loop {
if let Value::Pointer(target, needs_unique) = self.targets[place.local] {
let perform_opt = (self.can_perform_opt)(target, loc);
debug!(?place, ?target, ?needs_unique, ?perform_opt);

// This a reborrow chain, recursively allow the replacement.
//
// This also allows to detect cases where `target.local` is not replacable,
// and mark it as such.
if let &[PlaceElem::Deref] = &target.projection[..] {
assert!(perform_opt);
self.allowed_replacements.insert((target.local, loc));
place.local = target.local;
continue;
} else if perform_opt {
self.allowed_replacements.insert((target.local, loc));
} else if needs_unique {
// This mutable reference is not fully replacable, so drop it.
self.targets[place.local] = Value::Unknown;
}
}

break;
}
}
}
Expand Down Expand Up @@ -326,18 +351,23 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'tcx> {
}

fn visit_place(&mut self, place: &mut Place<'tcx>, ctxt: PlaceContext, loc: Location) {
if let Value::Pointer(target, _) = self.targets[place.local]
&& place.projection.first() == Some(&PlaceElem::Deref)
{
let perform_opt = matches!(ctxt, PlaceContext::NonUse(_))
|| self.allowed_replacements.contains(&(target.local, loc));

if perform_opt {
*place = target.project_deeper(&place.projection[1..], self.tcx);
self.any_replacement = true;
if place.projection.first() != Some(&PlaceElem::Deref) {
return;
}

loop {
if let Value::Pointer(target, _) = self.targets[place.local] {
let perform_opt = matches!(ctxt, PlaceContext::NonUse(_))
|| self.allowed_replacements.contains(&(target.local, loc));

if perform_opt {
*place = target.project_deeper(&place.projection[1..], self.tcx);
self.any_replacement = true;
continue;
}
}
} else {
self.super_place(place, ctxt, loc);

break;
}
}

Expand Down
74 changes: 74 additions & 0 deletions tests/mir-opt/reference_prop.mut_raw_then_mut_shr.ReferencePropagation.diff
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
- // MIR for `mut_raw_then_mut_shr` before ReferencePropagation
+ // MIR for `mut_raw_then_mut_shr` after ReferencePropagation

fn mut_raw_then_mut_shr() -> (i32, i32) {
let mut _0: (i32, i32); // return place in scope 0 at $DIR/reference_prop.rs:+0:30: +0:40
let mut _1: i32; // in scope 0 at $DIR/reference_prop.rs:+1:9: +1:14
let mut _4: *mut i32; // in scope 0 at $DIR/reference_prop.rs:+3:16: +3:36
let mut _5: &mut i32; // in scope 0 at $DIR/reference_prop.rs:+3:16: +3:26
let _8: (); // in scope 0 at $DIR/reference_prop.rs:+7:5: +7:26
let mut _9: i32; // in scope 0 at $DIR/reference_prop.rs:+8:6: +8:7
let mut _10: i32; // in scope 0 at $DIR/reference_prop.rs:+8:9: +8:10
scope 1 {
debug x => _1; // in scope 1 at $DIR/reference_prop.rs:+1:9: +1:14
let _2: &mut i32; // in scope 1 at $DIR/reference_prop.rs:+2:9: +2:13
scope 2 {
debug xref => _2; // in scope 2 at $DIR/reference_prop.rs:+2:9: +2:13
let _3: *mut i32; // in scope 2 at $DIR/reference_prop.rs:+3:9: +3:13
scope 3 {
debug xraw => _3; // in scope 3 at $DIR/reference_prop.rs:+3:9: +3:13
let _6: &i32; // in scope 3 at $DIR/reference_prop.rs:+4:9: +4:13
scope 4 {
debug xshr => _6; // in scope 4 at $DIR/reference_prop.rs:+4:9: +4:13
let _7: i32; // in scope 4 at $DIR/reference_prop.rs:+6:9: +6:10
scope 5 {
debug a => _7; // in scope 5 at $DIR/reference_prop.rs:+6:9: +6:10
scope 6 {
}
}
}
}
}
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/reference_prop.rs:+1:9: +1:14
_1 = const 2_i32; // scope 0 at $DIR/reference_prop.rs:+1:17: +1:18
- StorageLive(_2); // scope 1 at $DIR/reference_prop.rs:+2:9: +2:13
_2 = &mut _1; // scope 1 at $DIR/reference_prop.rs:+2:16: +2:22
StorageLive(_3); // scope 2 at $DIR/reference_prop.rs:+3:9: +3:13
- StorageLive(_4); // scope 2 at $DIR/reference_prop.rs:+3:16: +3:36
- StorageLive(_5); // scope 2 at $DIR/reference_prop.rs:+3:16: +3:26
- _5 = &mut (*_2); // scope 2 at $DIR/reference_prop.rs:+3:16: +3:26
- _4 = &raw mut (*_5); // scope 2 at $DIR/reference_prop.rs:+3:16: +3:26
+ _4 = &raw mut _1; // scope 2 at $DIR/reference_prop.rs:+3:16: +3:26
_3 = _4; // scope 2 at $DIR/reference_prop.rs:+3:16: +3:36
- StorageDead(_5); // scope 2 at $DIR/reference_prop.rs:+3:36: +3:37
- StorageDead(_4); // scope 2 at $DIR/reference_prop.rs:+3:36: +3:37
StorageLive(_6); // scope 3 at $DIR/reference_prop.rs:+4:9: +4:13
- _6 = &(*_2); // scope 3 at $DIR/reference_prop.rs:+4:16: +4:22
+ _6 = &_1; // scope 3 at $DIR/reference_prop.rs:+4:16: +4:22
StorageLive(_7); // scope 4 at $DIR/reference_prop.rs:+6:9: +6:10
- _7 = (*_6); // scope 4 at $DIR/reference_prop.rs:+6:13: +6:18
- StorageLive(_8); // scope 5 at $DIR/reference_prop.rs:+7:5: +7:26
- (*_3) = const 4_i32; // scope 6 at $DIR/reference_prop.rs:+7:14: +7:23
- _8 = const (); // scope 6 at $DIR/reference_prop.rs:+7:5: +7:26
- StorageDead(_8); // scope 5 at $DIR/reference_prop.rs:+7:25: +7:26
+ _7 = _1; // scope 4 at $DIR/reference_prop.rs:+6:13: +6:18
+ _1 = const 4_i32; // scope 6 at $DIR/reference_prop.rs:+7:14: +7:23
StorageLive(_9); // scope 5 at $DIR/reference_prop.rs:+8:6: +8:7
_9 = _7; // scope 5 at $DIR/reference_prop.rs:+8:6: +8:7
StorageLive(_10); // scope 5 at $DIR/reference_prop.rs:+8:9: +8:10
_10 = _1; // scope 5 at $DIR/reference_prop.rs:+8:9: +8:10
_0 = (move _9, move _10); // scope 5 at $DIR/reference_prop.rs:+8:5: +8:11
StorageDead(_10); // scope 5 at $DIR/reference_prop.rs:+8:10: +8:11
StorageDead(_9); // scope 5 at $DIR/reference_prop.rs:+8:10: +8:11
StorageDead(_7); // scope 4 at $DIR/reference_prop.rs:+9:1: +9:2
StorageDead(_6); // scope 3 at $DIR/reference_prop.rs:+9:1: +9:2
StorageDead(_3); // scope 2 at $DIR/reference_prop.rs:+9:1: +9:2
- StorageDead(_2); // scope 1 at $DIR/reference_prop.rs:+9:1: +9:2
StorageDead(_1); // scope 0 at $DIR/reference_prop.rs:+9:1: +9:2
return; // scope 0 at $DIR/reference_prop.rs:+9:2: +9:2
}
}

7 changes: 3 additions & 4 deletions tests/mir-opt/reference_prop.read_through_raw.ReferencePropagation.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@
let mut _5: *mut usize; // in scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL

bb0: {
_2 = &mut (*_1); // scope 0 at $DIR/reference_prop.rs:+10:13: +10:25
- _2 = &mut (*_1); // scope 0 at $DIR/reference_prop.rs:+10:13: +10:25
- _3 = &mut (*_2); // scope 0 at $DIR/reference_prop.rs:+11:13: +11:26
- _4 = &raw mut (*_2); // scope 0 at $DIR/reference_prop.rs:+12:13: +12:30
- _5 = &raw mut (*_3); // scope 0 at $DIR/reference_prop.rs:+13:13: +13:30
- _0 = (*_4); // scope 0 at $DIR/reference_prop.rs:+15:13: +15:22
- _0 = (*_5); // scope 0 at $DIR/reference_prop.rs:+16:13: +16:22
+ _3 = &mut (*_1); // scope 0 at $DIR/reference_prop.rs:+11:13: +11:26
+ _0 = (*_2); // scope 0 at $DIR/reference_prop.rs:+15:13: +15:22
+ _0 = (*_3); // scope 0 at $DIR/reference_prop.rs:+16:13: +16:22
+ _0 = (*_1); // scope 0 at $DIR/reference_prop.rs:+15:13: +15:22
+ _0 = (*_1); // scope 0 at $DIR/reference_prop.rs:+16:13: +16:22
return; // scope 0 at $DIR/reference_prop.rs:+17:13: +17:21
}
}
Expand Down
27 changes: 27 additions & 0 deletions tests/mir-opt/reference_prop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,29 @@ fn maybe_dead(m: bool) {
)
}

fn mut_raw_then_mut_shr() -> (i32, i32) {
let mut x = 2;
let xref = &mut x;
let xraw = &mut *xref as *mut _;
let xshr = &*xref;
// Verify that we completely replace with `x` in both cases.
let a = *xshr;
unsafe { *xraw = 4; }
(a, x)
}

fn unique_with_copies() {
let y = {
let mut a = 0;
let x = &raw mut a;
// `*y` is not replacable below, so we must not replace `*x`.
unsafe { opaque(*x) };
x
};
// But rewriting as `*x` is ok.
unsafe { opaque(*y) };
}

fn main() {
let mut x = 5_usize;
let mut y = 7_usize;
Expand All @@ -444,6 +467,8 @@ fn main() {
multiple_storage();
dominate_storage();
maybe_dead(true);
mut_raw_then_mut_shr();
unique_with_copies();
}

// EMIT_MIR reference_prop.reference_propagation.ReferencePropagation.diff
Expand All @@ -454,3 +479,5 @@ fn main() {
// EMIT_MIR reference_prop.multiple_storage.ReferencePropagation.diff
// EMIT_MIR reference_prop.dominate_storage.ReferencePropagation.diff
// EMIT_MIR reference_prop.maybe_dead.ReferencePropagation.diff
// EMIT_MIR reference_prop.mut_raw_then_mut_shr.ReferencePropagation.diff
// EMIT_MIR reference_prop.unique_with_copies.ReferencePropagation.diff
66 changes: 66 additions & 0 deletions tests/mir-opt/reference_prop.unique_with_copies.ReferencePropagation.diff
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
- // MIR for `unique_with_copies` before ReferencePropagation
+ // MIR for `unique_with_copies` after ReferencePropagation

fn unique_with_copies() -> () {
let mut _0: (); // return place in scope 0 at $DIR/reference_prop.rs:+0:25: +0:25
let _1: *mut i32; // in scope 0 at $DIR/reference_prop.rs:+1:9: +1:10
let mut _2: i32; // in scope 0 at $DIR/reference_prop.rs:+2:13: +2:18
let _4: (); // in scope 0 at $DIR/reference_prop.rs:+5:18: +5:28
let mut _5: i32; // in scope 0 at $DIR/reference_prop.rs:+5:25: +5:27
let _6: (); // in scope 0 at $DIR/reference_prop.rs:+9:14: +9:24
let mut _7: i32; // in scope 0 at $DIR/reference_prop.rs:+9:21: +9:23
scope 1 {
debug y => _1; // in scope 1 at $DIR/reference_prop.rs:+1:9: +1:10
scope 5 {
}
}
scope 2 {
debug a => _2; // in scope 2 at $DIR/reference_prop.rs:+2:13: +2:18
let _3: *mut i32; // in scope 2 at $DIR/reference_prop.rs:+3:13: +3:14
scope 3 {
debug x => _3; // in scope 3 at $DIR/reference_prop.rs:+3:13: +3:14
scope 4 {
}
}
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/reference_prop.rs:+1:9: +1:10
StorageLive(_2); // scope 0 at $DIR/reference_prop.rs:+2:13: +2:18
_2 = const 0_i32; // scope 0 at $DIR/reference_prop.rs:+2:21: +2:22
- StorageLive(_3); // scope 2 at $DIR/reference_prop.rs:+3:13: +3:14
_3 = &raw mut _2; // scope 2 at $DIR/reference_prop.rs:+3:17: +3:27
StorageLive(_4); // scope 3 at $DIR/reference_prop.rs:+5:9: +5:30
StorageLive(_5); // scope 4 at $DIR/reference_prop.rs:+5:25: +5:27
_5 = (*_3); // scope 4 at $DIR/reference_prop.rs:+5:25: +5:27
_4 = opaque::<i32>(move _5) -> bb1; // scope 4 at $DIR/reference_prop.rs:+5:18: +5:28
// mir::Constant
// + span: $DIR/reference_prop.rs:452:18: 452:24
// + literal: Const { ty: fn(i32) {opaque::<i32>}, val: Value(<ZST>) }
}

bb1: {
StorageDead(_5); // scope 4 at $DIR/reference_prop.rs:+5:27: +5:28
StorageDead(_4); // scope 3 at $DIR/reference_prop.rs:+5:30: +5:31
_1 = _3; // scope 3 at $DIR/reference_prop.rs:+6:9: +6:10
- StorageDead(_3); // scope 2 at $DIR/reference_prop.rs:+7:5: +7:6
StorageDead(_2); // scope 0 at $DIR/reference_prop.rs:+7:5: +7:6
StorageLive(_6); // scope 1 at $DIR/reference_prop.rs:+9:5: +9:26
StorageLive(_7); // scope 5 at $DIR/reference_prop.rs:+9:21: +9:23
- _7 = (*_1); // scope 5 at $DIR/reference_prop.rs:+9:21: +9:23
+ _7 = (*_3); // scope 5 at $DIR/reference_prop.rs:+9:21: +9:23
_6 = opaque::<i32>(move _7) -> bb2; // scope 5 at $DIR/reference_prop.rs:+9:14: +9:24
// mir::Constant
// + span: $DIR/reference_prop.rs:456:14: 456:20
// + literal: Const { ty: fn(i32) {opaque::<i32>}, val: Value(<ZST>) }
}

bb2: {
StorageDead(_7); // scope 5 at $DIR/reference_prop.rs:+9:23: +9:24
StorageDead(_6); // scope 1 at $DIR/reference_prop.rs:+9:26: +9:27
_0 = const (); // scope 0 at $DIR/reference_prop.rs:+0:25: +10:2
StorageDead(_1); // scope 0 at $DIR/reference_prop.rs:+10:1: +10:2
return; // scope 0 at $DIR/reference_prop.rs:+10:2: +10:2
}
}

0 comments on commit 3aec76e

Please sign in to comment.