WIP: Update for rustc validation fixes

rust-toolchain

@@ -1 +1 @@
-nightly-2018-10-10
+nightly-2018-10-11

src/fn_call.rs

@@ -252,12 +252,11 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, '
                 // Now we make a function call.  TODO: Consider making this re-usable?  EvalContext::step does sth. similar for the TLS dtors,
                 // and of course eval_main.
                 let mir = self.load_mir(f_instance.def)?;
-                let closure_dest = Place::null(&self);
                 self.push_stack_frame(
                     f_instance,
                     mir.span,
                     mir,
-                    closure_dest,
+                    None,
                     StackPopCleanup::Goto(Some(ret)), // directly return to caller
                 )?;
                 let mut args = self.frame().mir.args_iter();

src/intrinsic.rs

@@ -238,7 +238,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             "init" => {
                 // Check fast path: we don't want to force an allocation in case the destination is a simple value,
                 // but we also do not want to create a new allocation with 0s and then copy that over.
-                if !dest.layout.is_zst() { // notzhing to do for ZST
+                // FIXME: We do not properly validate in case of ZSTs and when doing it in memory!
+                // However, this only affects direct calls of the intrinsic; calls to the stable
+                // functions wrapping them do get their validation.
+                if !dest.layout.is_zst() { // nothing to do for ZST
                     match dest.layout.abi {
                         layout::Abi::Scalar(ref s) => {
                             let x = Scalar::from_int(0, s.value.size(&self));
@@ -252,7 +255,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
                         _ => {
                             // Do it in memory
                             let mplace = self.force_allocation(dest)?;
-                            assert!(mplace.extra.is_none());
+                            assert!(mplace.meta.is_none());
                             self.memory.write_repeat(mplace.ptr, 0, dest.layout.size)?;
                         }
                     }
@@ -338,7 +341,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
 
             "size_of_val" => {
                 let mplace = self.ref_to_mplace(self.read_value(args[0])?)?;
-                let (size, _) = self.size_and_align_of_mplace(mplace)?;
+                let (size, _) = self.size_and_align_of_mplace(mplace)?
+                    .expect("size_of_val called on extern type");
                 let ptr_size = self.pointer_size();
                 self.write_scalar(
                     Scalar::from_uint(size.bytes() as u128, ptr_size),
@@ -349,7 +353,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             "min_align_of_val" |
             "align_of_val" => {
                 let mplace = self.ref_to_mplace(self.read_value(args[0])?)?;
-                let (_, align) = self.size_and_align_of_mplace(mplace)?;
+                let (_, align) = self.size_and_align_of_mplace(mplace)?
+                    .expect("size_of_val called on extern type");
                 let ptr_size = self.pointer_size();
                 self.write_scalar(
                     Scalar::from_uint(align.abi(), ptr_size),
@@ -397,6 +402,9 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             "uninit" => {
                 // Check fast path: we don't want to force an allocation in case the destination is a simple value,
                 // but we also do not want to create a new allocation with 0s and then copy that over.
+                // FIXME: We do not properly validate in case of ZSTs and when doing it in memory!
+                // However, this only affects direct calls of the intrinsic; calls to the stable
+                // functions wrapping them do get their validation.
                 if !dest.layout.is_zst() { // nothing to do for ZST
                     match dest.layout.abi {
                         layout::Abi::Scalar(..) => {
@@ -410,7 +418,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
                         _ => {
                             // Do it in memory
                             let mplace = self.force_allocation(dest)?;
-                            assert!(mplace.extra.is_none());
+                            assert!(mplace.meta.is_none());
                             self.memory.mark_definedness(mplace.ptr.to_ptr()?, dest.layout.size, false)?;
                         }
                     }

src/lib.rs

@@ -13,6 +13,9 @@ extern crate rustc_mir;
 extern crate rustc_target;
 extern crate syntax;
 
+use std::collections::HashMap;
+use std::borrow::Cow;
+
 use rustc::ty::{self, TyCtxt, query::TyCtxtAt};
 use rustc::ty::layout::{TyLayout, LayoutOf, Size};
 use rustc::hir::def_id::DefId;
@@ -21,11 +24,10 @@ use rustc::mir;
 use syntax::ast::Mutability;
 use syntax::attr;
 
-use std::collections::HashMap;
 
 pub use rustc::mir::interpret::*;
 pub use rustc_mir::interpret::*;
-pub use rustc_mir::interpret;
+pub use rustc_mir::interpret::{self, AllocMap}; // resolve ambiguity
 
 mod fn_call;
 mod operator;
@@ -34,13 +36,15 @@ mod helpers;
 mod tls;
 mod locks;
 mod range_map;
+mod mono_hash_map;
 
 use fn_call::EvalContextExt as MissingFnsEvalContextExt;
 use operator::EvalContextExt as OperatorEvalContextExt;
 use intrinsic::EvalContextExt as IntrinsicEvalContextExt;
 use tls::{EvalContextExt as TlsEvalContextExt, TlsData};
 use range_map::RangeMap;
 use helpers::FalibleScalarExt;
+use mono_hash_map::MonoHashMap;
 
 pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
@@ -63,7 +67,6 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
                 .to_owned(),
         ));
     }
-    let ptr_size = ecx.memory.pointer_size();
 
     if let Some(start_id) = start_wrapper {
         let main_ret_ty = ecx.tcx.fn_sig(main_id).output();
@@ -85,16 +88,15 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
         }
 
         // Return value (in static memory so that it does not count as leak)
-        let size = ecx.tcx.data_layout.pointer_size;
-        let align = ecx.tcx.data_layout.pointer_align;
-        let ret_ptr = ecx.memory_mut().allocate(size, align, MiriMemoryKind::MutStatic.into())?;
+        let ret = ecx.layout_of(start_mir.return_ty())?;
+        let ret_ptr = ecx.allocate(ret, MiriMemoryKind::MutStatic.into())?;
 
         // Push our stack frame
         ecx.push_stack_frame(
             start_instance,
             start_mir.span,
             start_mir,
-            Place::from_ptr(ret_ptr, align),
+            Some(ret_ptr.into()),
             StackPopCleanup::None { cleanup: true },
         )?;
 
@@ -122,11 +124,12 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
 
         assert!(args.next().is_none(), "start lang item has more arguments than expected");
     } else {
+        let ret_place = MPlaceTy::dangling(ecx.layout_of(tcx.mk_unit())?, &ecx).into();
         ecx.push_stack_frame(
             main_instance,
             main_mir.span,
             main_mir,
-            Place::from_scalar_ptr(Scalar::from_int(1, ptr_size).into(), ty::layout::Align::from_bytes(1, 1).unwrap()),
+            Some(ret_place),
             StackPopCleanup::None { cleanup: true },
         )?;
 
@@ -231,9 +234,16 @@ pub struct Evaluator<'tcx> {
 impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
     type MemoryData = ();
     type MemoryKinds = MiriMemoryKind;
+    type PointerTag = (); // still WIP
+
+    type MemoryMap = MonoHashMap<AllocId, (MemoryKind<MiriMemoryKind>, Allocation<()>)>;
 
-    const MUT_STATIC_KIND: Option<MiriMemoryKind> = Some(MiriMemoryKind::MutStatic);
-    const ENFORCE_VALIDITY: bool = false; // this is still WIP
+    const STATIC_KIND: Option<MiriMemoryKind> = Some(MiriMemoryKind::MutStatic);
+
+    #[inline(always)]
+    fn enforce_validity(_ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool {
+        false // this is still WIP
+    }
 
     /// Returns Ok() when the function was handled, fail otherwise
     fn find_fn(
@@ -279,7 +289,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
             malloc,
             malloc_mir.span,
             malloc_mir,
-            *dest,
+            Some(dest),
             // Don't do anything when we are done.  The statement() function will increment
             // the old stack frame's stmt counter to the next statement, which means that when
             // exchange_malloc returns, we go on evaluating exactly where we want to be.
@@ -308,7 +318,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
     fn find_foreign_static(
         tcx: TyCtxtAt<'a, 'tcx, 'tcx>,
         def_id: DefId,
-    ) -> EvalResult<'tcx, &'tcx Allocation> {
+    ) -> EvalResult<'tcx, Cow<'tcx, Allocation>> {
         let attrs = tcx.get_attrs(def_id);
         let link_name = match attr::first_attr_value_str_by_name(&attrs, "link_name") {
             Some(name) => name.as_str(),
@@ -319,14 +329,13 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
             "__cxa_thread_atexit_impl" => {
                 // This should be all-zero, pointer-sized
                 let data = vec![0; tcx.data_layout.pointer_size.bytes() as usize];
-                let alloc = Allocation::from_bytes(&data[..], tcx.data_layout.pointer_align);
-                tcx.intern_const_alloc(alloc)
+                Allocation::from_bytes(&data[..], tcx.data_layout.pointer_align)
             }
             _ => return err!(Unimplemented(
                     format!("can't access foreign static: {}", link_name),
                 )),
         };
-        Ok(alloc)
+        Ok(Cow::Owned(alloc))
     }
 
     fn validation_op(
@@ -344,4 +353,11 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
         // We are not interested in detecting loops
         Ok(())
     }
+
+    fn static_with_default_tag(
+        alloc: &'_ Allocation
+    ) -> Cow<'_, Allocation<Self::PointerTag>> {
+        let alloc = alloc.clone();
+        Cow::Owned(alloc)
+    }
 }

src/mono_hash_map.rs

@@ -0,0 +1,91 @@
+//! This is a "monotonic HashMap": A HashMap that, when shared, can be pushed to but not
+//! otherwise mutated.  We also Box items in the map. This means we can safely provide
+//! shared references into existing items in the HashMap, because they will not be dropped
+//! (from being removed) or moved (because they are boxed).
+//! The API is is completely tailored to what `memory.rs` needs. It is still in
+//! a separate file to minimize the amount of code that has to care about the unsafety.
+
+use std::collections::hash_map::Entry;
+use std::cell::RefCell;
+use std::hash::Hash;
+use std::borrow::Borrow;
+
+use rustc_data_structures::fx::FxHashMap;
+
+use super::AllocMap;
+
+#[derive(Debug, Clone)]
+pub struct MonoHashMap<K: Hash + Eq, V>(RefCell<FxHashMap<K, Box<V>>>);
+
+impl<K: Hash + Eq, V> Default for MonoHashMap<K, V> {
+    fn default() -> Self {
+        MonoHashMap(RefCell::new(Default::default()))
+    }
+}
+
+impl<K: Hash + Eq, V> AllocMap<K, V> for MonoHashMap<K, V> {
+    #[inline(always)]
+    fn contains_key<Q: ?Sized + Hash + Eq>(&mut self, k: &Q) -> bool
+        where K: Borrow<Q>
+    {
+        self.0.get_mut().contains_key(k)
+    }
+
+    #[inline(always)]
+    fn insert(&mut self, k: K, v: V) -> Option<V>
+    {
+        self.0.get_mut().insert(k, Box::new(v)).map(|x| *x)
+    }
+
+    #[inline(always)]
+    fn remove<Q: ?Sized + Hash + Eq>(&mut self, k: &Q) -> Option<V>
+        where K: Borrow<Q>
+    {
+        self.0.get_mut().remove(k).map(|x| *x)
+    }
+
+    #[inline(always)]
+    fn filter_map_collect<T>(&self, mut f: impl FnMut(&K, &V) -> Option<T>) -> Vec<T> {
+        self.0.borrow()
+            .iter()
+            .filter_map(move |(k, v)| f(k, &*v))
+            .collect()
+    }
+
+    /// The most interesting method: Providing a shared ref without
+    /// holding the `RefCell` open, and inserting new data if the key
+    /// is not used yet.
+    /// `vacant` is called if the key is not found in the map;
+    /// if it returns a reference, that is used directly, if it
+    /// returns owned data, that is put into the map and returned.
+    #[inline(always)]
+    fn get_or<E>(
+        &self,
+        k: K,
+        vacant: impl FnOnce() -> Result<V, E>
+    ) -> Result<&V, E> {
+        let val: *const V = match self.0.borrow_mut().entry(k) {
+            Entry::Occupied(entry) => &**entry.get(),
+            Entry::Vacant(entry) => &**entry.insert(Box::new(vacant()?)),
+        };
+        // This is safe because `val` points into a `Box`, that we know will not move and
+        // will also not be dropped as long as the shared reference `self` is live.
+        unsafe { Ok(&*val) }
+    }
+
+    #[inline(always)]
+    fn get_mut_or<E>(
+        &mut self,
+        k: K,
+        vacant: impl FnOnce() -> Result<V, E>
+    ) -> Result<&mut V, E>
+    {
+        match self.0.get_mut().entry(k) {
+            Entry::Occupied(e) => Ok(e.into_mut()),
+            Entry::Vacant(e) => {
+                let v = vacant()?;
+                Ok(e.insert(Box::new(v)))
+            }
+        }
+    }
+}

src/tls.rs

@@ -1,9 +1,12 @@
 use std::collections::BTreeMap;
 
+use rustc_target::abi::LayoutOf;
 use rustc::{ty, ty::layout::HasDataLayout, mir};
 
-use super::{EvalResult, EvalErrorKind, Scalar, Evaluator,
-            Place, StackPopCleanup, EvalContext};
+use super::{
+    EvalResult, EvalErrorKind, StackPopCleanup, EvalContext, Evaluator,
+    MPlaceTy, Scalar,
+};
 
 pub type TlsKey = u128;
 
@@ -139,12 +142,12 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx> for EvalContext<'a, 'mir, '
             // TODO: Potentially, this has to support all the other possible instances?
             // See eval_fn_call in interpret/terminator/mod.rs
             let mir = self.load_mir(instance.def)?;
-            let ret = Place::null(&self);
+            let ret_place = MPlaceTy::dangling(self.layout_of(self.tcx.mk_unit())?, &self).into();
             self.push_stack_frame(
                 instance,
                 mir.span,
                 mir,
-                ret,
+                Some(ret_place),
                 StackPopCleanup::None { cleanup: true },
             )?;
             let arg_local = self.frame().mir.args_iter().next().ok_or_else(

tests/run-pass/ref-invalid-ptr.rs

@@ -1,7 +1,9 @@
 fn main() {
     let x = 2usize as *const u32;
+    // this is not aligned, but we immediately cast it to a raw ptr so that must be okay
     let _y = unsafe { &*x as *const u32 };
 
     let x = 0usize as *const u32;
+    // this is NULL, but we immediately cast it to a raw ptr so that must be okay
     let _y = unsafe { &*x as *const u32 };
 }