miri engine: Fix run-time validation

src/Cargo.lock

@@ -825,16 +825,6 @@ name = "getopts"
 version = "0.2.17"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
-[[package]]
-name = "getset"
-version = "0.0.6"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "proc-macro2 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
- "quote 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "syn 0.13.11 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
 [[package]]
 name = "git2"
 version = "0.7.5"
@@ -1303,7 +1293,7 @@ dependencies = [
  "compiletest_rs 0.3.13 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.5.12 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)",
- "vergen 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "vergen 3.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -3072,13 +3062,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "vergen"
-version = "2.0.0"
+version = "3.0.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "chrono 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)",
- "error-chain 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "getset 0.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "failure 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -3245,7 +3234,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 "checksum futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)" = "1a70b146671de62ec8c8ed572219ca5d594d9b06c0b364d5e67b722fc559b48c"
 "checksum fwdansi 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "34dd4c507af68d37ffef962063dfa1944ce0dd4d5b82043dbab1dabe088610c3"
 "checksum getopts 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)" = "b900c08c1939860ce8b54dc6a89e26e00c04c380fd0e09796799bd7f12861e05"
-"checksum getset 0.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "54c7f36a235738bb25904d6a2b3dbb28f6f5736cd3918c4bf80d6bb236200782"
 "checksum git2 0.7.5 (registry+https://github.com/rust-lang/crates.io-index)" = "591f8be1674b421644b6c030969520bc3fa12114d2eb467471982ed3e9584e71"
 "checksum git2-curl 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b502f6b1b467957403d168f0039e0c46fa6a1220efa2adaef25d5b267b5fe024"
 "checksum glob 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)" = "8be18de09a56b60ed0edf84bc9df007e30040691af7acd1c41874faac5895bfb"
@@ -3419,7 +3407,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 "checksum utf8-ranges 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "fd70f467df6810094968e2fce0ee1bd0e87157aceb026a8c083bcf5e25b9efe4"
 "checksum vcpkg 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)" = "def296d3eb3b12371b2c7d0e83bfe1403e4db2d7a0bba324a12b21c4ee13143d"
 "checksum vec_map 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "05c78687fb1a80548ae3250346c3db86a80a7cdd77bda190189f2d0a0987c81a"
-"checksum vergen 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9a16834fc61e1492c07dae49b6c14b55f8b1d43a5f5f9e9a2ecc063f47b9f93c"
+"checksum vergen 3.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "1b9696d96ec5d68984d060af80d7ba0ed4eb533978a0efb05ed4b8465f20d04f"
 "checksum version_check 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "7716c242968ee87e5542f8021178248f267f295a5c4803beae8b8b7fd9bc6051"
 "checksum void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d"
 "checksum wait-timeout 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "b9f3bf741a801531993db6478b95682117471f76916f5e690dd8d45395b09349"

src/librustc_mir/const_eval.rs

@@ -33,7 +33,7 @@ use rustc::mir::interpret::{
     Scalar, Allocation, AllocId, ConstValue,
 };
 use interpret::{self,
-    Place, PlaceTy, MemPlace, OpTy, Operand, Value,
+    PlaceTy, MemPlace, OpTy, Operand, Value,
     EvalContext, StackPopCleanup, MemoryKind,
     snapshot,
 };
@@ -55,13 +55,14 @@ pub fn mk_borrowck_eval_cx<'a, 'mir, 'tcx>(
     let param_env = tcx.param_env(instance.def_id());
     let mut ecx = EvalContext::new(tcx.at(span), param_env, CompileTimeInterpreter::new(), ());
     // insert a stack frame so any queries have the correct substs
+    // cannot use `push_stack_frame`; if we do `const_prop` explodes
     ecx.stack.push(interpret::Frame {
         block: mir::START_BLOCK,
         locals: IndexVec::new(),
         instance,
         span,
         mir,
-        return_place: Place::null(tcx),
+        return_place: None,
         return_to_block: StackPopCleanup::Goto(None), // never pop
         stmt: 0,
     });
@@ -82,7 +83,7 @@ pub fn mk_eval_cx<'a, 'tcx>(
         instance,
         mir.span,
         mir,
-        Place::null(tcx),
+        None,
         StackPopCleanup::Goto(None), // never pop
     )?;
     Ok(ecx)
@@ -187,7 +188,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
         cid.instance,
         mir.span,
         mir,
-        Place::Ptr(*ret),
+        Some(ret.into()),
         StackPopCleanup::None { cleanup: false },
     )?;
 
@@ -342,7 +343,11 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx>
     type MemoryMap = FxHashMap<AllocId, (MemoryKind<!>, Allocation<()>)>;
 
     const STATIC_KIND: Option<!> = None; // no copying of statics allowed
