Auto merge of #3968 - YohDeadfall:variadic-arg-helper, r=RalfJung

Added a variadic argument helper

`@RalfJung,` as you wished (:

src/tools/miri/src/helpers.rs

@@ -1179,6 +1179,21 @@ where
     throw_ub_format!("incorrect number of arguments: got {}, expected {}", args.len(), N)
 }
 
+/// Check that the number of args is at least the minumim what we expect.
+pub fn check_min_arg_count<'a, 'tcx, const N: usize>(
+    name: &'a str,
+    args: &'a [OpTy<'tcx>],
+) -> InterpResult<'tcx, &'a [OpTy<'tcx>; N]> {
+    if let Some((ops, _)) = args.split_first_chunk() {
+        return interp_ok(ops);
+    }
+    throw_ub_format!(
+        "incorrect number of arguments for `{name}`: got {}, expected at least {}",
+        args.len(),
+        N
+    )
+}
+
 pub fn isolation_abort_error<'tcx>(name: &str) -> InterpResult<'tcx> {
     throw_machine_stop!(TerminationInfo::UnsupportedInIsolation(format!(
         "{name} not available when isolation is enabled",

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

@@ -9,6 +9,7 @@ use std::rc::{Rc, Weak};
 
 use rustc_target::abi::Size;
 
+use crate::helpers::check_min_arg_count;
 use crate::shims::unix::linux::epoll::EpollReadyEvents;
 use crate::shims::unix::*;
 use crate::*;
@@ -481,56 +482,62 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> {
         let this = self.eval_context_mut();
 
-        let [fd_num, cmd, ..] = args else {
-            throw_ub_format!(
-                "incorrect number of arguments for fcntl: got {}, expected at least 2",
-                args.len()
-            );
-        };
+        let [fd_num, cmd] = check_min_arg_count("fcntl", args)?;
+
         let fd_num = this.read_scalar(fd_num)?.to_i32()?;
         let cmd = this.read_scalar(cmd)?.to_i32()?;
 
+        let f_getfd = this.eval_libc_i32("F_GETFD");
+        let f_dupfd = this.eval_libc_i32("F_DUPFD");
+        let f_dupfd_cloexec = this.eval_libc_i32("F_DUPFD_CLOEXEC");
+
         // We only support getting the flags for a descriptor.
-        if cmd == this.eval_libc_i32("F_GETFD") {
-            // Currently this is the only flag that `F_GETFD` returns. It is OK to just return the
-            // `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")
-            } else {
-                this.fd_not_found()?
-            }))
-        } else if cmd == this.eval_libc_i32("F_DUPFD")
-            || cmd == this.eval_libc_i32("F_DUPFD_CLOEXEC")
-        {
-            // Note that we always assume the FD_CLOEXEC flag is set for every open file, in part
-            // because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only
-            // differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor,
-            // thus they can share the same implementation here.
-            let [_, _, start, ..] = args else {
-                throw_ub_format!(
-                    "incorrect number of arguments for fcntl with cmd=`F_DUPFD`/`F_DUPFD_CLOEXEC`: got {}, expected at least 3",
-                    args.len()
-                );
-            };
-            let start = this.read_scalar(start)?.to_i32()?;
-
-            match this.machine.fds.get(fd_num) {
-                Some(fd) =>
-                    interp_ok(Scalar::from_i32(this.machine.fds.insert_with_min_num(fd, start))),
-                None => interp_ok(Scalar::from_i32(this.fd_not_found()?)),
+        match cmd {
+            cmd if cmd == f_getfd => {
+                // Currently this is the only flag that `F_GETFD` returns. It is OK to just return the
+                // `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")
+                } else {
+                    this.fd_not_found()?
+                }))
             }
-        } else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC") {
-            // Reject if isolation is enabled.
-            if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
-                this.reject_in_isolation("`fcntl`", reject_with)?;
-                return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
+            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
+                // because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only
+                // differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor,
+                // thus they can share the same implementation here.
+                let cmd_name = if cmd == f_dupfd {
+                    "fcntl(fd, F_DUPFD, ...)"
+                } else {
+                    "fcntl(fd, F_DUPFD_CLOEXEC, ...)"
+                };
+
+                let [_, _, start] = check_min_arg_count(cmd_name, args)?;
+                let start = this.read_scalar(start)?.to_i32()?;
+
+                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()?))
+                }
             }
+            cmd if this.tcx.sess.target.os == "macos"
+                && cmd == this.eval_libc_i32("F_FULLFSYNC") =>
+            {
+                // Reject if isolation is enabled.
+                if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
+                    this.reject_in_isolation("`fcntl`", reject_with)?;
+                    return this.set_last_error_and_return_i32(ErrorKind::PermissionDenied);
+                }
 
