Auto merge of #3923 - tiif:refactor, r=RalfJung

Refactor ``return_read_bytes_and_count`` and ``return_written_byte_count_or_error``

Fixes #3904

This PR
- separate the error logic from ``return_read_bytes_and_count`` and ``return_written_byte_count_or_error`` into a helper function ``set_last_error_and_return``.
This commit is contained in:
bors 2024-10-01 06:09:00 +00:00
commit c37539d149
5 changed files with 76 additions and 67 deletions

View File

@ -2,7 +2,7 @@
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 @@ fn set_last_error_from_io_error(&mut self, err: std::io::Error) -> InterpResult<
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.
fn set_last_error_and_return(
&mut self,
err: impl Into<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.

View File

@ -150,7 +150,10 @@ fn read<'tcx>(
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),
}
}
fn is_tty(&self, communicate_allowed: bool) -> bool {
@ -181,7 +184,10 @@ fn write<'tcx>(
// 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),
}
}
fn is_tty(&self, communicate_allowed: bool) -> bool {
@ -207,7 +213,10 @@ fn write<'tcx>(
// 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),
}
}
fn is_tty(&self, communicate_allowed: bool) -> bool {
@ -234,8 +243,7 @@ fn write<'tcx>(
ecx: &mut MiriInterpCx<'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 @@ fn write(
}
/// 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.
fn return_read_bytes_and_count(
/// `bytes` is written to `buf` and the size is written to `dest`.
fn return_read_success(
&mut self,
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())?;
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(read_bytes).unwrap(), dest)?;
this.write_int(u64::try_from(actual_read_size).unwrap(), dest)?;
Ok(())
}
Err(e) => {
this.set_last_error_from_io_error(e)?;
this.write_int(-1, 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.
fn return_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`
fn return_write_success(
&mut self,
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(())
}
}

View File

@ -41,7 +41,10 @@ fn read<'tcx>(
assert!(communicate_allowed, "isolation should have prevented even opening a file");
let mut 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),
}
}
fn write<'tcx>(
@ -56,7 +59,10 @@ fn write<'tcx>(
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),
}
}
fn pread<'tcx>(
@ -84,7 +90,10 @@ fn pread<'tcx>(
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),
}
}
fn pwrite<'tcx>(
@ -112,7 +121,10 @@ fn pwrite<'tcx>(
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),
}
}
fn seek<'tcx>(

View File

@ -1,7 +1,7 @@
//! Linux `eventfd` implementation.
use std::cell::{Cell, RefCell};
use std::io;
use std::io::{Error, ErrorKind};
use std::io::ErrorKind;
use crate::concurrency::VClock;
use crate::shims::unix::fd::FileDescriptionRef;
@ -66,9 +66,7 @@ fn read<'tcx>(
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)?;
return Ok(());
return ecx.set_last_error_and_return(ErrorKind::InvalidInput, dest);
}
// eventfd read at the size of u64.
@ -78,9 +76,7 @@ fn read<'tcx>(
let counter = self.counter.get();
if counter == 0 {
if self.is_nonblock {
ecx.set_last_error_from_io_error(Error::from(ErrorKind::WouldBlock))?;
ecx.write_int(-1, dest)?;
return Ok(());
return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest);
}
throw_unsup_format!("eventfd: blocking is unsupported");
@ -128,8 +124,7 @@ fn write<'tcx>(
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 @@ fn write<'tcx>(
// 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 @@ fn write<'tcx>(
}
None | Some(u64::MAX) =>
if self.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");
},

View File

@ -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 @@ fn read<'tcx>(
// 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);
}
let Some(readbuf) = &self.readbuf else {
@ -152,8 +151,7 @@ fn read<'tcx>(
if self.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 {
if self.is_nonblock {
// Non-blocking socketpair with writer and empty buffer.
@ -161,8 +159,7 @@ fn read<'tcx>(
// 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 @@ fn read<'tcx>(
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)
}
fn write<'tcx>(
@ -210,16 +206,14 @@ fn write<'tcx>(
// 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.
let Some(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);
};
let Some(writebuf) = &peer_fd.downcast::<AnonSocket>().unwrap().readbuf else {
@ -233,8 +227,7 @@ fn write<'tcx>(
if available_space == 0 {
if self.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 @@ fn write<'tcx>(
// 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)
}
}