From 51ff9fdaf684c89c12ac5bf41980a53eed44ee2d Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Thu, 17 Nov 2016 14:48:34 +0100 Subject: [PATCH] deallocate all locals on function exit and transitively freeze constants through pointers --- src/error.rs | 8 ++++- src/interpreter/mod.rs | 43 +++++++++++++++++++++-- src/interpreter/step.rs | 5 +-- src/interpreter/terminator/mod.rs | 7 ++-- src/memory.rs | 23 +++++++++++- tests/compile-fail/modifying_constants.rs | 6 ++++ tests/run-pass/move-arg-3-unique.rs | 18 ++++++++++ 7 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 tests/compile-fail/modifying_constants.rs create mode 100644 tests/run-pass/move-arg-3-unique.rs diff --git a/src/error.rs b/src/error.rs index 481815996a0..0fd35ff6565 100644 --- a/src/error.rs +++ b/src/error.rs @@ -7,7 +7,7 @@ use memory::Pointer; use rustc_const_math::ConstMathErr; use syntax::codemap::Span; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum EvalError<'tcx> { FunctionPointerTyMismatch(Abi, &'tcx FnSig<'tcx>, &'tcx BareFnTy<'tcx>), NoMirFor(String), @@ -48,6 +48,8 @@ pub enum EvalError<'tcx> { AssumptionNotHeld, InlineAsm, TypeNotPrimitive(Ty<'tcx>), + ReallocatedFrozenMemory, + DeallocatedFrozenMemory, } pub type EvalResult<'tcx, T> = Result>; @@ -110,6 +112,10 @@ impl<'tcx> Error for EvalError<'tcx> { "cannot evaluate inline assembly", EvalError::TypeNotPrimitive(_) => "expected primitive type, got nonprimitive", + EvalError::ReallocatedFrozenMemory => + "tried to reallocate frozen memory", + EvalError::DeallocatedFrozenMemory => + "tried to deallocate frozen memory", } } diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index ee503f97d78..dbc6d7bfb83 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -364,6 +364,20 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let global_value = self.globals .get_mut(&id) .expect("global should have been cached (freeze)"); + match global_value.data.expect("global should have been initialized") { + Value::ByRef(ptr) => self.memory.freeze(ptr.alloc_id)?, + Value::ByVal(val) => if let Some(alloc_id) = val.relocation { + self.memory.freeze(alloc_id)?; + }, + Value::ByValPair(a, b) => { + if let Some(alloc_id) = a.relocation { + self.memory.freeze(alloc_id)?; + } + if let Some(alloc_id) = b.relocation { + self.memory.freeze(alloc_id)?; + } + }, + } if let Value::ByRef(ptr) = global_value.data.expect("global should have been initialized") { self.memory.freeze(ptr.alloc_id)?; } @@ -375,7 +389,17 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { StackPopCleanup::Goto(target) => self.goto_block(target), StackPopCleanup::None => {}, } - // TODO(solson): Deallocate local variables. + // check that all locals have been deallocated through StorageDead + for (i, local) in frame.locals.into_iter().enumerate() { + if let Some(Value::ByRef(ptr)) = local { + trace!("deallocating local {}: {:?}", i + 1, ptr); + self.memory.dump(ptr.alloc_id); + match self.memory.deallocate(ptr) { + Ok(()) | Err(EvalError::DeallocatedFrozenMemory) => {}, + other => return other, + } + } + } Ok(()) } @@ -729,6 +753,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // Skip the initial 0 intended for LLVM GEP. for field_index in path { let field_offset = self.get_field_offset(ty, field_index)?; + trace!("field_path_offset_and_ty: {}, {}, {:?}, {:?}", field_index, ty, field_offset, offset); ty = self.get_field_ty(ty, field_index)?; offset = offset.checked_add(field_offset, &self.tcx.data_layout).unwrap(); } @@ -1595,8 +1620,20 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { ) -> EvalResult<'tcx, ()> { let val = self.stack[frame].get_local(local); let val = f(self, val)?; - // can't use `set_local` here, because that's only meant for going to an initialized value - self.stack[frame].locals[local.index() - 1] = val; + if let Some(val) = val { + self.stack[frame].set_local(local, val); + } else { + self.deallocate_local(frame, local)?; + } + Ok(()) + } + + pub fn deallocate_local(&mut self, frame: usize, local: mir::Local) -> EvalResult<'tcx, ()> { + if let Some(Value::ByRef(ptr)) = self.stack[frame].get_local(local) { + self.memory.deallocate(ptr)?; + } + // Subtract 1 because we don't store a value for the ReturnPointer, the local with index 0. + self.stack[frame].locals[local.index() - 1] = None; Ok(()) } } diff --git a/src/interpreter/step.rs b/src/interpreter/step.rs index 4c03b3c42aa..90564cd2fcc 100644 --- a/src/interpreter/step.rs +++ b/src/interpreter/step.rs @@ -81,8 +81,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Assign(ref lvalue, ref rvalue) => self.eval_rvalue_into_lvalue(rvalue, lvalue)?, SetDiscriminant { .. } => unimplemented!(), - // Miri can safely ignore these. Only translation needs them. - StorageLive(_) | StorageDead(_) => {} + // Miri can safely ignore these. Only translation needs it. + StorageLive(_) | + StorageDead(_) => {} // Defined to do nothing. These are added by optimization passes, to avoid changing the // size of MIR constantly. diff --git a/src/interpreter/terminator/mod.rs b/src/interpreter/terminator/mod.rs index a80ac216aa2..b0c24fc344f 100644 --- a/src/interpreter/terminator/mod.rs +++ b/src/interpreter/terminator/mod.rs @@ -144,7 +144,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Ok(()) } - fn eval_drop_impls(&mut self, drops: Vec<(DefId, Value, &'tcx Substs<'tcx>)>) -> EvalResult<'tcx, ()> { + pub fn eval_drop_impls(&mut self, drops: Vec<(DefId, Value, &'tcx Substs<'tcx>)>) -> EvalResult<'tcx, ()> { let span = self.frame().span; // add them to the stack in reverse order, because the impl that needs to run the last // is the one that needs to be at the bottom of the stack @@ -249,6 +249,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { fn read_discriminant_value(&self, adt_ptr: Pointer, adt_ty: Ty<'tcx>) -> EvalResult<'tcx, u64> { use rustc::ty::layout::Layout::*; let adt_layout = self.type_layout(adt_ty); + trace!("read_discriminant_value {:?}", adt_layout); let discr_val = match *adt_layout { General { discr, .. } | CEnum { discr, signed: false, .. } => { @@ -263,12 +264,14 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { RawNullablePointer { nndiscr, value } => { let discr_size = value.size(&self.tcx.data_layout).bytes() as usize; + trace!("rawnullablepointer with size {}", discr_size); self.read_nonnull_discriminant_value(adt_ptr, nndiscr, discr_size)? } StructWrappedNullablePointer { nndiscr, ref discrfield, .. } => { let (offset, ty) = self.nonnull_offset_and_ty(adt_ty, nndiscr, discrfield)?; let nonnull = adt_ptr.offset(offset.bytes() as isize); + trace!("struct wrapped nullable pointer type: {}", ty); // only the pointer part of a fat pointer is used for this space optimization let discr_size = self.type_size(ty).expect("bad StructWrappedNullablePointer discrfield"); self.read_nonnull_discriminant_value(nonnull, nndiscr, discr_size)? @@ -515,7 +518,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } /// push DefIds of drop impls and their argument on the given vector - fn drop( + pub fn drop( &mut self, lval: Lvalue<'tcx>, ty: Ty<'tcx>, diff --git a/src/memory.rs b/src/memory.rs index 6670ed66cf0..4421fe23eba 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -212,6 +212,9 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { if ptr.points_to_zst() { return self.allocate(new_size, align); } + if self.get(ptr.alloc_id).map(|alloc| alloc.immutable) == Ok(true) { + return Err(EvalError::ReallocatedFrozenMemory); + } let size = self.get(ptr.alloc_id)?.bytes.len(); @@ -242,6 +245,9 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { // TODO(solson): Report error about non-__rust_allocate'd pointer. return Err(EvalError::Unimplemented(format!("bad pointer offset: {}", ptr.offset))); } + if self.get(ptr.alloc_id).map(|alloc| alloc.immutable) == Ok(true) { + return Err(EvalError::DeallocatedFrozenMemory); + } if let Some(alloc) = self.alloc_map.remove(&ptr.alloc_id) { self.memory_usage -= alloc.bytes.len(); @@ -446,9 +452,24 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { /// Reading and writing impl<'a, 'tcx> Memory<'a, 'tcx> { pub fn freeze(&mut self, alloc_id: AllocId) -> EvalResult<'tcx, ()> { + // FIXME: the comment is wrong and do we really still need this check? // It's not possible to freeze the zero-sized allocation, because it doesn't exist. if alloc_id != ZST_ALLOC_ID { - self.get_mut(alloc_id)?.immutable = true; + // do not use `self.get_mut(alloc_id)` here, because we might have already frozen a + // sub-element or have circular pointers (e.g. `Rc`-cycles) + let allocs: Vec<_> = match self.alloc_map.get_mut(&alloc_id) { + Some(ref mut alloc) if !alloc.immutable => { + alloc.immutable = true; + alloc.relocations.values().cloned().collect() + }, + None if alloc_id == NEVER_ALLOC_ID || alloc_id == ZST_ALLOC_ID => Vec::new(), + None if !self.functions.contains_key(&alloc_id) => return Err(EvalError::DanglingPointerDeref), + _ => Vec::new(), + }; + // recurse into inner allocations + for alloc in allocs { + self.freeze(alloc)?; + } } Ok(()) } diff --git a/tests/compile-fail/modifying_constants.rs b/tests/compile-fail/modifying_constants.rs new file mode 100644 index 00000000000..5a8fd189aae --- /dev/null +++ b/tests/compile-fail/modifying_constants.rs @@ -0,0 +1,6 @@ +fn main() { + let x = &1; // the `&1` is promoted to a constant, but it used to be that only the pointer is frozen, not the pointee + let y = unsafe { &mut *(x as *const i32 as *mut i32) }; + *y = 42; //~ ERROR tried to modify constant memory + assert_eq!(*x, 42); +} diff --git a/tests/run-pass/move-arg-3-unique.rs b/tests/run-pass/move-arg-3-unique.rs new file mode 100644 index 00000000000..2e6320eb802 --- /dev/null +++ b/tests/run-pass/move-arg-3-unique.rs @@ -0,0 +1,18 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(unused_features, unused_variables)] +#![feature(box_syntax)] + +pub fn main() { + let x = box 10; + let y = x; + assert_eq!(*y, 10); +}