Auto merge of #54955 - RalfJung:miri-validate2, r=oli-obk

miri engine: Fix run-time validation

This fixes all false positives that came up when actually enabling this in miri.

r? @oli-obk

Author: bors

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();

src/librustc_mir/interpret/place.rs

@@ -256,12 +256,6 @@ impl<'tcx, Tag: ::std::fmt::Debug> Place<Tag> {
 }
 
 impl<'tcx, Tag: ::std::fmt::Debug> PlaceTy<'tcx, Tag> {
-    /// Produces a Place that will error if attempted to be read from or written to
-    #[inline]
-    pub fn null(cx: impl HasDataLayout, layout: TyLayout<'tcx>) -> Self {
-        PlaceTy { place: Place::from_scalar_ptr(Scalar::ptr_null(cx), layout.align), layout }
-    }
-
     #[inline]
     pub fn to_mem_place(self) -> MPlaceTy<'tcx, Tag> {
         MPlaceTy { mplace: self.place.to_mem_place(), layout: self.layout }
@@ -325,9 +319,9 @@ where
         // Offset may need adjustment for unsized fields
         let (meta, offset) = if field_layout.is_unsized() {
             // re-use parent metadata to determine dynamic field layout
-            let (_, align) = self.size_and_align_of(base.meta, field_layout)?;
+            let (_, align) = self.size_and_align_of(base.meta, field_layout)?
+                .expect("Fields cannot be extern types");
             (base.meta, offset.abi_align(align))
-
         } else {
             // base.meta could be present; we might be accessing a sized field of an unsized
             // struct.
@@ -565,9 +559,15 @@ where
     ) -> EvalResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
         use rustc::mir::Place::*;
         let place = match *mir_place {
-            Local(mir::RETURN_PLACE) => PlaceTy {
-                place: self.frame().return_place,
-                layout: self.layout_of_local(self.cur_frame(), mir::RETURN_PLACE)?,
+            Local(mir::RETURN_PLACE) => match self.frame().return_place {
+                Some(return_place) =>
+                    // We use our layout to verify our assumption; caller will validate
+                    // their layout on return.
+                    PlaceTy {
+                        place: *return_place,
+                        layout: self.layout_of_local(self.cur_frame(), mir::RETURN_PLACE)?,
+                    },
+                None => return err!(InvalidNullPointerUsage),
             },
             Local(local) => PlaceTy {
                 place: Place::Local {
@@ -599,18 +599,47 @@ where
     }
 
     /// Write a value to a place
+    #[inline(always)]
     pub fn write_value(
         &mut self,
         src_val: Value<M::PointerTag>,
         dest: PlaceTy<'tcx, M::PointerTag>,
     ) -> EvalResult<'tcx> {
-        trace!("write_value: {:?} <- {:?}", *dest, src_val);
-        // Check that the value actually is okay for that type
-        if M::ENFORCE_VALIDITY {
-            // Something changed somewhere, better make sure it matches the type!
-            let op = OpTy { op: Operand::Immediate(src_val), layout: dest.layout };
-            self.validate_operand(op, &mut vec![], None, /*const_mode*/false)?;
+        self.write_value_no_validate(src_val, dest)?;
+
+        if M::enforce_validity(self) {
+            // Data got changed, better make sure it matches the type!
+            self.validate_operand(self.place_to_op(dest)?, &mut vec![], None, /*const_mode*/false)?;
+        }
+
+        Ok(())
+    }
+
+    /// Write a value to a place.
+    /// If you use this you are responsible for validating that things got copied at the
+    /// right type.
+    fn write_value_no_validate(
+        &mut self,
+        src_val: Value<M::PointerTag>,
+        dest: PlaceTy<'tcx, M::PointerTag>,
+    ) -> EvalResult<'tcx> {
+        if cfg!(debug_assertions) {
+            // This is a very common path, avoid some checks in release mode
+            assert!(!dest.layout.is_unsized(), "Cannot write unsized data");
+            match src_val {
+                Value::Scalar(ScalarMaybeUndef::Scalar(Scalar::Ptr(_))) =>
+                    assert_eq!(self.pointer_size(), dest.layout.size,
+                        "Size mismatch when writing pointer"),
+                Value::Scalar(ScalarMaybeUndef::Scalar(Scalar::Bits { size, .. })) =>
+                    assert_eq!(Size::from_bytes(size.into()), dest.layout.size,
+                        "Size mismatch when writing bits"),
+                Value::Scalar(ScalarMaybeUndef::Undef) => {}, // undef can have any size
+                Value::ScalarPair(_, _) => {
+                    // FIXME: Can we check anything here?
+                }
+            }
         }
+        trace!("write_value: {:?} <- {:?}: {}", *dest, src_val, dest.layout.ty);
 
         // See if we can avoid an allocation. This is the counterpart to `try_read_value`,
         // but not factored as a separate function.
@@ -627,15 +656,16 @@ where
             },
             Place::Ptr(mplace) => mplace, // already in memory
         };
+        let dest = MPlaceTy { mplace, layout: dest.layout };
 
         // This is already in memory, write there.
-        let dest = MPlaceTy { mplace, layout: dest.layout };
-        self.write_value_to_mplace(src_val, dest)
+        self.write_value_to_mplace_no_validate(src_val, dest)
     }
 
-    /// Write a value to memory. This does NOT do validation, so you better had already
-    /// done that before calling this!
-    fn write_value_to_mplace(
+    /// Write a value to memory.
+    /// If you use this you are responsible for validating that things git copied at the
+    /// right type.
+    fn write_value_to_mplace_no_validate(
         &mut self,
         value: Value<M::PointerTag>,
         dest: MPlaceTy<'tcx, M::PointerTag>,
@@ -653,8 +683,17 @@ where
         }
 
         let ptr = ptr.to_ptr()?;
+        // FIXME: We should check that there are dest.layout.size many bytes available in
+        // memory.  The code below is not sufficient, with enough padding it might not
+        // cover all the bytes!
         match value {
             Value::Scalar(scalar) => {
+                match dest.layout.abi {
+                    layout::Abi::Scalar(_) => {}, // fine
+                    _ => bug!("write_value_to_mplace: invalid Scalar layout: {:#?}",
+                            dest.layout)
+                }
+
                 self.memory.write_scalar(
                     ptr, ptr_align.min(dest.layout.align), scalar, dest.layout.size
                 )
@@ -670,45 +709,109 @@ where
                 let b_offset = a_size.abi_align(b_align);
                 let b_ptr = ptr.offset(b_offset, &self)?.into();
 
+                // It is tempting to verify `b_offset` against `layout.fields.offset(1)`,
+                // but that does not work: We could be a newtype around a pair, then the
+                // fields do not match the `ScalarPair` components.
+
                 self.memory.write_scalar(ptr, ptr_align.min(a_align), a_val, a_size)?;
                 self.memory.write_scalar(b_ptr, ptr_align.min(b_align), b_val, b_size)
             }
         }
     }
 
-    /// Copy the data from an operand to a place
+    /// Copy the data from an operand to a place.  This does not support transmuting!
+    /// Use `copy_op_transmute` if the layouts could disagree.
+    #[inline(always)]
     pub fn copy_op(
         &mut self,
         src: OpTy<'tcx, M::PointerTag>,
         dest: PlaceTy<'tcx, M::PointerTag>,
     ) -> EvalResult<'tcx> {
-        assert!(!src.layout.is_unsized() && !dest.layout.is_unsized(),
+        self.copy_op_no_validate(src, dest)?;
+
+        if M::enforce_validity(self) {
+            // Data got changed, better make sure it matches the type!
+            self.validate_operand(self.place_to_op(dest)?, &mut vec![], None, /*const_mode*/false)?;
+        }
+
+        Ok(())
+    }
+
+    /// Copy the data from an operand to a place.  This does not support transmuting!
+    /// Use `copy_op_transmute` if the layouts could disagree.
+    /// Also, if you use this you are responsible for validating that things git copied at the
+    /// right type.
+    fn copy_op_no_validate(
+        &mut self,
+        src: OpTy<'tcx, M::PointerTag>,
+        dest: PlaceTy<'tcx, M::PointerTag>,
+    ) -> EvalResult<'tcx> {
+        debug_assert!(!src.layout.is_unsized() && !dest.layout.is_unsized(),
             "Cannot copy unsized data");
-        assert_eq!(src.layout.size, dest.layout.size,
-            "Size mismatch when copying!\nsrc: {:#?}\ndest: {:#?}", src, dest);
+        // We do NOT compare the types for equality, because well-typed code can
+        // actually "transmute" `&mut T` to `&T` in an assignment without a cast.
+        assert!(src.layout.details == dest.layout.details,
+            "Layout mismatch when copying!\nsrc: {:#?}\ndest: {:#?}", src, dest);
 
         // Let us see if the layout is simple so we take a shortcut, avoid force_allocation.
-        let (src_ptr, src_align) = match self.try_read_value(src)? {
-            Ok(src_val) =>
-                // Yay, we got a value that we can write directly.  We write with the
-                // *source layout*, because that was used to load, and if they do not match
-                // this is a transmute we want to support.
-                return self.write_value(src_val, PlaceTy { place: *dest, layout: src.layout }),
-            Err(mplace) => mplace.to_scalar_ptr_align(),
+        let src = match self.try_read_value(src)? {
+            Ok(src_val) => {
+                // Yay, we got a value that we can write directly.
+                return self.write_value_no_validate(src_val, dest);
+            }
+            Err(mplace) => mplace,
         };
         // Slow path, this does not fit into an immediate. Just memcpy.
-        trace!("copy_op: {:?} <- {:?}", *dest, *src);
+        trace!("copy_op: {:?} <- {:?}: {}", *dest, src, dest.layout.ty);
+
         let dest = self.force_allocation(dest)?;
+        let (src_ptr, src_align) = src.to_scalar_ptr_align();
         let (dest_ptr, dest_align) = dest.to_scalar_ptr_align();
         self.memory.copy(
             src_ptr, src_align,
             dest_ptr, dest_align,
-            src.layout.size, false
+            dest.layout.size, false
+        )?;
+
+        Ok(())
+    }
+
+    /// Copy the data from an operand to a place.  The layouts may disagree, but they must
+    /// have the same size.
+    pub fn copy_op_transmute(
+        &mut self,
+        src: OpTy<'tcx, M::PointerTag>,
+        dest: PlaceTy<'tcx, M::PointerTag>,
+    ) -> EvalResult<'tcx> {
+        if src.layout.details == dest.layout.details {
+            // Fast path: Just use normal `copy_op`
+            return self.copy_op(src, dest);
+        }
+        // We still require the sizes to match
+        debug_assert!(!src.layout.is_unsized() && !dest.layout.is_unsized(),
+            "Cannot copy unsized data");
+        assert!(src.layout.size == dest.layout.size,
+            "Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}", src, dest);
+
+        // The hard case is `ScalarPair`.  `src` is already read from memory in this case,
+        // using `src.layout` to figure out which bytes to use for the 1st and 2nd field.
+        // We have to write them to `dest` at the offsets they were *read at*, which is
+        // not necessarily the same as the offsets in `dest.layout`!
+        // Hence we do the copy with the source layout on both sides.  We also make sure to write
+        // into memory, because if `dest` is a local we would not even have a way to write
+        // at the `src` offsets; the fact that we came from a different layout would
+        // just be lost.
+        let dest = self.force_allocation(dest)?;
+        self.copy_op_no_validate(
+            src,
+            PlaceTy::from(MPlaceTy { mplace: *dest, layout: src.layout }),
         )?;
-        if M::ENFORCE_VALIDITY {
-            // Something changed somewhere, better make sure it matches the type!
+
+        if M::enforce_validity(self) {
+            // Data got changed, better make sure it matches the type!
             self.validate_operand(dest.into(), &mut vec![], None, /*const_mode*/false)?;
         }
+
         Ok(())
     }
 
@@ -734,7 +837,7 @@ where
                         let ptr = self.allocate(local_layout, MemoryKind::Stack)?;
                         // We don't have to validate as we can assume the local
                         // was already valid for its type.
-                        self.write_value_to_mplace(value, ptr)?;
+                        self.write_value_to_mplace_no_validate(value, ptr)?;
                         let mplace = ptr.mplace;
                         // Update the local
                         *self.stack[frame].locals[local].access_mut()? =

src/librustc_mir/interpret/snapshot.rs

@@ -337,7 +337,7 @@ struct FrameSnapshot<'a, 'tcx: 'a> {
     instance: &'a ty::Instance<'tcx>,
     span: &'a Span,
     return_to_block: &'a StackPopCleanup,
-    return_place: Place<(), AllocIdSnapshot<'a>>,
+    return_place: Option<Place<(), AllocIdSnapshot<'a>>>,
     locals: IndexVec<mir::Local, LocalValue<(), AllocIdSnapshot<'a>>>,
     block: &'a mir::BasicBlock,
     stmt: usize,
@@ -362,7 +362,7 @@ impl<'a, 'mir, 'tcx: 'mir> HashStable<StableHashingContext<'a>> for Frame<'mir,
         } = self;
 
         (mir, instance, span, return_to_block).hash_stable(hcx, hasher);
