From 64259a7b8dec493da7fcd1296eea86fafbadc484 Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Mon, 14 Oct 2024 00:19:39 +0300 Subject: [PATCH] Added a variadic argument helper --- src/tools/miri/src/helpers.rs | 15 +++ src/tools/miri/src/shims/unix/fd.rs | 95 ++++++++++--------- src/tools/miri/src/shims/unix/fs.rs | 18 +--- .../src/shims/unix/linux/foreign_items.rs | 35 +++---- .../fs/unix_open_missing_required_mode.rs | 2 +- .../fs/unix_open_missing_required_mode.stderr | 6 +- 6 files changed, 86 insertions(+), 85 deletions(-) diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 8bd429deefc..812cc4295fc 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -1180,6 +1180,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", diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index 34e29760da7..e3914640037 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/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()?)), - } - } 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); + 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()? + })) } + 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, ...)" + }; - this.ffullsync_fd(fd_num) - } else { - throw_unsup_format!("the {:#x} command is not supported for `fcntl`)", cmd); + 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) + } + cmd => { + throw_unsup_format!("fcntl: unsupported command {cmd:#x}"); + } } } diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 6c9a2beac2d..4b3ae8e0520 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/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)] { diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 1d31c12be6f..6c8345652c2 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/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). - 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}"); } } } diff --git a/src/tools/miri/tests/fail-dep/libc/fs/unix_open_missing_required_mode.rs b/src/tools/miri/tests/fail-dep/libc/fs/unix_open_missing_required_mode.rs index b763121080e..4b6f344a78e 100644 --- a/src/tools/miri/tests/fail-dep/libc/fs/unix_open_missing_required_mode.rs +++ b/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::(); - 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 } diff --git a/src/tools/miri/tests/fail-dep/libc/fs/unix_open_missing_required_mode.stderr b/src/tools/miri/tests/fail-dep/libc/fs/unix_open_missing_required_mode.stderr index 971a2d76053..ca9e3c6c4be 100644 --- a/src/tools/miri/tests/fail-dep/libc/fs/unix_open_missing_required_mode.stderr +++ b/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