Use new write_bytes method

Author: pvdrz

.appveyor.yml

@@ -12,6 +12,9 @@ branches:
     - auto
     - try
 
+matrix:
+  fast_finish: true     # set this flag to immediately finish build once one of the jobs fails.
+
 cache:
     - '%USERPROFILE%\.cargo'
     - '%USERPROFILE%\.rustup'

Cargo.lock

@@ -40,6 +40,7 @@ log = "0.4"
 shell-escape = "0.1.4"
 hex = "0.3.2"
 rand = "0.7"
+itertools = "0.8"
 
 # A noop dependency that changes in the Rust repository, it's a bit of a hack.
 # See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`

Cargo.toml

@@ -1 +1 @@
-7979016aff545f7b41cc517031026020b340989d
+6576f4be5af31a5e61dfc0cf50b7130e6c6dfb35

rust-version

@@ -257,8 +257,9 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) {
                 trace!("-------------------");
                 trace!("Frame {}", i);
                 trace!("    return: {:?}", frame.return_place.map(|p| *p));
-                for (i, local) in frame.locals.iter().enumerate() {
-                    trace!("    local {}: {:?}", i, local.value);
+                for (_i, _local) in frame.locals.iter().enumerate() {
+                    //trace!("    local {}: {:?}", i, local.value);
+                    //FIXME: enable this again when the LocalValue Debug impl is back
                 }
             }
         }

src/eval.rs

@@ -5,7 +5,7 @@ use rustc::hir::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc::mir;
 use rustc::ty::{
     self,
-    layout::{self, Align, LayoutOf, Size, TyLayout},
+    layout::{self, LayoutOf, Size, TyLayout},
 };
 
 use rand::RngCore;
@@ -95,13 +95,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         }
         let this = self.eval_context_mut();
 
-        // Don't forget the bounds check.
-        let ptr = this.memory.check_ptr_access(
-            ptr,
-            Size::from_bytes(len as u64),
-            Align::from_bytes(1).unwrap()
-        )?.expect("we already checked for size 0");
-
         let mut data = vec![0; len];
 
         if this.machine.communicate {
@@ -114,7 +107,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             rng.fill_bytes(&mut data);
         }
 
-        this.memory.get_mut(ptr.alloc_id)?.write_bytes(&*this.tcx, ptr, &data)
+        this.memory.write_bytes(ptr, data.iter().copied())
     }
 
     /// Visits the memory covered by `place`, sensitive to freezing: the 3rd parameter
@@ -420,27 +413,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
     /// Helper function to write an OsStr as a null-terminated sequence of bytes, which is what
     /// the Unix APIs usually handle.
-    fn write_os_str_to_c_string(&mut self, os_str: &OsStr, ptr: Pointer<Tag>, size: u64) -> InterpResult<'tcx> {
+    fn write_os_str_to_c_string(&mut self, os_str: &OsStr, scalar: Scalar<Tag>, size: u64) -> InterpResult<'tcx> {
         let bytes = os_str_to_bytes(os_str)?;
-        let len = bytes.len();
         // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null
         // terminator to memory using the `ptr` pointer would cause an overflow.
-        if size <= len as u64 {
-            throw_unsup_format!("OsString of length {} is too large for destination buffer of size {}", len, size)
+        if size <= bytes.len() as u64 {
+            throw_unsup_format!("OsString of length {} is too large for destination buffer of size {}", bytes.len(), size)
         }
-        let actual_len = (len as u64)
-            .checked_add(1)
-            .map(Size::from_bytes)
-            .ok_or_else(|| err_unsup_format!("OsString of length {} is too large", len))?;
-        let this = self.eval_context_mut();
-        this.memory.check_ptr_access(ptr.into(), actual_len, Align::from_bytes(1).unwrap())?;
-        let buffer = this.memory.get_mut(ptr.alloc_id)?.get_bytes_mut(&*this.tcx, ptr, actual_len)?;
-        buffer[..len].copy_from_slice(bytes);
-        // This is ok because the buffer was strictly larger than `bytes`, so after adding the
-        // null terminator, the buffer size is larger or equal to `bytes.len()`, meaning that
-        // `bytes` actually fit inside tbe buffer.
-        buffer[len] = 0;
-        Ok(())
+        // FIXME: We should use `Iterator::chain` instead when rust-lang/rust#65704 lands.
+        self.eval_context_mut().memory.write_bytes(scalar, [bytes, &[0]].concat())
     }
 }
 

src/helpers.rs

@@ -123,13 +123,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("getcwd")?;
 
