Replace set_last_error with set_last_error_and_return_i*

Add `set_last_error_and_return_i64`, change calls from
`this.libc_eval` to `LibcError`, and replace pairs of
`set_last_error` and returning values of the right type
with the new helper functions
This commit is contained in:
Noah Bright 2024-10-04 14:05:57 -04:00
parent 2775f3962a
commit 2b11c70690
5 changed files with 42 additions and 74 deletions

View File

@ -141,6 +141,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
interp_ok(Scalar::from_i32(-1)) interp_ok(Scalar::from_i32(-1))
} }
/// Sets the last OS error and return `-1` as a `i64`-typed Scalar
fn set_last_error_and_return_i64(
&mut self,
err: impl Into<IoError>,
) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
this.set_last_error(err)?;
interp_ok(Scalar::from_i64(-1))
}
/// Gets the last error variable. /// Gets the last error variable.
fn get_last_error(&mut self) -> InterpResult<'tcx, Scalar> { fn get_last_error(&mut self) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut(); let this = self.eval_context_mut();

View File

@ -561,8 +561,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
/// types (like `read`, that returns an `i64`). /// types (like `read`, that returns an `i64`).
fn fd_not_found<T: From<i32>>(&mut self) -> InterpResult<'tcx, T> { fn fd_not_found<T: From<i32>>(&mut self) -> InterpResult<'tcx, T> {
let this = self.eval_context_mut(); let this = self.eval_context_mut();
let ebadf = this.eval_libc("EBADF"); this.set_last_error(LibcError("EBADF"))?;
this.set_last_error(ebadf)?;
interp_ok((-1).into()) interp_ok((-1).into())
} }

View File

