diff --git a/src/shims/posix/fs.rs b/src/shims/posix/fs.rs index 288935576e0..11b9082da64 100644 --- a/src/shims/posix/fs.rs +++ b/src/shims/posix/fs.rs @@ -15,7 +15,6 @@ use rustc_middle::ty::{self, layout::LayoutOf}; use rustc_target::abi::{Align, Size}; use crate::*; -use helpers::check_arg_count; use shims::os_str::os_str_to_bytes; use shims::time::system_time_to_duration; @@ -479,16 +478,16 @@ fn maybe_sync_file( impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { fn open(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> { - if args.len() < 2 || args.len() > 3 { + if args.len() < 2 { throw_ub_format!( - "incorrect number of arguments for `open`: got {}, expected 2 or 3", + "incorrect number of arguments for `open`: got {}, expected at least 2", args.len() ); } let this = self.eval_context_mut(); - let path_op = &args[0]; + let path = this.read_pointer(&args[0])?; let flag = this.read_scalar(&args[1])?.to_i32()?; let mut options = OpenOptions::new(); @@ -541,7 +540,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.read_scalar(arg)?.to_u32()? } else { throw_ub_format!( - "incorrect number of arguments for `open` with `O_CREAT`: got {}, expected 3", + "incorrect number of arguments for `open` with `O_CREAT`: got {}, expected at least 3", args.len() ); }; @@ -572,7 +571,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx throw_unsup_format!("unsupported flags {:#x}", flag & !mirror); } - let path = this.read_path_from_c_str(this.read_pointer(path_op)?)?; + let path = this.read_path_from_c_str(path)?; // Reject if isolation is enabled. if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { @@ -614,7 +613,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // `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. - let &[_, _] = check_arg_count(args)?; if this.machine.file_handler.handles.contains_key(&fd) { Ok(this.eval_libc_i32("FD_CLOEXEC")?) } else { @@ -627,8 +625,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // 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 &[_, _, ref start] = check_arg_count(args)?; - let start = this.read_scalar(start)?.to_i32()?; + if args.len() < 3 { + 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(&args[2])?.to_i32()?; let fh = &mut this.machine.file_handler; @@ -646,7 +649,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx None => return this.handle_not_found(), } } else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC")? { - let &[_, _] = check_arg_count(args)?; if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { // FIXME: Support fullfsync for all FDs let FileHandle { file, writable } = file_descriptor.as_file_handle()?; @@ -919,15 +921,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx dirfd_op: &OpTy<'tcx, Tag>, // Should be an `int` pathname_op: &OpTy<'tcx, Tag>, // Should be a `const char *` flags_op: &OpTy<'tcx, Tag>, // Should be an `int` - _mask_op: &OpTy<'tcx, Tag>, // Should be an `unsigned int` + mask_op: &OpTy<'tcx, Tag>, // Should be an `unsigned int` statxbuf_op: &OpTy<'tcx, Tag>, // Should be a `struct statx *` ) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); this.assert_target_os("linux", "statx"); - let statxbuf_ptr = this.read_pointer(statxbuf_op)?; + let dirfd = this.read_scalar(dirfd_op)?.to_i32()?; let pathname_ptr = this.read_pointer(pathname_op)?; + let flags = this.read_scalar(flags_op)?.to_i32()?; + let _mask = this.read_scalar(mask_op)?.to_u32()?; + let statxbuf_ptr = this.read_pointer(statxbuf_op)?; // 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)? { @@ -953,9 +958,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let path = this.read_path_from_c_str(pathname_ptr)?.into_owned(); // See for a discussion of argument sizes. - let flags = this.read_scalar(flags_op)?.to_i32()?; let empty_path_flag = flags & this.eval_libc("AT_EMPTY_PATH")?.to_i32()? != 0; - let dirfd = this.read_scalar(dirfd_op)?.to_i32()?; // We only support: // * interpreting `path` as an absolute directory, // * interpreting `path` as a path relative to `dirfd` when the latter is `AT_FDCWD`, or diff --git a/src/shims/posix/linux/foreign_items.rs b/src/shims/posix/linux/foreign_items.rs index 280f24e9ea4..453c77f1b53 100644 --- a/src/shims/posix/linux/foreign_items.rs +++ b/src/shims/posix/linux/foreign_items.rs @@ -130,16 +130,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // count is checked bellow. this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; // The syscall variadic function is legal to call with more arguments than needed, - // extra arguments are simply ignored. However, all arguments need to be scalars; - // other types might be treated differently by the calling convention. - for arg in args { - if !matches!(arg.layout.abi, rustc_target::abi::Abi::Scalar(_)) { - throw_ub_format!( - "`syscall` arguments must all have scalar layout, but {} does not", - arg.layout.ty - ); - } - } + // extra arguments are simply ignored. The important check is that when we use an + // argument, we have to also check all arguments *before* it to ensure that they + // have the right type. let sys_getrandom = this.eval_libc("SYS_getrandom")?.to_machine_usize(this)?; @@ -181,7 +174,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } // `futex` is used by some synchonization primitives. id if id == sys_futex => { - futex(this, args, dest)?; + futex(this, &args[1..], dest)?; } id => { this.handle_unsupported(format!("can't execute syscall with ID {}", id))?; diff --git a/src/shims/posix/linux/sync.rs b/src/shims/posix/linux/sync.rs index 9350ad6ba94..362373bb7d3 100644 --- a/src/shims/posix/linux/sync.rs +++ b/src/shims/posix/linux/sync.rs @@ -4,6 +4,7 @@ use rustc_target::abi::{Align, Size}; use std::time::{Instant, SystemTime}; /// Implementation of the SYS_futex syscall. +/// `args` is the arguments *after* the syscall number. pub fn futex<'tcx>( this: &mut MiriEvalContext<'_, 'tcx>, args: &[OpTy<'tcx, Tag>], @@ -17,9 +18,9 @@ pub fn futex<'tcx>( // may or may not be left out from the `syscall()` call. // Therefore we don't use `check_arg_count` here, but only check for the // number of arguments to fall within a range. - if args.len() < 4 { + if args.len() < 3 { throw_ub_format!( - "incorrect number of arguments for `futex` syscall: got {}, expected at least 4", + "incorrect number of arguments for `futex` syscall: got {}, expected at least 3", args.len() ); } @@ -27,12 +28,13 @@ pub fn futex<'tcx>( // The first three arguments (after the syscall number itself) are the same to all futex operations: // (int *addr, int op, int val). // We checked above that these definitely exist. - let addr = this.read_immediate(&args[1])?; - let op = this.read_scalar(&args[2])?.to_i32()?; - let val = this.read_scalar(&args[3])?.to_i32()?; + let addr = this.read_immediate(&args[0])?; + let op = this.read_scalar(&args[1])?.to_i32()?; + let val = this.read_scalar(&args[2])?.to_i32()?; let thread = this.get_active_thread(); let addr_scalar = addr.to_scalar()?; + let addr_usize = addr_scalar.to_machine_usize(this)?; let futex_private = this.eval_libc_i32("FUTEX_PRIVATE_FLAG")?; let futex_wait = this.eval_libc_i32("FUTEX_WAIT")?; @@ -56,17 +58,19 @@ pub fn futex<'tcx>( let wait_bitset = op & !futex_realtime == futex_wait_bitset; let bitset = if wait_bitset { - if args.len() != 7 { + if args.len() < 6 { throw_ub_format!( - "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT_BITSET`: got {}, expected 7", + "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT_BITSET`: got {}, expected at least 6", args.len() ); } - this.read_scalar(&args[6])?.to_u32()? + let _timeout = this.read_pointer(&args[3])?; + let _uaddr2 = this.read_pointer(&args[4])?; + this.read_scalar(&args[5])?.to_u32()? } else { - if args.len() < 5 { + if args.len() < 4 { throw_ub_format!( - "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT`: got {}, expected at least 5", + "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT`: got {}, expected at least 4", args.len() ); } @@ -81,7 +85,7 @@ pub fn futex<'tcx>( } // `deref_operand` but not actually dereferencing the ptr yet (it might be NULL!). - let timeout = this.ref_to_mplace(&this.read_immediate(&args[4])?)?; + let timeout = this.ref_to_mplace(&this.read_immediate(&args[3])?)?; let timeout_time = if this.ptr_is_null(timeout.ptr)? { None } else { @@ -145,7 +149,7 @@ pub fn futex<'tcx>( if val == futex_val { // The value still matches, so we block the trait make it wait for FUTEX_WAKE. this.block_thread(thread); - this.futex_wait(addr_scalar.to_machine_usize(this)?, thread, bitset); + this.futex_wait(addr_usize, thread, bitset); // Succesfully waking up from FUTEX_WAIT always returns zero. this.write_scalar(Scalar::from_machine_isize(0, this), dest)?; // Register a timeout callback if a timeout was specified. @@ -157,7 +161,7 @@ pub fn futex<'tcx>( timeout_time, Box::new(move |this| { this.unblock_thread(thread); - this.futex_remove_waiter(addr_scalar.to_machine_usize(this)?, thread); + this.futex_remove_waiter(addr_usize, thread); let etimedout = this.eval_libc("ETIMEDOUT")?; this.set_last_error(etimedout)?; this.write_scalar(Scalar::from_machine_isize(-1, this), &dest)?; @@ -181,13 +185,15 @@ pub fn futex<'tcx>( // Same as FUTEX_WAKE, but allows you to specify a bitset to select which threads to wake up. op if op == futex_wake || op == futex_wake_bitset => { let bitset = if op == futex_wake_bitset { - if args.len() != 7 { + if args.len() < 6 { throw_ub_format!( - "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAKE_BITSET`: got {}, expected 7", + "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAKE_BITSET`: got {}, expected at least 6", args.len() ); } - this.read_scalar(&args[6])?.to_u32()? + let _timeout = this.read_pointer(&args[3])?; + let _uaddr2 = this.read_pointer(&args[4])?; + this.read_scalar(&args[5])?.to_u32()? } else { u32::MAX }; @@ -199,7 +205,7 @@ pub fn futex<'tcx>( } let mut n = 0; for _ in 0..val { - if let Some(thread) = this.futex_wake(addr_scalar.to_machine_usize(this)?, bitset) { + if let Some(thread) = this.futex_wake(addr_usize, bitset) { this.unblock_thread(thread); this.unregister_timeout_callback_if_exists(thread); n += 1; diff --git a/tests/run-pass/concurrency/linux-futex.rs b/tests/run-pass/concurrency/linux-futex.rs index fb7c022929b..4ac928398e2 100644 --- a/tests/run-pass/concurrency/linux-futex.rs +++ b/tests/run-pass/concurrency/linux-futex.rs @@ -32,8 +32,8 @@ fn wake_nobody() { &futex as *const i32, libc::FUTEX_WAKE, 1, - 0, - 0, + ptr::null::(), + 0usize, 0, ), 0); } @@ -121,7 +121,7 @@ fn wait_absolute_timeout() { libc::FUTEX_WAIT_BITSET, 123, &timeout, - 0, + 0usize, u32::MAX, ), -1); assert_eq!(*libc::__errno_location(), libc::ETIMEDOUT); @@ -173,8 +173,8 @@ fn wait_wake_bitset() { &FUTEX as *const i32, libc::FUTEX_WAKE_BITSET, 10, // Wake up at most 10 threads. - 0, - 0, + ptr::null::(), + 0usize, 0b1001, // bitset ), 0); // Didn't match any thread. } @@ -185,8 +185,8 @@ fn wait_wake_bitset() { &FUTEX as *const i32, libc::FUTEX_WAKE_BITSET, 10, // Wake up at most 10 threads. - 0, - 0, + ptr::null::(), + 0usize, 0b0110, // bitset ), 1); // Woken up one thread. } @@ -199,7 +199,7 @@ fn wait_wake_bitset() { libc::FUTEX_WAIT_BITSET, 0, ptr::null::(), - 0, + 0usize, 0b0100, // bitset ), 0); }