Skip to content

Commit 0ebdb0c

Browse files
borsbors
committed
Auto merge of #3941 - noahmbright:unix_shims, r=oli-obk
Replace set_last_error with set_last_error_and_return Took care of the simple patterns. Other patterns involved setting an error and then using `write_int` or setting metadata and returning -1. Unsure if those are in the scope of this change Looks like this has conflicts with #3779, so I can update when how to handle that is decided. Part of #3930.
2 parents da5eeb3 + ca5c26d commit 0ebdb0c

File tree

5 files changed

+42
-74
lines changed

5 files changed

+42
-74
lines changed

‎src/shims/io_error.rs

+10
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
141141
interp_ok(Scalar::from_i32(-1))
142142
}
143143

144+
/// Sets the last OS error and return `-1` as a `i64`-typed Scalar
145+
fn set_last_error_and_return_i64(
146+
&mut self,
147+
err: impl Into<IoError>,
148+
) -> InterpResult<'tcx, Scalar> {
149+
let this = self.eval_context_mut();
150+
this.set_last_error(err)?;
151+
interp_ok(Scalar::from_i64(-1))
152+
}
153+
144154
/// Gets the last error variable.
145155
fn get_last_error(&mut self) -> InterpResult<'tcx, Scalar> {
146156
let this = self.eval_context_mut();

‎src/shims/unix/fd.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
561561
/// types (like `read`, that returns an `i64`).
562562
fn fd_not_found<T: From<i32>>(&mut self) -> InterpResult<'tcx, T> {
563563
let this = self.eval_context_mut();
564-
let ebadf = this.eval_libc("EBADF");
565-
this.set_last_error(ebadf)?;
564+
this.set_last_error(LibcError("EBADF"))?;
566565
interp_ok((-1).into())
567566
}
568567

‎src/shims/unix/fs.rs

+25-57
Original file line numberDiff line numberDiff line change
@@ -528,8 +528,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
528528
let o_tmpfile = this.eval_libc_i32("O_TMPFILE");
529529
if flag & o_tmpfile == o_tmpfile {
530530
// if the flag contains `O_TMPFILE` then we return a graceful error
531-
this.set_last_error(LibcError("EOPNOTSUPP"))?;
532-
return interp_ok(Scalar::from_i32(-1));
531+
return this.set_last_error_and_return_i32(LibcError("EOPNOTSUPP"));
533532
}
534533
}
535534

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

572568
let fd = options
@@ -584,8 +580,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
584580
let seek_from = if whence == this.eval_libc_i32("SEEK_SET") {
585581
if offset < 0 {
586582
// Negative offsets return `EINVAL`.
587-
this.set_last_error(LibcError("EINVAL"))?;
588-
return interp_ok(Scalar::from_i64(-1));
583+
return this.set_last_error_and_return_i64(LibcError("EINVAL"));
589584
} else {
590585
SeekFrom::Start(u64::try_from(offset).unwrap())
591586
}
@@ -594,8 +589,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
594589
} else if whence == this.eval_libc_i32("SEEK_END") {
595590
SeekFrom::End(i64::try_from(offset).unwrap())
596591
} else {
597-
this.set_last_error(LibcError("EINVAL"))?;
598-
return interp_ok(Scalar::from_i64(-1));
592+
return this.set_last_error_and_return_i64(LibcError("EINVAL"));
599593
};
600594

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

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

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

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

714702
let metadata = match FileMetadata::from_path(this, &path, false)? {
@@ -766,9 +754,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
766754

767755
// If the statxbuf or pathname pointers are null, the function fails with `EFAULT`.
768756
if this.ptr_is_null(statxbuf_ptr)? || this.ptr_is_null(pathname_ptr)? {
769-
let efault = this.eval_libc("EFAULT");
770-
this.set_last_error(efault)?;
771-
return interp_ok(Scalar::from_i32(-1));
757+
return this.set_last_error_and_return_i32(LibcError("EFAULT"));
772758
}
773759

774760
let statxbuf = this.deref_pointer_as(statxbuf_op, this.libc_ty_layout("statx"))?;
@@ -809,8 +795,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
809795
assert!(empty_path_flag);
810796
this.eval_libc("EBADF")
811797
};
812-
this.set_last_error(ecode)?;
813-
return interp_ok(Scalar::from_i32(-1));
798+
return this.set_last_error_and_return_i32(ecode);
814799
}
815800

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

941926
if this.ptr_is_null(oldpath_ptr)? || this.ptr_is_null(newpath_ptr)? {
942-
let efault = this.eval_libc("EFAULT");
943-
this.set_last_error(efault)?;
944-
return interp_ok(Scalar::from_i32(-1));
927+
return this.set_last_error_and_return_i32(LibcError("EFAULT"));
945928
}
946929

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

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

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

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

