Skip to content

Add pread and pwrite shims #3743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 74 additions & 19 deletions src/shims/unix/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,30 @@ pub trait FileDescription: std::fmt::Debug + Any {
throw_unsup_format!("cannot write to {}", self.name());
}

/// Reads as much as possible into the given buffer from a given offset,
/// and returns the number of bytes read.
fn pread<'tcx>(
&mut self,
_communicate_allowed: bool,
_bytes: &mut [u8],
_offset: u64,
_ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
throw_unsup_format!("cannot pread from {}", self.name());
}

/// Writes as much as possible from the given buffer starting at a given offset,
/// and returns the number of bytes written.
fn pwrite<'tcx>(
&mut self,
_communicate_allowed: bool,
_bytes: &[u8],
_offset: u64,
_ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
throw_unsup_format!("cannot pwrite to {}", self.name());
}

/// Seeks to the given offset (which can be relative to the beginning, end, or current position).
/// Returns the new position from the start of the stream.
fn seek<'tcx>(
Expand Down Expand Up @@ -380,7 +404,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
Ok((-1).into())
}

fn read(&mut self, fd: i32, buf: Pointer, count: u64) -> InterpResult<'tcx, i64> {
/// Read data from `fd` into buffer specified by `buf` and `count`.
///
/// If `offset` is `None`, reads data from current cursor position associated with `fd`
/// and updates cursor position on completion. Otherwise, reads from the specified offset
/// and keeps the cursor unchanged.
fn read(
&mut self,
fd: i32,
buf: Pointer,
count: u64,
offset: Option<i128>,
) -> InterpResult<'tcx, i64> {
let this = self.eval_context_mut();

// Isolation check is done via `FileDescriptor` trait.
Expand All @@ -398,25 +433,31 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let communicate = this.machine.communicate();

// We temporarily dup the FD to be able to retain mutable access to `this`.
let Some(file_descriptor) = this.machine.fds.dup(fd) else {
let Some(fd) = this.machine.fds.dup(fd) else {
trace!("read: FD not found");
return this.fd_not_found();
};

trace!("read: FD mapped to {:?}", file_descriptor);
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`.
let mut bytes = vec![0; usize::try_from(count).unwrap()];
// `File::read` never returns a value larger than `count`,
// so this cannot fail.
let result = file_descriptor
.borrow_mut()
.read(communicate, &mut bytes, this)?
.map(|c| i64::try_from(c).unwrap());
drop(file_descriptor);

match result {
let result = match offset {
None => fd.borrow_mut().read(communicate, &mut bytes, this),
Some(offset) => {
let Ok(offset) = u64::try_from(offset) else {
let einval = this.eval_libc("EINVAL");
this.set_last_error(einval)?;
return Ok(-1);
};
fd.borrow_mut().pread(communicate, &mut bytes, offset, this)
}
};
drop(fd);

// `File::read` never returns a value larger than `count`, so this cannot fail.
match result?.map(|c| i64::try_from(c).unwrap()) {
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
Expand All @@ -434,7 +475,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}
}

fn write(&mut self, fd: i32, buf: Pointer, count: u64) -> InterpResult<'tcx, i64> {
fn write(
&mut self,
fd: i32,
buf: Pointer,
count: u64,
offset: Option<i128>,
) -> InterpResult<'tcx, i64> {
let this = self.eval_context_mut();

// Isolation check is done via `FileDescriptor` trait.
Expand All @@ -451,16 +498,24 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?.to_owned();
// We temporarily dup the FD to be able to retain mutable access to `this`.
let Some(file_descriptor) = this.machine.fds.dup(fd) else {
let Some(fd) = this.machine.fds.dup(fd) else {
return this.fd_not_found();
};

let result = file_descriptor
.borrow_mut()
.write(communicate, &bytes, this)?
.map(|c| i64::try_from(c).unwrap());
drop(file_descriptor);
let result = match offset {
None => fd.borrow_mut().write(communicate, &bytes, this),
Some(offset) => {
let Ok(offset) = u64::try_from(offset) else {
let einval = this.eval_libc("EINVAL");
this.set_last_error(einval)?;
return Ok(-1);
};
fd.borrow_mut().pwrite(communicate, &bytes, offset, this)
}
};
drop(fd);

let result = result?.map(|c| i64::try_from(c).unwrap());
this.try_unwrap_io_result(result)
}
}
44 changes: 42 additions & 2 deletions src/shims/unix/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +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)?;
let result = this.read(fd, buf, count, None)?;
this.write_scalar(Scalar::from_target_isize(result, this), dest)?;
}
"write" => {
Expand All @@ -101,7 +101,47 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let buf = this.read_pointer(buf)?;
let count = this.read_target_usize(n)?;
trace!("Called write({:?}, {:?}, {:?})", fd, buf, count);
let result = this.write(fd, buf, count)?;
let result = this.write(fd, buf, count, None)?;
// Now, `result` is the value we return back to the program.
this.write_scalar(Scalar::from_target_isize(result, this), dest)?;
}
"pread" => {
let [fd, buf, count, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let fd = this.read_scalar(fd)?.to_i32()?;
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(Scalar::from_target_isize(result, this), dest)?;
}
"pwrite" => {
let [fd, buf, n, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let fd = this.read_scalar(fd)?.to_i32()?;
let buf = this.read_pointer(buf)?;
let count = this.read_target_usize(n)?;
let offset = this.read_scalar(offset)?.to_int(this.libc_ty_layout("off_t").size)?;
trace!("Called pwrite({:?}, {:?}, {:?}, {:?})", fd, buf, count, offset);
let result = this.write(fd, buf, count, Some(offset))?;
// Now, `result` is the value we return back to the program.
this.write_scalar(Scalar::from_target_isize(result, this), dest)?;
}
"pread64" => {
let [fd, buf, count, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let fd = this.read_scalar(fd)?.to_i32()?;
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(Scalar::from_target_isize(result, this), dest)?;
}
"pwrite64" => {
let [fd, buf, n, offset] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let fd = this.read_scalar(fd)?.to_i32()?;
let buf = this.read_pointer(buf)?;
let count = this.read_target_usize(n)?;
let offset = this.read_scalar(offset)?.to_int(this.libc_ty_layout("off64_t").size)?;
trace!("Called pwrite64({:?}, {:?}, {:?}, {:?})", fd, buf, count, offset);
let result = this.write(fd, buf, count, Some(offset))?;
// Now, `result` is the value we return back to the program.
this.write_scalar(Scalar::from_target_isize(result, this), dest)?;
}
Expand Down
48 changes: 48 additions & 0 deletions src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,54 @@ impl FileDescription for FileHandle {
Ok(self.file.write(bytes))
}

fn pread<'tcx>(
&mut self,
communicate_allowed: bool,
bytes: &mut [u8],
offset: u64,
_ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
assert!(communicate_allowed, "isolation should have prevented even opening a file");
// Emulates pread using seek + read + seek to restore cursor position.
// Correctness of this emulation relies on sequential nature of Miri execution.
// The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`.
let mut f = || {
let cursor_pos = self.file.stream_position()?;
self.file.seek(SeekFrom::Start(offset))?;
let res = self.file.read(bytes);
// Attempt to restore cursor position even if the read has failed
self.file
.seek(SeekFrom::Start(cursor_pos))
.expect("failed to restore file position, this shouldn't be possible");
res
};
Ok(f())
}

