1
Fork 0

files: make write take callback to store result, rather than writing to 'dest' directly

This commit is contained in:
Ralf Jung 2025-02-02 17:47:22 +01:00
parent 35842d55be
commit bc5e839f07
5 changed files with 72 additions and 69 deletions

View file

@ -154,8 +154,8 @@ pub trait FileDescription: std::fmt::Debug + FileDescriptionExt {
_communicate_allowed: bool, _communicate_allowed: bool,
_ptr: Pointer, _ptr: Pointer,
_len: usize, _len: usize,
_dest: &MPlaceTy<'tcx>,
_ecx: &mut MiriInterpCx<'tcx>, _ecx: &mut MiriInterpCx<'tcx>,
_finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
throw_unsup_format!("cannot write to {}", self.name()); throw_unsup_format!("cannot write to {}", self.name());
} }
@ -234,22 +234,19 @@ impl FileDescription for io::Stdout {
_communicate_allowed: bool, _communicate_allowed: bool,
ptr: Pointer, ptr: Pointer,
len: usize, len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>, ecx: &mut MiriInterpCx<'tcx>,
finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?; // We allow writing to stdout even with isolation enabled.
// We allow writing to stderr even with isolation enabled. let result = ecx.write_to_host(&*self, len, ptr)?;
let result = Write::write(&mut &*self, bytes);
// Stdout is buffered, flush to make sure it appears on the // Stdout is buffered, flush to make sure it appears on the
// screen. This is the write() syscall of the interpreted // screen. This is the write() syscall of the interpreted
// program, we want it to correspond to a write() syscall on // program, we want it to correspond to a write() syscall on
// the host -- there is no good in adding extra buffering // the host -- there is no good in adding extra buffering
// here. // here.
io::stdout().flush().unwrap(); io::stdout().flush().unwrap();
match result {
Ok(write_size) => ecx.return_write_success(write_size, dest), finish.call(ecx, result)
Err(e) => ecx.set_last_error_and_return(e, dest),
}
} }
fn is_tty(&self, communicate_allowed: bool) -> bool { fn is_tty(&self, communicate_allowed: bool) -> bool {
@ -267,17 +264,13 @@ impl FileDescription for io::Stderr {
_communicate_allowed: bool, _communicate_allowed: bool,
ptr: Pointer, ptr: Pointer,
len: usize, len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>, ecx: &mut MiriInterpCx<'tcx>,
finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?;
// We allow writing to stderr even with isolation enabled. // We allow writing to stderr even with isolation enabled.
let result = ecx.write_to_host(&*self, len, ptr)?;
// No need to flush, stderr is not buffered. // No need to flush, stderr is not buffered.
let result = Write::write(&mut &*self, bytes); finish.call(ecx, result)
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 { fn is_tty(&self, communicate_allowed: bool) -> bool {
@ -299,11 +292,11 @@ impl FileDescription for NullOutput {
_communicate_allowed: bool, _communicate_allowed: bool,
_ptr: Pointer, _ptr: Pointer,
len: usize, len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>, ecx: &mut MiriInterpCx<'tcx>,
finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
// We just don't write anything, but report to the user that we did. // We just don't write anything, but report to the user that we did.
ecx.return_write_success(len, dest) finish.call(ecx, Ok(len))
} }
} }
@ -426,16 +419,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
} }
} }
/// Helper to implement `FileDescription::write`: /// Write data to a host `Write` type, withthe bytes taken from machine memory.
/// This function is only used when `write` is successful, and writes `actual_write_size` to `dest` fn write_to_host(
fn return_write_success(
&mut self, &mut self,
actual_write_size: usize, mut file: impl io::Write,
dest: &MPlaceTy<'tcx>, len: usize,
) -> InterpResult<'tcx> { ptr: Pointer,
) -> InterpResult<'tcx, Result<usize, IoError>> {
let this = self.eval_context_mut(); 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)?; let bytes = this.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?;
interp_ok(()) let result = file.write(bytes);
interp_ok(result.map_err(IoError::HostError))
} }
} }

View file