-    const ENFORCE_VALIDITY: bool = false; // for now, we don't
+
+    #[inline(always)]
+    fn enforce_validity(_ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool {
+        false // for now, we don't enforce validity
+    }
 
     fn find_fn(
         ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,

src/librustc_mir/interpret/cast.rs

@@ -322,7 +322,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 // For now, upcasts are limited to changes in marker
                 // traits, and hence never actually require an actual
                 // change to the vtable.
-                self.copy_op(src, dest)
+                let val = self.read_value(src)?;
+                self.write_value(*val, dest)
             }
             (_, &ty::Dynamic(ref data, _)) => {
                 // Initial cast from sized to dyn trait

src/librustc_mir/interpret/eval_context.rs

@@ -31,7 +31,7 @@ use rustc::mir::interpret::{
 use syntax::source_map::{self, Span};
 
 use super::{
-    Value, Operand, MemPlace, MPlaceTy, Place, ScalarMaybeUndef,
+    Value, Operand, MemPlace, MPlaceTy, Place, PlaceTy, ScalarMaybeUndef,
     Memory, Machine
 };
 
@@ -73,8 +73,9 @@ pub struct Frame<'mir, 'tcx: 'mir, Tag=()> {
     /// Work to perform when returning from this function
     pub return_to_block: StackPopCleanup,
 
-    /// The location where the result of the current stack frame should be written to.
-    pub return_place: Place<Tag>,
+    /// The location where the result of the current stack frame should be written to,
+    /// and its layout in the caller.
+    pub return_place: Option<PlaceTy<'tcx, Tag>>,
 
     /// The list of locals for this stack frame, stored in order as
     /// `[return_ptr, arguments..., variables..., temporaries...]`.
@@ -97,7 +98,8 @@ pub struct Frame<'mir, 'tcx: 'mir, Tag=()> {
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub enum StackPopCleanup {
     /// Jump to the next block in the caller, or cause UB if None (that's a function
-    /// that may never return).
+    /// that may never return). Also store layout of return place so
+    /// we can validate it at that layout.
     Goto(Option<mir::BasicBlock>),
     /// Just do nohing: Used by Main and for the box_alloc hook in miri.
     /// `cleanup` says whether locals are deallocated.  Static computation
@@ -330,22 +332,16 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
     }
 
     /// Return the actual dynamic size and alignment of the place at the given type.
-    /// Only the `meta` part of the place matters.
+    /// Only the "meta" (metadata) part of the place matters.
+    /// This can fail to provide an answer for extern types.
     pub(super) fn size_and_align_of(
         &self,
         metadata: Option<Scalar<M::PointerTag>>,
         layout: TyLayout<'tcx>,
-    ) -> EvalResult<'tcx, (Size, Align)> {
-        let metadata = match metadata {
-            None => {
-                assert!(!layout.is_unsized());
-                return Ok(layout.size_and_align())
-            }
-            Some(metadata) => {
-                assert!(layout.is_unsized());
-                metadata
-            }
-        };
+    ) -> EvalResult<'tcx, Option<(Size, Align)>> {
+        if !layout.is_unsized() {
+            return Ok(Some(layout.size_and_align()));
+        }
         match layout.ty.sty {
             ty::Adt(..) | ty::Tuple(..) => {
                 // First get the size of all statically known fields.
@@ -365,9 +361,11 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
                 );
 
                 // Recurse to get the size of the dynamically sized field (must be
-                // the last field).
+                // the last field).  Can't have foreign types here, how would we
+                // adjust alignment and size for them?
                 let field = layout.field(self, layout.fields.count() - 1)?;
-                let (unsized_size, unsized_align) = self.size_and_align_of(Some(metadata), field)?;
+                let (unsized_size, unsized_align) = self.size_and_align_of(metadata, field)?
+                    .expect("Fields cannot be extern types");
 
                 // FIXME (#26403, #27023): We should be adding padding
                 // to `sized_size` (to accommodate the `unsized_align`
@@ -394,18 +392,22 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
                 //
                 //   `(size + (align-1)) & -align`
 
-                Ok((size.abi_align(align), align))
+                Ok(Some((size.abi_align(align), align)))
             }
             ty::Dynamic(..) => {
-                let vtable = metadata.to_ptr()?;
+                let vtable = metadata.expect("dyn trait fat ptr must have vtable").to_ptr()?;
                 // the second entry in the vtable is the dynamic size of the object.
-                self.read_size_and_align_from_vtable(vtable)
+                Ok(Some(self.read_size_and_align_from_vtable(vtable)?))
             }
 
             ty::Slice(_) | ty::Str => {
-                let len = metadata.to_usize(self)?;
+                let len = metadata.expect("slice fat ptr must have vtable").to_usize(self)?;
                 let (elem_size, align) = layout.field(self, 0)?.size_and_align();
-                Ok((elem_size * len, align))
+                Ok(Some((elem_size * len, align)))
+            }
+
+            ty::Foreign(_) => {
+                Ok(None)
             }
 
             _ => bug!("size_and_align_of::<{:?}> not supported", layout.ty),
@@ -415,7 +417,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
     pub fn size_and_align_of_mplace(
         &self,
         mplace: MPlaceTy<'tcx, M::PointerTag>
-    ) -> EvalResult<'tcx, (Size, Align)> {
+    ) -> EvalResult<'tcx, Option<(Size, Align)>> {
         self.size_and_align_of(mplace.meta, mplace.layout)
     }
 
