Merge pull request rust-lang#212 from oli-obk/zero_sense_types

Remove the zst allocation

Author: oli-obk

src/error.rs

@@ -59,6 +59,8 @@ pub enum EvalError<'tcx> {
     ReallocatedStaticMemory,
     DeallocatedStaticMemory,
     Layout(layout::LayoutError<'tcx>),
+    HeapAllocZeroBytes,
+    HeapAllocNonPowerOfTwoAlignment(u64),
     Unreachable,
     Panic,
 }
@@ -89,7 +91,7 @@ impl<'tcx> Error for EvalError<'tcx> {
             EvalError::ReadBytesAsPointer =>
                 "a memory access tried to interpret some bytes as a pointer",
             EvalError::InvalidPointerMath =>
-                "attempted to do invalid arithmetic on pointers that would leak base addresses, e.g. compating pointers into different allocations",
+                "attempted to do invalid arithmetic on pointers that would leak base addresses, e.g. comparing pointers into different allocations",
             EvalError::ReadUndefBytes =>
                 "attempted to read undefined bytes",
             EvalError::DeadLocal =>
@@ -146,6 +148,10 @@ impl<'tcx> Error for EvalError<'tcx> {
                 "rustc layout computation failed",
             EvalError::UnterminatedCString(_) =>
                 "attempted to get length of a null terminated string, but no null found before end of allocation",
+            EvalError::HeapAllocZeroBytes =>
+                "tried to re-, de- or allocate zero bytes on the heap",
+            EvalError::HeapAllocNonPowerOfTwoAlignment(_) =>
+                "tried to re-, de-, or allocate heap memory with alignment that is not a power of two",
             EvalError::Unreachable =>
                 "entered unreachable code",
             EvalError::Panic =>

src/eval_context.rs

@@ -362,15 +362,19 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             StackPopCleanup::None => {},
             StackPopCleanup::Tls(key) => {
                 // either fetch the next dtor or start new from the beginning, if any are left with a non-null data
-                if let Some((instance, ptr, key)) = self.memory.fetch_tls_dtor(key).or_else(|| self.memory.fetch_tls_dtor(None)) {
+                let dtor = match self.memory.fetch_tls_dtor(key)? {
+                    dtor @ Some(_) => dtor,
+                    None => self.memory.fetch_tls_dtor(None)?,
+                };
+                if let Some((instance, ptr, key)) = dtor {
                     trace!("Running TLS dtor {:?} on {:?}", instance, ptr);
                     // TODO: Potentially, this has to support all the other possible instances? See eval_fn_call in terminator/mod.rs
                     let mir = self.load_mir(instance.def)?;
                     self.push_stack_frame(
                         instance,
                         mir.span,
                         mir,
-                        Lvalue::zst(),
+                        Lvalue::undef(),
                         StackPopCleanup::Tls(Some(key)),
                     )?;
                     let arg_local = self.frame().mir.args_iter().next().ok_or(EvalError::AbiViolation("TLS dtor does not take enough arguments.".to_owned()))?;
@@ -673,8 +677,14 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             }
 
             NullaryOp(mir::NullOp::Box, ty) => {
-                let ptr = self.alloc_ptr(ty)?;
-                self.write_primval(dest, PrimVal::Ptr(ptr), dest_ty)?;
+                // FIXME: call the `exchange_malloc` lang item if available
+                if self.type_size(ty)?.expect("box only works with sized types") == 0 {
+                    let align = self.type_align(ty)?;
+                    self.write_primval(dest, PrimVal::Bytes(align.into()), dest_ty)?;
+                } else {
+                    let ptr = self.alloc_ptr(ty)?;
+                    self.write_primval(dest, PrimVal::Ptr(ptr), dest_ty)?;
+                }
             }
 
             NullaryOp(mir::NullOp::SizeOf, ty) => {
@@ -904,11 +914,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         let pointee_size = self.type_size(pointee_ty)?.expect("cannot offset a pointer to an unsized type") as i64;
         return if let Some(offset) = offset.checked_mul(pointee_size) {
             let ptr = ptr.signed_offset(offset, self.memory.layout)?;
-            // Do not do bounds-checking for integers or ZST; they can never alias a normal pointer anyway.
+            // Do not do bounds-checking for integers; they can never alias a normal pointer anyway.
             if let PrimVal::Ptr(ptr) = ptr {
-                if !(ptr.points_to_zst() && (offset == 0 || pointee_size == 0)) {
-                    self.memory.check_bounds(ptr, false)?;
-                }
+                self.memory.check_bounds(ptr, false)?;
             } else if ptr.is_null()? {
                 // We moved *to* a NULL pointer.  That seems wrong, LLVM considers the NULL pointer its own small allocation.  Reject this, for now.
                 return Err(EvalError::NullPointerOutOfBounds);
@@ -1697,7 +1705,7 @@ pub fn eval_main<'a, 'tcx: 'a>(
                 main_instance,
                 main_mir.span,
                 main_mir,
-                Lvalue::zst(),
+                Lvalue::undef(),
                 StackPopCleanup::Tls(None),
             )?;
         }

src/lvalue.rs

@@ -8,7 +8,7 @@ use eval_context::{EvalContext};
 use memory::Pointer;
 use value::{PrimVal, Value};
 
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+#[derive(Copy, Clone, Debug)]
 pub enum Lvalue<'tcx> {
     /// An lvalue referring to a value allocated in the `Memory` system.
     Ptr {
@@ -73,10 +73,6 @@ impl<'tcx> Lvalue<'tcx> {
         Lvalue::Ptr { ptr, extra: LvalueExtra::None }
     }
 
-    pub fn zst() -> Self {
-        Self::from_ptr(Pointer::zst_ptr())
-    }
-
     pub fn from_ptr(ptr: Pointer) -> Self {
         Self::from_primval_ptr(PrimVal::Ptr(ptr))
     }

src/memory.rs

@@ -81,14 +81,6 @@ impl Pointer {
     pub fn offset<'tcx>(self, i: u64, layout: &TargetDataLayout) -> EvalResult<'tcx, Self> {
         Ok(Pointer::new(self.alloc_id, value::offset(self.offset, i, layout)?))
     }
-
-    pub fn points_to_zst(&self) -> bool {
-        self.alloc_id == ZST_ALLOC_ID
-    }
-
-    pub fn zst_ptr() -> Self {
-        Pointer::new(ZST_ALLOC_ID, 0)
-    }
 }
 
 pub type TlsKey = usize;
@@ -157,15 +149,13 @@ pub struct Memory<'a, 'tcx> {
     next_thread_local: TlsKey,
 }
 
-const ZST_ALLOC_ID: AllocId = AllocId(0);
-
 impl<'a, 'tcx> Memory<'a, 'tcx> {
     pub fn new(layout: &'a TargetDataLayout, max_memory: u64) -> Self {
         Memory {
             alloc_map: HashMap::new(),
             functions: HashMap::new(),
             function_alloc_cache: HashMap::new(),
-            next_id: AllocId(2),
+            next_id: AllocId(0),
             layout,
             memory_size: max_memory,
             memory_usage: 0,
@@ -206,10 +196,8 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
     }
 
     pub fn allocate(&mut self, size: u64, align: u64) -> EvalResult<'tcx, Pointer> {
-        if size == 0 {
-            return Ok(Pointer::zst_ptr());
-        }
         assert_ne!(align, 0);
+        assert!(align.is_power_of_two());
 
         if self.memory_size - self.memory_usage < size {
             return Err(EvalError::OutOfMemory {
@@ -236,13 +224,11 @@ 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> {
+        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.points_to_zst() {
-            return self.allocate(new_size, align);
-        }
         if self.get(ptr.alloc_id).ok().map_or(false, |alloc| alloc.static_kind != StaticKind::NotStatic) {
             return Err(EvalError::ReallocatedStaticMemory);
         }
@@ -253,13 +239,15 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
             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();
@@ -271,9 +259,6 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
 
     // TODO(solson): See comment on `reallocate`.
     pub fn deallocate(&mut self, ptr: Pointer) -> EvalResult<'tcx> {
-        if ptr.points_to_zst() {
-            return Ok(());
-        }
         if ptr.offset != 0 {
             // TODO(solson): Report error about non-__rust_allocate'd pointer.
             return Err(EvalError::Unimplemented(format!("bad pointer offset: {}", ptr.offset)));
@@ -419,22 +404,22 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
     /// with associated destructors, implementations may stop calling destructors,
     /// or they may continue calling destructors until no non-NULL values with
     /// associated destructors exist, even though this might result in an infinite loop.
-    pub(crate) fn fetch_tls_dtor(&mut self, key: Option<TlsKey>) -> Option<(ty::Instance<'tcx>, PrimVal, TlsKey)> {
+    pub(crate) fn fetch_tls_dtor(&mut self, key: Option<TlsKey>) -> EvalResult<'tcx, Option<(ty::Instance<'tcx>, PrimVal, TlsKey)>> {
         use std::collections::Bound::*;
         let start = match key {
             Some(key) => Excluded(key),
             None => Unbounded,
         };
         for (&key, &mut TlsEntry { ref mut data, dtor }) in self.thread_local.range_mut((start, Unbounded)) {
-            if *data != PrimVal::Bytes(0) {
+            if !data.is_null()? {
                 if let Some(dtor) = dtor {
                     let ret = Some((dtor, *data, key));
                     *data = PrimVal::Bytes(0);
-                    return ret;
+                    return Ok(ret);
                 }
             }
         }
-        return None;
+        return Ok(None);
     }
 }
 
@@ -459,7 +444,6 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
             Some(alloc) => Ok(alloc),
             None => match self.functions.get(&id) {
                 Some(_) => Err(EvalError::DerefFunctionPointer),
-                None if id == ZST_ALLOC_ID => Err(EvalError::InvalidMemoryAccess),
                 None => Err(EvalError::DanglingPointerDeref),
             }
         }
@@ -474,7 +458,6 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
             },
             None => match self.functions.get(&id) {
                 Some(_) => Err(EvalError::DerefFunctionPointer),
-                None if id == ZST_ALLOC_ID => Err(EvalError::InvalidMemoryAccess),
                 None => Err(EvalError::DanglingPointerDeref),
             }
         }
@@ -508,7 +491,6 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
         let mut allocs_seen = HashSet::new();
 
         while let Some(id) = allocs_to_print.pop_front() {
-            if id == ZST_ALLOC_ID { continue; }
             let mut msg = format!("Alloc {:<5} ", format!("{}:", id));
             let prefix_len = msg.len();
             let mut relocations = vec![];
@@ -556,10 +538,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
                 for (i, target_id) in relocations {
                     // this `as usize` is fine, since we can't print more chars than `usize::MAX`
                     write!(msg, "{:1$}", "", ((i - pos) * 3) as usize).unwrap();
-                    let target = match target_id {
-                        ZST_ALLOC_ID => String::from("zst"),
-                        _ => format!("({})", target_id),
-                    };
+                    let target = format!("({})", target_id);
                     // this `as usize` is fine, since we can't print more chars than `usize::MAX`
                     write!(msg, "└{0:─^1$}┘ ", target, relocation_width as usize).unwrap();
                     pos = i + self.pointer_size();
@@ -637,7 +616,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
     /// mark an allocation as being the entry point to a static (see `static_alloc` field)
     pub fn mark_static(&mut self, alloc_id: AllocId) {
         trace!("mark_static: {:?}", alloc_id);
-        if alloc_id != ZST_ALLOC_ID && !self.static_alloc.insert(alloc_id) {
+        if !self.static_alloc.insert(alloc_id) {
             bug!("tried to mark an allocation ({:?}) as static twice", alloc_id);
         }
     }
@@ -667,7 +646,6 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
                 // mark recursively
                 mem::replace(relocations, Default::default())
             },
-            None if alloc_id == ZST_ALLOC_ID => return Ok(()),
             None if !self.functions.contains_key(&alloc_id) => return Err(EvalError::DanglingPointerDeref),
             _ => return Ok(()),
         };

src/operator.rs

@@ -159,10 +159,22 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 },
                 // These work on anything
                 Eq if left_kind == right_kind => {
-                    return Ok((PrimVal::from_bool(left == right), false));
+                    let result = match (left, right) {
+                        (PrimVal::Bytes(left), PrimVal::Bytes(right)) => left == right,
+                        (PrimVal::Ptr(left), PrimVal::Ptr(right)) => left == right,
+                        (PrimVal::Undef, _) | (_, PrimVal::Undef) => return Err(EvalError::ReadUndefBytes),
+                        _ => false,
+                    };
+                    return Ok((PrimVal::from_bool(result), false));
                 }
                 Ne if left_kind == right_kind => {
-                    return Ok((PrimVal::from_bool(left != right), false));
+                    let result = match (left, right) {
+                        (PrimVal::Bytes(left), PrimVal::Bytes(right)) => left != right,
+                        (PrimVal::Ptr(left), PrimVal::Ptr(right)) => left != right,
+                        (PrimVal::Undef, _) | (_, PrimVal::Undef) => return Err(EvalError::ReadUndefBytes),
+                        _ => true,
+                    };
+                    return Ok((PrimVal::from_bool(result), false));
                 }
                 // These need both pointers to be in the same allocation
                 Lt | Le | Gt | Ge | Sub

src/terminator/drop.rs

@@ -49,7 +49,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             instance,
             span,
             mir,
-            Lvalue::zst(),
+            Lvalue::undef(),
             StackPopCleanup::None,
         )?;
 

src/terminator/mod.rs

@@ -596,13 +596,25 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             "__rust_allocate" => {
                 let size = self.value_to_primval(args[0], usize)?.to_u64()?;
                 let align = self.value_to_primval(args[1], usize)?.to_u64()?;
+                if size == 0 {
+                    return Err(EvalError::HeapAllocZeroBytes);
+                }
+                if !align.is_power_of_two() {
+                    return Err(EvalError::HeapAllocNonPowerOfTwoAlignment(align));
+                }
                 let ptr = self.memory.allocate(size, align)?;
                 self.write_primval(dest, PrimVal::Ptr(ptr), dest_ty)?;
             }
 
             "__rust_allocate_zeroed" => {
                 let size = self.value_to_primval(args[0], usize)?.to_u64()?;
                 let align = self.value_to_primval(args[1], usize)?.to_u64()?;
+                if size == 0 {
+                    return Err(EvalError::HeapAllocZeroBytes);
+                }
+                if !align.is_power_of_two() {
+                    return Err(EvalError::HeapAllocNonPowerOfTwoAlignment(align));
+                }
                 let ptr = self.memory.allocate(size, align)?;
                 self.memory.write_repeat(ptr, 0, size)?;
                 self.write_primval(dest, PrimVal::Ptr(ptr), dest_ty)?;
@@ -611,15 +623,27 @@ 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()?;
+                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 {
+                    return Err(EvalError::HeapAllocZeroBytes);
+                }
+                if !align.is_power_of_two() {
+                    return Err(EvalError::HeapAllocNonPowerOfTwoAlignment(align));
+                }
                 self.memory.deallocate(ptr)?;
             },
 
             "__rust_reallocate" => {
                 let ptr = args[0].read_ptr(&self.memory)?.to_ptr()?;
                 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 {
+                    return Err(EvalError::HeapAllocZeroBytes);
+                }
+                if !align.is_power_of_two() {
+                    return Err(EvalError::HeapAllocNonPowerOfTwoAlignment(align));
+                }
                 let new_ptr = self.memory.reallocate(ptr, size, align)?;
                 self.write_primval(dest, PrimVal::Ptr(new_ptr), dest_ty)?;
             }
@@ -640,7 +664,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     f_instance,
                     mir.span,
                     mir,
-                    Lvalue::zst(),
+                    Lvalue::undef(),
                     StackPopCleanup::Goto(dest_block),
                 )?;
 

src/value.rs

@@ -42,7 +42,7 @@ pub enum Value {
 /// `memory::Allocation`. It is in many ways like a small chunk of a `Allocation`, up to 8 bytes in
 /// size. Like a range of bytes in an `Allocation`, a `PrimVal` can either represent the raw bytes
 /// of a simple value, a pointer into another `Allocation`, or be undefined.
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+#[derive(Clone, Copy, Debug)]
 pub enum PrimVal {
     /// The raw bytes of a simple value.
     Bytes(u128),

tests/compile-fail/zst.rs

@@ -1,4 +1,4 @@
 fn main() {
     let x = &() as *const () as *const i32;
-    let _ = unsafe { *x }; //~ ERROR: tried to access memory through an invalid pointer
+    let _ = unsafe { *x }; //~ ERROR: tried to access memory with alignment 1, but alignment 4 is required
 }