Skip to content

Commit 920f4b7

Browse files
borsbors
committed
Auto merge of #142185 - saethlin:refprop-moves, r=<try>
Convert moves of references to copies in ReferencePropagation This is a fix for #141101. The root cause of this miscompile is that the SsaLocals analysis that MIR transforms use is supposed to detect locals that are only written to once, in their single assignment. But that analysis is subtly wrong; it does not consider `Operand::Move` to be a write even though the meaning ascribed to `Operand::Move` (at least as a function parameter) by Miri is that the callee may have done arbitrary writes to the operand in the caller (because `Move` is pass-by-pointer). So Miri conwiders `Operand::Move` to be a write but both the MIR visitor system considers it a read, and so does SsaLocals. I have tried fixing this by changing the `PlaceContext` that is ascribed to an `Operand::Move` to a `MutatingUseContext` but that seems to have borrow checker implications, and changing SsaLocals seems to have wide-ranging regressions in MIR optimizations. So instead of doing those, this PR adds a new kludge to ReferencePropagation, which follows the same line of thinking as the kludge in CopyProp that solves this same problem inside that pass: https://github.com/rust-lang/rust/blob/a5584a8fe16037dc01782064fa41424a6dbe9987/compiler/rustc_mir_transform/src/copy_prop.rs#L65-L98
2 parents a5584a8 + e41bd6c commit 920f4b7

11 files changed

+104
-47
lines changed

‎compiler/rustc_mir_transform/src/ref_prop.rs

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ impl<'tcx> crate::MirPass<'tcx> for ReferencePropagation {
7979
#[instrument(level = "trace", skip(self, tcx, body))]
8080
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
8181
debug!(def_id = ?body.source.def_id());
82+
move_to_copy_pointers(tcx, body);
8283
while propagate_ssa(tcx, body) {}
8384
}
8485

@@ -87,11 +88,38 @@ impl<'tcx> crate::MirPass<'tcx> for ReferencePropagation {
8788
}
8889
}
8990

91+
fn move_to_copy_pointers<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
92+
let mut visitor = MoveToCopyVisitor { tcx, local_decls: &body.local_decls };
93+
for (bb, data) in body.basic_blocks.as_mut_preserves_cfg().iter_enumerated_mut() {
94+
visitor.visit_basic_block_data(bb, data);
95+
}
96+
97+
struct MoveToCopyVisitor<'a, 'tcx> {
98+
tcx: TyCtxt<'tcx>,
99+
local_decls: &'a IndexVec<Local, LocalDecl<'tcx>>,
100+
}
101+
102+
impl<'a, 'tcx> MutVisitor<'tcx> for MoveToCopyVisitor<'a, 'tcx> {
103+
fn tcx(&self) -> TyCtxt<'tcx> {
104+
self.tcx
105+
}
106+
107+
fn visit_operand(&mut self, operand: &mut Operand<'tcx>, loc: Location) {
108+
if let Operand::Move(place) = *operand {
109+
if place.ty(self.local_decls, self.tcx).ty.is_any_ptr() {
110+
*operand = Operand::Copy(place);
111+
}
112+
}
113+
self.super_operand(operand, loc);
114+
}
115+
}
116+
}
117+
90118
fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
91119
let typing_env = body.typing_env(tcx);
92120
let ssa = SsaLocals::new(tcx, body, typing_env);
93121

94-
let mut replacer = compute_replacement(tcx, body, &ssa);
122+
let mut replacer = compute_replacement(tcx, body, ssa);
95123
debug!(?replacer.targets);
96124
debug!(?replacer.allowed_replacements);
97125
debug!(?replacer.storage_to_remove);
@@ -119,7 +147,7 @@ enum Value<'tcx> {
119147
fn compute_replacement<'tcx>(
120148
tcx: TyCtxt<'tcx>,
121149
body: &Body<'tcx>,
122-
ssa: &SsaLocals,
150+
ssa: SsaLocals,
123151
) -> Replacer<'tcx> {
124152
let always_live_locals = always_storage_live_locals(body);
125153

@@ -138,7 +166,7 @@ fn compute_replacement<'tcx>(
138166
// reborrowed references.
139167
let mut storage_to_remove = DenseBitSet::new_empty(body.local_decls.len());
140168

141-
let fully_replacable_locals = fully_replacable_locals(ssa);
169+
let fully_replacable_locals = fully_replacable_locals(&ssa);
142170

143171
// Returns true iff we can use `place` as a pointee.
144172
//

