mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

interpret: fix projecting into an unsized field of a local

Browse Source 
new invariant: Place::Local never refers to something unsized
This commit is contained in:
Ralf Jung 2023-08-04 21:42:35 +02:00
parent 61efe9d298
commit bdd5855b8e
6 changed files with 124 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
compiler/rustc_const_eval/src/interpret/eval_context.rs
View File

@ -177,7 +177,7 @@ pub enum LocalValue<Prov: Provenance = AllocId> {
impl<'tcx, Prov: Provenance + 'static> LocalState<'tcx, Prov> {
/// Read the local's value or error if the local is not yet live or not live anymore.
#[inline]
#[inline(always)]
pub fn access(&self) -> InterpResult<'tcx, &Operand<Prov>> {
match &self.value {
LocalValue::Dead => throw_ub!(DeadLocal), // could even be "invalid program"?
@ -190,7 +190,7 @@ impl<'tcx, Prov: Provenance + 'static> LocalState<'tcx, Prov> {
///
/// Note: This may only be invoked from the `Machine::access_local_mut` hook and not from
/// anywhere else. You may be invalidating machine invariants if you do!
#[inline]
#[inline(always)]
pub fn access_mut(&mut self) -> InterpResult<'tcx, &mut Operand<Prov>> {
match &mut self.value {
LocalValue::Dead => throw_ub!(DeadLocal), // could even be "invalid program"?

56
compiler/rustc_const_eval/src/interpret/operand.rs
View File

@ -291,23 +291,21 @@ impl<'tcx, Prov: Provenance> Projectable<'tcx, Prov> for ImmTy<'tcx, Prov> {
self.layout
}
fn meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
_ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, MemPlaceMeta<M::Provenance>> {
assert!(self.layout.is_sized()); // unsized ImmTy can only exist temporarily and should never reach this here
#[inline(always)]
fn meta(&self) -> InterpResult<'tcx, MemPlaceMeta<Prov>> {
debug_assert!(self.layout.is_sized()); // unsized ImmTy can only exist temporarily and should never reach this here
Ok(MemPlaceMeta::None)
}
fn offset_with_meta(
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
offset: Size,
meta: MemPlaceMeta<Prov>,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, Self> {
assert_matches!(meta, MemPlaceMeta::None); // we can't store this anywhere anyway
Ok(self.offset_(offset, layout, cx))
Ok(self.offset_(offset, layout, ecx))
}
fn to_op<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
@ -318,49 +316,39 @@ impl<'tcx, Prov: Provenance> Projectable<'tcx, Prov> for ImmTy<'tcx, Prov> {
}
}
impl<'tcx, Prov: Provenance> OpTy<'tcx, Prov> {
// Provided as inherent method since it doesn't need the `ecx` of `Projectable::meta`.
pub fn meta(&self) -> InterpResult<'tcx, MemPlaceMeta<Prov>> {
Ok(if self.layout.is_unsized() {
if matches!(self.op, Operand::Immediate(_)) {
// Unsized immediate OpTy cannot occur. We create a MemPlace for all unsized locals during argument passing.
// However, ConstProp doesn't do that, so we can run into this nonsense situation.
throw_inval!(ConstPropNonsense);
}
// There are no unsized immediates.
self.assert_mem_place().meta
} else {
MemPlaceMeta::None
})
}
}
impl<'tcx, Prov: Provenance + 'static> Projectable<'tcx, Prov> for OpTy<'tcx, Prov> {
#[inline(always)]
fn layout(&self) -> TyAndLayout<'tcx> {
self.layout
}
fn meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
_ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, MemPlaceMeta<M::Provenance>> {
self.meta()
fn meta(&self) -> InterpResult<'tcx, MemPlaceMeta<Prov>> {
Ok(match self.as_mplace_or_imm() {
Left(mplace) => mplace.meta,
Right(_) => {
if self.layout.is_unsized() {
// Unsized immediate OpTy cannot occur. We create a MemPlace for all unsized locals during argument passing.
// However, ConstProp doesn't do that, so we can run into this nonsense situation.
throw_inval!(ConstPropNonsense);
}
MemPlaceMeta::None
}
})
}
fn offset_with_meta(
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
offset: Size,
meta: MemPlaceMeta<Prov>,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, Self> {
match self.as_mplace_or_imm() {
Left(mplace) => Ok(mplace.offset_with_meta(offset, meta, layout, cx)?.into()),
Left(mplace) => Ok(mplace.offset_with_meta(offset, meta, layout, ecx)?.into()),
Right(imm) => {
assert!(!meta.has_meta()); // no place to store metadata here
// Every part of an uninit is uninit.
Ok(imm.offset(offset, layout, cx)?.into())
Ok(imm.offset(offset, layout, ecx)?.into())
}
}
}

66
compiler/rustc_const_eval/src/interpret/place.rs
View File

@ -111,6 +111,8 @@ pub enum Place<Prov: Provenance = AllocId> {
/// (Without that optimization, we'd just always be a `MemPlace`.)
/// Note that this only stores the frame index, not the thread this frame belongs to -- that is
/// implicit. This means a `Place` must never be moved across interpreter thread boundaries!
///
/// This variant shall not be used for unsized types -- those must always live in memory.
Local { frame: usize, local: mir::Local, offset: Option<Size> },
}
@ -157,7 +159,7 @@ impl<Prov: Provenance> MemPlace<Prov> {
}
/// Turn a mplace into a (thin or wide) pointer, as a reference, pointing to the same space.
#[inline(always)]
#[inline]
pub fn to_ref(self, cx: &impl HasDataLayout) -> Immediate<Prov> {
match self.meta {
MemPlaceMeta::None => Immediate::from(Scalar::from_maybe_pointer(self.ptr, cx)),
@ -220,22 +222,20 @@ impl<'tcx, Prov: Provenance + 'static> Projectable<'tcx, Prov> for MPlaceTy<'tcx
self.layout
}
fn meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
_ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, MemPlaceMeta<M::Provenance>> {
#[inline(always)]
fn meta(&self) -> InterpResult<'tcx, MemPlaceMeta<Prov>> {
Ok(self.meta)
}
fn offset_with_meta(
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
offset: Size,
meta: MemPlaceMeta<Prov>,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, Self> {
Ok(MPlaceTy {
mplace: self.mplace.offset_with_meta_(offset, meta, cx)?,
mplace: self.mplace.offset_with_meta_(offset, meta, ecx)?,
align: self.align.restrict_for_offset(offset),
layout,
})
@ -255,25 +255,33 @@ impl<'tcx, Prov: Provenance + 'static> Projectable<'tcx, Prov> for PlaceTy<'tcx,
self.layout
}
fn meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, MemPlaceMeta<M::Provenance>> {
ecx.place_meta(self)
fn meta(&self) -> InterpResult<'tcx, MemPlaceMeta<Prov>> {
Ok(match self.as_mplace_or_local() {
Left(mplace) => mplace.meta,
Right(_) => {
if self.layout.is_unsized() {
// Unsized `Place::Local` cannot occur. We create a MemPlace for all unsized locals during argument passing.
// However, ConstProp doesn't do that, so we can run into this nonsense situation.
throw_inval!(ConstPropNonsense);
}
MemPlaceMeta::None
}
})
}
fn offset_with_meta(
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
offset: Size,
meta: MemPlaceMeta<Prov>,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, Self> {
Ok(match self.as_mplace_or_local() {
Left(mplace) => mplace.offset_with_meta(offset, meta, layout, cx)?.into(),
Left(mplace) => mplace.offset_with_meta(offset, meta, layout, ecx)?.into(),
Right((frame, local, old_offset)) => {
debug_assert!(layout.is_sized(), "unsized locals should live in memory");
assert_matches!(meta, MemPlaceMeta::None); // we couldn't store it anyway...
let new_offset = cx
let new_offset = ecx
.data_layout()
.offset(old_offset.unwrap_or(Size::ZERO).bytes(), offset.bytes())?;
PlaceTy {
@ -399,20 +407,6 @@ where
Prov: Provenance + 'static,
M: Machine<'mir, 'tcx, Provenance = Prov>,
{
/// Get the metadata of the given place.
pub(super) fn place_meta(
&self,
place: &PlaceTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx, MemPlaceMeta<M::Provenance>> {
if place.layout.is_unsized() {
// For `Place::Local`, the metadata is stored with the local, not the place. So we have
// to look that up first.
self.place_to_op(place)?.meta()
} else {
Ok(MemPlaceMeta::None)
}
}
/// Take a value, which represents a (thin or wide) reference, and make it a place.
/// Alignment is just based on the type. This is the inverse of `mplace_to_ref()`.
///
@ -537,8 +531,14 @@ where
frame: usize,
local: mir::Local,
) -> InterpResult<'tcx, PlaceTy<'tcx, M::Provenance>> {
let layout = self.layout_of_local(&self.stack()[frame], local, None)?;
let place = Place::Local { frame, local, offset: None };
// Other parts of the system rely on `Place::Local` never being unsized.
// So we eagerly check here if this local has an MPlace, and if yes we use it.
let frame_ref = &self.stack()[frame];
let layout = self.layout_of_local(frame_ref, local, None)?;
let place = match frame_ref.locals[local].access()? {
Operand::Immediate(_) => Place::Local { frame, local, offset: None },
Operand::Indirect(mplace) => Place::Ptr(*mplace),
};
Ok(PlaceTy { place, layout, align: layout.align.abi })
}

66
compiler/rustc_const_eval/src/interpret/projection.rs
View File

@ -7,12 +7,13 @@
//! but we still need to do bounds checking and adjust the layout. To not duplicate that with MPlaceTy, we actually
//! implement the logic on OpTy, and MPlaceTy calls that.
use std::marker::PhantomData;
use std::ops::Range;
use rustc_middle::mir;
use rustc_middle::ty;
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
use rustc_middle::ty::Ty;
use rustc_middle::ty::TyCtxt;
use rustc_target::abi::HasDataLayout;
use rustc_target::abi::Size;
use rustc_target::abi::{self, VariantIdx};
@ -24,44 +25,42 @@ pub trait Projectable<'tcx, Prov: Provenance>: Sized + std::fmt::Debug {
fn layout(&self) -> TyAndLayout<'tcx>;
/// Get the metadata of a wide value.
fn meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, MemPlaceMeta<M::Provenance>>;
fn meta(&self) -> InterpResult<'tcx, MemPlaceMeta<Prov>>;
fn len<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, u64> {
self.meta(ecx)?.len(self.layout(), ecx)
self.meta()?.len(self.layout(), ecx)
}
/// Offset the value by the given amount, replacing the layout and metadata.
fn offset_with_meta(
fn offset_with_meta<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
offset: Size,
meta: MemPlaceMeta<Prov>,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, Self>;
fn offset(
fn offset<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
offset: Size,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, Self> {
assert!(layout.is_sized());
self.offset_with_meta(offset, MemPlaceMeta::None, layout, cx)
self.offset_with_meta(offset, MemPlaceMeta::None, layout, ecx)
}
fn transmute(
fn transmute<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&self,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, Self> {
assert!(self.layout().is_sized() && layout.is_sized());
assert_eq!(self.layout().size, layout.size);
self.offset_with_meta(Size::ZERO, MemPlaceMeta::None, layout, cx)
self.offset_with_meta(Size::ZERO, MemPlaceMeta::None, layout, ecx)
}
/// Convert this to an `OpTy`. This might be an irreversible transformation, but is useful for
@ -72,6 +71,28 @@ pub trait Projectable<'tcx, Prov: Provenance>: Sized + std::fmt::Debug {
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>>;
}
/// A type representing iteration over the elements of an array.
pub struct ArrayIterator<'tcx, 'a, Prov: Provenance + 'static, P: Projectable<'tcx, Prov>> {
base: &'a P,
range: Range<u64>,
stride: Size,
field_layout: TyAndLayout<'tcx>,
_phantom: PhantomData<Prov>, // otherwise it says `Prov` is never used...
}
impl<'tcx, 'a, Prov: Provenance + 'static, P: Projectable<'tcx, Prov>>
ArrayIterator<'tcx, 'a, Prov, P>
{
/// Should be the same `ecx` on each call, and match the one used to create the iterator.
pub fn next<'mir, M: Machine<'mir, 'tcx, Provenance = Prov>>(
&mut self,
ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, Option<(u64, P)>> {
let Some(idx) = self.range.next() else { return Ok(None) };
Ok(Some((idx, self.base.offset(self.stride * idx, self.field_layout, ecx)?)))
}
}
// FIXME: Working around https://github.com/rust-lang/rust/issues/54385
impl<'mir, 'tcx: 'mir, Prov, M> InterpCx<'mir, 'tcx, M>
where
@ -104,7 +125,7 @@ where
// But const-prop actually feeds us such nonsense MIR! (see test `const_prop/issue-86351.rs`)
throw_inval!(ConstPropNonsense);
}
let base_meta = base.meta(self)?;
let base_meta = base.meta()?;
// Re-use parent metadata to determine dynamic field layout.
// With custom DSTS, this *will* execute user-defined code, but the same
// happens at run-time so that's okay.
@ -132,7 +153,7 @@ where
base: &P,
variant: VariantIdx,
) -> InterpResult<'tcx, P> {
assert!(!base.meta(self)?.has_meta());
assert!(!base.meta()?.has_meta());
// Downcasts only change the layout.
// (In particular, no check about whether this is even the active variant -- that's by design,
// see https://github.com/rust-lang/rust/issues/93688#issuecomment-1032929496.)
@ -206,20 +227,13 @@ where
pub fn project_array_fields<'a, P: Projectable<'tcx, M::Provenance>>(
&self,
base: &'a P,
) -> InterpResult<'tcx, impl Iterator<Item = InterpResult<'tcx, P>> + 'a>
where
'tcx: 'a,
{
) -> InterpResult<'tcx, ArrayIterator<'tcx, 'a, M::Provenance, P>> {
let abi::FieldsShape::Array { stride, .. } = base.layout().fields else {
span_bug!(self.cur_span(), "operand_array_fields: expected an array layout");
};
let len = base.len(self)?;
let field_layout = base.layout().field(self, 0);
let tcx: TyCtxt<'tcx> = *self.tcx;
// `Size` multiplication
Ok((0..len).map(move |i| {
base.offset_with_meta(stride * i, MemPlaceMeta::None, field_layout, &tcx)
}))
Ok(ArrayIterator { base, range: 0..len, stride, field_layout, _phantom: PhantomData })
}
/// Subslicing

5
compiler/rustc_const_eval/src/interpret/visitor.rs
View File

@ -170,8 +170,9 @@ pub trait ValueVisitor<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized {
}
}
FieldsShape::Array { .. } => {
for (idx, field) in self.ecx().project_array_fields(v)?.enumerate() {
self.visit_field(v, idx, &field?)?;
let mut iter = self.ecx().project_array_fields(v)?;
while let Some((idx, field)) = iter.next(self.ecx())? {
self.visit_field(v, idx.try_into().unwrap(), &field)?;
}
}
}

24
src/tools/miri/tests/pass/unsized.rs
View File

@ -2,6 +2,7 @@
//@[tree]compile-flags: -Zmiri-tree-borrows
#![feature(unsized_tuple_coercion)]
#![feature(unsized_fn_params)]
#![feature(custom_mir, core_intrinsics)]
use std::mem;
@ -32,7 +33,30 @@ fn unsized_params() {
f3(*p);
}
fn unsized_field_projection() {
use std::intrinsics::mir::*;
pub struct S<T: ?Sized>(T);
#[custom_mir(dialect = "runtime", phase = "optimized")]
fn f(x: S<[u8]>) {
mir! {
{
let idx = 0;
// Project to an unsized field of an unsized local.
x.0[idx] = 0;
let _val = x.0[idx];
Return()
}
}
}
let x: Box<S<[u8]>> = Box::new(S([0]));
f(*x);
}
fn main() {
unsized_tuple();
unsized_params();
unsized_field_projection();
}