Check fixed args number for variadic function

[tiif]

This PR added

• check_shim_variadic to be used by variadic function shims
• check_min_vararg_count to retrieve varargs array from slice
• check_fixed_args_count to check if we got expected number of fixed args

cc #4013

[oli-obk]

Please squash

[tiif]

@rustbot author

[tiif]

So now there is this weird function definition in this PR

    fn open(
        &mut self,
        args: &[OpTy<'tcx>],
        fixed: &[OpTy<'tcx>; 2],
        _var: &[OpTy<'tcx>],
    ) -> InterpResult<'tcx, Scalar> {

where args is the full function argument, fixed is the fixed args, and _var is var args.

Since we already split fixed and var args through check_shim_variadic before we call this.open, I have tried to not pass args into open, but the problem is if I avoid using args and do this instead

            let [mode] = check_min_arg_count("open(pathname, O_CREAT, ...)", var)?

It will lead to diagnostic like this:

11 |     let _fd = unsafe { libc::open(name_ptr, libc::O_CREAT) }; 
   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ incorrect number of arguments for `open(pathname, O_CREAT, ...)`: got 0, expected at least 1

Or maybe we can just ignore the fixed args and var args returned by check_shim_variadic?

[RalfJung]

It will lead to diagnostic like this:

I would suggest to rename check_min_arg_count to check_min_vararg_count and make the error say something like "not enough variadic arguments for ...".

[tiif]

I didn't manage completely replace check_min_arg_count with check_min_vararg_count. There is only left with one check_min_arg_count, which is

miri/src/shims/backtrace.rs

Lines 32 to 42 in c2a3761

	fn handle_miri_get_backtrace(
	&mut self,
	abi: &FnAbi<'tcx, Ty<'tcx>>,
	link_name: Symbol,
	args: &[OpTy<'tcx>],
	dest: &MPlaceTy<'tcx>,
	) -> InterpResult<'tcx> {
	let this = self.eval_context_mut();
	let tcx = this.tcx;
	let [flags] = check_min_arg_count("miri_get_backtrace", args)?;

I think as a miri specific extern function, the argument here shouldn't be classified as a vararg?

[tiif]

@rustbot ready

[RalfJung]

I think as a miri specific extern function, the argument here shouldn't be classified as a vararg?

Oh dang.

We have a comment in the docs for this function saying "The flags argument must be 1."
So could you try making a PR that forces the function to have 2 arguments and flags == 1? We recently did the same with miri_resolve_frame, removing support for the old "version 0 protocol"; if we do the same here then we can get rid of this use of check_min_arg_count hopefully. :)

[RalfJung]

I left some comments on the first place where you used the new infra; they apply everywhere in the PR.

[RalfJung]

@rustbot author

[tiif]

So could you try making a PR that forces the function to have 2 arguments and flags == 1? We recently did the same with miri_resolve_frame, removing support for the old "version 0 protocol"; if we do the same here then we can get rid of this use of check_min_arg_count hopefully. :)

Alright, I can send a follow-up PR after this landed.

[tiif]

I am going for a panic here, but we can use other mechanism to prevent this being used at the wrong place.

[RalfJung]

Which other mechanism did you have in mind?

[RalfJung]

Actually yes this should not be a panic, this should be throw_ub_format!. It is UB to call a variadic function with a non-variadic caller-side signature.

[tiif]

t is UB to call a variadic function with a non-variadic caller-side signature.

I wonder if there is any way to test for this?

[tiif]

@rustbot ready

[RalfJung]

@rustbot author