@ -528,8 +528,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let o_tmpfile = this.eval_libc_i32("O_TMPFILE"); let o_tmpfile = this.eval_libc_i32("O_TMPFILE");
if flag & o_tmpfile == o_tmpfile { if flag & o_tmpfile == o_tmpfile {
// if the flag contains `O_TMPFILE` then we return a graceful error // if the flag contains `O_TMPFILE` then we return a graceful error
this.set_last_error(LibcError("EOPNOTSUPP"))?; return this.set_last_error_and_return_i32(LibcError("EOPNOTSUPP"));
return interp_ok(Scalar::from_i32(-1));
} }
} }
@ -548,9 +547,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// O_NOFOLLOW only fails when the trailing component is a symlink; // O_NOFOLLOW only fails when the trailing component is a symlink;
// the entire rest of the path can still contain symlinks. // the entire rest of the path can still contain symlinks.
if path.is_symlink() { if path.is_symlink() {
let eloop = this.eval_libc("ELOOP"); return this.set_last_error_and_return_i32(LibcError("ELOOP"));
this.set_last_error(eloop)?;
return interp_ok(Scalar::from_i32(-1));
} }
} }
mirror |= o_nofollow; mirror |= o_nofollow;
@ -565,8 +562,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`open`", reject_with)?; this.reject_in_isolation("`open`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?; return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
return interp_ok(Scalar::from_i32(-1));
} }
let fd = options let fd = options
@ -584,8 +580,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let seek_from = if whence == this.eval_libc_i32("SEEK_SET") { let seek_from = if whence == this.eval_libc_i32("SEEK_SET") {
if offset < 0 { if offset < 0 {
// Negative offsets return `EINVAL`. // Negative offsets return `EINVAL`.
this.set_last_error(LibcError("EINVAL"))?; return this.set_last_error_and_return_i64(LibcError("EINVAL"));
return interp_ok(Scalar::from_i64(-1));
} else { } else {
SeekFrom::Start(u64::try_from(offset).unwrap()) SeekFrom::Start(u64::try_from(offset).unwrap())
} }
@ -594,8 +589,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
} else if whence == this.eval_libc_i32("SEEK_END") { } else if whence == this.eval_libc_i32("SEEK_END") {
SeekFrom::End(i64::try_from(offset).unwrap()) SeekFrom::End(i64::try_from(offset).unwrap())
} else { } else {
this.set_last_error(LibcError("EINVAL"))?; return this.set_last_error_and_return_i64(LibcError("EINVAL"));
return interp_ok(Scalar::from_i64(-1));
}; };
let communicate = this.machine.communicate(); let communicate = this.machine.communicate();
@ -618,8 +612,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`unlink`", reject_with)?; this.reject_in_isolation("`unlink`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?; return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
return interp_ok(Scalar::from_i32(-1));
} }
let result = remove_file(path).map(|_| 0); let result = remove_file(path).map(|_| 0);
@ -649,8 +642,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`symlink`", reject_with)?; this.reject_in_isolation("`symlink`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?; return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
return interp_ok(Scalar::from_i32(-1));
} }
let result = create_link(&target, &linkpath).map(|_| 0); let result = create_link(&target, &linkpath).map(|_| 0);
@ -674,9 +666,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`stat`", reject_with)?; this.reject_in_isolation("`stat`", reject_with)?;
let eacc = this.eval_libc("EACCES"); return this.set_last_error_and_return_i32(LibcError("EACCES"));
this.set_last_error(eacc)?;
return interp_ok(Scalar::from_i32(-1));
} }
// `stat` always follows symlinks. // `stat` always follows symlinks.
@ -706,9 +696,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`lstat`", reject_with)?; this.reject_in_isolation("`lstat`", reject_with)?;
let eacc = this.eval_libc("EACCES"); return this.set_last_error_and_return_i32(LibcError("EACCES"));
this.set_last_error(eacc)?;
return interp_ok(Scalar::from_i32(-1));
} }
let metadata = match FileMetadata::from_path(this, &path, false)? { let metadata = match FileMetadata::from_path(this, &path, false)? {
@ -766,9 +754,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// If the statxbuf or pathname pointers are null, the function fails with `EFAULT`. // If the statxbuf or pathname pointers are null, the function fails with `EFAULT`.
if this.ptr_is_null(statxbuf_ptr)? || this.ptr_is_null(pathname_ptr)? { if this.ptr_is_null(statxbuf_ptr)? || this.ptr_is_null(pathname_ptr)? {
let efault = this.eval_libc("EFAULT"); return this.set_last_error_and_return_i32(LibcError("EFAULT"));
this.set_last_error(efault)?;
return interp_ok(Scalar::from_i32(-1));
} }
let statxbuf = this.deref_pointer_as(statxbuf_op, this.libc_ty_layout("statx"))?; let statxbuf = this.deref_pointer_as(statxbuf_op, this.libc_ty_layout("statx"))?;
@ -809,8 +795,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
assert!(empty_path_flag); assert!(empty_path_flag);
this.eval_libc("EBADF") this.eval_libc("EBADF")
}; };
this.set_last_error(ecode)?; return this.set_last_error_and_return_i32(ecode);
return interp_ok(Scalar::from_i32(-1));
} }
// the `_mask_op` parameter specifies the file information that the caller requested. // the `_mask_op` parameter specifies the file information that the caller requested.
@ -939,9 +924,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let newpath_ptr = this.read_pointer(newpath_op)?; let newpath_ptr = this.read_pointer(newpath_op)?;
if this.ptr_is_null(oldpath_ptr)? || this.ptr_is_null(newpath_ptr)? { if this.ptr_is_null(oldpath_ptr)? || this.ptr_is_null(newpath_ptr)? {
let efault = this.eval_libc("EFAULT"); return this.set_last_error_and_return_i32(LibcError("EFAULT"));
this.set_last_error(efault)?;
return interp_ok(Scalar::from_i32(-1));
} }
let oldpath = this.read_path_from_c_str(oldpath_ptr)?; let oldpath = this.read_path_from_c_str(oldpath_ptr)?;
@ -950,8 +933,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`rename`", reject_with)?; this.reject_in_isolation("`rename`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?; return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
return interp_ok(Scalar::from_i32(-1));
} }
let result = rename(oldpath, newpath).map(|_| 0); let result = rename(oldpath, newpath).map(|_| 0);
@ -974,8 +956,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`mkdir`", reject_with)?; this.reject_in_isolation("`mkdir`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?; return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
return interp_ok(Scalar::from_i32(-1));
} }
#[cfg_attr(not(unix), allow(unused_mut))] #[cfg_attr(not(unix), allow(unused_mut))]
@ -1002,8 +983,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`rmdir`", reject_with)?; this.reject_in_isolation("`rmdir`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?; return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
return interp_ok(Scalar::from_i32(-1));
} }
let result = remove_dir(path).map(|_| 0i32); let result = remove_dir(path).map(|_| 0i32);
@ -1019,8 +999,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`opendir`", reject_with)?; this.reject_in_isolation("`opendir`", reject_with)?;
let eacc = this.eval_libc("EACCES"); this.set_last_error(LibcError("EACCES"))?;
this.set_last_error(eacc)?;
return interp_ok(Scalar::null_ptr(this)); return interp_ok(Scalar::null_ptr(this));
} }
@ -1307,14 +1286,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
interp_ok(Scalar::from_i32(result)) interp_ok(Scalar::from_i32(result))
} else { } else {
drop(fd); drop(fd);
this.set_last_error(LibcError("EINVAL"))?; this.set_last_error_and_return_i32(LibcError("EINVAL"))
interp_ok(Scalar::from_i32(-1))
} }
} else { } else {
drop(fd); drop(fd);
// The file is not writable // The file is not writable
this.set_last_error(LibcError("EINVAL"))?; this.set_last_error_and_return_i32(LibcError("EINVAL"))
interp_ok(Scalar::from_i32(-1))
} }
} }
@ -1391,15 +1368,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let flags = this.read_scalar(flags_op)?.to_i32()?; let flags = this.read_scalar(flags_op)?.to_i32()?;
if offset < 0 || nbytes < 0 { if offset < 0 || nbytes < 0 {
this.set_last_error(LibcError("EINVAL"))?; return this.set_last_error_and_return_i32(LibcError("EINVAL"));
return interp_ok(Scalar::from_i32(-1));
} }
let allowed_flags = this.eval_libc_i32("SYNC_FILE_RANGE_WAIT_BEFORE") let allowed_flags = this.eval_libc_i32("SYNC_FILE_RANGE_WAIT_BEFORE")
| this.eval_libc_i32("SYNC_FILE_RANGE_WRITE") | this.eval_libc_i32("SYNC_FILE_RANGE_WRITE")
| this.eval_libc_i32("SYNC_FILE_RANGE_WAIT_AFTER"); | this.eval_libc_i32("SYNC_FILE_RANGE_WAIT_AFTER");
if flags & allowed_flags != flags { if flags & allowed_flags != flags {
this.set_last_error(LibcError("EINVAL"))?; return this.set_last_error_and_return_i32(LibcError("EINVAL"));
return interp_ok(Scalar::from_i32(-1));
} }
// Reject if isolation is enabled. // Reject if isolation is enabled.
@ -1436,8 +1411,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`readlink`", reject_with)?; this.reject_in_isolation("`readlink`", reject_with)?;
let eacc = this.eval_libc("EACCES"); this.set_last_error(LibcError("EACCES"))?;
this.set_last_error(eacc)?;
return interp_ok(-1); return interp_ok(-1);
} }
@ -1574,9 +1548,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled. // Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`mkstemp`", reject_with)?; this.reject_in_isolation("`mkstemp`", reject_with)?;
let eacc = this.eval_libc("EACCES"); return this.set_last_error_and_return_i32(LibcError("EACCES"));
this.set_last_error(eacc)?;
return interp_ok(Scalar::from_i32(-1));
} }
// Get the bytes of the suffix we expect in _target_ encoding. // Get the bytes of the suffix we expect in _target_ encoding.
@ -1592,8 +1564,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// If we don't find the suffix, it is an error. // If we don't find the suffix, it is an error.
if last_six_char_bytes != suffix_bytes { if last_six_char_bytes != suffix_bytes {
this.set_last_error(LibcError("EINVAL"))?; return this.set_last_error_and_return_i32(LibcError("EINVAL"));
return interp_ok(Scalar::from_i32(-1));
} }
// At this point we know we have 6 ASCII 'X' characters as a suffix. // At this point we know we have 6 ASCII 'X' characters as a suffix.
@ -1658,17 +1629,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
_ => { _ => {
// "On error, -1 is returned, and errno is set to // "On error, -1 is returned, and errno is set to
// indicate the error" // indicate the error"
this.set_last_error(e)?; return this.set_last_error_and_return_i32(e);
return interp_ok(Scalar::from_i32(-1));
} }
}, },
} }
} }
// We ran out of attempts to create the file, return an error. // We ran out of attempts to create the file, return an error.
let eexist = this.eval_libc("EEXIST"); this.set_last_error_and_return_i32(LibcError("EEXIST"))
this.set_last_error(eexist)?;
interp_ok(Scalar::from_i32(-1))
} }
} }

