Skip to content

Commit

Permalink
Proper error handling for Windows. Requires one-off changes to Linux …
Browse files Browse the repository at this point in the history
…handling
  • Loading branch information
CraftSpider committed Dec 6, 2024
1 parent 19acb93 commit 296d806
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 83 deletions.
48 changes: 34 additions & 14 deletions src/shims/files.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::any::Any;
use std::collections::BTreeMap;
use std::io::{IsTerminal, Read, SeekFrom, Write};
use std::io::{ErrorKind, IsTerminal, Read, SeekFrom, Write};
use std::ops::Deref;
use std::rc::{Rc, Weak};
use std::{fs, io};
Expand All @@ -25,7 +25,7 @@ pub trait FileDescription: std::fmt::Debug + Any {
_len: usize,
_dest: &MPlaceTy<'tcx>,
_ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
throw_unsup_format!("cannot read from {}", self.name());
}

Expand All @@ -40,7 +40,7 @@ pub trait FileDescription: std::fmt::Debug + Any {
_len: usize,
_dest: &MPlaceTy<'tcx>,
_ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
throw_unsup_format!("cannot write to {}", self.name());
}

Expand Down Expand Up @@ -97,7 +97,7 @@ impl FileDescription for io::Stdin {
len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
let mut bytes = vec![0; len];
if !communicate_allowed {
// We want isolation mode to be deterministic, so we have to disallow all reads, even stdin.
Expand All @@ -106,7 +106,10 @@ impl FileDescription for io::Stdin {
let result = Read::read(&mut { self }, &mut bytes);
match result {
Ok(read_size) => ecx.return_read_success(ptr, &bytes, read_size, dest),
Err(e) => ecx.set_last_error_and_return(e, dest),
Err(e) => {
ecx.write_int(-1, dest)?;
interp_ok(Err(e.kind()))
}
}
}

Expand All @@ -128,7 +131,7 @@ impl FileDescription for io::Stdout {
len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?;
// We allow writing to stderr even with isolation enabled.
let result = Write::write(&mut { self }, bytes);
Expand All @@ -140,7 +143,10 @@ impl FileDescription for io::Stdout {
io::stdout().flush().unwrap();
match result {
Ok(write_size) => ecx.return_write_success(write_size, dest),
Err(e) => ecx.set_last_error_and_return(e, dest),
Err(e) => {
ecx.write_int(-1, dest)?;
interp_ok(Err(e.kind()))
}
}
}

Expand All @@ -162,14 +168,17 @@ impl FileDescription for io::Stderr {
len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?;
// We allow writing to stderr even with isolation enabled.
// No need to flush, stderr is not buffered.
let result = Write::write(&mut { self }, bytes);
match result {
Ok(write_size) => ecx.return_write_success(write_size, dest),
Err(e) => ecx.set_last_error_and_return(e, dest),
Err(e) => {
ecx.write_int(-1, dest)?;
interp_ok(Err(e.kind()))
}
}
}

Expand All @@ -195,7 +204,7 @@ impl FileDescription for NullOutput {
len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
// We just don't write anything, but report to the user that we did.
ecx.return_write_success(len, dest)
}
Expand Down Expand Up @@ -384,7 +393,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
bytes: &[u8],
actual_read_size: usize,
dest: &MPlaceTy<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
let this = self.eval_context_mut();
// 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
Expand All @@ -393,7 +402,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

// 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)?;
interp_ok(())
interp_ok(Ok(()))
}

/// Helper to implement `FileDescription::write`:
Expand All @@ -402,10 +411,21 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
&mut self,
actual_write_size: usize,
dest: &MPlaceTy<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
let this = self.eval_context_mut();
// 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)?;
interp_ok(())
interp_ok(Ok(()))
}

fn return_io_error(
&mut self,
error: ErrorKind,
dest: &MPlaceTy<'tcx>,
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
let this = self.eval_context_mut();
this.set_last_error(error)?;
this.write_int(-1, dest)?;
interp_ok(Err(error))
}
}
12 changes: 10 additions & 2 deletions src/shims/unix/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// `usize::MAX` because it is bounded by the host's `isize`.

