diff options
| -rw-r--r-- | src/tools/miri/src/lib.rs | 2 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/unix/fd.rs | 36 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/unix/fs.rs | 79 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/unix/linux/epoll.rs | 12 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/unix/linux/sync.rs | 15 | ||||
| -rw-r--r-- | src/tools/miri/src/shims/windows/sync.rs | 2 |
6 files changed, 61 insertions, 85 deletions
diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 660f2e493bc..938d1ca319e 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -147,7 +147,7 @@ pub use crate::range_map::RangeMap; pub use crate::shims::EmulateItemResult; pub use crate::shims::env::{EnvVars, EvalContextExt as _}; pub use crate::shims::foreign_items::{DynSym, EvalContextExt as _}; -pub use crate::shims::io_error::{EvalContextExt as _, LibcError}; +pub use crate::shims::io_error::{EvalContextExt as _, IoError, LibcError}; pub use crate::shims::os_str::EvalContextExt as _; pub use crate::shims::panic::{CatchUnwindData, EvalContextExt as _}; pub use crate::shims::time::EvalContextExt as _; diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index c21982ee379..f3db56695fc 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -423,7 +423,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let Some(fd) = this.machine.fds.get(old_fd_num) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; interp_ok(Scalar::from_i32(this.machine.fds.insert(fd))) } @@ -432,7 +432,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let Some(fd) = this.machine.fds.get(old_fd_num) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; if new_fd_num != old_fd_num { // Close new_fd if it is previously opened. @@ -448,7 +448,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn flock(&mut self, fd_num: i32, op: i32) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); let Some(fd) = this.machine.fds.get(fd_num) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; // We need to check that there aren't unsupported options in `op`. @@ -498,11 +498,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'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. - interp_ok(Scalar::from_i32(if this.machine.fds.is_fd_num(fd_num) { - this.eval_libc_i32("FD_CLOEXEC") + if !this.machine.fds.is_fd_num(fd_num) { + this.set_last_error_and_return_i32(LibcError("EBADF")) } else { - this.fd_not_found()? - })) + interp_ok(this.eval_libc("FD_CLOEXEC")) + } } cmd if cmd == f_dupfd || cmd == f_dupfd_cloexec => { // Note that we always assume the FD_CLOEXEC flag is set for every open file, in part @@ -521,7 +521,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let Some(fd) = this.machine.fds.get(fd_num) { interp_ok(Scalar::from_i32(this.machine.fds.insert_with_min_num(fd, start))) } else { - interp_ok(Scalar::from_i32(this.fd_not_found()?)) + this.set_last_error_and_return_i32(LibcError("EBADF")) } } cmd if this.tcx.sess.target.os == "macos" @@ -547,7 +547,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let fd_num = this.read_scalar(fd_op)?.to_i32()?; let Some(fd) = this.machine.fds.remove(fd_num) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; let result = fd.close(this.machine.communicate(), this)?; // return `0` if close is successful @@ -555,16 +555,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { interp_ok(Scalar::from_i32(this.try_unwrap_io_result(result)?)) } - /// Function used when a file descriptor does not exist. It returns `Ok(-1)`and sets - /// the last OS error to `libc::EBADF` (invalid file descriptor). This function uses - /// `T: From<i32>` instead of `i32` directly because some fs functions return different integer - /// types (like `read`, that returns an `i64`). - fn fd_not_found<T: From<i32>>(&mut self) -> InterpResult<'tcx, T> { - let this = self.eval_context_mut(); - this.set_last_error(LibcError("EBADF"))?; - interp_ok((-1).into()) - } - /// Read data from `fd` into buffer specified by `buf` and `count`. /// /// If `offset` is `None`, reads data from current cursor position associated with `fd` @@ -598,9 +588,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // We temporarily dup the FD to be able to retain mutable access to `this`. let Some(fd) = this.machine.fds.get(fd_num) else { trace!("read: FD not found"); - let res: i32 = this.fd_not_found()?; - this.write_int(res, dest)?; - return interp_ok(()); + return this.set_last_error_and_return(LibcError("EBADF"), dest); }; trace!("read: FD mapped to {fd:?}"); @@ -645,9 +633,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // We temporarily dup the FD to be able to retain mutable access to `this`. let Some(fd) = this.machine.fds.get(fd_num) else { - let res: i32 = this.fd_not_found()?; - this.write_int(res, dest)?; - return interp_ok(()); + return this.set_last_error_and_return(LibcError("EBADF"), dest); }; match offset { diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 8c502c578f6..4a94d3807a6 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -595,7 +595,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let communicate = this.machine.communicate(); let Some(fd) = this.machine.fds.get(fd_num) else { - return interp_ok(Scalar::from_i64(this.fd_not_found()?)); + return this.set_last_error_and_return_i64(LibcError("EBADF")); }; let result = fd.seek(communicate, seek_from)?.map(|offset| i64::try_from(offset).unwrap()); drop(fd); @@ -671,8 +671,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // `stat` always follows symlinks. let metadata = match FileMetadata::from_path(this, &path, true)? { - Some(metadata) => metadata, - None => return interp_ok(Scalar::from_i32(-1)), // `FileMetadata` has set errno + Ok(metadata) => metadata, + Err(err) => return this.set_last_error_and_return_i32(err), }; interp_ok(Scalar::from_i32(this.macos_stat_write_buf(metadata, buf_op)?)) @@ -700,8 +700,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } let metadata = match FileMetadata::from_path(this, &path, false)? { - Some(metadata) => metadata, - None => return interp_ok(Scalar::from_i32(-1)), // `FileMetadata` has set errno + Ok(metadata) => metadata, + Err(err) => return this.set_last_error_and_return_i32(err), }; interp_ok(Scalar::from_i32(this.macos_stat_write_buf(metadata, buf_op)?)) @@ -724,12 +724,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`fstat`", reject_with)?; // Set error code as "EBADF" (bad fd) - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); } let metadata = match FileMetadata::from_fd_num(this, fd)? { - Some(metadata) => metadata, - None => return interp_ok(Scalar::from_i32(-1)), + Ok(metadata) => metadata, + Err(err) => return this.set_last_error_and_return_i32(err), }; interp_ok(Scalar::from_i32(this.macos_stat_write_buf(metadata, buf_op)?)) } @@ -816,8 +816,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { FileMetadata::from_path(this, &path, follow_symlink)? }; let metadata = match metadata { - Some(metadata) => metadata, - None => return interp_ok(Scalar::from_i32(-1)), + Ok(metadata) => metadata, + Err(err) => return this.set_last_error_and_return_i32(err), }; // The `mode` field specifies the type of the file and the permissions over the file for @@ -1131,7 +1131,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`readdir_r`", reject_with)?; // Set error code as "EBADF" (bad fd) - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); } let open_dir = this.machine.dirs.streams.get_mut(&dirp).ok_or_else(|| { @@ -1242,20 +1242,20 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let dirp = this.read_target_usize(dirp_op)?; // Reject if isolation is enabled. - interp_ok(Scalar::from_i32( - if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { - this.reject_in_isolation("`closedir`", reject_with)?; - this.fd_not_found()? - } else if let Some(open_dir) = this.machine.dirs.streams.remove(&dirp) { - if let Some(entry) = open_dir.entry { - this.deallocate_ptr(entry, None, MiriMemoryKind::Runtime.into())?; - } - drop(open_dir); - 0 - } else { - this.fd_not_found()? - }, - )) + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`closedir`", reject_with)?; + return this.set_last_error_and_return_i32(LibcError("EBADF")); + } + + let Some(open_dir) = this.machine.dirs.streams.remove(&dirp) else { + return this.set_last_error_and_return_i32(LibcError("EBADF")); + }; + if let Some(entry) = open_dir.entry { + this.deallocate_ptr(entry, None, MiriMemoryKind::Runtime.into())?; + } + drop(open_dir); + + interp_ok(Scalar::from_i32(0)) } fn ftruncate64(&mut self, fd_num: i32, length: i128) -> InterpResult<'tcx, Scalar> { @@ -1265,11 +1265,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`ftruncate64`", reject_with)?; // Set error code as "EBADF" (bad fd) - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); } let Some(fd) = this.machine.fds.get(fd_num) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; // FIXME: Support ftruncate64 for all FDs @@ -1308,7 +1308,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`fsync`", reject_with)?; // Set error code as "EBADF" (bad fd) - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); } self.ffullsync_fd(fd) @@ -1317,7 +1317,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn ffullsync_fd(&mut self, fd_num: i32) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); let Some(fd) = this.machine.fds.get(fd_num) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; // Only regular files support synchronization. let FileHandle { file, writable } = fd.downcast::<FileHandle>().ok_or_else(|| { @@ -1337,11 +1337,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`fdatasync`", reject_with)?; // Set error code as "EBADF" (bad fd) - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); } let Some(fd) = this.machine.fds.get(fd) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; // Only regular files support synchronization. let FileHandle { file, writable } = fd.downcast::<FileHandle>().ok_or_else(|| { @@ -1380,11 +1380,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`sync_file_range`", reject_with)?; // Set error code as "EBADF" (bad fd) - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); } let Some(fd) = this.machine.fds.get(fd) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; // Only regular files support synchronization. let FileHandle { file, writable } = fd.downcast::<FileHandle>().ok_or_else(|| { @@ -1667,7 +1667,7 @@ impl FileMetadata { ecx: &mut MiriInterpCx<'tcx>, path: &Path, follow_symlink: bool, - ) -> InterpResult<'tcx, Option<FileMetadata>> { + ) -> InterpResult<'tcx, Result<FileMetadata, IoError>> { let metadata = if follow_symlink { std::fs::metadata(path) } else { std::fs::symlink_metadata(path) }; @@ -1677,9 +1677,9 @@ impl FileMetadata { fn from_fd_num<'tcx>( ecx: &mut MiriInterpCx<'tcx>, fd_num: i32, - ) -> InterpResult<'tcx, Option<FileMetadata>> { + ) -> InterpResult<'tcx, Result<FileMetadata, IoError>> { let Some(fd) = ecx.machine.fds.get(fd_num) else { - return ecx.fd_not_found().map(|_: i32| None); + return interp_ok(Err(LibcError("EBADF"))); }; let file = &fd @@ -1699,12 +1699,11 @@ impl FileMetadata { fn from_meta<'tcx>( ecx: &mut MiriInterpCx<'tcx>, metadata: Result<std::fs::Metadata, std::io::Error>, - ) -> InterpResult<'tcx, Option<FileMetadata>> { + ) -> InterpResult<'tcx, Result<FileMetadata, IoError>> { let metadata = match metadata { Ok(metadata) => metadata, Err(e) => { - ecx.set_last_error(e)?; - return interp_ok(None); + return interp_ok(Err(e.into())); } }; @@ -1727,6 +1726,6 @@ impl FileMetadata { let modified = extract_sec_and_nsec(metadata.modified())?; // FIXME: Provide more fields using platform specific methods. - interp_ok(Some(FileMetadata { mode, size, created, accessed, modified })) + interp_ok(Ok(FileMetadata { mode, size, created, accessed, modified })) } } diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index d6788efbba4..de108665e9f 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -263,7 +263,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Check if epfd is a valid epoll file descriptor. let Some(epfd) = this.machine.fds.get(epfd_value) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; let epoll_file_description = epfd .downcast::<Epoll>() @@ -273,7 +273,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let ready_list = &epoll_file_description.ready_list; let Some(fd_ref) = this.machine.fds.get(fd) else { - return interp_ok(Scalar::from_i32(this.fd_not_found()?)); + return this.set_last_error_and_return_i32(LibcError("EBADF")); }; let id = fd_ref.get_id(); @@ -433,9 +433,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let timeout = this.read_scalar(timeout)?.to_i32()?; if epfd_value <= 0 || maxevents <= 0 { - this.set_last_error(LibcError("EINVAL"))?; - this.write_int(-1, dest)?; - return interp_ok(()); + return this.set_last_error_and_return(LibcError("EINVAL"), dest); } // This needs to come after the maxevents value check, or else maxevents.try_into().unwrap() @@ -446,9 +444,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { )?; let Some(epfd) = this.machine.fds.get(epfd_value) else { - let result_value: i32 = this.fd_not_found()?; - this.write_int(result_value, dest)?; - return interp_ok(()); + return this.set_last_error_and_return(LibcError("EBADF"), dest); }; // Create a weak ref of epfd and pass it to callback so we will make sure that epfd // is not close after the thread unblocks. diff --git a/src/tools/miri/src/shims/unix/linux/sync.rs b/src/tools/miri/src/shims/unix/linux/sync.rs index 8640ce2e2cd..c258be78f76 100644 --- a/src/tools/miri/src/shims/unix/linux/sync.rs +++ b/src/tools/miri/src/shims/unix/linux/sync.rs @@ -63,8 +63,7 @@ pub fn futex<'tcx>( }; if bitset == 0 { - this.set_last_error(LibcError("EINVAL"))?; - this.write_scalar(Scalar::from_target_isize(-1, this), dest)?; + this.set_last_error_and_return(LibcError("EINVAL"), dest)?; return interp_ok(()); } @@ -75,9 +74,7 @@ pub fn futex<'tcx>( let duration = match this.read_timespec(&timeout)? { Some(duration) => duration, None => { - this.set_last_error(LibcError("EINVAL"))?; - this.write_scalar(Scalar::from_target_isize(-1, this), dest)?; - return interp_ok(()); + return this.set_last_error_and_return(LibcError("EINVAL"), dest); } }; let timeout_clock = if op & futex_realtime == futex_realtime { @@ -153,12 +150,12 @@ pub fn futex<'tcx>( Scalar::from_target_isize(0, this), // retval_succ Scalar::from_target_isize(-1, this), // retval_timeout dest.clone(), - this.eval_libc("ETIMEDOUT"), + this.eval_libc("ETIMEDOUT"), // errno_timeout ); } else { // The futex value doesn't match the expected value, so we return failure // right away without sleeping: -1 and errno set to EAGAIN. - this.set_last_error_and_return(LibcError("EAGAIN"), dest)?; + return this.set_last_error_and_return(LibcError("EAGAIN"), dest); } } // FUTEX_WAKE: (int *addr, int op = FUTEX_WAKE, int val) @@ -178,9 +175,7 @@ pub fn futex<'tcx>( u32::MAX }; if bitset == 0 { - this.set_last_error(LibcError("EINVAL"))?; - this.write_scalar(Scalar::from_target_isize(-1, this), dest)?; - return interp_ok(()); + return this.set_last_error_and_return(LibcError("EINVAL"), dest); } // Together with the SeqCst fence in futex_wait, this makes sure that futex_wait // will see the latest value on addr which could be changed by our caller diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index f8861085fe5..f7566a8112d 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -202,7 +202,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { Scalar::from_i32(1), // retval_succ Scalar::from_i32(0), // retval_timeout dest.clone(), - this.eval_windows("c", "ERROR_TIMEOUT"), + this.eval_windows("c", "ERROR_TIMEOUT"), // errno_timeout ); } |