Refactor return_read_bytes_and_count and return_written_byte_count_or_error

Author: tiif

src/tools/miri/src/helpers.rs

@@ -2,7 +2,7 @@ use std::collections::BTreeSet;
 use std::num::NonZero;
 use std::sync::Mutex;
 use std::time::Duration;
-use std::{cmp, iter};
+use std::{cmp,io,iter};
 
 use rand::RngCore;
 use rustc_apfloat::Float;
@@ -839,6 +839,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 self.set_last_error(self.io_error_to_errnum(err)?)
 }
 
+/// Sets the last OS error using a `std::io::ErrorKind` and writes -1 to dest place.
+fnset_last_error_and_return(
+&mutself,
+err:implInto<io::Error>,
+dest:&MPlaceTy<'tcx>,
+) -> InterpResult<'tcx>{
+self.set_last_error(self.io_error_to_errnum(err.into())?)?;
+self.write_int(-1, dest)?;
+Ok(())
+}
+
 /// Helper function that consumes an `std::io::Result<T>` and returns an
 /// `InterpResult<'tcx,T>::Ok` instead. In case the result is an error, this function returns
 /// `Ok(-1)` and sets the last OS error accordingly.

src/tools/miri/src/shims/unix/fd.rs

@@ -150,7 +150,10 @@ impl FileDescription for io::Stdin {
  helpers::isolation_abort_error("`read` from stdin")?;
 }
 let result = Read::read(&mut{self},&mut bytes);
- ecx.return_read_bytes_and_count(ptr,&bytes, result, dest)
+match result {
+Ok(read_size) => ecx.return_read_success(ptr,&bytes, read_size, dest),
+Err(e) => ecx.set_last_error_and_return(e, dest),
+}
 }
 
 fnis_tty(&self,communicate_allowed:bool) -> bool{
@@ -181,7 +184,10 @@ impl FileDescription for io::Stdout {
 // the host -- there is no good in adding extra buffering
 // here.
  io::stdout().flush().unwrap();
- ecx.return_written_byte_count_or_error(result, dest)
+match result {
+Ok(write_size) => ecx.return_write_success(write_size, dest),
+Err(e) => ecx.set_last_error_and_return(e, dest),
+}
 }
 
 fnis_tty(&self,communicate_allowed:bool) -> bool{
@@ -207,7 +213,10 @@ impl FileDescription for io::Stderr {
 // We allow writing to stderr even with isolation enabled.
 // No need to flush, stderr is not buffered.
 let result = Write::write(&mut{self}, bytes);
- ecx.return_written_byte_count_or_error(result, dest)
+match result {
+Ok(write_size) => ecx.return_write_success(write_size, dest),
+Err(e) => ecx.set_last_error_and_return(e, dest),
+}
 }
 
 fnis_tty(&self,communicate_allowed:bool) -> bool{
@@ -234,8 +243,7 @@ impl FileDescription for NullOutput {
 ecx:&mutMiriInterpCx<'tcx>,
 ) -> InterpResult<'tcx>{
 // We just don't write anything, but report to the user that we did.
-let result = Ok(len);
- ecx.return_written_byte_count_or_error(result, dest)
+ ecx.return_write_success(len, dest)
 }
 }
 
@@ -655,46 +663,39 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 }
 
 /// Helper to implement `FileDescription::read`:
-/// `result` should be the return value of some underlying `read` call that used `bytes` as its output buffer.
+/// This is only used when `read` is successful.
+/// `actual_read_size` should be the return value of some underlying `read` call that used
+/// `bytes` as its output buffer.
 /// The length of `bytes` must not exceed either the host's or the target's `isize`.
-/// If `Result` indicates success, `bytes` is written to `buf` and the size is written to `dest`.
-/// Otherwise, `-1` is written to `dest` and the last libc error is set appropriately.
-fnreturn_read_bytes_and_count(
+/// `bytes` is written to `buf` and the size is written to `dest`.
+fnreturn_read_success(
 &mutself,
 buf:Pointer,
 bytes:&[u8],
-result: io::Result<usize>,
+actual_read_size:usize,
 dest:&MPlaceTy<'tcx>,
 ) -> InterpResult<'tcx>{
 let this = self.eval_context_mut();
-match result {
-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!
- this.write_bytes_ptr(buf, bytes[..read_bytes].iter().copied())?;
-// The actual read size is always less than what got originally requested so this cannot fail.
- this.write_int(u64::try_from(read_bytes).unwrap(), dest)?;
-Ok(())
-}
-Err(e) => {
- this.set_last_error_from_io_error(e)?;
- this.write_int(-1, dest)?;
-Ok(())
-}
-}
+// 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!
+ this.write_bytes_ptr(buf, bytes[..actual_read_size].iter().copied())?;
+
+// The actual read size is always less than what got originally requested so this cannot fail.
+ this.write_int(u64::try_from(actual_read_size).unwrap(), dest)?;
+Ok(())
 }
 