-        (return_place, locals, block, stmt).hash_stable(hcx, hasher);
+        (return_place.as_ref().map(|r| &**r), locals, block, stmt).hash_stable(hcx, hasher);
     }
 }
 impl<'a, 'mir, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a Frame<'mir, 'tcx>
@@ -388,7 +388,7 @@ impl<'a, 'mir, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a Frame<'mir, 'tcx>
             return_to_block,
             block,
             stmt: *stmt,
-            return_place: return_place.snapshot(ctx),
+            return_place: return_place.map(|r| r.snapshot(ctx)),
             locals: locals.iter().map(|local| local.snapshot(ctx)).collect(),
         }
     }

src/librustc_mir/interpret/terminator.rs

@@ -17,7 +17,7 @@ use rustc_target::spec::abi::Abi;
 
 use rustc::mir::interpret::{EvalResult, PointerArithmetic, EvalErrorKind, Scalar};
 use super::{
-    EvalContext, Machine, Value, OpTy, Place, PlaceTy, Operand, StackPopCleanup
+    EvalContext, Machine, Value, OpTy, PlaceTy, MPlaceTy, Operand, StackPopCleanup
 };
 
 impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
@@ -39,7 +39,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         use rustc::mir::TerminatorKind::*;
         match terminator.kind {
             Return => {
-                self.dump_place(self.frame().return_place);
+                self.frame().return_place.map(|r| self.dump_place(*r));
                 self.pop_stack_frame()?
             }
 
@@ -222,7 +222,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         if !Self::check_argument_compat(caller_arg.layout, callee_arg.layout) {
             return err!(FunctionArgMismatch(caller_arg.layout.ty, callee_arg.layout.ty));
         }
-        self.copy_op(caller_arg, callee_arg)
+        // We allow some transmutes here
+        self.copy_op_transmute(caller_arg, callee_arg)
     }
 
     /// Call this function -- pushing the stack frame and initializing the arguments.
