Merge pull request rust-lang#234 from RalfJung/reallocate

Make Reallocate & Deallocate less permissive

Author: oli-obk

src/error.rs

@@ -58,6 +58,9 @@ pub enum EvalError<'tcx> {
     TypeNotPrimitive(Ty<'tcx>),
     ReallocatedStaticMemory,
     DeallocatedStaticMemory,
+    ReallocateNonBasePtr,
+    DeallocateNonBasePtr,
+    IncorrectAllocationInformation,
     Layout(layout::LayoutError<'tcx>),
     HeapAllocZeroBytes,
     HeapAllocNonPowerOfTwoAlignment(u64),
@@ -72,98 +75,105 @@ pub type EvalResult<'tcx, T = ()> = Result<T, EvalError<'tcx>>;
 
 impl<'tcx> Error for EvalError<'tcx> {
     fn description(&self) -> &str {
+        use EvalError::*;
         match *self {
-            EvalError::FunctionPointerTyMismatch(..) =>
+            FunctionPointerTyMismatch(..) =>
                 "tried to call a function through a function pointer of a different type",
-            EvalError::InvalidMemoryAccess =>
+            InvalidMemoryAccess =>
                 "tried to access memory through an invalid pointer",
-            EvalError::DanglingPointerDeref =>
+            DanglingPointerDeref =>
                 "dangling pointer was dereferenced",
-            EvalError::InvalidFunctionPointer =>
+            InvalidFunctionPointer =>
                 "tried to use an integer pointer or a dangling pointer as a function pointer",
-            EvalError::InvalidBool =>
+            InvalidBool =>
                 "invalid boolean value read",
-            EvalError::InvalidDiscriminant =>
+            InvalidDiscriminant =>
                 "invalid enum discriminant value read",
-            EvalError::PointerOutOfBounds { .. } =>
+            PointerOutOfBounds { .. } =>
                 "pointer offset outside bounds of allocation",
-            EvalError::InvalidNullPointerUsage =>
+            InvalidNullPointerUsage =>
                 "invalid use of NULL pointer",
-            EvalError::ReadPointerAsBytes =>
+            ReadPointerAsBytes =>
                 "a raw memory access tried to access part of a pointer value as raw bytes",
-            EvalError::ReadBytesAsPointer =>
+            ReadBytesAsPointer =>
                 "a memory access tried to interpret some bytes as a pointer",
-            EvalError::InvalidPointerMath =>
+            InvalidPointerMath =>
                 "attempted to do invalid arithmetic on pointers that would leak base addresses, e.g. comparing pointers into different allocations",
-            EvalError::ReadUndefBytes =>
+            ReadUndefBytes =>
                 "attempted to read undefined bytes",
-            EvalError::DeadLocal =>
+            DeadLocal =>
                 "tried to access a dead local variable",
-            EvalError::InvalidBoolOp(_) =>
+            InvalidBoolOp(_) =>
                 "invalid boolean operation",
-            EvalError::Unimplemented(ref msg) => msg,
-            EvalError::DerefFunctionPointer =>
+            Unimplemented(ref msg) => msg,
+            DerefFunctionPointer =>
                 "tried to dereference a function pointer",
-            EvalError::ExecuteMemory =>
+            ExecuteMemory =>
                 "tried to treat a memory pointer as a function pointer",
-            EvalError::ArrayIndexOutOfBounds(..) =>
+            ArrayIndexOutOfBounds(..) =>
                 "array index out of bounds",
-            EvalError::Math(..) =>
+            Math(..) =>
                 "mathematical operation failed",
-            EvalError::Intrinsic(..) =>
+            Intrinsic(..) =>
                 "intrinsic failed",
-            EvalError::OverflowingMath =>
+            OverflowingMath =>
                 "attempted to do overflowing math",
-            EvalError::NoMirFor(..) =>
+            NoMirFor(..) =>
                 "mir not found",
-            EvalError::InvalidChar(..) =>
+            InvalidChar(..) =>
                 "tried to interpret an invalid 32-bit value as a char",
-            EvalError::OutOfMemory{..} =>
+            OutOfMemory{..} =>
                 "could not allocate more memory",
-            EvalError::ExecutionTimeLimitReached =>
+            ExecutionTimeLimitReached =>
                 "reached the configured maximum execution time",
-            EvalError::StackFrameLimitReached =>
+            StackFrameLimitReached =>
                 "reached the configured maximum number of stack frames",
-            EvalError::OutOfTls =>
+            OutOfTls =>
                 "reached the maximum number of representable TLS keys",
-            EvalError::TlsOutOfBounds =>
+            TlsOutOfBounds =>
                 "accessed an invalid (unallocated) TLS key",
-            EvalError::AbiViolation(ref msg) => msg,
-            EvalError::AlignmentCheckFailed{..} =>
+            AbiViolation(ref msg) => msg,
+            AlignmentCheckFailed{..} =>
                 "tried to execute a misaligned read or write",
-            EvalError::CalledClosureAsFunction =>
+            CalledClosureAsFunction =>
                 "tried to call a closure through a function pointer",
-            EvalError::VtableForArgumentlessMethod =>
+            VtableForArgumentlessMethod =>
                 "tried to call a vtable function without arguments",
-            EvalError::ModifiedConstantMemory =>
+            ModifiedConstantMemory =>
                 "tried to modify constant memory",
-            EvalError::AssumptionNotHeld =>
+            AssumptionNotHeld =>
                 "`assume` argument was false",
-            EvalError::InlineAsm =>
+            InlineAsm =>
                 "miri does not support inline assembly",
-            EvalError::TypeNotPrimitive(_) =>
+            TypeNotPrimitive(_) =>
                 "expected primitive type, got nonprimitive",
-            EvalError::ReallocatedStaticMemory =>
+            ReallocatedStaticMemory =>
                 "tried to reallocate static memory",
-            EvalError::DeallocatedStaticMemory =>
+            DeallocatedStaticMemory =>
                 "tried to deallocate static memory",
-            EvalError::Layout(_) =>
+            ReallocateNonBasePtr =>
+                "tried to reallocate with a pointer not to the beginning of an existing object",
+            DeallocateNonBasePtr =>
+                "tried to deallocate with a pointer not to the beginning of an existing object",
+            IncorrectAllocationInformation =>
+                "tried to deallocate or reallocate using incorrect alignment or size",
+            Layout(_) =>
                 "rustc layout computation failed",
-            EvalError::UnterminatedCString(_) =>
+            UnterminatedCString(_) =>
                 "attempted to get length of a null terminated string, but no null found before end of allocation",
-            EvalError::HeapAllocZeroBytes =>
+            HeapAllocZeroBytes =>
                 "tried to re-, de- or allocate zero bytes on the heap",
-            EvalError::HeapAllocNonPowerOfTwoAlignment(_) =>
+            HeapAllocNonPowerOfTwoAlignment(_) =>
                 "tried to re-, de-, or allocate heap memory with alignment that is not a power of two",
-            EvalError::Unreachable =>
+            Unreachable =>
                 "entered unreachable code",
-            EvalError::Panic =>
+            Panic =>
                 "the evaluated program panicked",
-            EvalError::NeedsRfc(_) =>
+            NeedsRfc(_) =>
                 "this feature needs an rfc before being allowed inside constants",
-            EvalError::NotConst(_) =>
+            NotConst(_) =>
                 "this feature is not compatible with constant evaluation",
-            EvalError::ReadFromReturnPointer =>
+            ReadFromReturnPointer =>
                 "tried to read from the return pointer",
         }
     }
@@ -173,36 +183,37 @@ impl<'tcx> Error for EvalError<'tcx> {
 
 impl<'tcx> fmt::Display for EvalError<'tcx> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use EvalError::*;
         match *self {
-            EvalError::PointerOutOfBounds { ptr, access, allocation_size } => {
+            PointerOutOfBounds { ptr, access, allocation_size } => {
                 write!(f, "{} at offset {}, outside bounds of allocation {} which has size {}",
                        if access { "memory access" } else { "pointer computed" },
                        ptr.offset, ptr.alloc_id, allocation_size)
             },
-            EvalError::NoMirFor(ref func) => write!(f, "no mir for `{}`", func),
-            EvalError::FunctionPointerTyMismatch(sig, got) =>
+            NoMirFor(ref func) => write!(f, "no mir for `{}`", func),
+            FunctionPointerTyMismatch(sig, got) =>
                 write!(f, "tried to call a function with sig {} through a function pointer of type {}", sig, got),
-            EvalError::ArrayIndexOutOfBounds(span, len, index) =>
+            ArrayIndexOutOfBounds(span, len, index) =>
                 write!(f, "index out of bounds: the len is {} but the index is {} at {:?}", len, index, span),
-            EvalError::Math(span, ref err) =>
+            Math(span, ref err) =>
                 write!(f, "{:?} at {:?}", err, span),
-            EvalError::Intrinsic(ref err) =>
+            Intrinsic(ref err) =>
                 write!(f, "{}", err),
-            EvalError::InvalidChar(c) =>
+            InvalidChar(c) =>
                 write!(f, "tried to interpret an invalid 32-bit value as a char: {}", c),
-            EvalError::OutOfMemory { allocation_size, memory_size, memory_usage } =>
+            OutOfMemory { allocation_size, memory_size, memory_usage } =>
                 write!(f, "tried to allocate {} more bytes, but only {} bytes are free of the {} byte memory",
                        allocation_size, memory_size - memory_usage, memory_size),
-            EvalError::AlignmentCheckFailed { required, has } =>
+            AlignmentCheckFailed { required, has } =>
                write!(f, "tried to access memory with alignment {}, but alignment {} is required",
                       has, required),
-            EvalError::TypeNotPrimitive(ty) =>
+            TypeNotPrimitive(ty) =>
                 write!(f, "expected primitive type, got {}", ty),
-            EvalError::Layout(ref err) =>
+            Layout(ref err) =>
                 write!(f, "rustc layout computation failed: {:?}", err),
-            EvalError::NeedsRfc(ref msg) =>
+            NeedsRfc(ref msg) =>
                 write!(f, "\"{}\" needs an rfc before being allowed inside constants", msg),
-            EvalError::NotConst(ref msg) =>
+            NotConst(ref msg) =>
                 write!(f, "Cannot evaluate within constants: \"{}\"", msg),
             _ => write!(f, "{}", self.description()),
         }

src/eval_context.rs

@@ -412,7 +412,7 @@ 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) {
+            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) => {},
@@ -1714,7 +1714,7 @@ pub fn eval_main<'a, 'tcx: 'a>(
 
         while ecx.step()? {}
         if let Some(cleanup_ptr) = cleanup_ptr {
-            ecx.memory.deallocate(cleanup_ptr)?;
+            ecx.memory.deallocate(cleanup_ptr, None)?;
         }
         return Ok(());
     }

src/memory.rs

@@ -223,58 +223,74 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
 
     // TODO(solson): Track which allocations were returned from __rust_allocate and report an error
     // when reallocating/deallocating any others.
-    pub fn reallocate(&mut self, ptr: Pointer, new_size: u64, align: u64) -> EvalResult<'tcx, Pointer> {
+    pub fn reallocate(&mut self, ptr: Pointer, old_size: u64, new_size: u64, align: u64) -> EvalResult<'tcx, Pointer> {
         assert!(align.is_power_of_two());
         // TODO(solson): Report error about non-__rust_allocate'd pointer.
-        if ptr.offset != 0 {
-            return Err(EvalError::Unimplemented(format!("bad pointer offset: {}", ptr.offset)));
+        if ptr.offset != 0 || self.get(ptr.alloc_id).is_err() {
+            return Err(EvalError::ReallocateNonBasePtr);
         }
         if self.get(ptr.alloc_id).ok().map_or(false, |alloc| alloc.static_kind != StaticKind::NotStatic) {
             return Err(EvalError::ReallocatedStaticMemory);
         }
 
         let size = self.get(ptr.alloc_id)?.bytes.len() as u64;
+        let real_align = self.get(ptr.alloc_id)?.align;
+        if size != old_size || real_align != align {
+            return Err(EvalError::IncorrectAllocationInformation);
+        }
 
         if new_size > size {
             let amount = new_size - size;
             self.memory_usage += amount;
             let alloc = self.get_mut(ptr.alloc_id)?;
-            // FIXME: check alignment here
             assert_eq!(amount as usize as u64, amount);
             alloc.bytes.extend(iter::repeat(0).take(amount as usize));
             alloc.undef_mask.grow(amount, false);
         } else if size > new_size {
             self.memory_usage -= size - new_size;
             self.clear_relocations(ptr.offset(new_size, self.layout)?, size - new_size)?;
             let alloc = self.get_mut(ptr.alloc_id)?;
-            // FIXME: check alignment here
             // `as usize` is fine here, since it is smaller than `size`, which came from a usize
             alloc.bytes.truncate(new_size as usize);
             alloc.bytes.shrink_to_fit();
             alloc.undef_mask.truncate(new_size);
         }
 
-        Ok(Pointer::new(ptr.alloc_id, 0))
+        // Change allocation ID.  We do this after the above to be able to re-use methods like `clear_relocations`.
+        let id = {
+            let alloc = self.alloc_map.remove(&ptr.alloc_id).expect("We already used this pointer above");
+            let id = self.next_id;
+            self.next_id.0 += 1;
+            self.alloc_map.insert(id, alloc);
+            id
+        };
+
+        Ok(Pointer::new(id, 0))
     }
 
     // TODO(solson): See comment on `reallocate`.
-    pub fn deallocate(&mut self, ptr: Pointer) -> EvalResult<'tcx> {
-        if ptr.offset != 0 {
+    pub fn deallocate(&mut self, ptr: Pointer, size_and_align: Option<(u64, u64)>) -> EvalResult<'tcx> {
+        if ptr.offset != 0 || self.get(ptr.alloc_id).is_err() {
             // 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).ok().map_or(false, |alloc| alloc.static_kind != StaticKind::NotStatic) {
-            return Err(EvalError::DeallocatedStaticMemory);
+            return Err(EvalError::DeallocateNonBasePtr);
         }
 
-        if let Some(alloc) = self.alloc_map.remove(&ptr.alloc_id) {
-            self.memory_usage -= alloc.bytes.len() as u64;
-        } else {
-            debug!("deallocated a pointer twice: {}", ptr.alloc_id);
-            // TODO(solson): Report error about erroneous free. This is blocked on properly tracking
-            // already-dropped state since this if-statement is entered even in safe code without
-            // it.
+        {
+            // deallocate_local in eval_context.rs relies on nothing actually having changed when this error occurs.
+            // So we do this test in advance.
+            let alloc = self.get(ptr.alloc_id)?;
+            if alloc.static_kind != StaticKind::NotStatic {
+                return Err(EvalError::DeallocatedStaticMemory);
+            }
+            if let Some((size, align)) = size_and_align {
+                if size != alloc.bytes.len() as u64 || align != alloc.align {
+                    return Err(EvalError::IncorrectAllocationInformation);
+                }
+            }
         }
+
+        let alloc = self.alloc_map.remove(&ptr.alloc_id).expect("already verified");
+        self.memory_usage -= alloc.bytes.len() as u64;
         debug!("deallocated : {}", ptr.alloc_id);
 
         Ok(())

src/terminator/mod.rs

@@ -589,7 +589,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             "free" => {
                 let ptr = args[0].read_ptr(&self.memory)?;
                 if !ptr.is_null()? {
-                    self.memory.deallocate(ptr.to_ptr()?)?;
+                    self.memory.deallocate(ptr.to_ptr()?, None)?;
                 }
             }
 
@@ -638,7 +638,6 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
 
             "__rust_deallocate" => {
                 let ptr = args[0].read_ptr(&self.memory)?.to_ptr()?;
-                // FIXME: insert sanity check for size and align?
                 let old_size = self.value_to_primval(args[1], usize)?.to_u64()?;
                 let align = self.value_to_primval(args[2], usize)?.to_u64()?;
                 if old_size == 0 {
@@ -647,20 +646,21 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 if !align.is_power_of_two() {
                     return Err(EvalError::HeapAllocNonPowerOfTwoAlignment(align));
                 }
-                self.memory.deallocate(ptr)?;
+                self.memory.deallocate(ptr, Some((old_size, align)))?;
             },
 
             "__rust_reallocate" => {
                 let ptr = args[0].read_ptr(&self.memory)?.to_ptr()?;
+                let old_size = self.value_to_primval(args[1], usize)?.to_u64()?;
                 let size = self.value_to_primval(args[2], usize)?.to_u64()?;
                 let align = self.value_to_primval(args[3], usize)?.to_u64()?;
-                if size == 0 {
+                if old_size == 0 || size == 0 {
                     return Err(EvalError::HeapAllocZeroBytes);
                 }
                 if !align.is_power_of_two() {
                     return Err(EvalError::HeapAllocNonPowerOfTwoAlignment(align));
                 }
-                let new_ptr = self.memory.reallocate(ptr, size, align)?;
+                let new_ptr = self.memory.reallocate(ptr, old_size, size, align)?;
                 self.write_primval(dest, PrimVal::Ptr(new_ptr), dest_ty)?;
             }
 
@@ -768,7 +768,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 }
                 if let Some(old) = success {
                     if let Some(var) = old {
-                        self.memory.deallocate(var)?;
+                        self.memory.deallocate(var, None)?;
                     }
                     self.write_primval(dest, PrimVal::Bytes(0), dest_ty)?;
                 } else {
@@ -795,7 +795,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     self.memory.write_bytes(PrimVal::Ptr(value_copy), &value)?;
                     self.memory.write_bytes(PrimVal::Ptr(value_copy.offset(value.len() as u64, self.memory.layout)?), &[0])?;
                     if let Some(var) = self.env_vars.insert(name.to_owned(), value_copy) {
-                        self.memory.deallocate(var)?;
+                        self.memory.deallocate(var, None)?;
                     }
                     self.write_primval(dest, PrimVal::Bytes(0), dest_ty)?;
                 } else {

tests/compile-fail/deallocate-bad-alignment.rs

@@ -0,0 +1,13 @@
+#![feature(alloc, heap_api)]
+
+extern crate alloc;
+
+// error-pattern: tried to deallocate or reallocate using incorrect alignment or size
+
+use alloc::heap::*;
+fn main() {
+    unsafe {
+        let x = allocate(1, 1);
+        deallocate(x, 1, 2);
+    }
+}