View File

@ -266,8 +266,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Throw EINVAL if epfd and fd have the same value. // Throw EINVAL if epfd and fd have the same value.
if epfd_value == fd { if epfd_value == fd {
this.set_last_error(LibcError("EINVAL"))?; return this.set_last_error_and_return_i32(LibcError("EINVAL"));
return interp_ok(Scalar::from_i32(-1));
} }
// Check if epfd is a valid epoll file descriptor. // Check if epfd is a valid epoll file descriptor.
@ -332,15 +331,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Check the existence of fd in the interest list. // Check the existence of fd in the interest list.
if op == epoll_ctl_add { if op == epoll_ctl_add {
if interest_list.contains_key(&epoll_key) { if interest_list.contains_key(&epoll_key) {
let eexist = this.eval_libc("EEXIST"); return this.set_last_error_and_return_i32(LibcError("EEXIST"));
this.set_last_error(eexist)?;
return interp_ok(Scalar::from_i32(-1));
} }
} else { } else {
if !interest_list.contains_key(&epoll_key) { if !interest_list.contains_key(&epoll_key) {
let enoent = this.eval_libc("ENOENT"); return this.set_last_error_and_return_i32(LibcError("ENOENT"));
this.set_last_error(enoent)?;
return interp_ok(Scalar::from_i32(-1));
} }
} }
@ -374,9 +369,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Remove epoll_event_interest from interest_list. // Remove epoll_event_interest from interest_list.
let Some(epoll_interest) = interest_list.remove(&epoll_key) else { let Some(epoll_interest) = interest_list.remove(&epoll_key) else {
let enoent = this.eval_libc("ENOENT"); return this.set_last_error_and_return_i32(LibcError("ENOENT"));
this.set_last_error(enoent)?;
return interp_ok(Scalar::from_i32(-1));
}; };
// All related Weak<EpollEventInterest> will fail to upgrade after the drop. // All related Weak<EpollEventInterest> will fail to upgrade after the drop.
drop(epoll_interest); drop(epoll_interest);

View File

@ -134,13 +134,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// as a dealloc. // as a dealloc.
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero #[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
if addr.addr().bytes() % this.machine.page_size != 0 { if addr.addr().bytes() % this.machine.page_size != 0 {
this.set_last_error(this.eval_libc("EINVAL"))?; return this.set_last_error_and_return_i32(LibcError("EINVAL"));
return interp_ok(Scalar::from_i32(-1));
} }
let Some(length) = length.checked_next_multiple_of(this.machine.page_size) else { let Some(length) = length.checked_next_multiple_of(this.machine.page_size) else {
this.set_last_error(this.eval_libc("EINVAL"))?; return this.set_last_error_and_return_i32(LibcError("EINVAL"));
return interp_ok(Scalar::from_i32(-1));
}; };
if length > this.target_usize_max() { if length > this.target_usize_max() {
this.set_last_error(this.eval_libc("EINVAL"))?; this.set_last_error(this.eval_libc("EINVAL"))?;