@@ -285,15 +286,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     None => return Ok(()),
                 };
 
-                let return_place = match dest {
-                    Some(place) => *place,
-                    None => Place::null(&self), // any access will error. good!
-                };
                 self.push_stack_frame(
                     instance,
                     span,
                     mir,
-                    return_place,
+                    dest,
                     StackPopCleanup::Goto(ret),
                 )?;
 
@@ -382,10 +379,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                             ));
                         }
                     } else {
-                        // FIXME: The caller thinks this function cannot return. How do
-                        // we verify that the callee agrees?
-                        // On the plus side, the the callee ever writes to its return place,
-                        // that will be detected as UB (because we set that to NULL above).
+                        let callee_layout =
+                            self.layout_of_local(self.cur_frame(), mir::RETURN_PLACE)?;
+                        if !callee_layout.abi.is_uninhabited() {
+                            return err!(FunctionRetMismatch(
+                                self.tcx.types.never, callee_layout.ty
+                            ));
+                        }
                     }
                     Ok(())
                 })();
@@ -451,14 +451,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         };
 
         let ty = self.tcx.mk_unit(); // return type is ()
-        let dest = PlaceTy::null(&self, self.layout_of(ty)?);
+        let dest = MPlaceTy::dangling(self.layout_of(ty)?, &self);
 
         self.eval_fn_call(
             instance,
             span,
             Abi::Rust,
             &[arg],
-            Some(dest),
+            Some(dest.into()),
             Some(target),
         )
     }