match offset {
None => fd.read(&fd, communicate, buf, count, dest, this)?,
None =>
match fd.read(&fd, communicate, buf, count, dest, this)? {
Ok(_) => (),
Err(e) => this.set_last_error(e)?,
},
Some(offset) => {
let Ok(offset) = u64::try_from(offset) else {
return this.set_last_error_and_return(LibcError("EINVAL"), dest);
Expand Down Expand Up @@ -286,7 +290,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
};

match offset {
None => fd.write(&fd, communicate, buf, count, dest, this)?,
None =>
match fd.write(&fd, communicate, buf, count, dest, this)? {
Ok(_) => (),
Err(e) => this.set_last_error(e)?,
},
Some(offset) => {
let Ok(offset) = u64::try_from(offset) else {
return this.set_last_error_and_return(LibcError("EINVAL"), dest);
Expand Down
18 changes: 12 additions & 6 deletions src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,16 @@ impl FileDescription for FileHandle {
len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), ErrorKind>> {
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);
match result {
Ok(read_size) => ecx.return_read_success(ptr, &bytes, read_size, dest),
Err(e) => ecx.set_last_error_and_return(e, dest),
Err(e) => {
ecx.set_last_error_and_return(e.kind(), dest)?;
interp_ok(Err(e.kind()))
}
}
}

Expand All @@ -56,13 +59,16 @@ impl FileDescription for FileHandle {
len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
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);
match result {
Ok(write_size) => ecx.return_write_success(write_size, dest),
Err(e) => ecx.set_last_error_and_return(e, dest),
Err(e) => {
ecx.set_last_error_and_return(e.kind(), dest)?;
interp_ok(Err(e.kind()))
}
}
}

Expand Down Expand Up @@ -141,7 +147,7 @@ impl UnixFileDescription for FileHandle {
};
let result = f();
match result {
Ok(read_size) => ecx.return_read_success(ptr, &bytes, read_size, dest),
Ok(read_size) => ecx.return_read_success(ptr, &bytes, read_size, dest).map(|_| ()),
Err(e) => ecx.set_last_error_and_return(e, dest),
}
}
Expand Down Expand Up @@ -172,7 +178,7 @@ impl UnixFileDescription for FileHandle {
};
let result = f();
match result {
Ok(write_size) => ecx.return_write_success(write_size, dest),
Ok(write_size) => ecx.return_write_success(write_size, dest).map(|_| ()),
Err(e) => ecx.set_last_error_and_return(e, dest),
}
}
Expand Down
42 changes: 26 additions & 16 deletions src/shims/unix/linux_like/eventfd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use std::io;
use std::io::ErrorKind;

use crate::concurrency::VClock;
use crate::shims::files::{FileDescription, FileDescriptionRef, WeakFileDescriptionRef};
use crate::shims::files::{
EvalContextExt as _, FileDescription, FileDescriptionRef, WeakFileDescriptionRef,
};
use crate::shims::unix::UnixFileDescription;
use crate::shims::unix::linux_like::epoll::{EpollReadyEvents, EvalContextExt as _};
use crate::*;
Expand Down Expand Up @@ -54,12 +56,12 @@ impl FileDescription for Event {
len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
// We're treating the buffer as a `u64`.
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() {
return ecx.set_last_error_and_return(ErrorKind::InvalidInput, dest);
return ecx.return_io_error(ErrorKind::InvalidInput, dest);
}

// eventfd read at the size of u64.
Expand Down Expand Up @@ -89,12 +91,12 @@ impl FileDescription for Event {
len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
// We're treating the buffer as a `u64`.
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() {
return ecx.set_last_error_and_return(ErrorKind::InvalidInput, dest);
return ecx.return_io_error(ErrorKind::InvalidInput, dest);
}

// Read the user-supplied value from the pointer.
Expand All @@ -103,7 +105,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 {
return ecx.set_last_error_and_return(ErrorKind::InvalidInput, dest);
return ecx.return_io_error(ErrorKind::InvalidInput, dest);
}
// If the addition does not let the counter to exceed the maximum value, update the counter.
// Else, block.
Expand Down Expand Up @@ -198,7 +200,7 @@ fn eventfd_write<'tcx>(
dest: &MPlaceTy<'tcx>,
weak_eventfd: WeakFileDescriptionRef,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
let Some(eventfd_ref) = weak_eventfd.upgrade() else {
throw_unsup_format!("eventfd FD got closed while blocking.")
};
Expand Down Expand Up @@ -232,12 +234,13 @@ fn eventfd_write<'tcx>(
}

// Return how many bytes we consumed from the user-provided buffer.
return ecx.write_int(buf_place.layout.size.bytes(), dest);
ecx.write_int(buf_place.layout.size.bytes(), dest)?;
return interp_ok(Ok(()));
}
None | Some(u64::MAX) => {
// We can't update the state, so we have to block.
if eventfd.is_nonblock {
return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest);
return ecx.return_io_error(ErrorKind::WouldBlock, dest);
}

let dest = dest.clone();
Expand All @@ -256,13 +259,16 @@ fn eventfd_write<'tcx>(
}
@unblock = |this| {
// When we get unblocked, try again.
eventfd_write(num, buf_place, &dest, weak_eventfd, this)
match eventfd_write(num, buf_place, &dest, weak_eventfd, this)? {
Ok(_) => interp_ok(()),
Err(e) => this.set_last_error(e),
}
}
),
);
}
};
interp_ok(())
interp_ok(Ok(()))
}

/// Block thread if the current counter is 0,
Expand All @@ -272,7 +278,7 @@ fn eventfd_read<'tcx>(
dest: &MPlaceTy<'tcx>,
weak_eventfd: WeakFileDescriptionRef,
ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx, Result<(), io::ErrorKind>> {
let Some(eventfd_ref) = weak_eventfd.upgrade() else {
throw_unsup_format!("eventfd FD got closed while blocking.")
};
Expand All @@ -287,7 +293,7 @@ fn eventfd_read<'tcx>(
// Block when counter == 0.
if counter == 0 {
if eventfd.is_nonblock {
return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest);
return ecx.return_io_error(ErrorKind::WouldBlock, dest);
}
let dest = dest.clone();

Expand All @@ -304,7 +310,10 @@ fn eventfd_read<'tcx>(
}
@unblock = |this| {
// When we get unblocked, try again.
eventfd_read(buf_place, &dest, weak_eventfd, this)
match eventfd_read(buf_place, &dest, weak_eventfd, this)? {
Ok(_) => interp_ok(()),
Err(e) => this.set_last_error(e),
}
}
),
);
Expand All @@ -330,7 +339,8 @@ fn eventfd_read<'tcx>(
}

// Tell userspace how many bytes we put into the buffer.
return ecx.write_int(buf_place.layout.size.bytes(), dest);
ecx.write_int(buf_place.layout.size.bytes(), dest)?;
return interp_ok(Ok(()));
}
interp_ok(())
interp_ok(Ok(()))
}
Loading

0 comments on commit 296d806

Please sign in to comment.