validate size and alignment on reallocate and deallocate

src/eval_context.rs

@@ -410,7 +410,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         if let Some(Value::ByRef(ptr)) = local {
             trace!("deallocating local");
             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) => {},
@@ -1724,7 +1724,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,10 +223,10 @@ 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 {
+        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) {
@@ -234,12 +234,15 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
         }
 
         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);
@@ -247,7 +250,6 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
             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();
@@ -267,21 +269,28 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
     }
 
     // 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::DeallocateNonBasePtr);
         }
-        if self.get(ptr.alloc_id).ok().map_or(false, |alloc| alloc.static_kind != StaticKind::NotStatic) {
-            return Err(EvalError::DeallocatedStaticMemory);
+
+        {
+            // Some code somewhere seems to rely on us *not* removing the allocation when we yield this kind of error.
+            // 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);
+                }
+            }
         }
 
-        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);
-            return Err(EvalError::DeallocateNonBasePtr);
-        }
+        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(value_copy, &value)?;
                     self.memory.write_bytes(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);
+    }
+}

tests/compile-fail/deallocate-bad-size.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);
+    }
+}

tests/compile-fail/reallocate-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);
+        let _y = reallocate(x, 1, 1, 2);
+    }
+}

tests/compile-fail/reallocate-bad-size.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);
+        let _y = reallocate(x, 2, 1, 1);
+    }
+}