Skip to content

Commit ab18da6

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #111441 - cjgillot:issue-111422, r=JakobDegen
Verify copies of mutable pointers in 2 stages in ReferencePropagation Fixes #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``````
2 parents d075b6c + 9fb1c73 commit ab18da6

File tree

5 files changed

+230
-34
lines changed

5 files changed

+230
-34
lines changed

‎compiler/rustc_mir_transform/src/ref_prop.rs‎

Lines changed: 60 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
8585
let ssa = SsaLocals::new(body);
8686

8787
let mut replacer = compute_replacement(tcx, body, &ssa);
88-
debug!(?replacer.targets, ?replacer.allowed_replacements, ?replacer.storage_to_remove);
88+
debug!(?replacer.targets);
89+
debug!(?replacer.allowed_replacements);
90+
debug!(?replacer.storage_to_remove);
8991

9092
replacer.visit_body_preserves_cfg(body);
9193

@@ -190,8 +192,11 @@ fn compute_replacement<'tcx>(
190192
continue;
191193
}
192194

195+
// Whether the current local is subject to the uniqueness rule.
196+
let needs_unique = ty.is_mutable_ptr();
197+
193198
// If this a mutable reference that we cannot fully replace, mark it as unknown.
194-
if ty.is_mutable_ptr() && !fully_replacable_locals.contains(local) {
199+
if needs_unique && !fully_replacable_locals.contains(local) {
195200
debug!("not fully replaceable");
196201
continue;
197202
}
@@ -203,32 +208,33 @@ fn compute_replacement<'tcx>(
203208
// have been visited before.
204209
Rvalue::Use(Operand::Copy(place) | Operand::Move(place))
205210
| Rvalue::CopyForDeref(place) => {
206-
if let Some(rhs) = place.as_local() {
211+
if let Some(rhs) = place.as_local() && ssa.is_ssa(rhs) {
207212
let target = targets[rhs];
208-
if matches!(target, Value::Pointer(..)) {
213+
// Only see through immutable reference and pointers, as we do not know yet if
214+
// mutable references are fully replaced.
215+
if !needs_unique && matches!(target, Value::Pointer(..)) {
209216
targets[local] = target;
210-
} else if ssa.is_ssa(rhs) {
211-
let refmut = body.local_decls[rhs].ty.is_mutable_ptr();
212-
targets[local] = Value::Pointer(tcx.mk_place_deref(rhs.into()), refmut);
217+
} else {
218+
targets[local] = Value::Pointer(tcx.mk_place_deref(rhs.into()), needs_unique);
213219
}
214220
}
215221
}
216222
Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) => {
217223
let mut place = *place;
218224
// Try to see through `place` in order to collapse reborrow chains.
219225
if place.projection.first() == Some(&PlaceElem::Deref)
220-
&& let Value::Pointer(target, refmut) = targets[place.local]
226+
&& let Value::Pointer(target, inner_needs_unique) = targets[place.local]
221227
// Only see through immutable reference and pointers, as we do not know yet if
222228
// mutable references are fully replaced.
223-
&& !refmut
229+
&& !inner_needs_unique
224230
// Only collapse chain if the pointee is definitely live.
225231
&& can_perform_opt(target, location)
226232
{
227233
place = target.project_deeper(&place.projection[1..], tcx);
228234
}
229235
assert_ne!(place.local, local);
230236
if is_constant_place(place) {
231-
targets[local] = Value::Pointer(place, ty.is_mutable_ptr());
237+
targets[local] = Value::Pointer(place, needs_unique);
232238
}
233239
}
234240
// We do not know what to do, so keep as not-a-pointer.
@@ -276,16 +282,35 @@ fn compute_replacement<'tcx>(
276282
return;
277283
}
278284

279-
if let Value::Pointer(target, refmut) = self.targets[place.local]
280-
&& place.projection.first() == Some(&PlaceElem::Deref)
281-
{
282-
let perform_opt = (self.can_perform_opt)(target, loc);
283-
if perform_opt {
284-
self.allowed_replacements.insert((target.local, loc));
285-
} else if refmut {
286-
// This mutable reference is not fully replacable, so drop it.
287-
self.targets[place.local] = Value::Unknown;
285+
if place.projection.first() != Some(&PlaceElem::Deref) {
286+
// This is not a dereference, nothing to do.
287+
return;
288+
}
289+
290+
let mut place = place.as_ref();
291+
loop {
292+
if let Value::Pointer(target, needs_unique) = self.targets[place.local] {
293+
let perform_opt = (self.can_perform_opt)(target, loc);
294+
debug!(?place, ?target, ?needs_unique, ?perform_opt);
295+
296+
// This a reborrow chain, recursively allow the replacement.
297+
//
298+
// This also allows to detect cases where `target.local` is not replacable,
299+
// and mark it as such.
300+
if let &[PlaceElem::Deref] = &target.projection[..] {
301+
assert!(perform_opt);
302+
self.allowed_replacements.insert((target.local, loc));
303+
place.local = target.local;
304+
continue;
305+
} else if perform_opt {
306+
self.allowed_replacements.insert((target.local, loc));
307+
} else if needs_unique {
308+
// This mutable reference is not fully replacable, so drop it.
309+
self.targets[place.local] = Value::Unknown;
310+
}
288311
}
312+
313+
break;
289314
}
290315
}
291316
}
@@ -326,18 +351,23 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'tcx> {
326351
}
327352