src/librustc_mir/interpret/validity.rs

@@ -208,7 +208,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 // for safe ptrs, also check the ptr values itself
                 if !ty.is_unsafe_ptr() {
                     // Make sure this is non-NULL and aligned
-                    let (size, align) = self.size_and_align_of(place.meta, place.layout)?;
+                    let (size, align) = self.size_and_align_of(place.meta, place.layout)?
+                        // for the purpose of validity, consider foreign types to have
+                        // alignment and size determined by the layout (size will be 0,
+                        // alignment should take attributes into account).
+                        .unwrap_or_else(|| place.layout.size_and_align());
                     match self.memory.check_align(place.ptr, align) {
                         Ok(_) => {},
                         Err(err) => match err.kind {
@@ -218,7 +222,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                                 return validation_failure!("unaligned reference", path),
                             _ =>
                                 return validation_failure!(
-                                    "dangling (deallocated) reference", path
+                                    "dangling (out-of-bounds) reference (might be NULL at \
+                                     run-time)",
+                                    path
                                 ),
                         }
                     }
@@ -490,9 +496,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     }
                     // Special handling for arrays/slices of builtin integer types
                     ty::Array(tys, ..) | ty::Slice(tys) if {
-                        // This optimization applies only for integer types
+                        // This optimization applies only for integer and floating point types
+                        // (i.e., types that can hold arbitrary bytes).
                         match tys.sty {
-                            ty::Int(..) | ty::Uint(..) => true,
+                            ty::Int(..) | ty::Uint(..) | ty::Float(..) => true,
                             _ => false,
                         }
                     } => {
@@ -504,9 +511,20 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                         // This is the size in bytes of the whole array.
                         let size = Size::from_bytes(ty_size * len);
 
-                        match self.memory.read_bytes(dest.ptr, size) {
+                        // In run-time mode, we accept pointers in here.  This is actually more
+                        // permissive than a per-element check would be, e.g. we accept
+                        // an &[u8] that contains a pointer even though bytewise checking would
+                        // reject it.  However, that's good: We don't inherently want
+                        // to reject those pointers, we just do not have the machinery to
+                        // talk about parts of a pointer.
+                        // We also accept undef, for consistency with the type-based checks.
+                        match self.memory.check_bytes(
+                            dest.ptr,
+                            size,
+                            /*allow_ptr_and_undef*/!const_mode,
+                        ) {
                             // In the happy case, we needn't check anything else.
-                            Ok(_) => {},
+                            Ok(()) => {},
                             // Some error happened, try to provide a more detailed description.
                             Err(err) => {
                                 // For some errors we might be able to provide extra information
@@ -553,11 +571,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         match layout.ty.sty {
             // generators and closures.
             ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => {
-                if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
-                    let freevar = self.tcx.with_freevars(node_id, |fv| fv[field]);
-                    PathElem::ClosureVar(self.tcx.hir.name(freevar.var_id()))
+                if let Some(upvar) = self.tcx.optimized_mir(def_id).upvar_decls.get(field) {
+                    PathElem::ClosureVar(upvar.debug_name)
                 } else {
-                    // The closure is not local, so we cannot get the name
+                    // Sometimes the index is beyond the number of freevars (seen
+                    // for a generator).
                     PathElem::ClosureVar(Symbol::intern(&field.to_string()))
                 }
             }

src/test/ui/consts/const-eval/ub-ref.rs

@@ -21,6 +21,9 @@ const NULL: &u16 = unsafe { mem::transmute(0usize) };
 const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
 //~^ ERROR this constant likely exhibits undefined behavior
 
+const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }];
+//~^ ERROR this constant likely exhibits undefined behavior
+
 const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
 //~^ ERROR this constant likely exhibits undefined behavior
 

src/test/ui/consts/const-eval/ub-ref.stderr

@@ -25,11 +25,19 @@ LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
 error[E0080]: this constant likely exhibits undefined behavior
   --> $DIR/ub-ref.rs:24:1
    |
+LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }];
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a raw memory access tried to access part of a pointer value as raw bytes
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
+
+error[E0080]: this constant likely exhibits undefined behavior
+  --> $DIR/ub-ref.rs:27:1
+   |
 LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered integer pointer in non-ZST reference
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors
 
 For more information about this error, try `rustc --explain E0080`.

src/test/ui/consts/const-eval/ub-upvars.rs

@@ -0,0 +1,21 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(const_transmute,const_let)]
+
+use std::mem;
+
+const BAD_UPVAR: &FnOnce() = &{ //~ ERROR this constant likely exhibits undefined behavior
+    let bad_ref: &'static u16 = unsafe { mem::transmute(0usize) };
+    let another_var = 13;
+    move || { let _ = bad_ref; let _ = another_var; }
+};
+
+fn main() {}

src/test/ui/consts/const-eval/ub-upvars.stderr

@@ -0,0 +1,15 @@
+error[E0080]: this constant likely exhibits undefined behavior
+  --> $DIR/ub-upvars.rs:15:1
+   |
+LL | / const BAD_UPVAR: &FnOnce() = &{ //~ ERROR this constant likely exhibits undefined behavior
+LL | |     let bad_ref: &'static u16 = unsafe { mem::transmute(0usize) };
+LL | |     let another_var = 13;
+LL | |     move || { let _ = bad_ref; let _ = another_var; }
+LL | | };
+   | |__^ type validation failed: encountered 0 at .<deref>.<closure-var(bad_ref)>, but expected something greater or equal to 1
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0080`.