[validate-mir] validate that all accessed locals are initialized

src/librustc_mir/dataflow/impls/init_locals.rs

@@ -8,7 +8,24 @@ use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::visit::{PlaceContext, Visitor};
 use rustc_middle::mir::{self, BasicBlock, Local, Location};
 
-pub struct MaybeInitializedLocals;
+pub struct MaybeInitializedLocals {
+    deinit_on_move: bool,
+}
+
+impl MaybeInitializedLocals {
+    /// Creates a new default `MaybeInitializedLocals` analysis.
+    pub fn new() -> Self {
+        Self { deinit_on_move: true }
+    }
+
+    /// Creates a new `MaybeInitializedLocals` analysis that does not consider a move from a local
+    /// to deinitialize it.
+    ///
+    /// `StorageDead` still deinitializes locals.
+    pub fn no_deinit_on_move() -> Self {
+        Self { deinit_on_move: false }
+    }
+}
 
 impl BottomValue for MaybeInitializedLocals {
     /// bottom = uninit
@@ -42,7 +59,8 @@ impl dataflow::GenKillAnalysis<'tcx> for MaybeInitializedLocals {
         statement: &mir::Statement<'tcx>,
         loc: Location,
     ) {
-        TransferFunction { trans }.visit_statement(statement, loc)
+        TransferFunction { trans, deinit_on_move: self.deinit_on_move }
+            .visit_statement(statement, loc)
     }
 
     fn terminator_effect(
@@ -51,7 +69,8 @@ impl dataflow::GenKillAnalysis<'tcx> for MaybeInitializedLocals {
         terminator: &mir::Terminator<'tcx>,
         loc: Location,
     ) {
-        TransferFunction { trans }.visit_terminator(terminator, loc)
+        TransferFunction { trans, deinit_on_move: self.deinit_on_move }
+            .visit_terminator(terminator, loc)
     }
 
     fn call_return_effect(
@@ -77,6 +96,7 @@ impl dataflow::GenKillAnalysis<'tcx> for MaybeInitializedLocals {
 }
 
 struct TransferFunction<'a, T> {
+    deinit_on_move: bool,
     trans: &'a mut T,
 }
 
@@ -95,8 +115,12 @@ where
 
             // If the local is moved out of, or if it gets marked `StorageDead`, consider it no
             // longer initialized.
-            PlaceContext::NonUse(NonUseContext::StorageDead)
-            | PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => self.trans.kill(local),
+            PlaceContext::NonUse(NonUseContext::StorageDead) => self.trans.kill(local),
+            PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => {
+                if self.deinit_on_move {
+                    self.trans.kill(local)
+                }
+            }
 
             // All other uses do not affect this analysis.
             PlaceContext::NonUse(

src/librustc_mir/transform/generator.rs

@@ -452,7 +452,7 @@ fn locals_live_across_suspend_points(
         .iterate_to_fixpoint()
         .into_results_cursor(body);
 
-    let mut init = MaybeInitializedLocals
+    let mut init = MaybeInitializedLocals::new()
         .into_engine(tcx, body, def_id)
         .iterate_to_fixpoint()
         .into_results_cursor(body);

src/librustc_mir/transform/validate.rs

@@ -1,12 +1,16 @@
 //! Validates the MIR to ensure that invariants are upheld.
 
 use super::{MirPass, MirSource};
-use rustc_middle::mir::visit::Visitor;
+use crate::dataflow::{impls::MaybeInitializedLocals, Analysis, ResultsCursor};
+use rustc_index::bit_set::BitSet;
+use rustc_middle::mir::visit::{MutatingUseContext, PlaceContext, Visitor};
+use rustc_middle::ty;
 use rustc_middle::{
-    mir::{Body, Location, Operand, Rvalue, Statement, StatementKind},
+    mir::{traversal, Body, Local, Location, Operand, Rvalue, Statement, StatementKind},
     ty::{ParamEnv, TyCtxt},
 };
 use rustc_span::{def_id::DefId, Span, DUMMY_SP};
+use ty::Ty;
 
 pub struct Validator {
     /// Describes at which point in the pipeline this validation is happening.
@@ -17,16 +21,32 @@ 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);
+
+        // Do not consider moves to deinitialize locals. Some MIR passes output MIR that violates
+        // this assumption and would lead to uses of uninitialized data.
+        let init = MaybeInitializedLocals::no_deinit_on_move()
+            .into_engine(tcx, body, def_id)
+            .iterate_to_fixpoint()
+            .into_results_cursor(body);
+
+        let mut checker = TypeChecker { when: &self.when, def_id, body, tcx, param_env, init };
+
+        // Only visit reachable blocks. Unreachable code may access uninitialized locals.
+        for (block, data) in traversal::preorder(body) {
+            checker.visit_basic_block_data(block, data);
+        }
     }
 }
 
 struct TypeChecker<'a, 'tcx> {
     when: &'a str,
+
     def_id: DefId,
     body: &'a Body<'tcx>,
     tcx: TyCtxt<'tcx>,
     param_env: ParamEnv<'tcx>,
+
+    init: ResultsCursor<'a, 'tcx, MaybeInitializedLocals>,
 }
 
 impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
@@ -46,7 +66,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
         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) {
+            if false && !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),
@@ -76,5 +96,50 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                 _ => {}
             }
         }
+
+        // Every local used by a statement must be initialized before the statement executes.
+        self.init.seek_before_primary_effect(location);
+        UsedLocalsAreInitialized {
+            checker: self,
+            init: self.init.get(),
+            span: statement.source_info.span,
+        }
+        .visit_statement(statement, location);
+    }
+}
+
+struct UsedLocalsAreInitialized<'a, 'tcx> {
+    checker: &'a TypeChecker<'a, 'tcx>,
+    init: &'a BitSet<Local>,
+    span: Span,
+}
+
+impl Visitor<'tcx> for UsedLocalsAreInitialized<'a, 'tcx> {
+    fn visit_local(&mut self, local: &Local, context: PlaceContext, location: Location) {
+        if context.is_use() && !context.is_place_assignment() && !self.init.contains(*local) {
+            if context == PlaceContext::MutatingUse(MutatingUseContext::Projection) {
+                // Ignore `_1.b`-like projections as they appear as assignment destinations, and
+                // `_1` doesn't have to be initialized there.
+                return;
+            }
+
+            if is_zst(
+                self.checker.tcx,
+                self.checker.def_id,
+                self.checker.body.local_decls[*local].ty,
+            ) {
+                // Known ZSTs don't have to be initialized at all, skip them.
+                return;
+            }
+
+            self.checker.fail(
+                self.span,
+                format!("use of uninitialized local {:?} at {:?}", local, location),
+            );
+        }
     }
 }
+
+fn is_zst<'tcx>(tcx: TyCtxt<'tcx>, did: DefId, ty: Ty<'tcx>) -> bool {
+    tcx.layout_of(tcx.param_env(did).and(ty)).map(|layout| layout.is_zst()).unwrap_or(false)
+}