Skip to content

Commit

Permalink
avoid reading from ZST locals
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Apr 8, 2019
1 parent 5731945 commit 4d79d39
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 56 deletions.
70 changes: 19 additions & 51 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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),
}
}

Expand All @@ -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(_)) |
Expand Down Expand Up @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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(
Expand Down Expand Up @@ -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)) => {
Expand Down
9 changes: 7 additions & 2 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
}

Expand All @@ -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 })
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/interpret/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 -> _,
});

Expand Down

0 comments on commit 4d79d39

Please sign in to comment.