Skip to content
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.

Sign up for GitHub

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

Replace set_last_error with set_last_error_and_return #3941

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Replace set_last_error with set_last_error_and_return #3941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions src/shims/io_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
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.
fn get_last_error(&mut self) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
Expand Down
3 changes: 1 addition & 2 deletions src/shims/unix/fd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,8 +561,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
/// 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();
let ebadf = this.eval_libc("EBADF");
this.set_last_error(ebadf)?;
this.set_last_error(LibcError("EBADF"))?;
interp_ok((-1).into())
}

Expand Down
82 changes: 25 additions & 57 deletions src/shims/unix/fs.rs
View file
Open in desktop
noahmbright marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let o_tmpfile = this.eval_libc_i32("O_TMPFILE");
if flag & o_tmpfile == o_tmpfile {
// if the flag contains `O_TMPFILE` then we return a graceful error
this.set_last_error(LibcError("EOPNOTSUPP"))?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EOPNOTSUPP"));
}
}

Expand All @@ -548,9 +547,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// O_NOFOLLOW only fails when the trailing component is a symlink;
// the entire rest of the path can still contain symlinks.
if path.is_symlink() {
let eloop = this.eval_libc("ELOOP");
this.set_last_error(eloop)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("ELOOP"));
}
}
mirror |= o_nofollow;
Expand All @@ -565,8 +562,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`open`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
}

let fd = options
Expand All @@ -584,8 +580,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let seek_from = if whence == this.eval_libc_i32("SEEK_SET") {
if offset < 0 {
// Negative offsets return `EINVAL`.
this.set_last_error(LibcError("EINVAL"))?;
return interp_ok(Scalar::from_i64(-1));
return this.set_last_error_and_return_i64(LibcError("EINVAL"));
} else {
SeekFrom::Start(u64::try_from(offset).unwrap())
}
Expand All @@ -594,8 +589,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
} else if whence == this.eval_libc_i32("SEEK_END") {
SeekFrom::End(i64::try_from(offset).unwrap())
} else {
this.set_last_error(LibcError("EINVAL"))?;
return interp_ok(Scalar::from_i64(-1));
return this.set_last_error_and_return_i64(LibcError("EINVAL"));
};

let communicate = this.machine.communicate();
Expand All @@ -618,8 +612,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`unlink`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
}

let result = remove_file(path).map(|_| 0);
Expand Down Expand Up @@ -649,8 +642,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`symlink`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
}

let result = create_link(&target, &linkpath).map(|_| 0);
Expand All @@ -674,9 +666,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`stat`", reject_with)?;
let eacc = this.eval_libc("EACCES");
this.set_last_error(eacc)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EACCES"));
}

// `stat` always follows symlinks.
Expand Down Expand Up @@ -706,9 +696,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`lstat`", reject_with)?;
let eacc = this.eval_libc("EACCES");
this.set_last_error(eacc)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EACCES"));
}

let metadata = match FileMetadata::from_path(this, &path, false)? {
Expand Down Expand Up @@ -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 this.ptr_is_null(statxbuf_ptr)? || this.ptr_is_null(pathname_ptr)? {
let efault = this.eval_libc("EFAULT");
this.set_last_error(efault)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EFAULT"));
}

let statxbuf = this.deref_pointer_as(statxbuf_op, this.libc_ty_layout("statx"))?;
Expand Down Expand Up @@ -809,8 +795,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
assert!(empty_path_flag);
this.eval_libc("EBADF")
};
this.set_last_error(ecode)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(ecode);
}

// the `_mask_op` parameter specifies the file information that the caller requested.
Expand Down Expand Up @@ -939,9 +924,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let newpath_ptr = this.read_pointer(newpath_op)?;

if this.ptr_is_null(oldpath_ptr)? || this.ptr_is_null(newpath_ptr)? {
let efault = this.eval_libc("EFAULT");
this.set_last_error(efault)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EFAULT"));
}

let oldpath = this.read_path_from_c_str(oldpath_ptr)?;
Expand All @@ -950,8 +933,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`rename`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
}

let result = rename(oldpath, newpath).map(|_| 0);
Expand All @@ -974,8 +956,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`mkdir`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
}

#[cfg_attr(not(unix), allow(unused_mut))]
Expand All @@ -1002,8 +983,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`rmdir`", reject_with)?;
this.set_last_error(ErrorKind::PermissionDenied)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
}