-            this.ffullsync_fd(fd_num)
-        } else {
-            throw_unsup_format!("the {:#x} command is not supported for `fcntl`)", cmd);
+                this.ffullsync_fd(fd_num)
+            }
+            cmd => {
+                throw_unsup_format!("fcntl: unsupported command {cmd:#x}");
+            }
         }
     }
 

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

@@ -13,6 +13,7 @@ use rustc_target::abi::Size;
 
 use self::fd::FlockOp;
 use self::shims::time::system_time_to_duration;
+use crate::helpers::check_min_arg_count;
 use crate::shims::os_str::bytes_to_os_str;
 use crate::shims::unix::fd::FileDescriptionRef;
 use crate::shims::unix::*;
@@ -433,12 +434,7 @@ fn maybe_sync_file(
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     fn open(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> {
-        let [path_raw, flag, ..] = args else {
-            throw_ub_format!(
-                "incorrect number of arguments for `open`: got {}, expected at least 2",
-                args.len()
-            );
-        };
+        let [path_raw, flag] = check_min_arg_count("open", args)?;
 
         let this = self.eval_context_mut();
 
@@ -492,14 +488,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             // Get the mode.  On macOS, the argument type `mode_t` is actually `u16`, but
             // C integer promotion rules mean that on the ABI level, it gets passed as `u32`
             // (see https://github.com/rust-lang/rust/issues/71915).
-            let mode = if let Some(arg) = args.get(2) {
-                this.read_scalar(arg)?.to_u32()?
-            } else {
-                throw_ub_format!(
-                    "incorrect number of arguments for `open` with `O_CREAT`: got {}, expected at least 3",
-                    args.len()
-                );
-            };
+            let [_, _, mode] = check_min_arg_count("open(pathname, O_CREAT, ...)", args)?;
+            let mode = this.read_scalar(mode)?.to_u32()?;
 
             #[cfg(unix)]
             {

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

@@ -5,6 +5,7 @@ use self::shims::unix::linux::epoll::EvalContextExt as _;
 use self::shims::unix::linux::eventfd::EvalContextExt as _;
 use self::shims::unix::linux::mem::EvalContextExt as _;
 use self::shims::unix::linux::sync::futex;
+use crate::helpers::check_min_arg_count;
 use crate::machine::{SIGRTMAX, SIGRTMIN};
 use crate::shims::unix::*;
 use crate::*;
@@ -127,23 +128,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 let sys_futex = this.eval_libc("SYS_futex").to_target_usize(this)?;
                 let sys_eventfd2 = this.eval_libc("SYS_eventfd2").to_target_usize(this)?;
 
-                if args.is_empty() {
-                    throw_ub_format!(
-                        "incorrect number of arguments for syscall: got 0, expected at least 1"
-                    );
-                }
-                match this.read_target_usize(&args[0])? {
+                let [op] = check_min_arg_count("syscall", args)?;
+                match this.read_target_usize(op)? {
                     // `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)`
                     // is called if a `HashMap` is created the regular way (e.g. HashMap<K, V>).
-                    id if id == sys_getrandom => {
+                    num if num == sys_getrandom => {
                         // Used by getrandom 0.1
                         // The first argument is the syscall id, so skip over it.
-                        let [_, ptr, len, flags, ..] = args else {
-                            throw_ub_format!(
-                                "incorrect number of arguments for `getrandom` syscall: got {}, expected at least 4",
-                                args.len()
-                            );
-                        };
+                        let [_, ptr, len, flags] =
+                            check_min_arg_count("syscall(SYS_getrandom, ...)", args)?;
 
                         let ptr = this.read_pointer(ptr)?;
                         let len = this.read_target_usize(len)?;
@@ -156,22 +149,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                         this.write_scalar(Scalar::from_target_usize(len, this), dest)?;
                     }
                     // `futex` is used by some synchronization primitives.
-                    id if id == sys_futex => {
+                    num if num == sys_futex => {
                         futex(this, &args[1..], dest)?;
                     }
-                    id if id == sys_eventfd2 => {
-                        let [_, initval, flags, ..] = args else {
-                            throw_ub_format!(
-                                "incorrect number of arguments for `eventfd2` syscall: got {}, expected at least 3",
-                                args.len()
-                            );
-                        };
+                    num if num == sys_eventfd2 => {
+                        let [_, initval, flags] =
+                            check_min_arg_count("syscall(SYS_evetfd2, ...)", args)?;
 
                         let result = this.eventfd(initval, flags)?;
                         this.write_int(result.to_i32()?, dest)?;
                     }
-                    id => {
-                        throw_unsup_format!("can't execute syscall with ID {id}");
+                    num => {
+                        throw_unsup_format!("syscall: unsupported syscall number {num}");
                     }
                 }
             }

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

@@ -8,5 +8,5 @@ fn main() {
 fn test_file_open_missing_needed_mode() {
     let name = b"missing_arg.txt\0";
     let name_ptr = name.as_ptr().cast::<libc::c_char>();
-    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
+    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
 }

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

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