miri: avoid a bunch of casts by offering usized-based field indexing

Author: RalfJung

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};
@@ -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)
@@ -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),
     };
 

src/librustc_mir/interpret/cast.rs

@@ -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 {

src/librustc_mir/interpret/intrinsics.rs

@@ -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)?;
                 }
             }
@@ -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),
         }

src/librustc_mir/interpret/operand.rs

@@ -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) => {
@@ -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();
@@ -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>,
@@ -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(_) => {
@@ -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);

src/librustc_mir/interpret/place.rs

@@ -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() {
@@ -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(
@@ -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 } => {
@@ -555,7 +561,7 @@ where
                     u64::from(offset)
                 };
 
-                self.mplace_field(base, index)?
+                self.mplace_index(base, index)?
             }
 
             Subslice { from, to, from_end } => {
@@ -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>,
@@ -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.
@@ -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 {
@@ -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)?;
                 }
             }

src/librustc_mir/interpret/step.rs

@@ -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;
@@ -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)?;
                     }
                 }

src/librustc_mir/interpret/terminator.rs

@@ -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>>>>(
                                     )?,
@@ -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 {

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};
@@ -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,
         );
@@ -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!(