@ -46,8 +46,8 @@ pub trait UnixFileDescription: FileDescription {
_ptr: Pointer, _ptr: Pointer,
_len: usize, _len: usize,
_offset: u64, _offset: u64,
_dest: &MPlaceTy<'tcx>,
_ecx: &mut MiriInterpCx<'tcx>, _ecx: &mut MiriInterpCx<'tcx>,
_finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
throw_unsup_format!("cannot pwrite to {}", self.name()); throw_unsup_format!("cannot pwrite to {}", self.name());
} }
@ -307,13 +307,33 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
return this.set_last_error_and_return(LibcError("EBADF"), dest); return this.set_last_error_and_return(LibcError("EBADF"), dest);
}; };
let finish = {
let dest = dest.clone();
callback!(
@capture<'tcx> {
count: usize,
dest: MPlaceTy<'tcx>,
}
|this, result: Result<usize, IoError>| {
match result {
Ok(write_size) => {
assert!(write_size <= count);
// This must fit since `count` fits.
this.write_int(u64::try_from(write_size).unwrap(), &dest)
}
Err(e) => {
this.set_last_error_and_return(e, &dest)
}
}}
)
};
match offset { match offset {
None => fd.write(communicate, buf, count, dest, this)?, None => fd.write(communicate, buf, count, this, finish)?,
Some(offset) => { Some(offset) => {
let Ok(offset) = u64::try_from(offset) else { let Ok(offset) = u64::try_from(offset) else {
return this.set_last_error_and_return(LibcError("EINVAL"), dest); return this.set_last_error_and_return(LibcError("EINVAL"), dest);
}; };
fd.as_unix().pwrite(communicate, buf, count, offset, dest, this)? fd.as_unix().pwrite(communicate, buf, count, offset, this, finish)?
} }
}; };
interp_ok(()) interp_ok(())

View file

@ -49,16 +49,13 @@ impl FileDescription for FileHandle {
communicate_allowed: bool, communicate_allowed: bool,
ptr: Pointer, ptr: Pointer,
len: usize, len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>, ecx: &mut MiriInterpCx<'tcx>,
finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
assert!(communicate_allowed, "isolation should have prevented even opening a file"); 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); let result = ecx.write_to_host(&self.file, len, ptr)?;
match result { finish.call(ecx, result)
Ok(write_size) => ecx.return_write_success(write_size, dest),
Err(e) => ecx.set_last_error_and_return(e, dest),
}
} }
fn seek<'tcx>( fn seek<'tcx>(
@ -153,8 +150,8 @@ impl UnixFileDescription for FileHandle {
ptr: Pointer, ptr: Pointer,
len: usize, len: usize,
offset: u64, offset: u64,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>, ecx: &mut MiriInterpCx<'tcx>,
finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
assert!(communicate_allowed, "isolation should have prevented even opening a file"); assert!(communicate_allowed, "isolation should have prevented even opening a file");
// Emulates pwrite using seek + write + seek to restore cursor position. // Emulates pwrite using seek + write + seek to restore cursor position.
@ -172,10 +169,7 @@ impl UnixFileDescription for FileHandle {
res res
}; };
let result = f(); let result = f();
match result { finish.call(ecx, result.map_err(IoError::HostError))
Ok(write_size) => ecx.return_write_success(write_size, dest),
Err(e) => ecx.set_last_error_and_return(e, dest),
}
} }
fn flock<'tcx>( fn flock<'tcx>(

View file

@ -84,20 +84,20 @@ impl FileDescription for EventFd {
_communicate_allowed: bool, _communicate_allowed: bool,
ptr: Pointer, ptr: Pointer,
len: usize, len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>, ecx: &mut MiriInterpCx<'tcx>,
finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
// We're treating the buffer as a `u64`. // We're treating the buffer as a `u64`.
let ty = ecx.machine.layouts.u64; let ty = ecx.machine.layouts.u64;
// Check the size of slice, and return error only if the size of the slice < 8. // Check the size of slice, and return error only if the size of the slice < 8.
if len < ty.layout.size.bytes_usize() { if len < ty.layout.size.bytes_usize() {
return ecx.set_last_error_and_return(ErrorKind::InvalidInput, dest); return finish.call(ecx, Err(ErrorKind::InvalidInput.into()));
} }
// Turn the pointer into a place at the right type. // Turn the pointer into a place at the right type.
let buf_place = ecx.ptr_to_mplace_unaligned(ptr, ty); let buf_place = ecx.ptr_to_mplace_unaligned(ptr, ty);
eventfd_write(buf_place, dest, self, ecx) eventfd_write(buf_place, self, ecx, finish)
} }
fn as_unix(&self) -> &dyn UnixFileDescription { fn as_unix(&self) -> &dyn UnixFileDescription {
@ -183,15 +183,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
/// else just add the user-supplied value to current counter. /// else just add the user-supplied value to current counter.
fn eventfd_write<'tcx>( fn eventfd_write<'tcx>(
buf_place: MPlaceTy<'tcx>, buf_place: MPlaceTy<'tcx>,
dest: &MPlaceTy<'tcx>,
eventfd: FileDescriptionRef<EventFd>, eventfd: FileDescriptionRef<EventFd>,
ecx: &mut MiriInterpCx<'tcx>, ecx: &mut MiriInterpCx<'tcx>,
finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
// Figure out which value we should add. // Figure out which value we should add.
let num = ecx.read_scalar(&buf_place)?.to_u64()?; let num = ecx.read_scalar(&buf_place)?.to_u64()?;
// u64::MAX as input is invalid because the maximum value of counter is u64::MAX - 1. // u64::MAX as input is invalid because the maximum value of counter is u64::MAX - 1.
if num == u64::MAX { if num == u64::MAX {
return ecx.set_last_error_and_return(ErrorKind::InvalidInput, dest); return finish.call(ecx, Err(ErrorKind::InvalidInput.into()));
} }
match eventfd.counter.get().checked_add(num) { match eventfd.counter.get().checked_add(num) {
@ -219,16 +219,14 @@ fn eventfd_write<'tcx>(
ecx.check_and_update_readiness(eventfd)?; ecx.check_and_update_readiness(eventfd)?;
// Return how many bytes we consumed from the user-provided buffer. // Return how many bytes we consumed from the user-provided buffer.
return ecx.write_int(buf_place.layout.size.bytes(), dest); return finish.call(ecx, Ok(buf_place.layout.size.bytes_usize()));
} }
None | Some(u64::MAX) => { None | Some(u64::MAX) => {
// We can't update the state, so we have to block. // We can't update the state, so we have to block.
if eventfd.is_nonblock { if eventfd.is_nonblock {
return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest); return finish.call(ecx, Err(ErrorKind::WouldBlock.into()));
} }
let dest = dest.clone();
eventfd.blocked_write_tid.borrow_mut().push(ecx.active_thread()); eventfd.blocked_write_tid.borrow_mut().push(ecx.active_thread());
let weak_eventfd = FileDescriptionRef::downgrade(&eventfd); let weak_eventfd = FileDescriptionRef::downgrade(&eventfd);
@ -239,7 +237,7 @@ fn eventfd_write<'tcx>(
@capture<'tcx> { @capture<'tcx> {
num: u64, num: u64,
buf_place: MPlaceTy<'tcx>, buf_place: MPlaceTy<'tcx>,
dest: MPlaceTy<'tcx>, finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
weak_eventfd: WeakFileDescriptionRef<EventFd>, weak_eventfd: WeakFileDescriptionRef<EventFd>,
} }
|this, unblock: UnblockKind| { |this, unblock: UnblockKind| {
@ -247,7 +245,7 @@ fn eventfd_write<'tcx>(
// When we get unblocked, try again. We know the ref is still valid, // When we get unblocked, try again. We know the ref is still valid,
// otherwise there couldn't be a `write` that unblocks us. // otherwise there couldn't be a `write` that unblocks us.
let eventfd_ref = weak_eventfd.upgrade().unwrap(); let eventfd_ref = weak_eventfd.upgrade().unwrap();
eventfd_write(buf_place, &dest, eventfd_ref, this) eventfd_write(buf_place, eventfd_ref, this, finish)
} }
), ),
); );

