Merge pull request rust-lang#245 from oli-obk/funky_allocs

Ensure that it is not possible to explicitly free stack memory

Author: oli-obk

src/const_eval.rs

@@ -1,5 +1,6 @@
 use rustc::traits::Reveal;
 use rustc::ty::{self, TyCtxt, Ty, Instance};
+use syntax::ast::Mutability;
 
 use error::{EvalError, EvalResult};
 use lvalue::{Global, GlobalId, Lvalue};
@@ -25,7 +26,12 @@ pub fn eval_body_as_primval<'a, 'tcx>(
                 ecx.tcx,
                 ty::ParamEnv::empty(Reveal::All),
                 mir.span);
-        let cleanup = StackPopCleanup::MarkStatic(mutable);
+        let mutability = if mutable {
+            Mutability::Mutable
+        } else {
+            Mutability::Immutable
+        };
+        let cleanup = StackPopCleanup::MarkStatic(mutability);
         let name = ty::tls::with(|tcx| tcx.item_path_str(instance.def_id()));
         trace!("pushing stack frame for global: {}", name);
         ecx.push_stack_frame(

src/error.rs

@@ -2,7 +2,7 @@ use std::error::Error;
 use std::fmt;
 use rustc::mir;
 use rustc::ty::{FnSig, Ty, layout};
-use memory::MemoryPointer;
+use memory::{MemoryPointer, Kind};
 use rustc_const_math::ConstMathErr;
 use syntax::codemap::Span;
 
@@ -12,6 +12,7 @@ pub enum EvalError<'tcx> {
     NoMirFor(String),
     UnterminatedCString(MemoryPointer),
     DanglingPointerDeref,
+    DoubleFree,
     InvalidMemoryAccess,
     InvalidFunctionPointer,
     InvalidBool,
@@ -56,8 +57,8 @@ pub enum EvalError<'tcx> {
     AssumptionNotHeld,
     InlineAsm,
     TypeNotPrimitive(Ty<'tcx>),
-    ReallocatedStaticMemory,
-    DeallocatedStaticMemory,
+    ReallocatedWrongMemoryKind(Kind, Kind),
+    DeallocatedWrongMemoryKind(Kind, Kind),
     ReallocateNonBasePtr,
     DeallocateNonBasePtr,
     IncorrectAllocationInformation,
@@ -84,6 +85,8 @@ impl<'tcx> Error for EvalError<'tcx> {
                 "tried to access memory through an invalid pointer",
             DanglingPointerDeref =>
                 "dangling pointer was dereferenced",
+            DoubleFree =>
+                "tried to deallocate dangling pointer",
             InvalidFunctionPointer =>
                 "tried to use an integer pointer or a dangling pointer as a function pointer",
             InvalidBool =>
@@ -148,10 +151,10 @@ impl<'tcx> Error for EvalError<'tcx> {
                 "miri does not support inline assembly",
             TypeNotPrimitive(_) =>
                 "expected primitive type, got nonprimitive",
-            ReallocatedStaticMemory =>
-                "tried to reallocate static memory",
-            DeallocatedStaticMemory =>
-                "tried to deallocate static memory",
+            ReallocatedWrongMemoryKind(_, _) =>
+                "tried to reallocate memory from one kind to another",
+            DeallocatedWrongMemoryKind(_, _) =>
+                "tried to deallocate memory of the wrong kind",
             ReallocateNonBasePtr =>
                 "tried to reallocate with a pointer not to the beginning of an existing object",
             DeallocateNonBasePtr =>
@@ -198,6 +201,10 @@ impl<'tcx> fmt::Display for EvalError<'tcx> {
                 write!(f, "tried to call a function with sig {} through a function pointer of type {}", sig, got),
             ArrayIndexOutOfBounds(span, len, index) =>
                 write!(f, "index out of bounds: the len is {} but the index is {} at {:?}", len, index, span),
+            ReallocatedWrongMemoryKind(old, new) =>
+                write!(f, "tried to reallocate memory from {:?} to {:?}", old, new),
+            DeallocatedWrongMemoryKind(old, new) =>
+                write!(f, "tried to deallocate {:?} memory but gave {:?} as the kind", old, new),
             Math(span, ref err) =>
                 write!(f, "{:?} at {:?}", err, span),
             Intrinsic(ref err) =>

src/eval_context.rs

@@ -12,7 +12,7 @@ use rustc::ty::{self, Ty, TyCtxt, TypeFoldable, Binder};
 use rustc::traits;
 use rustc_data_structures::indexed_vec::Idx;
 use syntax::codemap::{self, DUMMY_SP, Span};
-use syntax::ast;
+use syntax::ast::{self, Mutability};
 use syntax::abi::Abi;
 
 use error::{EvalError, EvalResult};
@@ -98,8 +98,7 @@ pub enum StackPopCleanup {
     /// isn't modifyable afterwards in case of constants.
     /// In case of `static mut`, mark the memory to ensure it's never marked as immutable through
     /// references or deallocated
-    /// The bool decides whether the value is mutable (true) or not (false)
-    MarkStatic(bool),
+    MarkStatic(Mutability),
     /// A regular stackframe added due to a function call will need to get forwarded to the next
     /// block
     Goto(mir::BasicBlock),
@@ -154,7 +153,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
     ) -> EvalResult<'tcx, MemoryPointer> {
         let size = self.type_size_with_substs(ty, substs)?.expect("cannot alloc memory for unsized type");
         let align = self.type_align_with_substs(ty, substs)?;
-        self.memory.allocate(size, align)
+        self.memory.allocate(size, align, ::memory::Kind::Stack)
     }
 
     pub fn memory(&self) -> &Memory<'a, 'tcx> {
@@ -370,7 +369,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 // see comment on `initialized` field
                 assert!(!global_value.initialized);
                 global_value.initialized = true;
-                assert!(global_value.mutable);
+                assert_eq!(global_value.mutable, Mutability::Mutable);
                 global_value.mutable = mutable;
             } else {
                 bug!("StackPopCleanup::MarkStatic on: {:?}", frame.return_lvalue);
@@ -414,11 +413,13 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             trace!("deallocating local");
             let ptr = ptr.to_ptr()?;
             self.memory.dump_alloc(ptr.alloc_id);
-            match self.memory.deallocate(ptr, None) {
-                // We could alternatively check whether the alloc_id is static before calling
-                // deallocate, but this is much simpler and is probably the rare case.
-                Ok(()) | Err(EvalError::DeallocatedStaticMemory) => {},
-                other => return other,
+            match self.memory.get(ptr.alloc_id)?.kind {
+                // for a constant like `const FOO: &i32 = &1;` the local containing
+                // the `1` is referred to by the global. We transitively marked everything
+                // the global refers to as static itself, so we don't free it here
+                ::memory::Kind::Static => {}
+                ::memory::Kind::Stack => self.memory.deallocate(ptr, None, ::memory::Kind::Stack)?,
+                other => bug!("local contained non-stack memory: {:?}", other),
             }
         };
         Ok(())
@@ -693,11 +694,13 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     return Err(EvalError::NeedsRfc("\"heap\" allocations".to_string()));
                 }
                 // FIXME: call the `exchange_malloc` lang item if available
-                if self.type_size(ty)?.expect("box only works with sized types") == 0 {
+                let size = self.type_size(ty)?.expect("box only works with sized types");
+                if size == 0 {
                     let align = self.type_align(ty)?;
                     self.write_primval(dest, PrimVal::Bytes(align.into()), dest_ty)?;
                 } else {
-                    let ptr = self.alloc_ptr(ty)?;
+                    let align = self.type_align(ty)?;
+                    let ptr = self.memory.allocate(size, align, ::memory::Kind::Rust)?;
                     self.write_primval(dest, PrimVal::Ptr(ptr), dest_ty)?;
                 }
             }
@@ -1022,7 +1025,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             }
             Lvalue::Ptr { .. } => lvalue,
             Lvalue::Global(cid) => {
-                let global_val = *self.globals.get(&cid).expect("global not cached");
+                let global_val = self.globals.get(&cid).expect("global not cached").clone();
                 match global_val.value {
                     Value::ByRef(ptr, aligned) =>
                         Lvalue::Ptr { ptr, aligned, extra: LvalueExtra::None },
@@ -1108,8 +1111,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
 
         match dest {
             Lvalue::Global(cid) => {
-                let dest = *self.globals.get_mut(&cid).expect("global should be cached");
-                if !dest.mutable {
+                let dest = self.globals.get_mut(&cid).expect("global should be cached").clone();
+                if dest.mutable == Mutability::Immutable {
                     return Err(EvalError::ModifiedConstantMemory);
                 }
                 let write_dest = |this: &mut Self, val| {
@@ -1594,8 +1597,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
     pub fn modify_global<F>(&mut self, cid: GlobalId<'tcx>, f: F) -> EvalResult<'tcx>
         where F: FnOnce(&mut Self, Value) -> EvalResult<'tcx, Value>,
     {
-        let mut val = *self.globals.get(&cid).expect("global not cached");
-        if !val.mutable {
+        let mut val = self.globals.get(&cid).expect("global not cached").clone();
+        if val.mutable == Mutability::Immutable {
             return Err(EvalError::ModifiedConstantMemory);
         }
         val.value = f(self, val.value)?;
@@ -1686,7 +1689,7 @@ pub fn eval_main<'a, 'tcx: 'a>(
             }
 
             // Return value
-            let ret_ptr = ecx.memory.allocate(ecx.tcx.data_layout.pointer_size.bytes(), ecx.tcx.data_layout.pointer_align.abi())?;
+            let ret_ptr = ecx.memory.allocate(ecx.tcx.data_layout.pointer_size.bytes(), ecx.tcx.data_layout.pointer_align.abi(), ::memory::Kind::Stack)?;
             cleanup_ptr = Some(ret_ptr);
 
             // Push our stack frame
@@ -1728,7 +1731,7 @@ pub fn eval_main<'a, 'tcx: 'a>(
 
         while ecx.step()? {}
         if let Some(cleanup_ptr) = cleanup_ptr {
-            ecx.memory.deallocate(cleanup_ptr, None)?;
+            ecx.memory.deallocate(cleanup_ptr, None, ::memory::Kind::Stack)?;
         }
         return Ok(());
     }

src/lvalue.rs

@@ -2,6 +2,7 @@ use rustc::mir;
 use rustc::ty::layout::{Size, Align};
 use rustc::ty::{self, Ty};
 use rustc_data_structures::indexed_vec::Idx;
+use syntax::ast::Mutability;
 
 use error::{EvalError, EvalResult};
 use eval_context::EvalContext;
@@ -51,15 +52,15 @@ pub struct GlobalId<'tcx> {
     pub(super) promoted: Option<mir::Promoted>,
 }
 
-#[derive(Copy, Clone, Debug)]
+#[derive(Clone, Debug)]
 pub struct Global<'tcx> {
     pub(super) value: Value,
     /// Only used in `force_allocation` to ensure we don't mark the memory
     /// before the static is initialized. It is possible to convert a
     /// global which initially is `Value::ByVal(PrimVal::Undef)` and gets
     /// lifted to an allocation before the static is fully initialized
     pub(super) initialized: bool,
-    pub(super) mutable: bool,
+    pub(super) mutable: Mutability,
     pub(super) ty: Ty<'tcx>,
 }
 
@@ -113,13 +114,13 @@ impl<'tcx> Global<'tcx> {
     pub(super) fn uninitialized(ty: Ty<'tcx>) -> Self {
         Global {
             value: Value::ByVal(PrimVal::Undef),
-            mutable: true,
+            mutable: Mutability::Mutable,
             ty,
             initialized: false,
         }
     }
 
-    pub(super) fn initialized(ty: Ty<'tcx>, value: Value, mutable: bool) -> Self {
+    pub(super) fn initialized(ty: Ty<'tcx>, value: Value, mutable: Mutability) -> Self {
         Global {
             value,
             mutable,