-/// This function writes the number of written bytes (given in `result`) to `dest`, or sets the
-/// last libc error and writes -1 to dest.
-fnreturn_written_byte_count_or_error(
+/// Helper to implement `FileDescription::write`:
+/// This function is only used when `write` is successful, and writes `actual_write_size` to `dest`
+fnreturn_write_success(
 &mutself,
-result: io::Result<usize>,
+actual_write_size:usize,
 dest:&MPlaceTy<'tcx>,
 ) -> InterpResult<'tcx>{
 let this = self.eval_context_mut();
-let result = this.try_unwrap_io_result(result.map(|c| i64::try_from(c).unwrap()))?;
- this.write_int(result, dest)?;
+// The actual write size is always less than what got originally requested so this cannot fail.
+ this.write_int(u64::try_from(actual_write_size).unwrap(), dest)?;
 Ok(())
 }
 }

src/tools/miri/src/shims/unix/fs.rs

@@ -41,7 +41,10 @@ impl FileDescription for FileHandle {
 assert!(communicate_allowed,"isolation should have prevented even opening a file");
 letmut bytes = vec![0; len];
 let result = (&mut&self.file).read(&mut bytes);
- ecx.return_read_bytes_and_count(ptr,&bytes, result, dest)
+match result {
+Ok(read_size) => ecx.return_read_success(ptr,&bytes, read_size, dest),
+Err(e) => ecx.set_last_error_and_return(e, dest),
+}
 }
 
 fnwrite<'tcx>(
@@ -56,7 +59,10 @@ impl FileDescription for FileHandle {
 assert!(communicate_allowed,"isolation should have prevented even opening a file");
 let bytes = ecx.read_bytes_ptr_strip_provenance(ptr,Size::from_bytes(len))?;
 let result = (&mut&self.file).write(bytes);
- ecx.return_written_byte_count_or_error(result, dest)
+match result {
+Ok(write_size) => ecx.return_write_success(write_size, dest),
+Err(e) => ecx.set_last_error_and_return(e, dest),
+}
 }
 
 fnpread<'tcx>(
@@ -84,7 +90,10 @@ impl FileDescription for FileHandle {
  res
 };
 let result = f();
- ecx.return_read_bytes_and_count(ptr,&bytes, result, dest)
+match result {
+Ok(read_size) => ecx.return_read_success(ptr,&bytes, read_size, dest),
+Err(e) => ecx.set_last_error_and_return(e, dest),
+}
 }
 
 fnpwrite<'tcx>(
@@ -112,7 +121,10 @@ impl FileDescription for FileHandle {
  res
 };
 let result = f();
- ecx.return_written_byte_count_or_error(result, dest)
+match result {
+Ok(write_size) => ecx.return_write_success(write_size, dest),
+Err(e) => ecx.set_last_error_and_return(e, dest),
+}
 }
 
 fnseek<'tcx>(

src/tools/miri/src/shims/unix/linux/eventfd.rs

@@ -1,7 +1,7 @@
 //! Linux `eventfd` implementation.
 use std::cell::{Cell,RefCell};
 use std::io;
-use std::io::{Error,ErrorKind};
+use std::io::ErrorKind;
 
 usecrate::concurrency::VClock;
 usecrate::shims::unix::fd::FileDescriptionRef;