View file

@ -7,8 +7,6 @@ use std::collections::VecDeque;
use std::io; use std::io;
use std::io::ErrorKind; use std::io::ErrorKind;
use rustc_abi::Size;
use crate::concurrency::VClock; use crate::concurrency::VClock;
use crate::shims::files::{ use crate::shims::files::{
EvalContextExt as _, FileDescription, FileDescriptionRef, WeakFileDescriptionRef, EvalContextExt as _, FileDescription, FileDescriptionRef, WeakFileDescriptionRef,
@ -103,10 +101,10 @@ impl FileDescription for AnonSocket {
_communicate_allowed: bool, _communicate_allowed: bool,
ptr: Pointer, ptr: Pointer,
len: usize, len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>, ecx: &mut MiriInterpCx<'tcx>,
finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
anonsocket_write(self, ptr, len, dest, ecx) anonsocket_write(self, ptr, len, ecx, finish)
} }
fn as_unix(&self) -> &dyn UnixFileDescription { fn as_unix(&self) -> &dyn UnixFileDescription {
@ -119,25 +117,25 @@ fn anonsocket_write<'tcx>(
self_ref: FileDescriptionRef<AnonSocket>, self_ref: FileDescriptionRef<AnonSocket>,
ptr: Pointer, ptr: Pointer,
len: usize, len: usize,
dest: &MPlaceTy<'tcx>,
ecx: &mut MiriInterpCx<'tcx>, ecx: &mut MiriInterpCx<'tcx>,
finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
// Always succeed on write size 0. // 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 count is zero and fd refers to a file other than a regular file, the results are not specified.")
if len == 0 { if len == 0 {
return ecx.return_write_success(0, dest); return finish.call(ecx, Ok(0));
} }
// We are writing to our peer's readbuf. // We are writing to our peer's readbuf.
let Some(peer_fd) = self_ref.peer_fd().upgrade() else { let Some(peer_fd) = self_ref.peer_fd().upgrade() else {
// If the upgrade from Weak to Rc fails, it indicates that all read ends have been // If the upgrade from Weak to Rc fails, it indicates that all read ends have been
// closed. It is an error to write even if there would be space. // closed. It is an error to write even if there would be space.
return ecx.set_last_error_and_return(ErrorKind::BrokenPipe, dest); return finish.call(ecx, Err(ErrorKind::BrokenPipe.into()));
}; };
let Some(writebuf) = &peer_fd.readbuf else { let Some(writebuf) = &peer_fd.readbuf else {
// Writing to the read end of a pipe. // Writing to the read end of a pipe.
return ecx.set_last_error_and_return(IoError::LibcError("EBADF"), dest); return finish.call(ecx, Err(IoError::LibcError("EBADF")));
}; };
// Let's see if we can write. // Let's see if we can write.
@ -145,13 +143,12 @@ fn anonsocket_write<'tcx>(
if available_space == 0 { if available_space == 0 {
if self_ref.is_nonblock { if self_ref.is_nonblock {
// Non-blocking socketpair with a full buffer. // Non-blocking socketpair with a full buffer.
return ecx.set_last_error_and_return(ErrorKind::WouldBlock, dest); return finish.call(ecx, Err(ErrorKind::WouldBlock.into()));
} else { } else {
self_ref.blocked_write_tid.borrow_mut().push(ecx.active_thread()); self_ref.blocked_write_tid.borrow_mut().push(ecx.active_thread());
// Blocking socketpair with a full buffer. // Blocking socketpair with a full buffer.
// Block the current thread; only keep a weak ref for this. // Block the current thread; only keep a weak ref for this.
let weak_self_ref = FileDescriptionRef::downgrade(&self_ref); let weak_self_ref = FileDescriptionRef::downgrade(&self_ref);
let dest = dest.clone();
ecx.block_thread( ecx.block_thread(
BlockReason::UnnamedSocket, BlockReason::UnnamedSocket,
None, None,
@ -160,14 +157,14 @@ fn anonsocket_write<'tcx>(
weak_self_ref: WeakFileDescriptionRef<AnonSocket>, weak_self_ref: WeakFileDescriptionRef<AnonSocket>,
ptr: Pointer, ptr: Pointer,
len: usize, len: usize,
dest: MPlaceTy<'tcx>, finish: DynMachineCallback<'tcx, Result<usize, IoError>>,
} }
|this, unblock: UnblockKind| { |this, unblock: UnblockKind| {
assert_eq!(unblock, UnblockKind::Ready); assert_eq!(unblock, UnblockKind::Ready);
// If we got unblocked, then our peer successfully upgraded its weak // If we got unblocked, then our peer successfully upgraded its weak
// ref to us. That means we can also upgrade our weak ref. // ref to us. That means we can also upgrade our weak ref.
let self_ref = weak_self_ref.upgrade().unwrap(); let self_ref = weak_self_ref.upgrade().unwrap();
anonsocket_write(self_ref, ptr, len, &dest, this) anonsocket_write(self_ref, ptr, len, this, finish)
} }
), ),
); );
@ -180,9 +177,9 @@ fn anonsocket_write<'tcx>(
writebuf.clock.join(clock); writebuf.clock.join(clock);
}); });
// Do full write / partial write based on the space available. // Do full write / partial write based on the space available.
let actual_write_size = len.min(available_space); let write_size = len.min(available_space);
let bytes = ecx.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?; let actual_write_size = ecx.write_to_host(&mut writebuf.buf, write_size, ptr)?.unwrap();
writebuf.buf.extend(&bytes[..actual_write_size]); assert_eq!(actual_write_size, write_size);
// Need to stop accessing peer_fd so that it can be notified. // Need to stop accessing peer_fd so that it can be notified.
drop(writebuf); drop(writebuf);
@ -197,7 +194,7 @@ fn anonsocket_write<'tcx>(
// The kernel does this even if the fd was already readable before, so we follow suit. // The kernel does this even if the fd was already readable before, so we follow suit.
ecx.check_and_update_readiness(peer_fd)?; ecx.check_and_update_readiness(peer_fd)?;
return ecx.return_write_success(actual_write_size, dest); return finish.call(ecx, Ok(write_size));
} }
interp_ok(()) interp_ok(())
} }