Skip to content

Commit

Permalink
Auto merge of rust-lang#99013 - RalfJung:dont-poison-my-places, r=oli…
Browse files Browse the repository at this point in the history
…-obk

interpret: get rid of MemPlaceMeta::Poison

This is achieved by refactoring the projection code (`{mplace,place,operand}_{downcast,field,index,...}`) so that we no longer need to call `assert_mem_place` in the operand handling.
  • Loading branch information
bors committed Jul 15, 2022
2 parents 522abf6 + e3ef4fd commit 6077b7c
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 22 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_const_eval/src/const_eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`"),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)?;
Expand Down
25 changes: 9 additions & 16 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,6 @@ pub enum MemPlaceMeta<Tag: Provenance = AllocId> {
Meta(Scalar<Tag>),
/// `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"))]
Expand All @@ -38,15 +33,16 @@ impl<Tag: Provenance> MemPlaceMeta<Tag> {
pub fn unwrap_meta(self) -> Scalar<Tag> {
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,
}
}
}
Expand Down Expand Up @@ -163,10 +159,6 @@ impl<Tag: Provenance> MemPlace<Tag> {
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"
),
}
}

Expand Down Expand Up @@ -195,13 +187,15 @@ impl<Tag: Provenance> Place<Tag> {
}

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.
/// 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
// `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]
Expand Down Expand Up @@ -273,7 +267,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)),
}
}
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::fake_alloc_zst(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),
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ impl<Tag> Allocation<Tag> {

/// 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
Expand Down

0 comments on commit 6077b7c

Please sign in to comment.