From 4d79d391b0aa1175f493e3544d8f66e6600fbfc6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 8 Apr 2019 13:28:51 +0200 Subject: [PATCH] avoid reading from ZST locals --- src/librustc_mir/interpret/eval_context.rs | 70 ++++++---------------- src/librustc_mir/interpret/operand.rs | 9 ++- src/librustc_mir/interpret/snapshot.rs | 6 +- 3 files changed, 29 insertions(+), 56 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 7c900bd596ac8..32f7ecd97b2ef 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/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, + pub value: LocalValue, /// Don't modify if `Some`, this is only used to prevent computing the layout twice pub layout: Cell>>, } -/// 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 { /// This local is not currently alive, and cannot be used at all. Dead, @@ -131,27 +131,13 @@ pub enum LocalValue { Live(Operand), } -impl LocalValue { - /// 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 { - // 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> { - 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> { + 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, MemPlace>> { - 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,7 +512,7 @@ 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; } _ => {} } @@ -534,23 +520,6 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tc } }, } - // 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)) => { diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 4ece062f380d6..65cd7be8fd4b2 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/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>, ) -> 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 }) } diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index f440e2966063c..0bb8b1d9d02ca 100644 --- a/src/librustc_mir/interpret/snapshot.rs +++ b/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 -> _, });