328353
fn visit_place(&mut self, place: &mut Place<'tcx>, ctxt: PlaceContext, loc: Location) {
329-
if let Value::Pointer(target, _) = self.targets[place.local]
330-
&& place.projection.first() == Some(&PlaceElem::Deref)
331-
{
332-
let perform_opt = matches!(ctxt, PlaceContext::NonUse(_))
333-
|| self.allowed_replacements.contains(&(target.local, loc));
334-
335-
if perform_opt {
336-
*place = target.project_deeper(&place.projection[1..], self.tcx);
337-
self.any_replacement = true;
354+
if place.projection.first() != Some(&PlaceElem::Deref) {
355+
return;
356+
}
357+
358+
loop {
359+
if let Value::Pointer(target, _) = self.targets[place.local] {
360+
let perform_opt = matches!(ctxt, PlaceContext::NonUse(_))
361+
|| self.allowed_replacements.contains(&(target.local, loc));
362+
363+
if perform_opt {
364+
*place = target.project_deeper(&place.projection[1..], self.tcx);
365+
self.any_replacement = true;
366+
continue;
367+
}
338368
}
339-
} else {
340-
self.super_place(place, ctxt, loc);
369+
370+
break;
341371
}
342372
}
343373

‎tests/mir-opt/reference_prop.mut_raw_then_mut_shr.ReferencePropagation.diff‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
- // MIR for `mut_raw_then_mut_shr` before ReferencePropagation
2+
+ // MIR for `mut_raw_then_mut_shr` after ReferencePropagation
3+
4+
fn mut_raw_then_mut_shr() -> (i32, i32) {
5+
let mut _0: (i32, i32); // return place in scope 0 at $DIR/reference_prop.rs:+0:30: +0:40
6+
let mut _1: i32; // in scope 0 at $DIR/reference_prop.rs:+1:9: +1:14
7+
let mut _4: *mut i32; // in scope 0 at $DIR/reference_prop.rs:+3:16: +3:36
8+
let mut _5: &mut i32; // in scope 0 at $DIR/reference_prop.rs:+3:16: +3:26
9+
let _8: (); // in scope 0 at $DIR/reference_prop.rs:+7:5: +7:26
10+
let mut _9: i32; // in scope 0 at $DIR/reference_prop.rs:+8:6: +8:7
11+
let mut _10: i32; // in scope 0 at $DIR/reference_prop.rs:+8:9: +8:10
12+
scope 1 {
13+
debug x => _1; // in scope 1 at $DIR/reference_prop.rs:+1:9: +1:14
14+
let _2: &mut i32; // in scope 1 at $DIR/reference_prop.rs:+2:9: +2:13
15+
scope 2 {
16+
debug xref => _2; // in scope 2 at $DIR/reference_prop.rs:+2:9: +2:13
17+
let _3: *mut i32; // in scope 2 at $DIR/reference_prop.rs:+3:9: +3:13
18+
scope 3 {
19+
debug xraw => _3; // in scope 3 at $DIR/reference_prop.rs:+3:9: +3:13
20+
let _6: &i32; // in scope 3 at $DIR/reference_prop.rs:+4:9: +4:13
21+
scope 4 {
22+
debug xshr => _6; // in scope 4 at $DIR/reference_prop.rs:+4:9: +4:13
23+
let _7: i32; // in scope 4 at $DIR/reference_prop.rs:+6:9: +6:10
24+
scope 5 {
25+
debug a => _7; // in scope 5 at $DIR/reference_prop.rs:+6:9: +6:10
26+
scope 6 {
27+
}
28+
}
29+
}
30+
}
31+
}
32+
}
33+
34+
bb0: {
35+
StorageLive(_1); // scope 0 at $DIR/reference_prop.rs:+1:9: +1:14
36+
_1 = const 2_i32; // scope 0 at $DIR/reference_prop.rs:+1:17: +1:18
37+
- StorageLive(_2); // scope 1 at $DIR/reference_prop.rs:+2:9: +2:13
38+
_2 = &mut _1; // scope 1 at $DIR/reference_prop.rs:+2:16: +2:22
39+
StorageLive(_3); // scope 2 at $DIR/reference_prop.rs:+3:9: +3:13
40+
- StorageLive(_4); // scope 2 at $DIR/reference_prop.rs:+3:16: +3:36
41+
- StorageLive(_5); // scope 2 at $DIR/reference_prop.rs:+3:16: +3:26
42+
- _5 = &mut (*_2); // scope 2 at $DIR/reference_prop.rs:+3:16: +3:26
43+
- _4 = &raw mut (*_5); // scope 2 at $DIR/reference_prop.rs:+3:16: +3:26
44+
+ _4 = &raw mut _1; // scope 2 at $DIR/reference_prop.rs:+3:16: +3:26
45+
_3 = _4; // scope 2 at $DIR/reference_prop.rs:+3:16: +3:36
46+
- StorageDead(_5); // scope 2 at $DIR/reference_prop.rs:+3:36: +3:37
47+
- StorageDead(_4); // scope 2 at $DIR/reference_prop.rs:+3:36: +3:37
48+
StorageLive(_6); // scope 3 at $DIR/reference_prop.rs:+4:9: +4:13
49+
- _6 = &(*_2); // scope 3 at $DIR/reference_prop.rs:+4:16: +4:22
50+
+ _6 = &_1; // scope 3 at $DIR/reference_prop.rs:+4:16: +4:22
51+
StorageLive(_7); // scope 4 at $DIR/reference_prop.rs:+6:9: +6:10
52+
- _7 = (*_6); // scope 4 at $DIR/reference_prop.rs:+6:13: +6:18
53+
- StorageLive(_8); // scope 5 at $DIR/reference_prop.rs:+7:5: +7:26
54+
- (*_3) = const 4_i32; // scope 6 at $DIR/reference_prop.rs:+7:14: +7:23
55+
- _8 = const (); // scope 6 at $DIR/reference_prop.rs:+7:5: +7:26
56+
- StorageDead(_8); // scope 5 at $DIR/reference_prop.rs:+7:25: +7:26
57+
+ _7 = _1; // scope 4 at $DIR/reference_prop.rs:+6:13: +6:18
58+
+ _1 = const 4_i32; // scope 6 at $DIR/reference_prop.rs:+7:14: +7:23
59+
StorageLive(_9); // scope 5 at $DIR/reference_prop.rs:+8:6: +8:7
60+
_9 = _7; // scope 5 at $DIR/reference_prop.rs:+8:6: +8:7
61+
StorageLive(_10); // scope 5 at $DIR/reference_prop.rs:+8:9: +8:10
62+
_10 = _1; // scope 5 at $DIR/reference_prop.rs:+8:9: +8:10
63+
_0 = (move _9, move _10); // scope 5 at $DIR/reference_prop.rs:+8:5: +8:11
64+
StorageDead(_10); // scope 5 at $DIR/reference_prop.rs:+8:10: +8:11
65+
StorageDead(_9); // scope 5 at $DIR/reference_prop.rs:+8:10: +8:11
66+
StorageDead(_7); // scope 4 at $DIR/reference_prop.rs:+9:1: +9:2
67+
StorageDead(_6); // scope 3 at $DIR/reference_prop.rs:+9:1: +9:2
68+
StorageDead(_3); // scope 2 at $DIR/reference_prop.rs:+9:1: +9:2
69+
- StorageDead(_2); // scope 1 at $DIR/reference_prop.rs:+9:1: +9:2
70+
StorageDead(_1); // scope 0 at $DIR/reference_prop.rs:+9:1: +9:2
71+
return; // scope 0 at $DIR/reference_prop.rs:+9:2: +9:2
72+
}
73+
}
74+

‎tests/mir-opt/reference_prop.read_through_raw.ReferencePropagation.diff‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,14 @@
99
let mut _5: *mut usize; // in scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL
1010

1111
bb0: {
12-
_2 = &mut (*_1); // scope 0 at $DIR/reference_prop.rs:+10:13: +10:25
12+
- _2 = &mut (*_1); // scope 0 at $DIR/reference_prop.rs:+10:13: +10:25
1313
- _3 = &mut (*_2); // scope 0 at $DIR/reference_prop.rs:+11:13: +11:26
1414
- _4 = &raw mut (*_2); // scope 0 at $DIR/reference_prop.rs:+12:13: +12:30
1515
- _5 = &raw mut (*_3); // scope 0 at $DIR/reference_prop.rs:+13:13: +13:30
1616
- _0 = (*_4); // scope 0 at $DIR/reference_prop.rs:+15:13: +15:22
1717
- _0 = (*_5); // scope 0 at $DIR/reference_prop.rs:+16:13: +16:22
18-
+ _3 = &mut (*_1); // scope 0 at $DIR/reference_prop.rs:+11:13: +11:26
19-
+ _0 = (*_2); // scope 0 at $DIR/reference_prop.rs:+15:13: +15:22
20-
+ _0 = (*_3); // scope 0 at $DIR/reference_prop.rs:+16:13: +16:22
18+
+ _0 = (*_1); // scope 0 at $DIR/reference_prop.rs:+15:13: +15:22
19+
+ _0 = (*_1); // scope 0 at $DIR/reference_prop.rs:+16:13: +16:22
2120
return; // scope 0 at $DIR/reference_prop.rs:+17:13: +17:21
2221
}
2322
}

‎tests/mir-opt/reference_prop.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,29 @@ fn maybe_dead(m: bool) {
433433
)
434434
}
435435