fn pwrite<'tcx>(
&mut self,
communicate_allowed: bool,
bytes: &[u8],
offset: u64,
_ecx: &mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
assert!(communicate_allowed, "isolation should have prevented even opening a file");
// Emulates pwrite using seek + write + seek to restore cursor position.
// Correctness of this emulation relies on sequential nature of Miri execution.
// The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`.
let mut f = || {
let cursor_pos = self.file.stream_position()?;
self.file.seek(SeekFrom::Start(offset))?;
let res = self.file.write(bytes);
// Attempt to restore cursor position even if the write has failed
self.file
.seek(SeekFrom::Start(cursor_pos))
.expect("failed to restore file position, this shouldn't be possible");
res
};
Ok(f())
}

fn seek<'tcx>(
&mut self,
communicate_allowed: bool,
Expand Down
41 changes: 41 additions & 0 deletions tests/pass/shims/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ fn main() {
test_directory();
test_canonicalize();
test_from_raw_os_error();
#[cfg(unix)]
test_pread_pwrite();
}

fn test_path_conversion() {
Expand Down Expand Up @@ -303,3 +305,42 @@ fn test_from_raw_os_error() {
// Make sure we can also format this.
let _ = format!("{error:?}");
}

#[cfg(unix)]
fn test_pread_pwrite() {
use std::os::unix::fs::FileExt;

let bytes = b"hello world";
let path = utils::prepare_with_content("miri_test_fs_pread_pwrite.txt", bytes);
let mut f = OpenOptions::new().read(true).write(true).open(path).unwrap();

let mut buf1 = [0u8; 3];
f.seek(SeekFrom::Start(5)).unwrap();

// Check that we get expected result after seek
f.read_exact(&mut buf1).unwrap();
assert_eq!(&buf1, b" wo");
f.seek(SeekFrom::Start(5)).unwrap();

// Check pread
f.read_exact_at(&mut buf1, 2).unwrap();
assert_eq!(&buf1, b"llo");
f.read_exact_at(&mut buf1, 6).unwrap();
assert_eq!(&buf1, b"wor");

// Ensure that cursor position is not changed
f.read_exact(&mut buf1).unwrap();
assert_eq!(&buf1, b" wo");
f.seek(SeekFrom::Start(5)).unwrap();

// Check pwrite
f.write_all_at(b" mo", 6).unwrap();

let mut buf2 = [0u8; 11];
f.read_exact_at(&mut buf2, 0).unwrap();
assert_eq!(&buf2, b"hello mold");

// Ensure that cursor position is not changed
f.read_exact(&mut buf1).unwrap();
assert_eq!(&buf1, b" m");
}