let result = remove_dir(path).map(|_| 0i32);
Expand All @@ -1019,8 +999,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`opendir`", reject_with)?;
let eacc = this.eval_libc("EACCES");
this.set_last_error(eacc)?;
this.set_last_error(LibcError("EACCES"))?;
return interp_ok(Scalar::null_ptr(this));
}

Expand Down Expand Up @@ -1307,14 +1286,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
interp_ok(Scalar::from_i32(result))
} else {
drop(fd);
this.set_last_error(LibcError("EINVAL"))?;
interp_ok(Scalar::from_i32(-1))
this.set_last_error_and_return_i32(LibcError("EINVAL"))
}
} else {
drop(fd);
// The file is not writable
this.set_last_error(LibcError("EINVAL"))?;
interp_ok(Scalar::from_i32(-1))
this.set_last_error_and_return_i32(LibcError("EINVAL"))
}
}

Expand Down Expand Up @@ -1391,15 +1368,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let flags = this.read_scalar(flags_op)?.to_i32()?;

if offset < 0 || nbytes < 0 {
this.set_last_error(LibcError("EINVAL"))?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
}
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_WAIT_AFTER");
if flags & allowed_flags != flags {
this.set_last_error(LibcError("EINVAL"))?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
}

// Reject if isolation is enabled.
Expand Down Expand Up @@ -1436,8 +1411,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`readlink`", reject_with)?;
let eacc = this.eval_libc("EACCES");
this.set_last_error(eacc)?;
this.set_last_error(LibcError("EACCES"))?;
return interp_ok(-1);
}

Expand Down Expand Up @@ -1574,9 +1548,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`mkstemp`", reject_with)?;
let eacc = this.eval_libc("EACCES");
this.set_last_error(eacc)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EACCES"));
}

// Get the bytes of the suffix we expect in _target_ encoding.
Expand All @@ -1592,8 +1564,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

// If we don't find the suffix, it is an error.
if last_six_char_bytes != suffix_bytes {
this.set_last_error(LibcError("EINVAL"))?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
}

// At this point we know we have 6 ASCII 'X' characters as a suffix.
Expand Down Expand Up @@ -1658,17 +1629,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
_ => {
// "On error, -1 is returned, and errno is set to
// indicate the error"
this.set_last_error(e)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(e);
}
},
}
}

// We ran out of attempts to create the file, return an error.
let eexist = this.eval_libc("EEXIST");
this.set_last_error(eexist)?;
interp_ok(Scalar::from_i32(-1))
this.set_last_error_and_return_i32(LibcError("EEXIST"))
}
}

Expand Down
15 changes: 4 additions & 11 deletions src/shims/unix/linux/epoll.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

// Throw EINVAL if epfd and fd have the same value.
if epfd_value == fd {
this.set_last_error(LibcError("EINVAL"))?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
}

// Check if epfd is a valid epoll file descriptor.
Expand Down Expand Up @@ -332,15 +331,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Check the existence of fd in the interest list.
if op == epoll_ctl_add {
if interest_list.contains_key(&epoll_key) {
let eexist = this.eval_libc("EEXIST");
this.set_last_error(eexist)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EEXIST"));
}
} else {
if !interest_list.contains_key(&epoll_key) {
let enoent = this.eval_libc("ENOENT");
this.set_last_error(enoent)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("ENOENT"));
}
}

Expand Down Expand Up @@ -374,9 +369,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

// Remove epoll_event_interest from interest_list.
let Some(epoll_interest) = interest_list.remove(&epoll_key) else {
let enoent = this.eval_libc("ENOENT");
this.set_last_error(enoent)?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("ENOENT"));
};
// All related Weak<EpollEventInterest> will fail to upgrade after the drop.
drop(epoll_interest);
Expand Down
6 changes: 2 additions & 4 deletions src/shims/unix/mem.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// as a dealloc.
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
if addr.addr().bytes() % this.machine.page_size != 0 {
this.set_last_error(this.eval_libc("EINVAL"))?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
}

let Some(length) = length.checked_next_multiple_of(this.machine.page_size) else {
this.set_last_error(this.eval_libc("EINVAL"))?;
return interp_ok(Scalar::from_i32(-1));
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
};
if length > this.target_usize_max() {
this.set_last_error(this.eval_libc("EINVAL"))?;
Expand Down