Merge pull request #85 from oli-obk/storage_dead

deallocate all locals on function exit and transitively freeze constants through pointers

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<T, EvalError<'tcx>>;
@@ -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",
         }
     }
 

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,21 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             StackPopCleanup::Goto(target) => self.goto_block(target),
             StackPopCleanup::None => {},
         }
-        // TODO(solson): Deallocate local variables.
+        // deallocate all locals that are backed by an allocation
+        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) {
+                    // Any frozen memory means that it belongs to a constant or something referenced
+                    // by a constant. We could alternatively check whether the alloc_id is frozen
+                    // before calling deallocate, but this is much simpler and is probably the
+                    // rare case.
+                    Ok(()) | Err(EvalError::DeallocatedFrozenMemory) => {},
+                    other => return other,
+                }
+            }
+        }
         Ok(())
     }
 
@@ -729,6 +757,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 +1624,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
+        if let Some(val) = val {
-        self.stack[frame].locals[local.index() - 1] = 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(())
     }
 }

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.
+            // Miri can safely ignore these. Only translation needs it.
-            StorageLive(_) | StorageDead(_) => {}
+            StorageLive(_) |
+            StorageDead(_) => {}
 
             // Defined to do nothing. These are added by optimization passes, to avoid changing the
             // size of MIR constantly.

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>,

src/memory.rs

@@ -1,7 +1,7 @@
 use byteorder::{ReadBytesExt, WriteBytesExt, LittleEndian, BigEndian, self};
 use std::collections::Bound::{Included, Excluded};
 use std::collections::{btree_map, BTreeMap, HashMap, HashSet, VecDeque};
-use std::{fmt, iter, ptr};
+use std::{fmt, iter, ptr, mem};
 
 use rustc::hir::def_id::DefId;
 use rustc::ty::{self, BareFnTy, ClosureTy, ClosureSubsts, TyCtxt};
@@ -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();
@@ -333,11 +339,8 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
 
         while let Some(id) = allocs_to_print.pop_front() {
             allocs_seen.insert(id);
+            if id == ZST_ALLOC_ID || id == NEVER_ALLOC_ID { continue; }
             let mut msg = format!("Alloc {:<5} ", format!("{}:", id));
-            if id == ZST_ALLOC_ID {
-                trace!("{} zst allocation", msg);
-                continue;
-            }
             let prefix_len = msg.len();
             let mut relocations = vec![];
 
@@ -379,7 +382,12 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
                 let relocation_width = (self.pointer_size() - 1) * 3;
                 for (i, target_id) in relocations {
                     write!(msg, "{:1$}", "", (i - pos) * 3).unwrap();
-                    write!(msg, "└{0:─^1$}┘ ", format!("({})", target_id), relocation_width).unwrap();
+                    let target = match target_id {
+                        ZST_ALLOC_ID => String::from("zst"),
+                        NEVER_ALLOC_ID => String::from("int ptr"),
+                        _ => format!("({})", target_id),
+                    };
+                    write!(msg, "└{0:─^1$}┘ ", target, relocation_width).unwrap();
                     pos = i + self.pointer_size();
                 }
                 trace!("{}", msg);
@@ -446,10 +454,25 @@ 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, ()> {
-        // It's not possible to freeze the zero-sized allocation, because it doesn't exist.
+        // do not use `self.get_mut(alloc_id)` here, because we might have already frozen a
-        if alloc_id != ZST_ALLOC_ID {
+        // sub-element or have circular pointers (e.g. `Rc`-cycles)
-            self.get_mut(alloc_id)?.immutable = true;
+        let relocations = match self.alloc_map.get_mut(&alloc_id) {
+            Some(ref mut alloc) if !alloc.immutable => {
+                alloc.immutable = true;
+                // take out the relocations vector to free the borrow on self, so we can call
+                // freeze recursively
+                mem::replace(&mut alloc.relocations, Default::default())
+            },
+            None if alloc_id == NEVER_ALLOC_ID || alloc_id == ZST_ALLOC_ID => return Ok(()),
+            None if !self.functions.contains_key(&alloc_id) => return Err(EvalError::DanglingPointerDeref),
+            _ => return Ok(()),
+        };
+        // recurse into inner allocations
+        for &alloc in relocations.values() {
+            self.freeze(alloc)?;
         }
+        // put back the relocations
+        self.alloc_map.get_mut(&alloc_id).expect("checked above").relocations = relocations;
         Ok(())
     }
 

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);
+}

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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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);
+}