Skip to content

Commit 2b020bf

Browse files
borsbors
committed
Auto merge of #3968 - YohDeadfall:variadic-arg-helper, r=RalfJung
Added a variadic argument helper `@RalfJung,` as you wished (:
2 parents d3c1036 + 64259a7 commit 2b020bf

File tree

6 files changed

+85
-84
lines changed

6 files changed

+85
-84
lines changed

‎src/tools/miri/src/helpers.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,6 +1179,21 @@ where
11791179
throw_ub_format!("incorrect number of arguments: got {}, expected {}", args.len(), N)
11801180
}
11811181

1182+
/// Check that the number of args is at least the minumim what we expect.
1183+
pub fn check_min_arg_count<'a, 'tcx, const N: usize>(
1184+
name: &'a str,
1185+
args: &'a [OpTy<'tcx>],
1186+
) -> InterpResult<'tcx, &'a [OpTy<'tcx>; N]> {
1187+
if let Some((ops, _)) = args.split_first_chunk() {
1188+
return interp_ok(ops);
1189+
}
1190+
throw_ub_format!(
1191+
"incorrect number of arguments for `{name}`: got {}, expected at least {}",
1192+
args.len(),
1193+
N
1194+
)
1195+
}
1196+
11821197
pub fn isolation_abort_error<'tcx>(name: &str) -> InterpResult<'tcx> {
11831198
throw_machine_stop!(TerminationInfo::UnsupportedInIsolation(format!(
11841199
"{name} not available when isolation is enabled",

‎src/tools/miri/src/shims/unix/fd.rs‎

Lines changed: 50 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::rc::{Rc, Weak};
99

1010
use rustc_target::abi::Size;
1111

12+
use crate::helpers::check_min_arg_count;
1213
use crate::shims::unix::linux::epoll::EpollReadyEvents;
1314
use crate::shims::unix::*;
1415
use crate::*;
@@ -481,56 +482,62 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
481482
fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> {
482483
let this = self.eval_context_mut();
483484

484-
let [fd_num, cmd, ..] = args else {
485-
throw_ub_format!(
486-
"incorrect number of arguments for fcntl: got {}, expected at least 2",
487-
args.len()
488-
);
489-
};
485+
let [fd_num, cmd] = check_min_arg_count("fcntl", args)?;
486+
490487
let fd_num = this.read_scalar(fd_num)?.to_i32()?;
491488
let cmd = this.read_scalar(cmd)?.to_i32()?;
492489

490+
let f_getfd = this.eval_libc_i32("F_GETFD");
491+
let f_dupfd = this.eval_libc_i32("F_DUPFD");
492+
let f_dupfd_cloexec = this.eval_libc_i32("F_DUPFD_CLOEXEC");
493+
493494
// We only support getting the flags for a descriptor.
494-
if cmd == this.eval_libc_i32("F_GETFD") {
495-
// Currently this is the only flag that `F_GETFD` returns. It is OK to just return the
496-
// `FD_CLOEXEC` value without checking if the flag is set for the file because `std`
497-
// always sets this flag when opening a file. However we still need to check that the
498-
// file itself is open.
499-
interp_ok(Scalar::from_i32(if this.machine.fds.is_fd_num(fd_num) {
500-
this.eval_libc_i32("FD_CLOEXEC")
501-
} else {
502-
this.fd_not_found()?
503-
}))
504-
} else if cmd == this.eval_libc_i32("F_DUPFD")
505-
|| cmd == this.eval_libc_i32("F_DUPFD_CLOEXEC")
506-
{
507-
// Note that we always assume the FD_CLOEXEC flag is set for every open file, in part
508-
// because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only
509-
// differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor,
510-
// thus they can share the same implementation here.
511-
let [_, _, start, ..] = args else {
512-
throw_ub_format!(
513-
"incorrect number of arguments for fcntl with cmd=`F_DUPFD`/`F_DUPFD_CLOEXEC`: got {}, expected at least 3",
514-
args.len()
515-
);
516-
};
517-
let start = this.read_scalar(start)?.to_i32()?;
518-
519-
match this.machine.fds.get(fd_num) {
520-
Some(fd) =>
521-
interp_ok(Scalar::from_i32(this.machine.fds.insert_with_min_num(fd, start))),
522-
None => interp_ok(Scalar::from_i32(this.fd_not_found()?)),
495+
match cmd {
496+
cmd if cmd == f_getfd => {
497+
// Currently this is the only flag that `F_GETFD` returns. It is OK to just return the
498+
// `FD_CLOEXEC` value without checking if the flag is set for the file because `std`
499+
// always sets this flag when opening a file. However we still need to check that the
500+
// file itself is open.
501+
interp_ok(Scalar::from_i32(if this.machine.fds.is_fd_num(fd_num) {
502+
this.eval_libc_i32("FD_CLOEXEC")
503+
} else {
504+
this.fd_not_found()?
505+
}))
523506
}
524-
} else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC") {
525-
// Reject if isolation is enabled.
526-
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
527-
this.reject_in_isolation("`fcntl`", reject_with)?;
528-
return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
507+
cmd if cmd == f_dupfd || cmd == f_dupfd_cloexec => {
508+
// Note that we always assume the FD_CLOEXEC flag is set for every open file, in part
509+
// because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only
510+
// differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor,
511+
// thus they can share the same implementation here.
512+
let cmd_name = if cmd == f_dupfd {
513+
"fcntl(fd, F_DUPFD, ...)"
514+
} else {
515+
"fcntl(fd, F_DUPFD_CLOEXEC, ...)"
516+
};
517+
518+
let [_, _, start] = check_min_arg_count(cmd_name, args)?;
519+
let start = this.read_scalar(start)?.to_i32()?;
520+
521+
if let Some(fd) = this.machine.fds.get(fd_num) {
522+
interp_ok(Scalar::from_i32(this.machine.fds.insert_with_min_num(fd, start)))
523+
} else {
524+
interp_ok(Scalar::from_i32(this.fd_not_found()?))
525+
}
529526
}
527+
cmd if this.tcx.sess.target.os == "macos"
528+
&& cmd == this.eval_libc_i32("F_FULLFSYNC") =>
529+
{
530+
// Reject if isolation is enabled.
531+
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
532+
this.reject_in_isolation("`fcntl`", reject_with)?;
533+
return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
534+
}
530535

531-
this.ffullsync_fd(fd_num)
532-
} else {
533-
throw_unsup_format!("the {:#x} command is not supported for `fcntl`)", cmd);
536+
this.ffullsync_fd(fd_num)
537+
}
538+
cmd => {
539+
throw_unsup_format!("fcntl: unsupported command {cmd:#x}");
540+
}
534541
}
535542
}
536543

‎src/tools/miri/src/shims/unix/fs.rs‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use rustc_target::abi::Size;
1313

1414
use self::fd::FlockOp;
1515
use self::shims::time::system_time_to_duration;
16+
use crate::helpers::check_min_arg_count;
1617
use crate::shims::os_str::bytes_to_os_str;
1718
use crate::shims::unix::fd::FileDescriptionRef;
1819
use crate::shims::unix::*;
@@ -433,12 +434,7 @@ fn maybe_sync_file(
433434
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
434435
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
435436
fn open(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> {
436-
let [path_raw, flag, ..] = args else {
437-
throw_ub_format!(
438-
"incorrect number of arguments for `open`: got {}, expected at least 2",
439-
args.len()
440-
);
441-
};
437+
let [path_raw, flag] = check_min_arg_count("open", args)?;
442438

443439
let this = self.eval_context_mut();
444440

@@ -492,14 +488,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
492488
// Get the mode. On macOS, the argument type `mode_t` is actually `u16`, but
493489
// C integer promotion rules mean that on the ABI level, it gets passed as `u32`
494490
// (see https://github.com/rust-lang/rust/issues/71915).
495-
let mode = if let Some(arg) = args.get(2) {
496-
this.read_scalar(arg)?.to_u32()?
497-
} else {
498-
throw_ub_format!(
499-
"incorrect number of arguments for `open` with `O_CREAT`: got {}, expected at least 3",
500-
args.len()
501-
);
502-
};
491+
let [_, _, mode] = check_min_arg_count("open(pathname, O_CREAT, ...)", args)?;
492+
let mode = this.read_scalar(mode)?.to_u32()?;
503493

504494
#[cfg(unix)]
505495
{

‎src/tools/miri/src/shims/unix/linux/foreign_items.rs‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use self::shims::unix::linux::epoll::EvalContextExt as _;
55
use self::shims::unix::linux::eventfd::EvalContextExt as _;
66
use self::shims::unix::linux::mem::EvalContextExt as _;
77
use self::shims::unix::linux::sync::futex;
8+
use crate::helpers::check_min_arg_count;
89
use crate::machine::{SIGRTMAX, SIGRTMIN};
910
use crate::shims::unix::*;
1011
use crate::*;
@@ -127,23 +128,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
127128
let sys_futex = this.eval_libc("SYS_futex").to_target_usize(this)?;
128129
let sys_eventfd2 = this.eval_libc("SYS_eventfd2").to_target_usize(this)?;
129130

130-
if args.is_empty() {
131-
throw_ub_format!(
132-
"incorrect number of arguments for syscall: got 0, expected at least 1"
133-
);
134-
}
135-
match this.read_target_usize(&args[0])? {
131+
let [op] = check_min_arg_count("syscall", args)?;
132+
match this.read_target_usize(op)? {
136133
// `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)`
137134
// is called if a `HashMap` is created the regular way (e.g. HashMap<K, V>).
138-
id if id == sys_getrandom => {
135+
num if num == sys_getrandom => {
139136
// Used by getrandom 0.1
140137
// The first argument is the syscall id, so skip over it.
141-
let [_, ptr, len, flags, ..] = args else {
142-
throw_ub_format!(
143-
"incorrect number of arguments for `getrandom` syscall: got {}, expected at least 4",
144-
args.len()
145-
);
146-
};
138+
let [_, ptr, len, flags] =
139+
check_min_arg_count("syscall(SYS_getrandom, ...)", args)?;
147140

148141
let ptr = this.read_pointer(ptr)?;
149142
let len = this.read_target_usize(len)?;
@@ -156,22 +149,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
156149
this.write_scalar(Scalar::from_target_usize(len, this), dest)?;
157150
}
158151
// `futex` is used by some synchronization primitives.
159-
id if id == sys_futex => {
152+
num if num == sys_futex => {
160153
futex(this, &args[1..], dest)?;
161154
}
162-
id if id == sys_eventfd2 => {
163-
let [_, initval, flags, ..] = args else {
164-
throw_ub_format!(
165-
"incorrect number of arguments for `eventfd2` syscall: got {}, expected at least 3",
166-
args.len()
167-
);
168-
};
155+
num if num == sys_eventfd2 => {
156+
let [_, initval, flags] =
157+
check_min_arg_count("syscall(SYS_evetfd2, ...)", args)?;
169158

170159
let result = this.eventfd(initval, flags)?;
171160
this.write_int(result.to_i32()?, dest)?;
172161
}
173-
id => {
174-
throw_unsup_format!("can't execute syscall with ID {id}");
162+
num => {
163+
throw_unsup_format!("syscall: unsupported syscall number {num}");
175164
}
176165
}
177166
}

‎src/tools/miri/tests/fail-dep/libc/fs/unix_open_missing_required_mode.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ fn main() {
88
fn test_file_open_missing_needed_mode() {
99
let name = b"missing_arg.txt\0";
1010
let name_ptr = name.as_ptr().cast::<libc::c_char>();
11-
let _fd = unsafe { libc::open(name_ptr, libc::O_CREAT) }; //~ ERROR: Undefined Behavior: incorrect number of arguments for `open` with `O_CREAT`: got 2, expected at least 3
11+
let _fd = unsafe { libc::open(name_ptr, libc::O_CREAT) }; //~ ERROR: Undefined Behavior: incorrect number of arguments for `open(pathname, O_CREAT, ...)`: got 2, expected at least 3
1212
}

‎src/tools/miri/tests/fail-dep/libc/fs/unix_open_missing_required_mode.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: incorrect number of arguments for `open` with `O_CREAT`: got 2, expected at least 3
1+
error: Undefined Behavior: incorrect number of arguments for `open(pathname, O_CREAT, ...)`: got 2, expected at least 3
22
--> tests/fail-dep/libc/fs/unix_open_missing_required_mode.rs:LL:CC
33
|
4-
LL | ...safe { libc::open(name_ptr, libc::O_CREAT) };
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ incorrect number of arguments for `open` with `O_CREAT`: got 2, expected at least 3
4+
LL | ... { libc::open(name_ptr, libc::O_CREAT) };
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ incorrect number of arguments for `open(pathname, O_CREAT, ...)`: got 2, expected at least 3
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

0 commit comments

Comments
 (0)