From 874a130ca01eb8a915b3ba0898aaeff35578758a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Jul 2022 11:46:10 -0400 Subject: [PATCH 1/2] get rid of MemPlaceMeta::Poison MPlaceTy::dangling still exists, but now it is only called in places that actually conceptually allocate something new, so that's fine. --- .../src/const_eval/eval_queries.rs | 1 + .../rustc_const_eval/src/const_eval/mod.rs | 1 - .../src/interpret/eval_context.rs | 2 +- .../rustc_const_eval/src/interpret/place.rs | 21 ++++++------------- .../src/interpret/terminator.rs | 6 ++---- .../src/mir/interpret/allocation.rs | 2 ++ 6 files changed, 12 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index b18976302b4ff..f03ceb54830c8 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -165,6 +165,7 @@ pub(super) fn op_to_const<'tcx>( Ok(ref mplace) => to_const_value(mplace), // see comment on `let try_as_immediate` above Err(imm) => match *imm { + _ if imm.layout.is_zst() => ConstValue::ZeroSized, Immediate::Scalar(x) => match x { ScalarMaybeUninit::Scalar(s) => ConstValue::Scalar(s), ScalarMaybeUninit::Uninit => to_const_value(&op.assert_mem_place()), diff --git a/compiler/rustc_const_eval/src/const_eval/mod.rs b/compiler/rustc_const_eval/src/const_eval/mod.rs index bf65fdc54ca48..edc4c13b6e8f6 100644 --- a/compiler/rustc_const_eval/src/const_eval/mod.rs +++ b/compiler/rustc_const_eval/src/const_eval/mod.rs @@ -147,7 +147,6 @@ pub(crate) fn deref_mir_constant<'tcx>( let ty = match mplace.meta { MemPlaceMeta::None => mplace.layout.ty, - MemPlaceMeta::Poison => bug!("poison metadata in `deref_mir_constant`: {:#?}", mplace), // In case of unsized types, figure out the real type behind. MemPlaceMeta::Meta(scalar) => match mplace.layout.ty.kind() { ty::Str => bug!("there's no sized equivalent of a `str`"), diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 2e47cf8921073..bacf5d5a59f2a 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -987,7 +987,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> std::fmt::Debug " by {} ref {:?}:", match mplace.meta { MemPlaceMeta::Meta(meta) => format!(" meta({:?})", meta), - MemPlaceMeta::Poison | MemPlaceMeta::None => String::new(), + MemPlaceMeta::None => String::new(), }, mplace.ptr, )?; diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 2001359d199cf..84ecfbabad369 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -24,11 +24,6 @@ pub enum MemPlaceMeta { Meta(Scalar), /// `Sized` types or unsized `extern type` None, - /// The address of this place may not be taken. This protects the `MemPlace` from coming from - /// a ZST Operand without a backing allocation and being converted to an integer address. This - /// should be impossible, because you can't take the address of an operand, but this is a second - /// protection layer ensuring that we don't mess up. - Poison, } #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] @@ -38,15 +33,16 @@ impl MemPlaceMeta { pub fn unwrap_meta(self) -> Scalar { match self { Self::Meta(s) => s, - Self::None | Self::Poison => { + Self::None => { bug!("expected wide pointer extra data (e.g. slice length or trait object vtable)") } } } + pub fn has_meta(self) -> bool { match self { Self::Meta(_) => true, - Self::None | Self::Poison => false, + Self::None => false, } } } @@ -163,10 +159,6 @@ impl MemPlace { MemPlaceMeta::Meta(meta) => { Immediate::ScalarPair(Scalar::from_maybe_pointer(self.ptr, cx).into(), meta.into()) } - MemPlaceMeta::Poison => bug!( - "MPlaceTy::dangling may never be used to produce a \ - place that will have the address of its pointee taken" - ), } } @@ -195,13 +187,13 @@ impl Place { } impl<'tcx, Tag: Provenance> MPlaceTy<'tcx, Tag> { - /// Produces a MemPlace that works for ZST but nothing else + /// Produces a MemPlace that works for ZST but nothing else. #[inline] pub fn dangling(layout: TyAndLayout<'tcx>) -> Self { + assert!(layout.is_zst()); let align = layout.align.abi; let ptr = Pointer::from_addr(align.bytes()); // no provenance, absolute address - // `Poison` this to make sure that the pointer value `ptr` is never observable by the program. - MPlaceTy { mplace: MemPlace { ptr, meta: MemPlaceMeta::Poison }, layout, align } + MPlaceTy { mplace: MemPlace { ptr, meta: MemPlaceMeta::None }, layout, align } } #[inline] @@ -273,7 +265,6 @@ impl<'tcx, Tag: Provenance> OpTy<'tcx, Tag> { Operand::Indirect(mplace) => { Ok(MPlaceTy { mplace, layout: self.layout, align: self.align.unwrap() }) } - Operand::Immediate(_) if self.layout.is_zst() => Ok(MPlaceTy::dangling(self.layout)), Operand::Immediate(imm) => Err(ImmTy::from_immediate(imm, self.layout)), } } diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 9e74b99ecd73b..9ff405b993944 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -617,16 +617,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { place.to_ref(self), self.layout_of(self.tcx.mk_mut_ptr(place.layout.ty))?, ); - - let ty = self.tcx.mk_unit(); // return type is () - let dest = MPlaceTy::dangling(self.layout_of(ty)?); + let ret = MPlaceTy::dangling(self.layout_of(self.tcx.types.unit)?); self.eval_fn_call( FnVal::Instance(instance), (Abi::Rust, fn_abi), &[arg.into()], false, - &dest.into(), + &ret.into(), Some(target), match unwind { Some(cleanup) => StackPopUnwind::Cleanup(cleanup), diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index ae333846f067c..eed52ca3eeaa6 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -225,6 +225,8 @@ impl Allocation { /// Try to create an Allocation of `size` bytes, failing if there is not enough memory /// available to the compiler to do so. + /// + /// If `panic_on_fail` is true, this will never return `Err`. pub fn uninit<'tcx>(size: Size, align: Align, panic_on_fail: bool) -> InterpResult<'tcx, Self> { let bytes = Box::<[u8]>::try_new_zeroed_slice(size.bytes_usize()).map_err(|_| { // This results in an error that can happen non-deterministically, since the memory From e3ef4fdac9bc83ebc30d421d9629c9df990d04c2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 14 Jul 2022 11:40:47 -0400 Subject: [PATCH 2/2] rename MPlaceTy::dangling to fake_alloc_zst --- compiler/rustc_const_eval/src/interpret/place.rs | 4 +++- compiler/rustc_const_eval/src/interpret/terminator.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 84ecfbabad369..f4b11ea196757 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -188,8 +188,10 @@ impl Place { impl<'tcx, Tag: Provenance> MPlaceTy<'tcx, Tag> { /// Produces a MemPlace that works for ZST but nothing else. + /// Conceptually this is a new allocation, but it doesn't actually create an allocation so you + /// don't need to worry about memory leaks. #[inline] - pub fn dangling(layout: TyAndLayout<'tcx>) -> Self { + pub fn fake_alloc_zst(layout: TyAndLayout<'tcx>) -> Self { assert!(layout.is_zst()); let align = layout.align.abi; let ptr = Pointer::from_addr(align.bytes()); // no provenance, absolute address diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 9ff405b993944..20122f8131c33 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -617,7 +617,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { place.to_ref(self), self.layout_of(self.tcx.mk_mut_ptr(place.layout.ty))?, ); - let ret = MPlaceTy::dangling(self.layout_of(self.tcx.types.unit)?); + let ret = MPlaceTy::fake_alloc_zst(self.layout_of(self.tcx.types.unit)?); self.eval_fn_call( FnVal::Instance(instance),