@@ -424,7 +426,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         instance: ty::Instance<'tcx>,
         span: source_map::Span,
         mir: &'mir mir::Mir<'tcx>,
-        return_place: Place<M::PointerTag>,
+        return_place: Option<PlaceTy<'tcx, M::PointerTag>>,
         return_to_block: StackPopCleanup,
     ) -> EvalResult<'tcx> {
         ::log_settings::settings().indentation += 1;
@@ -509,15 +511,38 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
             }
             StackPopCleanup::None { cleanup } => {
                 if !cleanup {
-                    // Leak the locals
+                    // Leak the locals. Also skip validation, this is only used by
+                    // static/const computation which does its own (stronger) final
+                    // validation.
                     return Ok(());
                 }
             }
         }
-        // deallocate all locals that are backed by an allocation
+        // Deallocate all locals that are backed by an allocation.
         for local in frame.locals {
             self.deallocate_local(local)?;
         }
+        // Validate the return value.
+        if let Some(return_place) = frame.return_place {
+            if M::enforce_validity(self) {
+                // Data got changed, better make sure it matches the type!
+                // It is still possible that the return place held invalid data while
+                // the function is running, but that's okay because nobody could have
+                // accessed that same data from the "outside" to observe any broken
+                // invariant -- that is, unless a function somehow has a ptr to
+                // its return place... but the way MIR is currently generated, the
+                // return place is always a local and then this cannot happen.
+                self.validate_operand(
+                    self.place_to_op(return_place)?,
+                    &mut vec![],
+                    None,
+                    /*const_mode*/false,
+                )?;
+            }
+        } else {
+            // Uh, that shouln't happen... the function did not intend to return
+            return err!(Unreachable);
+        }
 
         Ok(())
     }

src/librustc_mir/interpret/intrinsics.rs

@@ -151,11 +151,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 self.write_scalar(val, dest)?;
             }
             "transmute" => {
-                // Go through an allocation, to make sure the completely different layouts
-                // do not pose a problem.  (When the user transmutes through a union,
-                // there will not be a layout mismatch.)
-                let dest = self.force_allocation(dest)?;
-                self.copy_op(args[0], dest.into())?;
+                self.copy_op_transmute(args[0], dest)?;
             }
 
             _ => return Ok(false),

src/librustc_mir/interpret/machine.rs

@@ -86,7 +86,7 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
     const STATIC_KIND: Option<Self::MemoryKinds>;
 
     /// Whether to enforce the validity invariant
-    const ENFORCE_VALIDITY: bool;
+    fn enforce_validity(ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool;
 
     /// Called before a basic block terminator is executed.
     /// You can use this to detect endlessly running programs.

src/librustc_mir/interpret/memory.rs

@@ -650,7 +650,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
     }
 
     /// It is the caller's responsibility to handle undefined and pointer bytes.
-    /// However, this still checks that there are no relocations on the egdes.
+    /// However, this still checks that there are no relocations on the *egdes*.
     #[inline]
     fn get_bytes_with_undef_and_ptr(
         &self,
@@ -842,6 +842,29 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         }
     }
 
+    pub fn check_bytes(
+        &self,
+        ptr: Scalar<M::PointerTag>,
+        size: Size,
+        allow_ptr_and_undef: bool,
+    ) -> EvalResult<'tcx> {
+        // Empty accesses don't need to be valid pointers, but they should still be non-NULL
+        let align = Align::from_bytes(1, 1).unwrap();
+        if size.bytes() == 0 {
+            self.check_align(ptr, align)?;
+            return Ok(());
+        }
+        let ptr = ptr.to_ptr()?;
+        // Check bounds, align and relocations on the edges
+        self.get_bytes_with_undef_and_ptr(ptr, size, align)?;
+        // Check undef and ptr
+        if !allow_ptr_and_undef {
+            self.check_defined(ptr, size)?;
+            self.check_relocations(ptr, size)?;
+        }
+        Ok(())
+    }
+
     pub fn read_bytes(&self, ptr: Scalar<M::PointerTag>, size: Size) -> EvalResult<'tcx, &[u8]> {
         // Empty accesses don't need to be valid pointers, but they should still be non-NULL
         let align = Align::from_bytes(1, 1).unwrap();