436+
fn mut_raw_then_mut_shr() -> (i32, i32) {
437+
let mut x = 2;
438+
let xref = &mut x;
439+
let xraw = &mut *xref as *mut _;
440+
let xshr = &*xref;
441+
// Verify that we completely replace with `x` in both cases.
442+
let a = *xshr;
443+
unsafe { *xraw = 4; }
444+
(a, x)
445+
}
446+
447+
fn unique_with_copies() {
448+
let y = {
449+
let mut a = 0;
450+
let x = &raw mut a;
451+
// `*y` is not replacable below, so we must not replace `*x`.
452+
unsafe { opaque(*x) };
453+
x
454+
};
455+
// But rewriting as `*x` is ok.
456+
unsafe { opaque(*y) };
457+
}
458+
436459
fn main() {
437460
let mut x = 5_usize;
438461
let mut y = 7_usize;
@@ -444,6 +467,8 @@ fn main() {
444467
multiple_storage();
445468
dominate_storage();
446469
maybe_dead(true);
470+
mut_raw_then_mut_shr();
471+
unique_with_copies();
447472
}
448473

449474
// EMIT_MIR reference_prop.reference_propagation.ReferencePropagation.diff
@@ -454,3 +479,5 @@ fn main() {
454479
// EMIT_MIR reference_prop.multiple_storage.ReferencePropagation.diff
455480
// EMIT_MIR reference_prop.dominate_storage.ReferencePropagation.diff
456481
// EMIT_MIR reference_prop.maybe_dead.ReferencePropagation.diff
482+
// EMIT_MIR reference_prop.mut_raw_then_mut_shr.ReferencePropagation.diff
483+
// EMIT_MIR reference_prop.unique_with_copies.ReferencePropagation.diff

‎tests/mir-opt/reference_prop.unique_with_copies.ReferencePropagation.diff‎

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

0 commit comments

Comments
 (0)