diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index fd18d9feeea..1ce32f8c0a8 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 }, )?; diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index f6944b2a9ae..b0b8962b76a 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 @@ -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 { + // 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/place.rs b/src/librustc_mir/interpret/place.rs index 707857c809b..55077c8b6f9 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 } @@ -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 { diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index 06aee8605c6..11d5785bc56 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 d8ec014b852..a339fa34ae1 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()? } @@ -286,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), )?; @@ -375,17 +371,19 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> return err!(FunctionArgCountMismatch); } // Don't forget to check the return type! - let callee_ret = self.eval_place(&mir::Place::Local(mir::RETURN_PLACE))?; if let Some(caller_ret) = dest { + let callee_ret = self.eval_place(&mir::Place::Local(mir::RETURN_PLACE))?; if !Self::check_argument_compat(caller_ret.layout, callee_ret.layout) { return err!(FunctionRetMismatch( caller_ret.layout.ty, callee_ret.layout.ty )); } } else { - if !callee_ret.layout.abi.is_uninhabited() { + 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_ret.layout.ty + self.tcx.types.never, callee_layout.ty )); } } @@ -453,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), ) }