diff --git a/src/shims/fs.rs b/src/shims/fs.rs index 764f345904..4de59ad32f 100644 --- a/src/shims/fs.rs +++ b/src/shims/fs.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; -use std::fs::{File, OpenOptions, remove_file}; +use std::convert::TryFrom; +use std::fs::{remove_file, File, OpenOptions}; use std::io::{Read, Write}; use rustc::ty::layout::Size; @@ -125,8 +126,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // `FD_CLOEXEC` value without checking if the flag is set for the file because `std` // always sets this flag when opening a file. However we still need to check that the // file itself is open. - let fd_cloexec = this.eval_libc_i32("FD_CLOEXEC")?; - this.get_handle_and(fd, |_| Ok(fd_cloexec)) + if this.machine.file_handler.handles.contains_key(&fd) { + Ok(this.eval_libc_i32("FD_CLOEXEC")?) + } else { + this.handle_not_found() + } } else { throw_unsup_format!("The {:#x} command is not supported for `fcntl`)", cmd); } @@ -139,9 +143,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let fd = this.read_scalar(fd_op)?.to_i32()?; - this.remove_handle_and(fd, |handle, this| { - this.try_unwrap_io_result(handle.file.sync_all().map(|_| 0i32)) - }) + if let Some(handle) = this.machine.file_handler.handles.remove(&fd) { + // `File::sync_all` does the checks that are done when closing a file. We do this to + // to handle possible errors correctly. + let result = this.try_unwrap_io_result(handle.file.sync_all().map(|_| 0i32)); + // Now we actually close the file. + drop(handle); + // And return the result. + result + } else { + this.handle_not_found() + } } fn read( @@ -154,27 +166,50 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.check_no_isolation("read")?; - let count = this.read_scalar(count_op)?.to_machine_usize(&*this.tcx)?; + let ptr_size = this.pointer_size().bits(); + + // We cap the number of read bytes to the largest value that we are able to fit in both the + // host's and target's `isize`. + let count = this + .read_scalar(count_op)? + .to_machine_usize(&*this.tcx)? + .min((1 << (ptr_size - 1)) - 1) // max value of target `isize` + .min(isize::max_value() as u64); // Reading zero bytes should not change `buf`. if count == 0 { return Ok(0); } let fd = this.read_scalar(fd_op)?.to_i32()?; - let buf_scalar = this.read_scalar(buf_op)?.not_undef()?; - - // Remove the file handle to avoid borrowing issues. - this.remove_handle_and(fd, |mut handle, this| { - // Don't use `?` to avoid returning before reinserting the handle. - let bytes = this.force_ptr(buf_scalar).and_then(|buf| { - // FIXME: Don't use raw methods - this.memory - .get_raw_mut(buf.alloc_id)? - .get_bytes_mut(&*this.tcx, buf, Size::from_bytes(count)) - .map(|buffer| handle.file.read(buffer)) - }); - this.machine.file_handler.handles.insert(fd, handle).unwrap_none(); - this.try_unwrap_io_result(bytes?.map(|bytes| bytes as i64)) - }) + let buf = this.read_scalar(buf_op)?.not_undef()?; + + if let Some(handle) = this.machine.file_handler.handles.get_mut(&fd) { + // This can never fail because `count` was capped to be smaller than + // `isize::max_value()`. + let count = isize::try_from(count).unwrap(); + // 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_value()` because it is a host's `isize`. + let mut bytes = vec![0; count as usize]; + let result = handle + .file + .read(&mut bytes) + // `File::read` never returns a value larger than `count`, so this cannot fail. + .map(|c| i64::try_from(c).unwrap()); + + match result { + Ok(read_bytes) => { + // If reading to `bytes` did not fail, we write those bytes to the buffer. + this.memory.write_bytes(buf, bytes)?; + Ok(read_bytes) + } + Err(e) => { + this.set_last_error_from_io_error(e)?; + Ok(-1) + } + } + } else { + this.handle_not_found() + } } fn write( @@ -187,27 +222,32 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.check_no_isolation("write")?; - let count = this.read_scalar(count_op)?.to_machine_usize(&*this.tcx)?; + let ptr_size = this.pointer_size().bits(); + + // We cap the number of read bytes to the largest value that we are able to fit in both the + // host's and target's `isize`. + let count = this + .read_scalar(count_op)? + .to_machine_usize(&*this.tcx)? + .min((1 << (ptr_size - 1)) - 1) // max value of target `isize` + .min(isize::max_value() as u64); // Writing zero bytes should not change `buf`. if count == 0 { return Ok(0); } let fd = this.read_scalar(fd_op)?.to_i32()?; - let buf = this.force_ptr(this.read_scalar(buf_op)?.not_undef()?)?; - - this.remove_handle_and(fd, |mut handle, this| { - // FIXME: Don't use raw methods - let bytes = this.memory.get_raw(buf.alloc_id).and_then(|alloc| { - alloc - .get_bytes(&*this.tcx, buf, Size::from_bytes(count)) - .map(|bytes| handle.file.write(bytes).map(|bytes| bytes as i64)) - }); - this.machine.file_handler.handles.insert(fd, handle).unwrap_none(); - this.try_unwrap_io_result(bytes?) - }) + let buf = this.read_scalar(buf_op)?.not_undef()?; + + if let Some(handle) = this.machine.file_handler.handles.get_mut(&fd) { + let bytes = this.memory.read_bytes(buf, Size::from_bytes(count))?; + let result = handle.file.write(&bytes).map(|c| i64::try_from(c).unwrap()); + this.try_unwrap_io_result(result) + } else { + this.handle_not_found() + } } - fn unlink( &mut self, path_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { + fn unlink(&mut self, path_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); this.check_no_isolation("unlink")?; @@ -219,49 +259,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.try_unwrap_io_result(result) } - /// Helper function that gets a `FileHandle` immutable reference and allows to manipulate it - /// using the `f` closure. - /// - /// If the `fd` file descriptor does not correspond to a file, this functions returns `Ok(-1)` - /// and sets `Evaluator::last_error` to `libc::EBADF` (invalid file descriptor). - /// - /// This function uses `T: From` instead of `i32` directly because some IO related - /// functions return different integer types (like `read`, that returns an `i64`). - fn get_handle_and>(&mut self, fd: i32, f: F) -> InterpResult<'tcx, T> - where - F: Fn(&FileHandle) -> InterpResult<'tcx, T>, - { + /// Function used when a handle is not found inside `FileHandler`. It returns `Ok(-1)`and sets + /// the last OS error to `libc::EBADF` (invalid file descriptor). This function uses + /// `T: From` instead of `i32` directly because some fs functions return different integer + /// types (like `read`, that returns an `i64`). + fn handle_not_found>(&mut self) -> InterpResult<'tcx, T> { let this = self.eval_context_mut(); - if let Some(handle) = this.machine.file_handler.handles.get(&fd) { - f(handle) - } else { - let ebadf = this.eval_libc("EBADF")?; - this.set_last_error(ebadf)?; - Ok((-1).into()) - } - } - - /// Helper function that removes a `FileHandle` and allows to manipulate it using the `f` - /// closure. This function is quite useful when you need to modify a `FileHandle` but you need - /// to modify `MiriEvalContext` at the same time, so you can modify the handle and reinsert it - /// using `f`. - /// - /// If the `fd` file descriptor does not correspond to a file, this functions returns `Ok(-1)` - /// and sets `Evaluator::last_error` to `libc::EBADF` (invalid file descriptor). - /// - /// This function uses `T: From` instead of `i32` directly because some IO related - /// functions return different integer types (like `read`, that returns an `i64`). - fn remove_handle_and>(&mut self, fd: i32, mut f: F) -> InterpResult<'tcx, T> - where - F: FnMut(FileHandle, &mut MiriEvalContext<'mir, 'tcx>) -> InterpResult<'tcx, T>, - { - let this = self.eval_context_mut(); - if let Some(handle) = this.machine.file_handler.handles.remove(&fd) { - f(handle, this) - } else { - let ebadf = this.eval_libc("EBADF")?; - this.set_last_error(ebadf)?; - Ok((-1).into()) - } + let ebadf = this.eval_libc("EBADF")?; + this.set_last_error(ebadf)?; + Ok((-1).into()) } }