avoid reading from ZST locals

Author: RalfJung

src/librustc_mir/interpret/eval_context.rs

@@ -108,13 +108,13 @@ pub enum StackPopCleanup {
 /// State of a local variable including a memoized layout
 #[derive(Clone, PartialEq, Eq)]
 pub struct LocalState<'tcx, Tag=(), Id=AllocId> {
-    pub state: LocalValue<Tag, Id>,
+    pub value: LocalValue<Tag, Id>,
     /// Don't modify if `Some`, this is only used to prevent computing the layout twice
     pub layout: Cell<Option<TyLayout<'tcx>>>,
 }
 
-/// State of a local variable
-#[derive(Copy, Clone, PartialEq, Eq, Hash)]
+/// Current value of a local variable
+#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
 pub enum LocalValue<Tag=(), Id=AllocId> {
     /// This local is not currently alive, and cannot be used at all.
     Dead,
@@ -131,27 +131,13 @@ pub enum LocalValue<Tag=(), Id=AllocId> {
     Live(Operand<Tag, Id>),
 }
 
-impl<Tag: Copy> LocalValue<Tag> {
-    /// The initial value of a local: ZST get "initialized" because they can be read from without
-    /// ever having been written to.
-    fn uninit_local(
-        layout: TyLayout<'_>
-    ) -> LocalValue<Tag> {
-        // FIXME: Can we avoid this ZST special case? That would likely require MIR
-        // generation changes.
-        if layout.is_zst() {
-            LocalValue::Live(Operand::Immediate(Immediate::Scalar(Scalar::zst().into())))
-        } else {
-            LocalValue::Uninitialized
-        }
-    }
-}
-
-impl<'tcx, Tag: Copy> LocalState<'tcx, Tag> {
-    pub fn access(&self) -> EvalResult<'tcx, &Operand<Tag>> {
-        match self.state {
-            LocalValue::Dead | LocalValue::Uninitialized => err!(DeadLocal),
-            LocalValue::Live(ref val) => Ok(val),
+impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> {
+    pub fn access(&self) -> EvalResult<'tcx, Operand<Tag>> {
+        match self.value {
+            LocalValue::Dead => err!(DeadLocal),
+            LocalValue::Uninitialized =>
+                bug!("The type checker should prevent reading from a never-written local"),
+            LocalValue::Live(val) => Ok(val),
         }
     }
 
@@ -160,7 +146,7 @@ impl<'tcx, Tag: Copy> LocalState<'tcx, Tag> {
     pub fn access_mut(
         &mut self,
     ) -> EvalResult<'tcx, Result<&mut LocalValue<Tag>, MemPlace<Tag>>> {
-        match self.state {
+        match self.value {
             LocalValue::Dead => err!(DeadLocal),
             LocalValue::Live(Operand::Indirect(mplace)) => Ok(Err(mplace)),
             ref mut local @ LocalValue::Live(Operand::Immediate(_)) |
@@ -507,13 +493,13 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tc
         if mir.local_decls.len() > 1 {
             // Locals are initially uninitialized.
             let dummy = LocalState {
-                state: LocalValue::Uninitialized,
+                value: LocalValue::Uninitialized,
                 layout: Cell::new(None),
             };
             let mut locals = IndexVec::from_elem(dummy, &mir.local_decls);
             // Return place is handled specially by the `eval_place` functions, and the
             // entry in `locals` should never be used. Make it dead, to be sure.
-            locals[mir::RETURN_PLACE].state = LocalValue::Dead;
+            locals[mir::RETURN_PLACE].value = LocalValue::Dead;
             // Now mark those locals as dead that we do not want to initialize
             match self.tcx.describe_def(instance.def_id()) {
                 // statics and constants don't have `Storage*` statements, no need to look for them
@@ -526,31 +512,14 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tc
                             match stmt.kind {
                                 StorageLive(local) |
                                 StorageDead(local) => {
-                                    locals[local].state = LocalValue::Dead;
+                                    locals[local].value = LocalValue::Dead;
                                 }
                                 _ => {}
                             }
                         }
                     }
                 },
             }
-            // The remaining locals are uninitialized, fill them with `uninit_local`.
-            // (For ZST this is not a NOP.)
-            for (idx, local) in locals.iter_enumerated_mut() {
-                match local.state {
-                    LocalValue::Uninitialized => {
-                        // This needs to be properly initialized.
-                        let ty = self.monomorphize(mir.local_decls[idx].ty)?;
-                        let layout = self.layout_of(ty)?;
-                        local.state = LocalValue::uninit_local(layout);
-                        local.layout = Cell::new(Some(layout));
-                    }
-                    LocalValue::Dead => {
-                        // Nothing to do
-                    }
-                    LocalValue::Live(_) => bug!("Locals cannot be live yet"),
-                }
-            }
             // done
             self.frame_mut().locals = locals;
         }
@@ -585,7 +554,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tc
         }
         // Deallocate all locals that are backed by an allocation.
         for local in frame.locals {
-            self.deallocate_local(local.state)?;
+            self.deallocate_local(local.value)?;
         }
         // Validate the return value. Do this after deallocating so that we catch dangling
         // references.
@@ -633,10 +602,9 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tc
         assert!(local != mir::RETURN_PLACE, "Cannot make return place live");
         trace!("{:?} is now live", local);
 
-        let layout = self.layout_of_local(self.frame(), local, None)?;
-        let local_val = LocalValue::uninit_local(layout);
+        let local_val = LocalValue::Uninitialized;
         // StorageLive *always* kills the value that's currently stored
-        Ok(mem::replace(&mut self.frame_mut().locals[local].state, local_val))
+        Ok(mem::replace(&mut self.frame_mut().locals[local].value, local_val))
     }
 
     /// Returns the old value of the local.
@@ -645,7 +613,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tc
         assert!(local != mir::RETURN_PLACE, "Cannot make return place dead");
         trace!("{:?} is now dead", local);
 
-        mem::replace(&mut self.frame_mut().locals[local].state, LocalValue::Dead)
+        mem::replace(&mut self.frame_mut().locals[local].value, LocalValue::Dead)
     }
 
     pub(super) fn deallocate_local(
@@ -698,7 +666,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tc
                 }
                 write!(msg, ":").unwrap();
 
