Auto merge of #72093 - jonas-schievink:unmut, r=oli-obk

Avoid `Operand::Copy` with `&mut T`

This is generally unsound to do, as the copied type is assumed to implement
`Copy`.

Closes https://github.com/rust-lang/rust/issues/46420

src/librustc_interface/tests.rs

@@ -511,6 +511,7 @@ fn test_debugging_options_tracking_hash() {
     untracked!(ui_testing, true);
     untracked!(unpretty, Some("expanded".to_string()));
     untracked!(unstable_options, true);
+    untracked!(validate_mir, true);
     untracked!(verbose, true);
 
     macro_rules! tracked {

src/librustc_mir/shim.rs

@@ -700,7 +700,7 @@ fn build_call_shim<'tcx>(
 
     let rcvr = rcvr_adjustment.map(|rcvr_adjustment| match rcvr_adjustment {
         Adjustment::Identity => Operand::Move(rcvr_place()),
-        Adjustment::Deref => Operand::Copy(tcx.mk_place_deref(rcvr_place())),
+        Adjustment::Deref => Operand::Move(tcx.mk_place_deref(rcvr_place())), // Can't copy `&mut`
         Adjustment::DerefMove => Operand::Move(tcx.mk_place_deref(rcvr_place())),
         Adjustment::RefMut => {
             // let rcvr = &mut rcvr;

src/librustc_mir/transform/instcombine.rs

@@ -1,11 +1,11 @@
 //! Performs various peephole optimizations.
 
 use crate::transform::{MirPass, MirSource};
-use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_data_structures::fx::FxHashMap;
 use rustc_index::vec::Idx;
 use rustc_middle::mir::visit::{MutVisitor, Visitor};
 use rustc_middle::mir::{
-    Body, Constant, Local, Location, Operand, Place, PlaceRef, ProjectionElem, Rvalue,
+    Body, Constant, Local, Location, Mutability, Operand, Place, PlaceRef, ProjectionElem, Rvalue,
 };
 use rustc_middle::ty::{self, TyCtxt};
 use std::mem;
@@ -39,7 +39,7 @@ impl<'tcx> MutVisitor<'tcx> for InstCombineVisitor<'tcx> {
     }
 
     fn visit_rvalue(&mut self, rvalue: &mut Rvalue<'tcx>, location: Location) {
-        if self.optimizations.and_stars.remove(&location) {
+        if let Some(mtbl) = self.optimizations.and_stars.remove(&location) {
             debug!("replacing `&*`: {:?}", rvalue);
             let new_place = match rvalue {
                 Rvalue::Ref(_, _, place) => {
@@ -57,7 +57,10 @@ impl<'tcx> MutVisitor<'tcx> for InstCombineVisitor<'tcx> {
                 }
                 _ => bug!("Detected `&*` but didn't find `&*`!"),
             };
-            *rvalue = Rvalue::Use(Operand::Copy(new_place))
+            *rvalue = Rvalue::Use(match mtbl {
+                Mutability::Mut => Operand::Move(new_place),
+                Mutability::Not => Operand::Copy(new_place),
+            });
         }
 
         if let Some(constant) = self.optimizations.arrays_lengths.remove(&location) {
@@ -88,8 +91,10 @@ impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> {
             if let PlaceRef { local, projection: &[ref proj_base @ .., ProjectionElem::Deref] } =
                 place.as_ref()
             {
-                if Place::ty_from(local, proj_base, self.body, self.tcx).ty.is_region_ptr() {
-                    self.optimizations.and_stars.insert(location);
+                // The dereferenced place must have type `&_`.
+                let ty = Place::ty_from(local, proj_base, self.body, self.tcx).ty;
+                if let ty::Ref(_, _, mtbl) = ty.kind {
+                    self.optimizations.and_stars.insert(location, mtbl);
                 }
             }
         }
@@ -109,6 +114,6 @@ impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> {
 
 #[derive(Default)]
 struct OptimizationList<'tcx> {
-    and_stars: FxHashSet<Location>,
+    and_stars: FxHashMap<Location, Mutability>,
     arrays_lengths: FxHashMap<Location, Constant<'tcx>>,
 }

src/librustc_mir/transform/mod.rs

@@ -39,6 +39,7 @@ pub mod simplify_branches;
 pub mod simplify_try;
 pub mod uninhabited_enum_branching;
 pub mod unreachable_prop;
+pub mod validate;
 
 pub(crate) fn provide(providers: &mut Providers<'_>) {
     self::check_unsafety::provide(providers);
@@ -147,12 +148,18 @@ pub fn run_passes(
     passes: &[&[&dyn MirPass<'tcx>]],
 ) {
     let phase_index = mir_phase.phase_index();
+    let source = MirSource { instance, promoted };
+    let validate = tcx.sess.opts.debugging_opts.validate_mir;
 
     if body.phase >= mir_phase {
         return;
     }
 
-    let source = MirSource { instance, promoted };
+    if validate {
+        validate::Validator { when: format!("input to phase {:?}", mir_phase) }
+            .run_pass(tcx, source, body);
+    }
+
     let mut index = 0;
     let mut run_pass = |pass: &dyn MirPass<'tcx>| {
         let run_hooks = |body: &_, index, is_after| {
@@ -169,6 +176,11 @@ pub fn run_passes(
         pass.run_pass(tcx, source, body);
         run_hooks(body, index, true);
 
+        if validate {
+            validate::Validator { when: format!("after {} in phase {:?}", pass.name(), mir_phase) }
+                .run_pass(tcx, source, body);
+        }
+
         index += 1;
     };
 
@@ -179,6 +191,11 @@ pub fn run_passes(
     }
 
     body.phase = mir_phase;
+
+    if mir_phase == MirPhase::Optimized {
+        validate::Validator { when: format!("end of phase {:?}", mir_phase) }
+            .run_pass(tcx, source, body);
+    }
 }
 
 fn mir_const_qualif(tcx: TyCtxt<'_>, def_id: DefId) -> ConstQualifs {

src/librustc_mir/transform/validate.rs

@@ -0,0 +1,80 @@
+//! Validates the MIR to ensure that invariants are upheld.
+
+use super::{MirPass, MirSource};
+use rustc_middle::mir::visit::Visitor;
+use rustc_middle::{
+    mir::{Body, Location, Operand, Rvalue, Statement, StatementKind},
+    ty::{ParamEnv, TyCtxt},
+};
+use rustc_span::{def_id::DefId, Span, DUMMY_SP};
+
+pub struct Validator {
+    /// Describes at which point in the pipeline this validation is happening.
+    pub when: String,
+}
+
+impl<'tcx> MirPass<'tcx> for Validator {
+    fn run_pass(&self, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, body: &mut Body<'tcx>) {
+        let def_id = source.def_id();
+        let param_env = tcx.param_env(def_id);
+        TypeChecker { when: &self.when, def_id, body, tcx, param_env }.visit_body(body);
+    }
+}
+
+struct TypeChecker<'a, 'tcx> {
+    when: &'a str,
+    def_id: DefId,
+    body: &'a Body<'tcx>,
+    tcx: TyCtxt<'tcx>,
+    param_env: ParamEnv<'tcx>,
+}
+
+impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
+    fn fail(&self, span: Span, msg: impl AsRef<str>) {
+        // We use `delay_span_bug` as we might see broken MIR when other errors have already
+        // occurred.
+        self.tcx.sess.diagnostic().delay_span_bug(
+            span,
+            &format!("broken MIR in {:?} ({}): {}", self.def_id, self.when, msg.as_ref()),
+        );
+    }
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
+    fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) {
+        // `Operand::Copy` is only supposed to be used with `Copy` types.
+        if let Operand::Copy(place) = operand {
+            let ty = place.ty(&self.body.local_decls, self.tcx).ty;
+
+            if !ty.is_copy_modulo_regions(self.tcx, self.param_env, DUMMY_SP) {
+                self.fail(
+                    DUMMY_SP,
+                    format!("`Operand::Copy` with non-`Copy` type {} at {:?}", ty, location),
+                );
+            }
+        }
+
+        self.super_operand(operand, location);
+    }
+
+    fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
+        // The sides of an assignment must not alias. Currently this just checks whether the places
+        // are identical.
+        if let StatementKind::Assign(box (dest, rvalue)) = &statement.kind {
+            match rvalue {
+                Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) => {
+                    if dest == src {
+                        self.fail(
+                            DUMMY_SP,
+                            format!(
+                                "encountered `Assign` statement with overlapping memory at {:?}",
+                                location
+                            ),
+                        );
+                    }
+                }
+                _ => {}
+            }
+        }
+    }
+}

src/librustc_session/options.rs

@@ -1045,6 +1045,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
         "adds unstable command line options to rustc interface (default: no)"),
     use_ctors_section: Option<bool> = (None, parse_opt_bool, [TRACKED],
         "use legacy .ctors section for initializers rather than .init_array"),
+    validate_mir: bool = (false, parse_bool, [UNTRACKED],
+        "validate MIR after each transformation"),
     verbose: bool = (false, parse_bool, [UNTRACKED],
         "in general, enable more debug printouts (default: no)"),
     verify_llvm_ir: bool = (false, parse_bool, [TRACKED],

src/test/mir-opt/inline/issue-58867-inline-as-ref-as-mut/rustc.a.Inline.after.mir

@@ -15,7 +15,7 @@ fn a(_1: &mut [T]) -> &mut [T] {
         StorageLive(_3);                 // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:15
         StorageLive(_4);                 // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:6
         _4 = &mut (*_1);                 // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:6
-        _3 = _4;                         // scope 1 at $SRC_DIR/libcore/convert/mod.rs:LL:COL
+        _3 = move _4;                    // scope 1 at $SRC_DIR/libcore/convert/mod.rs:LL:COL
         _2 = &mut (*_3);                 // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:15
         StorageDead(_4);                 // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:14: 3:15
         _0 = &mut (*_2);                 // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:3:5: 3:15

src/test/mir-opt/inline/issue-58867-inline-as-ref-as-mut/rustc.b.Inline.after.mir

@@ -18,7 +18,7 @@ fn b(_1: &mut std::boxed::Box<T>) -> &mut T {
         _4 = &mut (*_1);                 // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:5: 8:6
         StorageLive(_5);                 // scope 1 at $SRC_DIR/liballoc/boxed.rs:LL:COL
         _5 = &mut (*(*_4));              // scope 1 at $SRC_DIR/liballoc/boxed.rs:LL:COL
-        _3 = _5;                         // scope 1 at $SRC_DIR/liballoc/boxed.rs:LL:COL
+        _3 = move _5;                    // scope 1 at $SRC_DIR/liballoc/boxed.rs:LL:COL
         StorageDead(_5);                 // scope 1 at $SRC_DIR/liballoc/boxed.rs:LL:COL
         _2 = &mut (*_3);                 // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:5: 8:15
         StorageDead(_4);                 // scope 0 at $DIR/issue-58867-inline-as-ref-as-mut.rs:8:14: 8:15