diff --git a/src/Cargo.lock b/src/Cargo.lock index c01086b3f6f2b..abe736a4fe613 100644 --- a/src/Cargo.lock +++ b/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" diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index fd18d9feeea91..2cfd058831f05 100644 --- a/src/librustc_mir/const_eval.rs +++ b/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, 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>, diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index bfc7e6801fc46..b5137e914dc91 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/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 diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index f6944b2a9ae85..85a8376134aa4 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/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, + /// The location where the result of the current stack frame should be written to, + /// and its layout in the caller. + pub return_place: Option>, /// 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), /// 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>, 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, + return_place: Option>, 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(()) } diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index a669b2aafc2b8..5fa0fef36935d 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/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), diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index a444f0bafd23c..560698f3f57a2 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -86,7 +86,7 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { const STATIC_KIND: Option; /// 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. diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 7d3ae19e1a30c..4b0c0c3ee6173 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/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, + 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, 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(); diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 8b9c6a5a27053..e4055947b6421 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -256,12 +256,6 @@ impl<'tcx, Tag: ::std::fmt::Debug> Place { } 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, 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, + 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, 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()? = diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index 06aee8605c6e1..11d5785bc565d 100644 --- a/src/librustc_mir/interpret/snapshot.rs +++ b/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>>, locals: IndexVec>>, block: &'a mir::BasicBlock, stmt: usize, @@ -362,7 +362,7 @@ impl<'a, 'mir, 'tcx: 'mir> HashStable> 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(), } } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index e599608b2dac9..a339fa34ae1f6 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/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), ) } diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 9dc035a3e20b8..c446980d04995 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/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())) } } diff --git a/src/test/ui/consts/const-eval/ub-ref.rs b/src/test/ui/consts/const-eval/ub-ref.rs index 7ee13f20dd2d9..584dc0691698a 100644 --- a/src/test/ui/consts/const-eval/ub-ref.rs +++ b/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 diff --git a/src/test/ui/consts/const-eval/ub-ref.stderr b/src/test/ui/consts/const-eval/ub-ref.stderr index 9907c780d2ccb..8bcb6d190b89a 100644 --- a/src/test/ui/consts/const-eval/ub-ref.stderr +++ b/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`. diff --git a/src/test/ui/consts/const-eval/ub-upvars.rs b/src/test/ui/consts/const-eval/ub-upvars.rs new file mode 100644 index 0000000000000..309211d19d461 --- /dev/null +++ b/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 or the MIT license +// , 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() {} diff --git a/src/test/ui/consts/const-eval/ub-upvars.stderr b/src/test/ui/consts/const-eval/ub-upvars.stderr new file mode 100644 index 0000000000000..3ae140d6e1c20 --- /dev/null +++ b/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 .., 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`. diff --git a/src/tools/miri b/src/tools/miri index cc275c63a90d4..26f9d617c3471 160000 --- a/src/tools/miri +++ b/src/tools/miri @@ -1 +1 @@ -Subproject commit cc275c63a90d4bea394e76607b2e10611eb1be36 +Subproject commit 26f9d617c347185433b77c481a5c50c55d9b72ce