Pass the dest place into read and move the actual writing to pointer to a free function

Author: tiif

src/shims/unix/fd.rs

@@ -538,7 +538,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         buf: Pointer,
         count: u64,
         offset: Option<i128>,
-    ) -> InterpResult<'tcx, Scalar> {
+        dest: &MPlaceTy<'tcx>,
+    ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
 
         // Isolation check is done via `FileDescriptor` trait.
@@ -558,50 +559,33 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // We temporarily dup the FD to be able to retain mutable access to `this`.
         let Some(fd) = this.machine.fds.get(fd) else {
             trace!("read: FD not found");
-            return Ok(Scalar::from_target_isize(this.fd_not_found()?, this));
+            let res = Scalar::from_target_isize(this.fd_not_found()?, this);
+            this.write_scalar(res, dest)?;
+            return Ok(());
         };
 
         trace!("read: FD mapped to {fd:?}");
         // We want to read at most `count` bytes. We are sure that `count` is not negative
         // because it was a target's `usize`. Also we are sure that its smaller than
         // `usize::MAX` because it is bounded by the host's `isize`.
 
-        // TODO: shouldn't pass vec, just pass pointer and write it.
+        // TODO: shouldn't pass vec, just pass pointer and directly write to it.
         let mut bytes = vec![0; usize::try_from(count).unwrap()];
         let result = match offset {
             None => fd.read(&fd, communicate, &mut bytes, buf, this),
             Some(offset) => {
                 let Ok(offset) = u64::try_from(offset) else {
                     let einval = this.eval_libc("EINVAL");
                     this.set_last_error(einval)?;
-                    return Ok(Scalar::from_target_isize(-1, this));
+                    this.write_scalar(Scalar::from_target_isize(-1, this), dest)?;
+                    return Ok(());
                 };
                 fd.pread(communicate, &mut bytes, offset, this)
             }
         };
-
-        // `File::read` never returns a value larger than `count`, so this cannot fail.
-        match result?.map(|c| i64::try_from(c).unwrap()) {
-            // TODO: get the byte out the result, then write the ptr.
-            // try to pass this the write_ptr inside write
-            // Pass the pointer inside the write function.
-            Ok(read_bytes) => {
-                // If reading to `bytes` did not fail, we write those bytes to the buffer.
-                // Crucially, if fewer than `bytes.len()` bytes were read, only write
-                // that much into the output buffer!
-
-                // TODO: write to pointer here.
-                this.write_bytes_ptr(
-                    buf,
-                    bytes[..usize::try_from(read_bytes).unwrap()].iter().copied(),
-                )?;
-                Ok(Scalar::from_target_isize(read_bytes, this))
-            }
-            Err(e) => {
-                this.set_last_error_from_io_error(e)?;
-                Ok(Scalar::from_target_isize(-1, this))
-            }
-        }
+        let res = write_byte_helper(buf, bytes, result, this)?;
+        this.write_scalar(res, dest)?;
+        Ok(())
     }
 
     fn write(
@@ -647,3 +631,32 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         Ok(Scalar::from_target_isize(this.try_unwrap_io_result(result)?, this))
     }
 }
+// TODO: move this as a free function later
+fn write_byte_helper<'tcx>(
+    buf: Pointer,
+    bytes: Vec<u8>,
+    result: InterpResult<'tcx, io::Result<usize>>,
+    ecx: &mut MiriInterpCx<'tcx>,
+) -> InterpResult<'tcx, Scalar> {
+    // `File::read` never returns a value larger than `count`, so this cannot fail.
+    match result?.map(|c| i64::try_from(c).unwrap()) {
+        // TODO: get the byte out the result, then write the ptr.
+        // try to pass this the write_ptr inside write
+        // Pass the pointer inside the write function.
+        Ok(read_bytes) => {
+            // If reading to `bytes` did not fail, we write those bytes to the buffer.
+            // Crucially, if fewer than `bytes.len()` bytes were read, only write
+            // that much into the output buffer!
+            // TODO: write to pointer here.
+            ecx.write_bytes_ptr(
+                buf,
+                bytes[..usize::try_from(read_bytes).unwrap()].iter().copied(),
+            )?;
+            Ok(Scalar::from_target_isize(read_bytes, ecx))
+        }
+        Err(e) => {
+            ecx.set_last_error_from_io_error(e)?;
+            Ok(Scalar::from_target_isize(-1, ecx))
+        }
+    }
+}

src/shims/unix/foreign_items.rs

@@ -92,8 +92,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 let fd = this.read_scalar(fd)?.to_i32()?;
                 let buf = this.read_pointer(buf)?;
                 let count = this.read_target_usize(count)?;
-                let result = this.read(fd, buf, count, None)?;
-                this.write_scalar(result, dest)?;
+                this.read(fd, buf, count, None, dest)?;
             }
             "write" => {
                 let [fd, buf, n] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
@@ -111,8 +110,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 let buf = this.read_pointer(buf)?;
                 let count = this.read_target_usize(count)?;
                 let offset = this.read_scalar(offset)?.to_int(this.libc_ty_layout("off_t").size)?;
-                let result = this.read(fd, buf, count, Some(offset))?;
-                this.write_scalar(result, dest)?;
+                this.read(fd, buf, count, Some(offset), dest)?;
             }
             "pwrite" => {
                 let [fd, buf, n, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
@@ -131,8 +129,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 let buf = this.read_pointer(buf)?;
                 let count = this.read_target_usize(count)?;
                 let offset = this.read_scalar(offset)?.to_int(this.libc_ty_layout("off64_t").size)?;
-                let result = this.read(fd, buf, count, Some(offset))?;
-                this.write_scalar(result, dest)?;
+                this.read(fd, buf, count, Some(offset), dest)?;
             }
             "pwrite64" => {
                 let [fd, buf, n, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;