@@ -1307,14 +1286,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
13071286
interp_ok(Scalar::from_i32(result))
13081287
} else {
13091288
drop(fd);
1310-
this.set_last_error(LibcError("EINVAL"))?;
1311-
interp_ok(Scalar::from_i32(-1))
1289+
this.set_last_error_and_return_i32(LibcError("EINVAL"))
13121290
}
13131291
} else {
13141292
drop(fd);
13151293
// The file is not writable
1316-
this.set_last_error(LibcError("EINVAL"))?;
1317-
interp_ok(Scalar::from_i32(-1))
1294+
this.set_last_error_and_return_i32(LibcError("EINVAL"))
13181295
}
13191296
}
13201297

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

13931370
if offset < 0 || nbytes < 0 {
1394-
this.set_last_error(LibcError("EINVAL"))?;
1395-
return interp_ok(Scalar::from_i32(-1));
1371+
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
13961372
}
13971373
let allowed_flags = this.eval_libc_i32("SYNC_FILE_RANGE_WAIT_BEFORE")
13981374
| this.eval_libc_i32("SYNC_FILE_RANGE_WRITE")
13991375
| this.eval_libc_i32("SYNC_FILE_RANGE_WAIT_AFTER");
14001376
if flags & allowed_flags != flags {
1401-
this.set_last_error(LibcError("EINVAL"))?;
1402-
return interp_ok(Scalar::from_i32(-1));
1377+
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
14031378
}
14041379

14051380
// Reject if isolation is enabled.
@@ -1436,8 +1411,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
14361411
// Reject if isolation is enabled.
14371412
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
14381413
this.reject_in_isolation("`readlink`", reject_with)?;
1439-
let eacc = this.eval_libc("EACCES");
1440-
this.set_last_error(eacc)?;
1414+
this.set_last_error(LibcError("EACCES"))?;
14411415
return interp_ok(-1);
14421416
}
14431417

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

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

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

15991570
// 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> {
16581629
_ => {
16591630
// "On error, -1 is returned, and errno is set to
16601631
// indicate the error"
1661-
this.set_last_error(e)?;
1662-
return interp_ok(Scalar::from_i32(-1));
1632+
return this.set_last_error_and_return_i32(e);
16631633
}
16641634
},
16651635
}
16661636
}
16671637

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

‎src/shims/unix/linux/epoll.rs

+4-11
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
266266

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

273272
// Check if epfd is a valid epoll file descriptor.
@@ -332,15 +331,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
332331
// Check the existence of fd in the interest list.
333332
if op == epoll_ctl_add {
334333
if interest_list.contains_key(&epoll_key) {
335-
let eexist = this.eval_libc("EEXIST");
336-
this.set_last_error(eexist)?;
337-
return interp_ok(Scalar::from_i32(-1));
334+
return this.set_last_error_and_return_i32(LibcError("EEXIST"));
338335
}
339336
} else {
340337
if !interest_list.contains_key(&epoll_key) {
341-
let enoent = this.eval_libc("ENOENT");
342-
this.set_last_error(enoent)?;
343-
return interp_ok(Scalar::from_i32(-1));
338+
return this.set_last_error_and_return_i32(LibcError("ENOENT"));
344339
}
345340
}
346341

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

375370
// Remove epoll_event_interest from interest_list.
376371
let Some(epoll_interest) = interest_list.remove(&epoll_key) else {
377-
let enoent = this.eval_libc("ENOENT");
378-
this.set_last_error(enoent)?;
379-
return interp_ok(Scalar::from_i32(-1));
372+
return this.set_last_error_and_return_i32(LibcError("ENOENT"));
380373
};
381374
// All related Weak<EpollEventInterest> will fail to upgrade after the drop.
382375
drop(epoll_interest);

‎src/shims/unix/mem.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
134134
// as a dealloc.
135135
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
136136
if addr.addr().bytes() % this.machine.page_size != 0 {
137-
this.set_last_error(this.eval_libc("EINVAL"))?;
138-
return interp_ok(Scalar::from_i32(-1));
137+
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
139138
}
140139

141140
let Some(length) = length.checked_next_multiple_of(this.machine.page_size) else {
142-
this.set_last_error(this.eval_libc("EINVAL"))?;
143-
return interp_ok(Scalar::from_i32(-1));
141+
return this.set_last_error_and_return_i32(LibcError("EINVAL"));
144142
};
145143
if length > this.target_usize_max() {
146144
this.set_last_error(this.eval_libc("EINVAL"))?;

0 commit comments

Comments
 (0)