‎tests/mir-opt/building/index_array_and_slice.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ struct WithSliceTail(f64, [i32]);
5555
fn index_custom(custom: &WithSliceTail, index: usize) -> &i32 {
5656
// CHECK: bb0:
5757
// CHECK: [[PTR:_.+]] = &raw const (fake) ((*_1).1: [i32]);
58-
// CHECK: [[LEN:_.+]] = PtrMetadata(move [[PTR]]);
58+
// CHECK: [[LEN:_.+]] = PtrMetadata(copy [[PTR]]);
5959
// CHECK: [[LT:_.+]] = Lt(copy _2, copy [[LEN]]);
6060
// CHECK: assert(move [[LT]], "index out of bounds{{.+}}", move [[LEN]], copy _2) -> [success: bb1,
6161

‎tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-unwind.diff

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
StorageDead(_6);
4747
_4 = copy _5 as *mut [u8] (Transmute);
4848
StorageDead(_5);
49-
_3 = move _4 as *mut u8 (PtrToPtr);
49+
_3 = copy _4 as *mut u8 (PtrToPtr);
5050
StorageDead(_4);
5151
StorageDead(_3);
5252
- StorageDead(_1);

‎tests/mir-opt/pre-codegen/slice_index.slice_get_mut_usize.PreCodegen.after.panic-unwind.mir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ fn slice_get_mut_usize(_1: &mut [u32], _2: usize) -> Option<&mut u32> {
3030
StorageDead(_3);
3131
StorageLive(_5);
3232
_5 = &mut (*_1)[_2];
33-
_0 = Option::<&mut u32>::Some(move _5);
33+
_0 = Option::<&mut u32>::Some(copy _5);
3434
StorageDead(_5);
3535
goto -> bb3;
3636
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@
9999
_13 = &(*_26);
100100
StorageLive(_15);
101101
_15 = RangeFull;
102-
_12 = <[i32; 10] as Index<RangeFull>>::index(move _13, move _15) -> [return: bb5, unwind continue];
102+
- _12 = <[i32; 10] as Index<RangeFull>>::index(move _13, move _15) -> [return: bb5, unwind continue];
103+
+ _12 = <[i32; 10] as Index<RangeFull>>::index(copy _13, move _15) -> [return: bb5, unwind continue];
103104
}
104105

105106
bb5: {

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,9 @@
218218
- StorageLive(_14);
219219
- _14 = &_11;
220220
- _13 = &(*_14);
221+
- _12 = move _13;
221222
+ _13 = &_11;
222-
_12 = move _13;
223+
+ _12 = copy _13;
223224
StorageDead(_13);
224225
- StorageDead(_14);
225226
StorageLive(_15);
@@ -252,7 +253,8 @@
252253
StorageLive(_23);
253254
StorageLive(_24);
254255
_24 = copy _21;
255-
_23 = opaque::<&&usize>(move _24) -> [return: bb3, unwind continue];
256+
- _23 = opaque::<&&usize>(move _24) -> [return: bb3, unwind continue];
257+
+ _23 = opaque::<&&usize>(copy _24) -> [return: bb3, unwind continue];
256258
}
257259

258260
bb3: {
@@ -276,7 +278,8 @@
276278
StorageLive(_30);
277279
StorageLive(_31);
278280
_31 = copy _28;
279-
_30 = opaque::<*mut &usize>(move _31) -> [return: bb4, unwind continue];
281+
- _30 = opaque::<*mut &usize>(move _31) -> [return: bb4, unwind continue];
282+
+ _30 = opaque::<*mut &usize>(copy _31) -> [return: bb4, unwind continue];
280283
}
281284

282285
bb4: {
@@ -299,7 +302,8 @@
299302
StorageLive(_36);
300303
StorageLive(_37);
301304
_37 = copy _34;
302-
_36 = opaque::<&usize>(move _37) -> [return: bb5, unwind continue];
305+
- _36 = opaque::<&usize>(move _37) -> [return: bb5, unwind continue];
306+
+ _36 = opaque::<&usize>(copy _37) -> [return: bb5, unwind continue];
303307
}
304308

305309
bb5: {
@@ -328,7 +332,8 @@
328332
StorageLive(_45);
329333
StorageLive(_46);
330334
_46 = copy _44;
331-
_45 = opaque::<&usize>(move _46) -> [return: bb6, unwind continue];
335+
- _45 = opaque::<&usize>(move _46) -> [return: bb6, unwind continue];
336+
+ _45 = opaque::<&usize>(copy _46) -> [return: bb6, unwind continue];
332337
}
333338

334339
bb6: {
@@ -368,8 +373,9 @@
368373
- StorageLive(_55);
369374
- _55 = &(*_1);
370375
- _54 = &(*_55);
376+
- _2 = move _54;
371377
+ _54 = &(*_1);
372-
_2 = move _54;
378+
+ _2 = copy _54;
373379
StorageDead(_54);
374380
- StorageDead(_55);
375381
StorageLive(_56);

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,8 @@
233233
_12 = &raw const _10;
234234
StorageLive(_13);
235235
_13 = &raw const _11;
236-
_12 = move _13;
236+
- _12 = move _13;
237+
+ _12 = copy _13;
237238
StorageDead(_13);
238239
StorageLive(_14);
239240
_14 = copy (*_12);
@@ -265,7 +266,8 @@
265266
StorageLive(_22);
266267
StorageLive(_23);
267268
_23 = copy _20;
268-
_22 = opaque::<&*const usize>(move _23) -> [return: bb3, unwind continue];
269+
- _22 = opaque::<&*const usize>(move _23) -> [return: bb3, unwind continue];
270+
+ _22 = opaque::<&*const usize>(copy _23) -> [return: bb3, unwind continue];
269271
}
270272

271273
bb3: {
@@ -289,7 +291,8 @@
289291
StorageLive(_29);
290292
StorageLive(_30);
291293
_30 = copy _27;
292-
_29 = opaque::<*mut *const usize>(move _30) -> [return: bb4, unwind continue];
294+
- _29 = opaque::<*mut *const usize>(move _30) -> [return: bb4, unwind continue];
295+
+ _29 = opaque::<*mut *const usize>(copy _30) -> [return: bb4, unwind continue];
293296
}
294297

295298
bb4: {
@@ -312,7 +315,8 @@
312315
StorageLive(_35);
313316
StorageLive(_36);
314317
_36 = copy _33;
315-
_35 = opaque::<*const usize>(move _36) -> [return: bb5, unwind continue];
318+
- _35 = opaque::<*const usize>(move _36) -> [return: bb5, unwind continue];
319+
+ _35 = opaque::<*const usize>(copy _36) -> [return: bb5, unwind continue];
316320
}
317321

318322
bb5: {
@@ -341,7 +345,8 @@
341345
StorageLive(_44);
342346
StorageLive(_45);
343347
_45 = copy _43;
344-
_44 = opaque::<*const usize>(move _45) -> [return: bb6, unwind continue];
348+
- _44 = opaque::<*const usize>(move _45) -> [return: bb6, unwind continue];
349+
+ _44 = opaque::<*const usize>(copy _45) -> [return: bb6, unwind continue];
345350
}
346351

347352
bb6: {
@@ -379,7 +384,8 @@
379384
_52 = &raw const (*_2);
380385
StorageLive(_53);
381386
_53 = &raw const (*_1);
382-
_2 = move _53;
387+
- _2 = move _53;
388+
+ _2 = copy _53;
383389
StorageDead(_53);
384390
StorageLive(_54);
385391
_54 = copy (*_52);

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

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,9 @@
218218
- StorageLive(_14);
219219
- _14 = &mut _11;
220220
- _13 = &mut (*_14);
221+
- _12 = move _13;
221222
+ _13 = &mut _11;
222-
_12 = move _13;
223+
+ _12 = copy _13;
223224
StorageDead(_13);
224225
- StorageDead(_14);
225226
StorageLive(_15);
@@ -251,7 +252,8 @@
251252
StorageLive(_23);
252253
StorageLive(_24);
253254
_24 = copy _21;
254-
_23 = opaque::<&&mut usize>(move _24) -> [return: bb3, unwind continue];
255+
- _23 = opaque::<&&mut usize>(move _24) -> [return: bb3, unwind continue];
256+
+ _23 = opaque::<&&mut usize>(copy _24) -> [return: bb3, unwind continue];
255257
}
256258

257259
bb3: {
@@ -275,7 +277,8 @@
275277
StorageLive(_30);
276278
StorageLive(_31);
277279
_31 = copy _28;
278-
_30 = opaque::<*mut &mut usize>(move _31) -> [return: bb4, unwind continue];
280+
- _30 = opaque::<*mut &mut usize>(move _31) -> [return: bb4, unwind continue];
281+
+ _30 = opaque::<*mut &mut usize>(copy _31) -> [return: bb4, unwind continue];
279282
}
280283

281284
bb4: {
@@ -296,8 +299,10 @@
296299
_35 = copy (*_34);
297300
StorageLive(_36);
298301
StorageLive(_37);
299-
_37 = move _34;
300-
_36 = opaque::<&mut usize>(move _37) -> [return: bb5, unwind continue];
302+
- _37 = move _34;
303+
- _36 = opaque::<&mut usize>(move _37) -> [return: bb5, unwind continue];
304+
+ _37 = copy _34;
305+
+ _36 = opaque::<&mut usize>(copy _37) -> [return: bb5, unwind continue];
301306
}
302307

303308
bb5: {
@@ -316,15 +321,19 @@
316321
StorageLive(_41);
317322
_41 = copy (*_40);
318323
StorageLive(_42);
319-
_42 = move _40;
324+
- _42 = move _40;
325+
+ _42 = copy _40;
320326
StorageLive(_43);
321327
_43 = copy (*_42);
322328
StorageLive(_44);
323-
_44 = move _42;
329+
- _44 = move _42;
330+
+ _44 = copy _42;
324331
StorageLive(_45);
325332
StorageLive(_46);
326-
_46 = move _44;
327-
_45 = opaque::<&mut usize>(move _46) -> [return: bb6, unwind continue];
333+
- _46 = move _44;
334+
- _45 = opaque::<&mut usize>(move _46) -> [return: bb6, unwind continue];
335+
+ _46 = copy _44;
336+
+ _45 = opaque::<&mut usize>(copy _46) -> [return: bb6, unwind continue];
328337
}
329338

330339
bb6: {
@@ -364,8 +373,9 @@
364373
- StorageLive(_55);
365374
- _55 = &mut (*_1);
366375
- _54 = &mut (*_55);
376+
- _2 = move _54;
367377
+ _54 = &mut (*_1);
368-
_2 = move _54;
378+
+ _2 = copy _54;
369379
StorageDead(_54);
370380
- StorageDead(_55);
371381
StorageLive(_56);

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,8 @@
214214
_12 = &raw mut _10;
215215
StorageLive(_13);
216216
_13 = &raw mut _11;
217-
_12 = move _13;
217+
- _12 = move _13;
218+
+ _12 = copy _13;
218219
StorageDead(_13);
219220
StorageLive(_14);
220221
_14 = copy (*_12);
@@ -245,7 +246,8 @@
245246
StorageLive(_22);
246247
StorageLive(_23);
247248
_23 = copy _20;
248-
_22 = opaque::<&*mut usize>(move _23) -> [return: bb3, unwind continue];
249+
- _22 = opaque::<&*mut usize>(move _23) -> [return: bb3, unwind continue];
250+
+ _22 = opaque::<&*mut usize>(copy _23) -> [return: bb3, unwind continue];
249251
}
250252

251253
bb3: {
@@ -269,7 +271,8 @@
269271
StorageLive(_29);
270272
StorageLive(_30);
271273
_30 = copy _27;
272-
_29 = opaque::<*mut *mut usize>(move _30) -> [return: bb4, unwind continue];
274+
- _29 = opaque::<*mut *mut usize>(move _30) -> [return: bb4, unwind continue];
275+
+ _29 = opaque::<*mut *mut usize>(copy _30) -> [return: bb4, unwind continue];
273276
}
274277

275278
bb4: {
@@ -291,7 +294,8 @@
291294
StorageLive(_35);
292295
StorageLive(_36);
293296
_36 = copy _33;
294-
_35 = opaque::<*mut usize>(move _36) -> [return: bb5, unwind continue];
297+
- _35 = opaque::<*mut usize>(move _36) -> [return: bb5, unwind continue];
298+
+ _35 = opaque::<*mut usize>(copy _36) -> [return: bb5, unwind continue];
295299
}
296300

297301
bb5: {
@@ -318,7 +322,8 @@
318322
StorageLive(_44);
319323
StorageLive(_45);
320324
_45 = copy _43;
321-
_44 = opaque::<*mut usize>(move _45) -> [return: bb6, unwind continue];
325+
- _44 = opaque::<*mut usize>(move _45) -> [return: bb6, unwind continue];
326+
+ _44 = opaque::<*mut usize>(copy _45) -> [return: bb6, unwind continue];
322327
}
323328

324329
bb6: {
@@ -356,7 +361,8 @@
356361
_52 = &raw mut (*_2);
357362
StorageLive(_53);
358363
_53 = &raw mut (*_1);
359-
_2 = move _53;
364+
- _2 = move _53;
365+
+ _2 = copy _53;
360366
StorageDead(_53);
361367
StorageLive(_54);
362368
_54 = copy (*_52);

0 commit comments

Comments
 (0)