Skip to content

Commit

Permalink
Auto merge of rust-lang#3603 - Luv-Ray:issue-3572, r=RalfJung
Browse files Browse the repository at this point in the history
Give `FileDescription::{read, write}` access to the `MiriInterpCx `

fixes rust-lang#3572
  • Loading branch information
bors committed May 17, 2024
2 parents fffc8e9 + 99c6b2e commit 5b2fdb6
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 19 deletions.
37 changes: 24 additions & 13 deletions src/tools/miri/src/shims/unix/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::collections::BTreeMap;
use std::io::{self, ErrorKind, IsTerminal, Read, SeekFrom, Write};
use std::rc::Rc;

use rustc_middle::ty::TyCtxt;
use rustc_target::abi::Size;

use crate::shims::unix::*;
Expand All @@ -22,7 +21,7 @@ pub trait FileDescription: std::fmt::Debug + Any {
&mut self,
_communicate_allowed: bool,
_bytes: &mut [u8],
_tcx: TyCtxt<'tcx>,
_ecx: &mut MiriInterpCx<'_, 'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
throw_unsup_format!("cannot read from {}", self.name());
}
Expand All @@ -32,7 +31,7 @@ pub trait FileDescription: std::fmt::Debug + Any {
&mut self,
_communicate_allowed: bool,
_bytes: &[u8],
_tcx: TyCtxt<'tcx>,
_ecx: &mut MiriInterpCx<'_, 'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
throw_unsup_format!("cannot write to {}", self.name());
}
Expand Down Expand Up @@ -82,7 +81,7 @@ impl FileDescription for io::Stdin {
&mut self,
communicate_allowed: bool,
bytes: &mut [u8],
_tcx: TyCtxt<'tcx>,
_ecx: &mut MiriInterpCx<'_, 'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
if !communicate_allowed {
// We want isolation mode to be deterministic, so we have to disallow all reads, even stdin.
Expand All @@ -105,7 +104,7 @@ impl FileDescription for io::Stdout {
&mut self,
_communicate_allowed: bool,
bytes: &[u8],
_tcx: TyCtxt<'tcx>,
_ecx: &mut MiriInterpCx<'_, 'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
// We allow writing to stderr even with isolation enabled.
let result = Write::write(self, bytes);
Expand Down Expand Up @@ -133,7 +132,7 @@ impl FileDescription for io::Stderr {
&mut self,
_communicate_allowed: bool,
bytes: &[u8],
_tcx: TyCtxt<'tcx>,
_ecx: &mut MiriInterpCx<'_, 'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
// We allow writing to stderr even with isolation enabled.
// No need to flush, stderr is not buffered.
Expand All @@ -158,7 +157,7 @@ impl FileDescription for NullOutput {
&mut self,
_communicate_allowed: bool,
bytes: &[u8],
_tcx: TyCtxt<'tcx>,
_ecx: &mut MiriInterpCx<'_, 'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
// We just don't write anything, but report to the user that we did.
Ok(Ok(bytes.len()))
Expand All @@ -173,6 +172,14 @@ impl FileDescriptor {
FileDescriptor(Rc::new(RefCell::new(Box::new(fd))))
}

pub fn borrow(&self) -> Ref<'_, dyn FileDescription> {
Ref::map(self.0.borrow(), |fd| fd.as_ref())
}

pub fn borrow_mut(&self) -> RefMut<'_, dyn FileDescription> {
RefMut::map(self.0.borrow_mut(), |fd| fd.as_mut())
}

pub fn close<'ctx>(self, communicate_allowed: bool) -> InterpResult<'ctx, io::Result<()>> {
// Destroy this `Rc` using `into_inner` so we can call `close` instead of
// implicitly running the destructor of the file description.
Expand Down Expand Up @@ -242,12 +249,12 @@ impl FdTable {

pub fn get(&self, fd: i32) -> Option<Ref<'_, dyn FileDescription>> {
let fd = self.fds.get(&fd)?;
Some(Ref::map(fd.0.borrow(), |fd| fd.as_ref()))
Some(fd.borrow())
}

pub fn get_mut(&self, fd: i32) -> Option<RefMut<'_, dyn FileDescription>> {
let fd = self.fds.get(&fd)?;
Some(RefMut::map(fd.0.borrow_mut(), |fd| fd.as_mut()))
Some(fd.borrow_mut())
}

pub fn dup(&self, fd: i32) -> Option<FileDescriptor> {
Expand Down Expand Up @@ -370,7 +377,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
.min(u64::try_from(isize::MAX).unwrap());
let communicate = this.machine.communicate();

let Some(mut file_descriptor) = this.machine.fds.get_mut(fd) else {
// We temporarily dup the FD to be able to retain mutable access to `this`.
let Some(file_descriptor) = this.machine.fds.dup(fd) else {
trace!("read: FD not found");
return this.fd_not_found();
};
Expand All @@ -383,7 +391,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// `File::read` never returns a value larger than `count`,
// so this cannot fail.
let result = file_descriptor
.read(communicate, &mut bytes, *this.tcx)?
.borrow_mut()
.read(communicate, &mut bytes, this)?
.map(|c| i64::try_from(c).unwrap());
drop(file_descriptor);

Expand Down Expand Up @@ -421,12 +430,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let communicate = this.machine.communicate();

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

let result = file_descriptor
.write(communicate, &bytes, *this.tcx)?
.borrow_mut()
.write(communicate, &bytes, this)?
.map(|c| i64::try_from(c).unwrap());
drop(file_descriptor);

Expand Down
5 changes: 2 additions & 3 deletions src/tools/miri/src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use std::path::{Path, PathBuf};
use std::time::SystemTime;

use rustc_data_structures::fx::FxHashMap;
use rustc_middle::ty::TyCtxt;
use rustc_target::abi::Size;

use crate::shims::os_str::bytes_to_os_str;
Expand All @@ -34,7 +33,7 @@ impl FileDescription for FileHandle {
&mut self,
communicate_allowed: bool,
bytes: &mut [u8],
_tcx: TyCtxt<'tcx>,
_ecx: &mut MiriInterpCx<'_, 'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
assert!(communicate_allowed, "isolation should have prevented even opening a file");
Ok(self.file.read(bytes))
Expand All @@ -44,7 +43,7 @@ impl FileDescription for FileHandle {
&mut self,
communicate_allowed: bool,
bytes: &[u8],
_tcx: TyCtxt<'tcx>,
_ecx: &mut MiriInterpCx<'_, 'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
assert!(communicate_allowed, "isolation should have prevented even opening a file");
Ok(self.file.write(bytes))
Expand Down
5 changes: 2 additions & 3 deletions src/tools/miri/src/shims/unix/linux/eventfd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
//! Currently just a stub.
use std::io;

use rustc_middle::ty::TyCtxt;
use rustc_target::abi::Endian;

use crate::shims::unix::*;
Expand Down Expand Up @@ -52,11 +51,11 @@ impl FileDescription for Event {
&mut self,
_communicate_allowed: bool,
bytes: &[u8],
tcx: TyCtxt<'tcx>,
ecx: &mut MiriInterpCx<'_, 'tcx>,
) -> InterpResult<'tcx, io::Result<usize>> {
let bytes: [u8; 8] = bytes.try_into().unwrap(); // FIXME fail gracefully when this has the wrong size
// Convert from target endianness to host endianness.
let num = match tcx.sess.target.endian {
let num = match ecx.tcx.sess.target.endian {
Endian::Little => u64::from_le_bytes(bytes),
Endian::Big => u64::from_be_bytes(bytes),
};
Expand Down

0 comments on commit 5b2fdb6

Please sign in to comment.