diff options
| -rw-r--r-- | src/librustc_middle/mir/interpret/error.rs | 3 | ||||
| -rw-r--r-- | src/librustc_mir/interpret/eval_context.rs | 51 | ||||
| -rw-r--r-- | src/librustc_mir/interpret/operand.rs | 16 | ||||
| -rw-r--r-- | src/librustc_mir/interpret/place.rs | 37 | ||||
| -rw-r--r-- | src/librustc_mir/interpret/terminator.rs | 1 | ||||
| -rw-r--r-- | src/test/ui/consts/const-eval/ub-nonnull.stderr | 2 | ||||
| -rw-r--r-- | src/test/ui/consts/miri_unleashed/mutable_const.stderr | 2 |
7 files changed, 42 insertions, 70 deletions
diff --git a/src/librustc_middle/mir/interpret/error.rs b/src/librustc_middle/mir/interpret/error.rs index c56dc5196c6..2510dbcea0b 100644 --- a/src/librustc_middle/mir/interpret/error.rs +++ b/src/librustc_middle/mir/interpret/error.rs @@ -361,8 +361,6 @@ pub enum UndefinedBehaviorInfo { InvalidUndefBytes(Option<Pointer>), /// Working with a local that is not currently live. DeadLocal, - /// Trying to read from the return place of a function. - ReadFromReturnPlace, } impl fmt::Debug for UndefinedBehaviorInfo { @@ -424,7 +422,6 @@ impl fmt::Debug for UndefinedBehaviorInfo { "using uninitialized data, but this operation requires initialized memory" ), DeadLocal => write!(f, "accessing a dead local variable"), - ReadFromReturnPlace => write!(f, "reading from return place"), } } } diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index b2a041874d0..283f9e14251 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -623,35 +623,30 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let frame = M::init_frame_extra(self, pre_frame)?; self.stack_mut().push(frame); - // don't allocate at all for trivial constants - if body.local_decls.len() > 1 { - // Locals are initially uninitialized. - let dummy = LocalState { value: LocalValue::Uninitialized, layout: Cell::new(None) }; - let mut locals = IndexVec::from_elem(dummy, &body.local_decls); - // Return place is handled specially by the `eval_place` functions, and the - // entry in `locals` should never be used. Make it dead, to be sure. - locals[mir::RETURN_PLACE].value = LocalValue::Dead; - // Now mark those locals as dead that we do not want to initialize - match self.tcx.def_kind(instance.def_id()) { - // statics and constants don't have `Storage*` statements, no need to look for them - // - // FIXME: The above is likely untrue. See - // <https://github.com/rust-lang/rust/pull/70004#issuecomment-602022110>. Is it - // okay to ignore `StorageDead`/`StorageLive` annotations during CTFE? - Some(DefKind::Static | DefKind::Const | DefKind::AssocConst) => {} - _ => { - // Mark locals that use `Storage*` annotations as dead on function entry. - let always_live = AlwaysLiveLocals::new(self.body()); - for local in locals.indices() { - if !always_live.contains(local) { - locals[local].value = LocalValue::Dead; - } + // Locals are initially uninitialized. + let dummy = LocalState { value: LocalValue::Uninitialized, layout: Cell::new(None) }; + let mut locals = IndexVec::from_elem(dummy, &body.local_decls); + + // Now mark those locals as dead that we do not want to initialize + match self.tcx.def_kind(instance.def_id()) { + // statics and constants don't have `Storage*` statements, no need to look for them + // + // FIXME: The above is likely untrue. See + // <https://github.com/rust-lang/rust/pull/70004#issuecomment-602022110>. Is it + // okay to ignore `StorageDead`/`StorageLive` annotations during CTFE? + Some(DefKind::Static | DefKind::Const | DefKind::AssocConst) => {} + _ => { + // Mark locals that use `Storage*` annotations as dead on function entry. + let always_live = AlwaysLiveLocals::new(self.body()); + for local in locals.indices() { + if !always_live.contains(local) { + locals[local].value = LocalValue::Dead; } } } - // done - self.frame_mut().locals = locals; } + // done + self.frame_mut().locals = locals; M::after_stack_push(self)?; info!("ENTERING({}) {}", self.frame_idx(), self.frame().instance); @@ -729,6 +724,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let frame = self.stack_mut().pop().expect("tried to pop a stack frame, but there were none"); + if let Some(return_place) = frame.return_place { + // Copy the return value to the caller's stack frame. + let op = self.access_local(&frame, mir::RETURN_PLACE, None)?; + self.copy_op(op, return_place)?; + } + // Now where do we jump next? // Usually we want to clean up (deallocate locals), but in a few rare cases we don't. diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 31e6fbdceee..d5864d1d4db 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -419,7 +419,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { local: mir::Local, layout: Option<TyAndLayout<'tcx>>, ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { - assert_ne!(local, mir::RETURN_PLACE); let layout = self.layout_of_local(frame, local, layout)?; let op = if layout.is_zst() { // Do not read from ZST, they might not be initialized @@ -454,15 +453,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { place: mir::Place<'tcx>, layout: Option<TyAndLayout<'tcx>>, ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { - let base_op = match place.local { - mir::RETURN_PLACE => throw_ub!(ReadFromReturnPlace), - local => { - // Do not use the layout passed in as argument if the base we are looking at - // here is not the entire place. - let layout = if place.projection.is_empty() { layout } else { None }; - - self.access_local(self.frame(), local, layout)? - } + let base_op = { + // Do not use the layout passed in as argument if the base we are looking at + // here is not the entire place. + let layout = if place.projection.is_empty() { layout } else { None }; + + self.access_local(self.frame(), place.local, layout)? }; let op = place diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index af3a9da2f6c..ca7148714ac 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -276,6 +276,10 @@ impl<Tag: ::std::fmt::Debug> Place<Tag> { } impl<'tcx, Tag: ::std::fmt::Debug> PlaceTy<'tcx, Tag> { + pub fn null(cx: &impl HasDataLayout, layout: TyAndLayout<'tcx>) -> Self { + Self { place: Place::null(cx), layout } + } + #[inline] pub fn assert_mem_place(self) -> MPlaceTy<'tcx, Tag> { MPlaceTy { mplace: self.place.assert_mem_place(), layout: self.layout } @@ -636,35 +640,10 @@ where &mut self, place: mir::Place<'tcx>, ) -> InterpResult<'tcx, PlaceTy<'tcx, M::PointerTag>> { - let mut place_ty = match place.local { - mir::RETURN_PLACE => { - // `return_place` has the *caller* layout, but we want to use our - // `layout to verify our assumption. The caller will validate - // their layout on return. - PlaceTy { - place: match self.frame().return_place { - Some(p) => *p, - // Even if we don't have a return place, we sometimes need to - // create this place, but any attempt to read from / write to it - // (even a ZST read/write) needs to error, so let us make this - // a NULL place. - // - // FIXME: Ideally we'd make sure that the place projections also - // bail out. - None => Place::null(&*self), - }, - layout: self.layout_of( - self.subst_from_current_frame_and_normalize_erasing_regions( - self.frame().body.return_ty(), - ), - )?, - } - } - local => PlaceTy { - // This works even for dead/uninitialized locals; we check further when writing - place: Place::Local { frame: self.frame_idx(), local }, - layout: self.layout_of_local(self.frame(), local, None)?, - }, + let mut place_ty = PlaceTy { + // This works even for dead/uninitialized locals; we check further when writing + place: Place::Local { frame: self.frame_idx(), local: place.local }, + layout: self.layout_of_local(self.frame(), place.local, None)?, }; for elem in place.projection.iter() { diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 7157225e5c9..777a4381cda 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -19,7 +19,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { use rustc_middle::mir::TerminatorKind::*; match terminator.kind { Return => { - self.frame().return_place.map(|r| self.dump_place(*r)); self.pop_stack_frame(/* unwinding */ false)? } diff --git a/src/test/ui/consts/const-eval/ub-nonnull.stderr b/src/test/ui/consts/const-eval/ub-nonnull.stderr index adad1b4f7fa..b6c2572cb8d 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.stderr @@ -13,7 +13,7 @@ LL | / const OUT_OF_BOUNDS_PTR: NonNull<u8> = { unsafe { LL | | let ptr: &[u8; 256] = mem::transmute(&0u8); // &0 gets promoted so it does not dangle LL | | // Use address-of-element for pointer arithmetic. This could wrap around to NULL! LL | | let out_of_bounds_ptr = &ptr[255]; - | | ^^^^^^^^^ Memory access failed: pointer must be in-bounds at offset 256, but is outside bounds of alloc8 which has size 1 + | | ^^^^^^^^^ Memory access failed: pointer must be in-bounds at offset 256, but is outside bounds of alloc11 which has size 1 LL | | mem::transmute(out_of_bounds_ptr) LL | | } }; | |____- diff --git a/src/test/ui/consts/miri_unleashed/mutable_const.stderr b/src/test/ui/consts/miri_unleashed/mutable_const.stderr index 8b847e4bf73..54a9eda2146 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_const.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_const.stderr @@ -11,7 +11,7 @@ LL | / const MUTATING_BEHIND_RAW: () = { LL | | // Test that `MUTABLE_BEHIND_RAW` is actually immutable, by doing this at const time. LL | | unsafe { LL | | *MUTABLE_BEHIND_RAW = 99 - | | ^^^^^^^^^^^^^^^^^^^^^^^^ writing to alloc1 which is read-only + | | ^^^^^^^^^^^^^^^^^^^^^^^^ writing to alloc2 which is read-only LL | | } LL | | }; | |__- |