Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For variadic functions, accept arbitrary trailing arguments #2058

Merged
merged 2 commits into from
Apr 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions src/shims/posix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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()
);
};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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;

Expand All @@ -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()?;
Expand Down Expand Up @@ -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)? {
Expand All @@ -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 <https://github.com/rust-lang/rust/pull/79196> 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
Expand Down
21 changes: 7 additions & 14 deletions src/shims/posix/linux/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx

// 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" => {
Expand All @@ -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)?;

Expand Down Expand Up @@ -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))?;
Expand Down
40 changes: 23 additions & 17 deletions src/shims/posix/linux/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>],
Expand All @@ -17,22 +18,23 @@ 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()
);
}

// 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")?;
Expand All @@ -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()
);
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand All @@ -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)?;
Expand All @@ -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
};
Expand All @@ -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;
Expand Down
36 changes: 25 additions & 11 deletions src/shims/posix/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,28 +97,42 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, '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
// byte, we need to truncate to 15.
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);
Expand Down
2 changes: 1 addition & 1 deletion tests/compile-fail/fs/unix_open_missing_required_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<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 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
}
16 changes: 0 additions & 16 deletions tests/compile-fail/fs/unix_open_too_many_args.rs

This file was deleted.

16 changes: 8 additions & 8 deletions tests/run-pass/concurrency/linux-futex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ fn wake_nobody() {
&futex as *const i32,
libc::FUTEX_WAKE,
1,
0,
0,
ptr::null::<libc::timespec>(),
0usize,
0,
), 0);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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::<libc::timespec>(),
0usize,
0b1001, // bitset
), 0); // Didn't match any thread.
}
Expand All @@ -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::<libc::timespec>(),
0usize,
0b0110, // bitset
), 1); // Woken up one thread.
}
Expand All @@ -199,7 +199,7 @@ fn wait_wake_bitset() {
libc::FUTEX_WAIT_BITSET,
0,
ptr::null::<libc::timespec>(),
0,
0usize,
0b0100, // bitset
), 0);
}
Expand Down