From: bors Date: Fri, 8 Apr 2022 13:57:45 +0000 (+0000) Subject: Auto merge of #2055 - RalfJung:rustup, r=RalfJung X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=be72564a643758afcc1de152ead2359d489149c0;hp=3a59a15af718339675273b397b822ef1e3f4290b;p=rust.git Auto merge of #2055 - RalfJung:rustup, r=RalfJung Rustup Fixes https://github.com/rust-lang/miri/issues/1717 --- diff --git a/src/shims/posix/fs.rs b/src/shims/posix/fs.rs index 288935576e0..5395d0f0bf1 100644 --- a/src/shims/posix/fs.rs +++ b/src/shims/posix/fs.rs @@ -15,7 +15,6 @@ 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 @@ fn open(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> { 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 @@ fn open(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> { 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 @@ fn fcntl(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> { // `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 @@ fn fcntl(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> { // 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 @@ fn fcntl(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> { 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 @@ fn linux_statx( 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 @@ fn linux_statx( 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..339fb04dae3 100644 --- a/src/shims/posix/linux/foreign_items.rs +++ b/src/shims/posix/linux/foreign_items.rs @@ -106,9 +106,9 @@ fn emulate_foreign_item_by_name( // Threading "prctl" => { - let &[ref option, ref arg2, ref arg3, ref arg4, ref arg5] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let result = this.prctl(option, arg2, arg3, arg4, arg5)?; + // prctl is variadic. (It is not documented like that in the manpage, but defined like that in the libc crate.) + this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; + let result = this.prctl(args)?; this.write_scalar(Scalar::from_i32(result), dest)?; } "pthread_condattr_setclock" => { @@ -130,16 +130,9 @@ fn emulate_foreign_item_by_name( // 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 @@ fn emulate_foreign_item_by_name( } // `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 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/src/shims/posix/thread.rs b/src/shims/posix/thread.rs index 58d48028f62..69875a9ffc4 100644 --- a/src/shims/posix/thread.rs +++ b/src/shims/posix/thread.rs @@ -97,20 +97,27 @@ fn pthread_self(&mut self, dest: &PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> { this.write_scalar(Scalar::from_uint(thread_id.to_u32(), dest.layout.size), dest) } - fn prctl( - &mut self, - option: &OpTy<'tcx, Tag>, - arg2: &OpTy<'tcx, Tag>, - _arg3: &OpTy<'tcx, Tag>, - _arg4: &OpTy<'tcx, Tag>, - _arg5: &OpTy<'tcx, Tag>, - ) -> InterpResult<'tcx, i32> { + fn prctl(&mut self, args: &[OpTy<'tcx, Tag>]) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); this.assert_target_os("linux", "prctl"); - let option = this.read_scalar(option)?.to_i32()?; + if args.len() < 1 { + throw_ub_format!( + "incorrect number of arguments for `prctl`: got {}, expected at least 1", + args.len() + ); + } + + let option = this.read_scalar(&args[0])?.to_i32()?; if option == this.eval_libc_i32("PR_SET_NAME")? { - let address = this.read_pointer(arg2)?; + if args.len() < 2 { + throw_ub_format!( + "incorrect number of arguments for `prctl` with `PR_SET_NAME`: got {}, expected at least 2", + args.len() + ); + } + + let address = this.read_pointer(&args[1])?; let mut name = this.read_c_str(address)?.to_owned(); // The name should be no more than 16 bytes, including the null // byte. Since `read_c_str` returns the string without the null @@ -118,7 +125,14 @@ fn prctl( name.truncate(15); this.set_active_thread_name(name); } else if option == this.eval_libc_i32("PR_GET_NAME")? { - let address = this.read_pointer(arg2)?; + if args.len() < 2 { + throw_ub_format!( + "incorrect number of arguments for `prctl` with `PR_SET_NAME`: got {}, expected at least 2", + args.len() + ); + } + + let address = this.read_pointer(&args[1])?; let mut name = this.get_active_thread_name().to_vec(); name.push(0u8); assert!(name.len() <= 16); diff --git a/tests/compile-fail/fs/unix_open_missing_required_mode.rs b/tests/compile-fail/fs/unix_open_missing_required_mode.rs index 2bac7291254..bd2ae6094be 100644 --- a/tests/compile-fail/fs/unix_open_missing_required_mode.rs +++ b/tests/compile-fail/fs/unix_open_missing_required_mode.rs @@ -12,5 +12,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 3 + 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 } diff --git a/tests/compile-fail/fs/unix_open_too_many_args.rs b/tests/compile-fail/fs/unix_open_too_many_args.rs deleted file mode 100644 index 9df7415d313..00000000000 --- a/tests/compile-fail/fs/unix_open_too_many_args.rs +++ /dev/null @@ -1,16 +0,0 @@ -// ignore-windows: No libc on Windows -// compile-flags: -Zmiri-disable-isolation - -#![feature(rustc_private)] - -extern crate libc; - -fn main() { - test_open_too_many_args(); -} - -fn test_open_too_many_args() { - let name = b"too_many_args.txt\0"; - let name_ptr = name.as_ptr().cast::(); - let _fd = unsafe { libc::open(name_ptr, libc::O_RDONLY, 0, 0) }; //~ ERROR Undefined Behavior: incorrect number of arguments for `open`: got 4, expected 2 or 3 -} 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); }