Auto merge of #53903 - GabrielMajeri:opt-miri-array-slice, r=oli-obk

Optimize miri checking of integer array/slices

This pull request implements the optimization described in #53845 (the  `E-easy` part of that issue, not the refactoring). Instead of checking every element of an integral array, we can check the whole memory range at once.

r? @RalfJung

Author: bors

src/librustc/ich/impls_ty.rs

@@ -521,7 +521,6 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
             ReadBytesAsPointer |
             ReadForeignStatic |
             InvalidPointerMath |
-            ReadUndefBytes |
             DeadLocal |
             StackFrameLimitReached |
             OutOfTls |
@@ -548,6 +547,7 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
             GeneratorResumedAfterReturn |
             GeneratorResumedAfterPanic |
             InfiniteLoop => {}
+            ReadUndefBytes(offset) => offset.hash_stable(hcx, hasher),
             InvalidDiscriminant(val) => val.hash_stable(hcx, hasher),
             Panic { ref msg, ref file, line, col } => {
                 msg.hash_stable(hcx, hasher);

src/librustc/mir/interpret/error.rs

@@ -205,7 +205,7 @@ pub enum EvalErrorKind<'tcx, O> {
     ReadBytesAsPointer,
     ReadForeignStatic,
     InvalidPointerMath,
-    ReadUndefBytes,
+    ReadUndefBytes(Size),
     DeadLocal,
     InvalidBoolOp(mir::BinOp),
     Unimplemented(String),
@@ -331,7 +331,7 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
             InvalidPointerMath =>
                 "attempted to do invalid arithmetic on pointers that would leak base addresses, \
                 e.g. comparing pointers into different allocations",
-            ReadUndefBytes =>
+            ReadUndefBytes(_) =>
                 "attempted to read undefined bytes",
             DeadLocal =>
                 "tried to access a dead local variable",

src/librustc/mir/interpret/mod.rs

@@ -645,16 +645,23 @@ impl UndefMask {
     }
 
     /// Check whether the range `start..end` (end-exclusive) is entirely defined.
-    pub fn is_range_defined(&self, start: Size, end: Size) -> bool {
+    ///
+    /// Returns `Ok(())` if it's defined. Otherwise returns the index of the byte
+    /// at which the first undefined access begins.
+    #[inline]
+    pub fn is_range_defined(&self, start: Size, end: Size) -> Result<(), Size> {
         if end > self.len {
-            return false;
+            return Err(self.len);
         }
-        for i in start.bytes()..end.bytes() {
-            if !self.get(Size::from_bytes(i)) {
-                return false;
-            }
+
+        let idx = (start.bytes()..end.bytes())
+            .map(|i| Size::from_bytes(i))
+            .find(|&i| !self.get(i));
+
+        match idx {
+            Some(idx) => Err(idx),
+            None => Ok(())
         }
-        true
     }
 
     pub fn set_range(&mut self, start: Size, end: Size, new_state: bool) {

src/librustc/mir/interpret/value.rs

@@ -359,7 +359,7 @@ impl<'tcx> ScalarMaybeUndef {
     pub fn not_undef(self) -> EvalResult<'static, Scalar> {
         match self {
             ScalarMaybeUndef::Scalar(scalar) => Ok(scalar),
-            ScalarMaybeUndef::Undef => err!(ReadUndefBytes),
+            ScalarMaybeUndef::Undef => err!(ReadUndefBytes(Size::from_bytes(0))),
         }
     }
 

src/librustc/ty/structural_impls.rs

@@ -511,7 +511,7 @@ impl<'a, 'tcx, O: Lift<'tcx>> Lift<'tcx> for interpret::EvalErrorKind<'a, O> {
             ReadBytesAsPointer => ReadBytesAsPointer,
             ReadForeignStatic => ReadForeignStatic,
             InvalidPointerMath => InvalidPointerMath,
-            ReadUndefBytes => ReadUndefBytes,
+            ReadUndefBytes(offset) => ReadUndefBytes(offset),
             DeadLocal => DeadLocal,
             InvalidBoolOp(bop) => InvalidBoolOp(bop),
             Unimplemented(ref s) => Unimplemented(s.clone()),

src/librustc_mir/interpret/memory.rs

@@ -400,7 +400,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
                     }
                     relocations.push((i, target_id));
                 }
-                if alloc.undef_mask.is_range_defined(i, i + Size::from_bytes(1)) {
+                if alloc.undef_mask.is_range_defined(i, i + Size::from_bytes(1)).is_ok() {
                     // this `as usize` is fine, since `i` came from a `usize`
                     write!(msg, "{:02x} ", alloc.bytes[i.bytes() as usize]).unwrap();
                 } else {
@@ -736,7 +736,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         )?;
         // Undef check happens *after* we established that the alignment is correct.
         // We must not return Ok() for unaligned pointers!
-        if !self.is_defined(ptr, size)? {
+        if self.check_defined(ptr, size).is_err() {
             // this inflates undefined bytes to the entire scalar, even if only a few
             // bytes are undefined
             return Ok(ScalarMaybeUndef::Undef);
@@ -938,21 +938,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         Ok(())
     }
 
-    fn is_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx, bool> {
+    /// Checks that a range of bytes is defined. If not, returns the `ReadUndefBytes`
+    /// error which will report the first byte which is undefined.
+    #[inline]
+    fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> {
         let alloc = self.get(ptr.alloc_id)?;
-        Ok(alloc.undef_mask.is_range_defined(
+        alloc.undef_mask.is_range_defined(
             ptr.offset,
             ptr.offset + size,
-        ))
-    }
-
-    #[inline]
-    fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> {
-        if self.is_defined(ptr, size)? {
-            Ok(())
-        } else {
-            err!(ReadUndefBytes)
-        }
+        ).or_else(|idx| err!(ReadUndefBytes(idx)))
     }
 
     pub fn mark_definedness(

src/librustc_mir/interpret/validity.rs

@@ -290,7 +290,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                             Ok(val) => val,
                             Err(err) => match err.kind {
                                 EvalErrorKind::PointerOutOfBounds { .. } |
-                                EvalErrorKind::ReadUndefBytes =>
+                                EvalErrorKind::ReadUndefBytes(_) =>
                                     return validation_failure!(
                                         "uninitialized or out-of-bounds memory", path
                                     ),
@@ -333,16 +333,16 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                 // The fields don't need to correspond to any bit pattern of the union's fields.
                 // See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389
             },
-            layout::FieldPlacement::Array { .. } if !dest.layout.is_zst() => {
+            layout::FieldPlacement::Array { stride, .. } if !dest.layout.is_zst() => {
                 let dest = dest.to_mem_place(); // non-ZST array/slice/str cannot be immediate
-                // Special handling for strings to verify UTF-8
                 match dest.layout.ty.sty {
+                    // Special handling for strings to verify UTF-8
                     ty::Str => {
                         match self.read_str(dest) {
                             Ok(_) => {},
                             Err(err) => match err.kind {
                                 EvalErrorKind::PointerOutOfBounds { .. } |
-                                EvalErrorKind::ReadUndefBytes =>
+                                EvalErrorKind::ReadUndefBytes(_) =>
                                     // The error here looks slightly different than it does
                                     // for slices, because we do not report the index into the
                                     // str at which we are OOB.
@@ -356,6 +356,55 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                             }
                         }
                     }
+                    // Special handling for arrays/slices of builtin integer types
+                    ty::Array(tys, ..) | ty::Slice(tys) if {
+                        // This optimization applies only for integer types
+                        match tys.sty {
+                            ty::Int(..) | ty::Uint(..) => true,
+                            _ => false,
+                        }
+                    } => {
+                        // This is the length of the array/slice.
+                        let len = dest.len(self)?;
+                        // Since primitive types are naturally aligned and tightly packed in arrays,
+                        // we can use the stride to get the size of the integral type.
+                        let ty_size = stride.bytes();
+                        // This is the size in bytes of the whole array.
+                        let size = Size::from_bytes(ty_size * len);
+
+                        match self.memory.read_bytes(dest.ptr, size) {
+                            // In the happy case, we needn't check anything else.
+                            Ok(_) => {},
+                            // Some error happened, try to provide a more detailed description.
+                            Err(err) => {
+                                // For some errors we might be able to provide extra information
+                                match err.kind {
+                                    EvalErrorKind::ReadUndefBytes(offset) => {
+                                        // Some byte was undefined, determine which
+                                        // element that byte belongs to so we can
+                                        // provide an index.
+                                        let i = (offset.bytes() / ty_size) as usize;
+                                        path.push(PathElem::ArrayElem(i));
+
+                                        return validation_failure!(
+                                            "undefined bytes", path
+                                        )
+                                    },
+                                    EvalErrorKind::PointerOutOfBounds { allocation_size, .. } => {
+                                        // If the array access is out-of-bounds, the first
+                                        // undefined access is the after the end of the array.
+                                        let i = (allocation_size.bytes() * ty_size) as usize;
+                                        path.push(PathElem::ArrayElem(i));
+                                    },
+                                    _ => (),
+                                }
+
+                                return validation_failure!(
+                                    "uninitialized or out-of-bounds memory", path
+                                )
+                            }
+                        }
+                    },
                     _ => {
                         // This handles the unsized case correctly as well, as well as
                         // SIMD an all sorts of other array-like types.

src/librustc_mir/transform/const_prop.rs

@@ -180,7 +180,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
                     | InvalidMemoryLockRelease { .. }
                     | DeallocatedLockedMemory { .. }
                     | InvalidPointerMath
-                    | ReadUndefBytes
+                    | ReadUndefBytes(_)
                     | DeadLocal
                     | InvalidBoolOp(_)
                     | DerefFunctionPointer