-        let buf = this.force_ptr(this.read_scalar(buf_op)?.not_undef()?)?;
+        let buf = this.read_scalar(buf_op)?.not_undef()?;
         let size = this.read_scalar(size_op)?.to_usize(&*this.tcx)?;
         // If we cannot get the current directory, we return null
         match env::current_dir() {
             Ok(cwd) => {
                 if this.write_os_str_to_c_string(&OsString::from(cwd), buf, size).is_ok() {
-                    return Ok(Scalar::Ptr(buf));
+                    return Ok(buf);
                 }
                 let erange = this.eval_libc("ERANGE")?;
                 this.set_last_error(erange)?;

src/shims/env.rs

@@ -52,9 +52,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             if zero_init {
                 // We just allocated this, the access is definitely in-bounds.
                 this.memory
-                    .get_mut(ptr.alloc_id)
-                    .unwrap()
-                    .write_repeat(&*this.tcx, ptr, 0, Size::from_bytes(size))
+                    .write_bytes(ptr.into(), itertools::repeat_n(0u8, size as usize))
                     .unwrap();
             }
             Scalar::Ptr(ptr)
@@ -229,9 +227,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 );
                 // We just allocated this, the access is definitely in-bounds.
                 this.memory
-                    .get_mut(ptr.alloc_id)
-                    .unwrap()
-                    .write_repeat(tcx, ptr, 0, Size::from_bytes(size))
+                    .write_bytes(ptr.into(), itertools::repeat_n(0u8, size as usize))
                     .unwrap();
                 this.write_scalar(Scalar::Ptr(ptr), dest)?;
             }
@@ -841,25 +837,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             }
             "GetSystemInfo" => {
                 let system_info = this.deref_operand(args[0])?;
-                let system_info_ptr = this
-                    .check_mplace_access(system_info, None)?
-                    .expect("cannot be a ZST");
-                // We rely on `deref_operand` doing bounds checks for us.
                 // Initialize with `0`.
                 this.memory
-                    .get_mut(system_info_ptr.alloc_id)?
-                    .write_repeat(tcx, system_info_ptr, 0, system_info.layout.size)?;
+                    .write_bytes(system_info.ptr, itertools::repeat_n(0, system_info.layout.size.bytes() as usize))?;
                 // Set number of processors.
                 let dword_size = Size::from_bytes(4);
-                let offset = 2 * dword_size + 3 * tcx.pointer_size();
-                this.memory
-                    .get_mut(system_info_ptr.alloc_id)?
-                    .write_scalar(
-                        tcx,
-                        system_info_ptr.offset(offset, tcx)?,
-                        Scalar::from_int(NUM_CPUS, dword_size).into(),
-                        dword_size,
-                    )?;
+                let num_cpus = this.mplace_field(system_info, 6)?;
+                this.write_scalar(
+                    Scalar::from_int(NUM_CPUS, dword_size),
+                    num_cpus.into(),
+                )?;
             }
 
             "TlsAlloc" => {

src/shims/foreign_items.rs

@@ -356,11 +356,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                         _ => {
                             // Do it in memory
                             let mplace = this.force_allocation(dest)?;
-                            mplace.meta.unwrap_none();
-                            // not a zst, must be valid pointer
-                            let ptr = mplace.ptr.to_ptr()?;
-                            // we know the return place is in-bounds
-                            this.memory.get_mut(ptr.alloc_id)?.write_repeat(tcx, ptr, 0, dest.layout.size)?;
+                            mplace.meta.unwrap_none(); // must be sized
+                            this.memory.write_bytes(mplace.ptr, itertools::repeat_n(0, dest.layout.size.bytes() as usize))?;
                         }
                     }
                 }
@@ -565,16 +562,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let ptr = this.read_scalar(args[0])?.not_undef()?;
                 let count = this.read_scalar(args[2])?.to_usize(this)?;
                 let byte_count = ty_layout.size * count;
-                match this.memory.check_ptr_access(ptr, byte_count, ty_layout.align.abi)? {
-                    Some(ptr) => {
-                        this.memory
-                            .get_mut(ptr.alloc_id)?
-                            .write_repeat(tcx, ptr, val_byte, byte_count)?;
-                    }
-                    None => {
-                        // Size is 0, nothing to do.
-                    }
-                }
+                this.memory.write_bytes(ptr, itertools::repeat_n(val_byte, byte_count.bytes() as usize))?;
             }
 
             name => throw_unsup_format!("unimplemented intrinsic: {}", name),

src/shims/intrinsics.rs

@@ -1,19 +1,20 @@
 #![feature(untagged_unions)]
-#![allow(unions_with_drop_fields)]
 
 #[repr(C)]
+#[derive(Clone, Copy)]
 struct Pair<T, U>(T, U);
 #[repr(C)]
+#[derive(Clone, Copy)]
 struct Triple<T>(T, T, T);
 
 #[repr(C)]
-union U<A, B> {
+union U<A: Copy, B: Copy> {
     a: Pair<A, A>,
     b: B,
 }
 
 #[repr(C)]
-union W<A, B> {
+union W<A: Copy, B: Copy> {
     a: A,
     b: B,
 }