@@ -66,9 +66,7 @@ impl FileDescription for Event {
 let ty = ecx.machine.layouts.u64;
 // Check the size of slice, and return error only if the size of the slice < 8.
 if len < ty.size.bytes_usize(){
- ecx.set_last_error_from_io_error(Error::from(ErrorKind::InvalidInput))?;
- ecx.write_int(-1, dest)?;
-returnOk(());
+return ecx.set_last_error_and_return(ErrorKind::InvalidInput, dest);
 }
 
 // eventfd read at the size of u64.
@@ -78,9 +76,7 @@ impl FileDescription for Event {
 let counter = self.counter.get();
 if counter == 0{
 ifself.is_nonblock{
- ecx.set_last_error_from_io_error(Error::from(ErrorKind::WouldBlock))?;
- ecx.write_int(-1, dest)?;
-returnOk(());
+return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest);
 }
 
 throw_unsup_format!("eventfd: blocking is unsupported");
@@ -128,8 +124,7 @@ impl FileDescription for Event {
 let ty = ecx.machine.layouts.u64;
 // Check the size of slice, and return error only if the size of the slice < 8.
 if len < ty.layout.size.bytes_usize(){
-let result = Err(Error::from(ErrorKind::InvalidInput));
-return ecx.return_written_byte_count_or_error(result, dest);
+return ecx.set_last_error_and_return(ErrorKind::InvalidInput, dest);
 }
 
 // Read the user supplied value from the pointer.
@@ -138,8 +133,7 @@ impl FileDescription for Event {
 
 // u64::MAX as input is invalid because the maximum value of counter is u64::MAX - 1.
 if num == u64::MAX{
-let result = Err(Error::from(ErrorKind::InvalidInput));
-return ecx.return_written_byte_count_or_error(result, dest);
+return ecx.set_last_error_and_return(ErrorKind::InvalidInput, dest);
 }
 // If the addition does not let the counter to exceed the maximum value, update the counter.
 // Else, block.
@@ -153,8 +147,7 @@ impl FileDescription for Event {
 }
 None | Some(u64::MAX) =>
 ifself.is_nonblock{
-let result = Err(Error::from(ErrorKind::WouldBlock));
-return ecx.return_written_byte_count_or_error(result, dest);
+return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest);
 }else{
 throw_unsup_format!("eventfd: blocking is unsupported");
 },

src/tools/miri/src/shims/unix/unnamed_socket.rs

@@ -5,7 +5,7 @@
 use std::cell::{Cell,OnceCell,RefCell};
 use std::collections::VecDeque;
 use std::io;
-use std::io::{Error,ErrorKind,Read};
+use std::io::{ErrorKind,Read};
 
 use rustc_target::abi::Size;
 
@@ -138,8 +138,7 @@ impl FileDescription for AnonSocket {
 
 // Always succeed on read size 0.
 if len == 0{
-let result = Ok(0);
-return ecx.return_read_bytes_and_count(ptr,&bytes, result, dest);
+return ecx.return_read_success(ptr,&bytes,0, dest);
 }
 
 letSome(readbuf) = &self.readbufelse{
@@ -152,17 +151,15 @@ impl FileDescription for AnonSocket {
 ifself.peer_fd().upgrade().is_none(){
 // Socketpair with no peer and empty buffer.
 // 0 bytes successfully read indicates end-of-file.
-let result = Ok(0);
-return ecx.return_read_bytes_and_count(ptr,&bytes, result, dest);
+return ecx.return_read_success(ptr,&bytes,0, dest);
 }else{
 ifself.is_nonblock{
 // Non-blocking socketpair with writer and empty buffer.
 // https://linux.die.net/man/2/read
 // EAGAIN or EWOULDBLOCK can be returned for socket,
 // POSIX.1-2001 allows either error to be returned for this case.
 // Since there is no ErrorKind for EAGAIN, WouldBlock is used.
-let result = Err(Error::from(ErrorKind::WouldBlock));
-return ecx.return_read_bytes_and_count(ptr,&bytes, result, dest);
+return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest);
 }else{
 // Blocking socketpair with writer and empty buffer.
 // FIXME: blocking is currently not supported
@@ -194,8 +191,7 @@ impl FileDescription for AnonSocket {
  ecx.check_and_update_readiness(&peer_fd)?;
 }
 
-let result = Ok(actual_read_size);
- ecx.return_read_bytes_and_count(ptr,&bytes, result, dest)
+ ecx.return_read_success(ptr,&bytes, actual_read_size, dest)
 }
 
 fnwrite<'tcx>(
@@ -210,16 +206,14 @@ impl FileDescription for AnonSocket {
 // Always succeed on write size 0.
 // ("If count is zero and fd refers to a file other than a regular file, the results are not specified.")
 if len == 0{
-let result = Ok(0);
-return ecx.return_written_byte_count_or_error(result, dest);
+return ecx.return_write_success(0, dest);
 }
 
 // We are writing to our peer's readbuf.
 letSome(peer_fd) = self.peer_fd().upgrade()else{
 // If the upgrade from Weak to Rc fails, it indicates that all read ends have been
 // closed.
-let result = Err(Error::from(ErrorKind::BrokenPipe));
-return ecx.return_written_byte_count_or_error(result, dest);
+return ecx.set_last_error_and_return(ErrorKind::BrokenPipe, dest);
 };
 
 letSome(writebuf) = &peer_fd.downcast::<AnonSocket>().unwrap().readbufelse{
@@ -233,8 +227,7 @@ impl FileDescription for AnonSocket {
 if available_space == 0{
 ifself.is_nonblock{
 // Non-blocking socketpair with a full buffer.
-let result = Err(Error::from(ErrorKind::WouldBlock));
-return ecx.return_written_byte_count_or_error(result, dest);
+return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest);
 }else{
 // Blocking socketpair with a full buffer.
 throw_unsup_format!("socketpair write: blocking isn't supported yet");
@@ -256,8 +249,7 @@ impl FileDescription for AnonSocket {
 // The kernel does this even if the fd was already readable before, so we follow suit.
  ecx.check_and_update_readiness(&peer_fd)?;
 
-let result = Ok(actual_write_size);
- ecx.return_written_byte_count_or_error(result, dest)
+ ecx.return_write_success(actual_write_size, dest)
 }
 }