-                match self.stack[frame].locals[local].state {
+                match self.stack[frame].locals[local].value {
                     LocalValue::Dead => write!(msg, " is dead").unwrap(),
                     LocalValue::Uninitialized => write!(msg, " is uninitialized").unwrap(),
                     LocalValue::Live(Operand::Indirect(mplace)) => {

src/librustc_mir/interpret/operand.rs

@@ -459,8 +459,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M>
         layout: Option<TyLayout<'tcx>>,
     ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> {
         assert_ne!(local, mir::RETURN_PLACE);
-        let op = *frame.locals[local].access()?;
         let layout = self.layout_of_local(frame, local, layout)?;
+        let op = if layout.is_zst() {
+            // Do not read from ZST, they might not be initialized
+            Operand::Immediate(Immediate::Scalar(Scalar::zst().into()))
+        } else {
+            frame.locals[local].access()?
+        };
         Ok(OpTy { op, layout })
     }
 
@@ -475,7 +480,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M>
                 Operand::Indirect(mplace)
             }
             Place::Local { frame, local } =>
-                *self.stack[frame].locals[local].access()?
+                *self.access_local(&self.stack[frame], local, None)?
         };
         Ok(OpTy { op, layout: place.layout })
     }

src/librustc_mir/interpret/snapshot.rs

@@ -363,13 +363,13 @@ impl<'a, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a LocalState<'tcx>
     type Item = LocalValue<(), AllocIdSnapshot<'a>>;
 
     fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
-        let LocalState { state, layout: _ } = self;
-        state.snapshot(ctx)
+        let LocalState { value, layout: _ } = self;
+        value.snapshot(ctx)
     }
 }
 
 impl_stable_hash_for!(struct LocalState<'tcx> {
-    state,
+    value,
     layout -> _,
 });