diff --git a/src/librustc_middle/mir/interpret/error.rs b/src/librustc_middle/mir/interpret/error.rs index c56dc5196c6c2..2510dbcea0bdc 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), /// 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_middle/mir/interpret/value.rs b/src/librustc_middle/mir/interpret/value.rs index f3c1c87dad484..25fa3e5e8e0e3 100644 --- a/src/librustc_middle/mir/interpret/value.rs +++ b/src/librustc_middle/mir/interpret/value.rs @@ -188,11 +188,6 @@ impl<'tcx, Tag> Scalar { } } - #[inline] - pub fn null_ptr(cx: &impl HasDataLayout) -> Self { - Scalar::Raw { data: 0, size: cx.data_layout().pointer_size.bytes() as u8 } - } - #[inline] pub fn zst() -> Self { Scalar::Raw { data: 0, size: 0 } diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index b2a041874d09d..41083c839ff60 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 - // . 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 + // . 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,17 @@ 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 !unwinding { + // Copy the return value to the caller's stack frame. + if let Some(return_place) = frame.return_place { + let op = self.access_local(&frame, mir::RETURN_PLACE, None)?; + self.copy_op_transmute(op, return_place)?; + self.dump_place(*return_place); + } else { + throw_ub!(Unreachable); + } + } + // Now where do we jump next? // Usually we want to clean up (deallocate locals), but in a few rare cases we don't. @@ -754,7 +760,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.deallocate_local(local.value)?; } - let return_place = frame.return_place; if M::after_stack_pop(self, frame, unwinding)? == StackPopJump::NoJump { // The hook already did everything. // We want to skip the `info!` below, hence early return. @@ -767,25 +772,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.unwind_to_block(unwind); } else { // Follow the normal return edge. - // Validate the return value. Do this after deallocating so that we catch dangling - // references. - if let Some(return_place) = 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)?)?; - } - } else { - // Uh, that shouldn't happen... the function did not intend to return - throw_ub!(Unreachable); - } - - // Jump to new block -- *after* validation so that the spans make more sense. if let Some(ret) = next_block { self.return_to_block(ret)?; } diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 31e6fbdceee4a..8188106b5f187 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>, ) -> 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,16 +453,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { place: mir::Place<'tcx>, layout: Option>, ) -> 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)? - } - }; + // 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 }; + + let base_op = self.access_local(self.frame(), place.local, layout)?; let op = place .projection diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index af3a9da2f6ca6..e961d4fce04c4 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -135,12 +135,6 @@ impl MemPlace { MemPlace { ptr, align, meta: MemPlaceMeta::None } } - /// Produces a Place that will error if attempted to be read from or written to - #[inline(always)] - fn null(cx: &impl HasDataLayout) -> Self { - Self::from_scalar_ptr(Scalar::null_ptr(cx), Align::from_bytes(1).unwrap()) - } - #[inline(always)] pub fn from_ptr(ptr: Pointer, align: Align) -> Self { Self::from_scalar_ptr(ptr.into(), align) @@ -260,12 +254,6 @@ impl<'tcx, Tag: ::std::fmt::Debug + Copy> OpTy<'tcx, Tag> { } impl Place { - /// Produces a Place that will error if attempted to be read from or written to - #[inline(always)] - fn null(cx: &impl HasDataLayout) -> Self { - Place::Ptr(MemPlace::null(cx)) - } - #[inline] pub fn assert_mem_place(self) -> MemPlace { match self { @@ -636,35 +624,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 7157225e5c9bb..777a4381cda72 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/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 79dba2c5db8fb..39de685b1735b 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -331,7 +331,6 @@ struct ConstPropagator<'mir, 'tcx> { // by accessing them through `ecx` instead. source_scopes: IndexVec, local_decls: IndexVec>, - ret: Option>, // Because we have `MutVisitor` we can't obtain the `SourceInfo` from a `Location`. So we store // the last known `SourceInfo` here and just keep revisiting it. source_info: Option, @@ -403,22 +402,19 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { source_scopes: body.source_scopes.clone(), //FIXME(wesleywiser) we can't steal this because `Visitor::super_visit_body()` needs it local_decls: body.local_decls.clone(), - ret: ret.map(Into::into), source_info: None, } } fn get_const(&self, local: Local) -> Option> { - if local == RETURN_PLACE { - // Try to read the return place as an immediate so that if it is representable as a - // scalar, we can handle it as such, but otherwise, just return the value as is. - return match self.ret.map(|ret| self.ecx.try_read_immediate(ret)) { - Some(Ok(Ok(imm))) => Some(imm.into()), - _ => self.ret, - }; - } + let op = self.ecx.access_local(self.ecx.frame(), local, None).ok(); - self.ecx.access_local(self.ecx.frame(), local, None).ok() + // Try to read the local as an immediate so that if it is representable as a scalar, we can + // handle it as such, but otherwise, just return the value as is. + match op.map(|ret| self.ecx.try_read_immediate(ret)) { + Some(Ok(Ok(imm))) => Some(imm.into()), + _ => op, + } } fn remove_const(&mut self, local: Local) { diff --git a/src/test/codegen/consts.rs b/src/test/codegen/consts.rs index e53e75b339bef..ed93af2f993d3 100644 --- a/src/test/codegen/consts.rs +++ b/src/test/codegen/consts.rs @@ -10,11 +10,11 @@ // CHECK: @STATIC = {{.*}}, align 4 // This checks the constants from inline_enum_const -// CHECK: @alloc5 = {{.*}}, align 2 +// CHECK: @alloc7 = {{.*}}, align 2 // This checks the constants from {low,high}_align_const, they share the same // constant, but the alignment differs, so the higher one should be used -// CHECK: [[LOW_HIGH:@[0-9]+]] = {{.*}} getelementptr inbounds (<{ [8 x i8] }>, <{ [8 x i8] }>* @alloc15, i32 0, i32 0, i32 0), {{.*}} +// CHECK: [[LOW_HIGH:@[0-9]+]] = {{.*}} getelementptr inbounds (<{ [8 x i8] }>, <{ [8 x i8] }>* @alloc19, i32 0, i32 0, i32 0), {{.*}} #[derive(Copy, Clone)] // repr(i16) is required for the {low,high}_align_const test diff --git a/src/test/incremental/hashes/enum_constructors.rs b/src/test/incremental/hashes/enum_constructors.rs index 99c50f7e17356..2c07cbcb2054b 100644 --- a/src/test/incremental/hashes/enum_constructors.rs +++ b/src/test/incremental/hashes/enum_constructors.rs @@ -274,14 +274,14 @@ pub enum Clike2 { // Change constructor path (C-like) -------------------------------------- #[cfg(cfail1)] pub fn change_constructor_path_c_like() { - let _ = Clike::B; + let _x = Clike::B; } #[cfg(not(cfail1))] #[rustc_clean(cfg="cfail2", except="hir_owner_nodes,optimized_mir,mir_built,typeck_tables_of")] #[rustc_clean(cfg="cfail3")] pub fn change_constructor_path_c_like() { - let _ = Clike2::B; + let _x = Clike2::B; } @@ -289,14 +289,14 @@ pub fn change_constructor_path_c_like() { // Change constructor variant (C-like) -------------------------------------- #[cfg(cfail1)] pub fn change_constructor_variant_c_like() { - let _ = Clike::A; + let _x = Clike::A; } #[cfg(not(cfail1))] #[rustc_clean(cfg="cfail2", except="hir_owner_nodes,optimized_mir,mir_built")] #[rustc_clean(cfg="cfail3")] pub fn change_constructor_variant_c_like() { - let _ = Clike::C; + let _x = Clike::C; } diff --git a/src/test/mir-opt/const_allocation2/32bit/rustc.main.ConstProp.after.mir b/src/test/mir-opt/const_allocation2/32bit/rustc.main.ConstProp.after.mir index 4105d673218a0..efd14ea140fe8 100644 --- a/src/test/mir-opt/const_allocation2/32bit/rustc.main.ConstProp.after.mir +++ b/src/test/mir-opt/const_allocation2/32bit/rustc.main.ConstProp.after.mir @@ -30,41 +30,41 @@ fn main() -> () { } alloc0 (static: FOO, size: 8, align: 4) { - ╾alloc24+0╼ 03 00 00 00 │ ╾──╼.... + ╾alloc25+0╼ 03 00 00 00 │ ╾──╼.... } -alloc24 (size: 48, align: 4) { - 0x00 │ 00 00 00 00 __ __ __ __ ╾alloc9+0─╼ 00 00 00 00 │ ....░░░░╾──╼.... - 0x10 │ 00 00 00 00 __ __ __ __ ╾alloc14+0╼ 02 00 00 00 │ ....░░░░╾──╼.... - 0x20 │ 01 00 00 00 2a 00 00 00 ╾alloc22+0╼ 03 00 00 00 │ ....*...╾──╼.... +alloc25 (size: 48, align: 4) { + 0x00 │ 00 00 00 00 __ __ __ __ ╾alloc10+0╼ 00 00 00 00 │ ....░░░░╾──╼.... + 0x10 │ 00 00 00 00 __ __ __ __ ╾alloc15+0╼ 02 00 00 00 │ ....░░░░╾──╼.... + 0x20 │ 01 00 00 00 2a 00 00 00 ╾alloc23+0╼ 03 00 00 00 │ ....*...╾──╼.... } -alloc9 (size: 0, align: 4) {} +alloc10 (size: 0, align: 4) {} -alloc14 (size: 8, align: 4) { - ╾alloc12+0╼ ╾alloc13+0╼ │ ╾──╼╾──╼ +alloc15 (size: 8, align: 4) { + ╾alloc13+0╼ ╾alloc14+0╼ │ ╾──╼╾──╼ } -alloc12 (size: 1, align: 1) { +alloc13 (size: 1, align: 1) { 05 │ . } -alloc13 (size: 1, align: 1) { +alloc14 (size: 1, align: 1) { 06 │ . } -alloc22 (size: 12, align: 4) { - ╾alloc18+3╼ ╾alloc19+0╼ ╾alloc21+2╼ │ ╾──╼╾──╼╾──╼ +alloc23 (size: 12, align: 4) { + ╾alloc19+3╼ ╾alloc20+0╼ ╾alloc22+2╼ │ ╾──╼╾──╼╾──╼ } -alloc18 (size: 4, align: 1) { +alloc19 (size: 4, align: 1) { 2a 45 15 6f │ *E.o } -alloc19 (size: 1, align: 1) { +alloc20 (size: 1, align: 1) { 2a │ * } -alloc21 (size: 4, align: 1) { +alloc22 (size: 4, align: 1) { 2a 45 15 6f │ *E.o } diff --git a/src/test/mir-opt/const_allocation2/64bit/rustc.main.ConstProp.after.mir b/src/test/mir-opt/const_allocation2/64bit/rustc.main.ConstProp.after.mir index e61f0a8b69fa7..3b649ee7a24bd 100644 --- a/src/test/mir-opt/const_allocation2/64bit/rustc.main.ConstProp.after.mir +++ b/src/test/mir-opt/const_allocation2/64bit/rustc.main.ConstProp.after.mir @@ -30,44 +30,44 @@ fn main() -> () { } alloc0 (static: FOO, size: 16, align: 8) { - ╾──────alloc24+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........ + ╾──────alloc25+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........ } -alloc24 (size: 72, align: 8) { - 0x00 │ 00 00 00 00 __ __ __ __ ╾──────alloc9+0───────╼ │ ....░░░░╾──────╼ +alloc25 (size: 72, align: 8) { + 0x00 │ 00 00 00 00 __ __ __ __ ╾──────alloc10+0──────╼ │ ....░░░░╾──────╼ 0x10 │ 00 00 00 00 00 00 00 00 00 00 00 00 __ __ __ __ │ ............░░░░ - 0x20 │ ╾──────alloc14+0──────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........ - 0x30 │ 01 00 00 00 2a 00 00 00 ╾──────alloc22+0──────╼ │ ....*...╾──────╼ + 0x20 │ ╾──────alloc15+0──────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........ + 0x30 │ 01 00 00 00 2a 00 00 00 ╾──────alloc23+0──────╼ │ ....*...╾──────╼ 0x40 │ 03 00 00 00 00 00 00 00 │ ........ } -alloc9 (size: 0, align: 8) {} +alloc10 (size: 0, align: 8) {} -alloc14 (size: 16, align: 8) { - ╾──────alloc12+0──────╼ ╾──────alloc13+0──────╼ │ ╾──────╼╾──────╼ +alloc15 (size: 16, align: 8) { + ╾──────alloc13+0──────╼ ╾──────alloc14+0──────╼ │ ╾──────╼╾──────╼ } -alloc12 (size: 1, align: 1) { +alloc13 (size: 1, align: 1) { 05 │ . } -alloc13 (size: 1, align: 1) { +alloc14 (size: 1, align: 1) { 06 │ . } -alloc22 (size: 24, align: 8) { - 0x00 │ ╾──────alloc18+3──────╼ ╾──────alloc19+0──────╼ │ ╾──────╼╾──────╼ - 0x10 │ ╾──────alloc21+2──────╼ │ ╾──────╼ +alloc23 (size: 24, align: 8) { + 0x00 │ ╾──────alloc19+3──────╼ ╾──────alloc20+0──────╼ │ ╾──────╼╾──────╼ + 0x10 │ ╾──────alloc22+2──────╼ │ ╾──────╼ } -alloc18 (size: 4, align: 1) { +alloc19 (size: 4, align: 1) { 2a 45 15 6f │ *E.o } -alloc19 (size: 1, align: 1) { +alloc20 (size: 1, align: 1) { 2a │ * } -alloc21 (size: 4, align: 1) { +alloc22 (size: 4, align: 1) { 2a 45 15 6f │ *E.o } diff --git a/src/test/ui/consts/const-eval/ub-nonnull.stderr b/src/test/ui/consts/const-eval/ub-nonnull.stderr index adad1b4f7fafe..b6c2572cb8dc3 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 = { 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 8b847e4bf731f..54a9eda214660 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 | | }; | |__-