Skip to content

Commit

Permalink
miri: avoid a bunch of casts by offering usized-based field indexing
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Mar 25, 2020
1 parent cd15b65 commit d7e2650
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 68 deletions.
11 changes: 7 additions & 4 deletions src/librustc_mir/const_eval/mod.rs
@@ -1,5 +1,7 @@
// Not in interpret to make sure we do not use private implementation details

use std::convert::TryFrom;

use rustc::mir;
use rustc::ty::layout::VariantIdx;
use rustc::ty::{self, TyCtxt};
Expand Down Expand Up @@ -37,7 +39,7 @@ pub(crate) fn const_field<'tcx>(
Some(variant) => ecx.operand_downcast(op, variant).unwrap(),
};
// then project
let field = ecx.operand_field(down, field.index() as u64).unwrap();
let field = ecx.operand_field(down, field.index()).unwrap();
// and finally move back to the const world, always normalizing because
// this is not called for statics.
op_to_const(&ecx, field)
Expand Down Expand Up @@ -68,10 +70,11 @@ pub(crate) fn destructure_const<'tcx>(

let variant = ecx.read_discriminant(op).unwrap().1;

// We go to `usize` as we cannot allocate anything bigger anyway.
let field_count = match val.ty.kind {
ty::Array(_, len) => len.eval_usize(tcx, param_env),
ty::Adt(def, _) => def.variants[variant].fields.len() as u64,
ty::Tuple(substs) => substs.len() as u64,
ty::Array(_, len) => usize::try_from(len.eval_usize(tcx, param_env)).unwrap(),
ty::Adt(def, _) => def.variants[variant].fields.len(),
ty::Tuple(substs) => substs.len(),
_ => bug!("cannot destructure constant {:?}", val),
};

Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/cast.rs
Expand Up @@ -320,11 +320,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Example: `Arc<T>` -> `Arc<Trait>`
// here we need to increase the size of every &T thin ptr field to a fat ptr
for i in 0..src.layout.fields.count() {
let dst_field = self.place_field(dest, i as u64)?;
let dst_field = self.place_field(dest, i)?;
if dst_field.layout.is_zst() {
continue;
}
let src_field = self.operand_field(src, i as u64)?;
let src_field = self.operand_field(src, i)?;
if src_field.layout.ty == dst_field.layout.ty {
self.copy_op(src_field, dst_field)?;
} else {
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/interpret/intrinsics.rs
Expand Up @@ -350,8 +350,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
);

for i in 0..len {
let place = self.place_field(dest, i)?;
let value = if i == index { elem } else { self.operand_field(input, i)? };
let place = self.place_index(dest, i)?;
let value = if i == index { elem } else { self.operand_index(input, i)? };
self.copy_op(value, place)?;
}
}
Expand All @@ -370,7 +370,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
"Return type `{}` must match vector element type `{}`",
dest.layout.ty, e_ty
);
self.copy_op(self.operand_field(args[0], index)?, dest)?;
self.copy_op(self.operand_index(args[0], index)?, dest)?;
}
_ => return Ok(false),
}
Expand Down
22 changes: 18 additions & 4 deletions src/librustc_mir/interpret/operand.rs
Expand Up @@ -351,7 +351,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn operand_field(
&self,
op: OpTy<'tcx, M::PointerTag>,
field: u64,
field: usize,
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
let base = match op.try_as_mplace(self) {
Ok(mplace) => {
Expand All @@ -362,7 +362,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Err(value) => value,
};

let field = field.try_into().unwrap();
let field_layout = op.layout.field(self, field)?;
if field_layout.is_zst() {
let immediate = Scalar::zst().into();
Expand All @@ -384,6 +383,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Ok(OpTy { op: Operand::Immediate(immediate), layout: field_layout })
}

pub fn operand_index(
&self,
op: OpTy<'tcx, M::PointerTag>,
index: u64,
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
if let Ok(index) = usize::try_from(index) {
// We can just treat this as a field.
self.operand_field(op, index)
} else {
// Indexing into a big array. This must be an mplace.
let mplace = op.assert_mem_place(self);
Ok(self.mplace_index(mplace, index)?.into())
}
}

pub fn operand_downcast(
&self,
op: OpTy<'tcx, M::PointerTag>,
Expand All @@ -406,7 +420,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
use rustc::mir::ProjectionElem::*;
Ok(match *proj_elem {
Field(field, _) => self.operand_field(base, u64::try_from(field.index()).unwrap())?,
Field(field, _) => self.operand_field(base, field.index())?,
Downcast(_, variant) => self.operand_downcast(base, variant)?,
Deref => self.deref_operand(base)?.into(),
Subslice { .. } | ConstantIndex { .. } | Index(_) => {
Expand Down Expand Up @@ -593,7 +607,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
};

// read raw discriminant value
let discr_op = self.operand_field(rval, u64::try_from(discr_index).unwrap())?;
let discr_op = self.operand_field(rval, discr_index)?;
let discr_val = self.read_immediate(discr_op)?;
let raw_discr = discr_val.to_scalar_or_undef();
trace!("discr value: {:?}", raw_discr);
Expand Down
93 changes: 54 additions & 39 deletions src/librustc_mir/interpret/place.rs
Expand Up @@ -386,43 +386,20 @@ where
Ok(place)
}

/// Offset a pointer to project to a field. Unlike `place_field`, this is always
/// possible without allocating, so it can take `&self`. Also return the field's layout.
/// Offset a pointer to project to a field of a struct/union. Unlike `place_field`, this is
/// always possible without allocating, so it can take `&self`. Also return the field's layout.
/// This supports both struct and array fields.
///
/// This also works for arrays, but then the `usize` index type is restricting.
/// For indexing into arrays, use `mplace_index`.
#[inline(always)]
pub fn mplace_field(
&self,
base: MPlaceTy<'tcx, M::PointerTag>,
field: u64,
field: usize,
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
// Not using the layout method because we want to compute on u64
let (offset, field_layout) = match base.layout.fields {
layout::FieldPlacement::Arbitrary { ref offsets, .. } => {
let field = usize::try_from(field).unwrap();
(offsets[field], base.layout.field(self, field)?)
}
layout::FieldPlacement::Array { stride, .. } => {
let len = base.len(self)?;
if field >= len {
// This can only be reached in ConstProp and non-rustc-MIR.
throw_ub!(BoundsCheckFailed { len, index: field });
}
// All fields have the same layout.
(Size::mul(stride, field), base.layout.field(self, 9)?)
}
layout::FieldPlacement::Union(count) => {
let field = usize::try_from(field).unwrap();
assert!(
field < count,
"Tried to access field {} of union {:#?} with {} fields",
field,
base.layout,
count
);
// Offset is always 0
(Size::from_bytes(0), base.layout.field(self, field)?)
}
};
let offset = base.layout.fields.offset(field);
let field_layout = base.layout.field(self, field)?;

// Offset may need adjustment for unsized fields.
let (meta, offset) = if field_layout.is_unsized() {
Expand Down Expand Up @@ -452,6 +429,32 @@ where
base.offset(offset, meta, field_layout, self)
}

/// Index into an array.
#[inline(always)]
pub fn mplace_index(
&self,
base: MPlaceTy<'tcx, M::PointerTag>,
index: u64,
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
// Not using the layout method because we want to compute on u64
match base.layout.fields {
layout::FieldPlacement::Array { stride, .. } => {
let len = base.len(self)?;
if index >= len {
// This can only be reached in ConstProp and non-rustc-MIR.
throw_ub!(BoundsCheckFailed { len, index });
}
let offset = Size::mul(stride, index);
// All fields have the same layout.
let field_layout = base.layout.field(self, 0)?;

assert!(!field_layout.is_unsized());
base.offset(offset, MemPlaceMeta::None, field_layout, self)
}
_ => bug!("`mplace_index` called on non-array type {:?}", base.layout.ty),
}
}

// Iterates over all fields of an array. Much more efficient than doing the
// same by repeatedly calling `mplace_array`.
pub(super) fn mplace_array_fields(
Expand Down Expand Up @@ -528,16 +531,19 @@ where
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
use rustc::mir::ProjectionElem::*;
Ok(match *proj_elem {
Field(field, _) => self.mplace_field(base, u64::try_from(field.index()).unwrap())?,
Field(field, _) => self.mplace_field(base, field.index())?,
Downcast(_, variant) => self.mplace_downcast(base, variant)?,
Deref => self.deref_operand(base.into())?,

Index(local) => {
let layout = self.layout_of(self.tcx.types.usize)?;
let n = self.access_local(self.frame(), local, Some(layout))?;
let n = self.read_scalar(n)?;
let n = self.force_bits(n.not_undef()?, self.tcx.data_layout.pointer_size)?;
self.mplace_field(base, u64::try_from(n).unwrap())?
let n = u64::try_from(
self.force_bits(n.not_undef()?, self.tcx.data_layout.pointer_size)?,
)
.unwrap();
self.mplace_index(base, n)?
}

ConstantIndex { offset, min_length, from_end } => {
Expand All @@ -555,7 +561,7 @@ where
u64::from(offset)
};

self.mplace_field(base, index)?
self.mplace_index(base, index)?
}

Subslice { from, to, from_end } => {
Expand All @@ -571,14 +577,23 @@ where
pub fn place_field(
&mut self,
base: PlaceTy<'tcx, M::PointerTag>,
field: u64,
field: usize,
) -> InterpResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
// FIXME: We could try to be smarter and avoid allocation for fields that span the
// entire place.
let mplace = self.force_allocation(base)?;
Ok(self.mplace_field(mplace, field)?.into())
}

pub fn place_index(
&mut self,
base: PlaceTy<'tcx, M::PointerTag>,
index: u64,
) -> InterpResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
let mplace = self.force_allocation(base)?;
Ok(self.mplace_index(mplace, index)?.into())
}

pub fn place_downcast(
&self,
base: PlaceTy<'tcx, M::PointerTag>,
Expand All @@ -604,7 +619,7 @@ where
) -> InterpResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
use rustc::mir::ProjectionElem::*;
Ok(match *proj_elem {
Field(field, _) => self.place_field(base, u64::try_from(field.index()).unwrap())?,
Field(field, _) => self.place_field(base, field.index())?,
Downcast(_, variant) => self.place_downcast(base, variant)?,
Deref => self.deref_operand(self.place_to_op(base)?)?.into(),
// For the other variants, we have to force an allocation.
Expand Down Expand Up @@ -1073,7 +1088,7 @@ where
let size = discr_layout.value.size(self);
let discr_val = truncate(discr_val, size);

let discr_dest = self.place_field(dest, u64::try_from(discr_index).unwrap())?;
let discr_dest = self.place_field(dest, discr_index)?;
self.write_scalar(Scalar::from_uint(discr_val, size), discr_dest)?;
}
layout::Variants::Multiple {
Expand Down Expand Up @@ -1104,7 +1119,7 @@ where
niche_start_val,
)?;
// Write result.
let niche_dest = self.place_field(dest, u64::try_from(discr_index).unwrap())?;
let niche_dest = self.place_field(dest, discr_index)?;
self.write_immediate(*discr_val, niche_dest)?;
}
}
Expand Down
5 changes: 1 addition & 4 deletions src/librustc_mir/interpret/step.rs
Expand Up @@ -2,8 +2,6 @@
//!
//! The main entry point is the `step` method.

use std::convert::TryFrom;

use rustc::mir;
use rustc::mir::interpret::{InterpResult, PointerArithmetic, Scalar};
use rustc::ty::layout::LayoutOf;
Expand Down Expand Up @@ -194,8 +192,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Ignore zero-sized fields.
if !op.layout.is_zst() {
let field_index = active_field_index.unwrap_or(i);
let field_dest =
self.place_field(dest, u64::try_from(field_index).unwrap())?;
let field_dest = self.place_field(dest, field_index)?;
self.copy_op(op, field_dest)?;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/terminator.rs
Expand Up @@ -309,7 +309,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
.map(|&a| Ok(a))
.chain(
(0..untuple_arg.layout.fields.count())
.map(|i| self.operand_field(untuple_arg, i as u64)),
.map(|i| self.operand_field(untuple_arg, i)),
)
.collect::<InterpResult<'_, Vec<OpTy<'tcx, M::PointerTag>>>>(
)?,
Expand All @@ -332,7 +332,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
if Some(local) == body.spread_arg {
// Must be a tuple
for i in 0..dest.layout.fields.count() {
let dest = self.place_field(dest, i as u64)?;
let dest = self.place_field(dest, i)?;
self.pass_argument(rust_abi, &mut caller_iter, dest)?;
}
} else {
Expand Down
7 changes: 4 additions & 3 deletions src/librustc_mir/interpret/traits.rs
@@ -1,3 +1,4 @@
use std::convert::TryFrom;
use std::ops::Mul;

use rustc::mir::interpret::{InterpResult, Pointer, PointerArithmetic, Scalar};
Expand Down Expand Up @@ -56,7 +57,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// `get_vtable` in `rust_codegen_llvm/meth.rs`.
// /////////////////////////////////////////////////////////////////////////////////////////
let vtable = self.memory.allocate(
ptr_size * (3 + methods.len() as u64),
Size::mul(ptr_size, u64::try_from(methods.len()).unwrap().checked_add(3).unwrap()),
ptr_align,
MemoryKind::Vtable,
);
Expand Down Expand Up @@ -172,10 +173,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
.expect("cannot be a ZST");
let alloc = self.memory.get_raw(vtable.alloc_id)?;
let size = alloc.read_ptr_sized(self, vtable.offset(pointer_size, self)?)?.not_undef()?;
let size = self.force_bits(size, pointer_size)? as u64;
let size = u64::try_from(self.force_bits(size, pointer_size)?).unwrap();
let align =
alloc.read_ptr_sized(self, vtable.offset(pointer_size * 2, self)?)?.not_undef()?;
let align = self.force_bits(align, pointer_size)? as u64;
let align = u64::try_from(self.force_bits(align, pointer_size)?).unwrap();

if size >= self.tcx.data_layout().obj_size_bound() {
throw_ub_format!(
Expand Down

0 comments on commit d7e2650

Please sign in to comment.