Auto merge of #1570 - RalfJung:syscalls, r=RalfJung

check that all syscall arguments are scalars

`@m-ou-se` do you think this check makes sense?

The `abi` of a layout contains everything needed for the calling convention (as far as I know), so this should ensure that all the ignored arguments are passed like integers and do not otherwise mess up argument passing.

The one thing I am not sure about is what happens when more arguments are passed than fit in registers. The `syscall` man page says that all calling conventions support at least 6 arguments, except for "mips/o32" where it says that
```
       [1] The mips/o32 system call convention passes arguments 5 through 8 on the user stack.
```
If we assume that passing these extra arguments to futex is legal even with that calling convention, that would mean that those user stack arguments are just silently ignored as well, which seems plausible to me -- but I know very little about calling conventions.

Author: bors

src/bin/miri.rs

@@ -221,7 +221,7 @@ fn main() {
                             ),
                             FromHexError::OddLength =>
                                 panic!("-Zmiri-seed should have an even number of digits"),
-                            err => panic!("Unknown error decoding -Zmiri-seed as hex: {:?}", err),
+                            err => panic!("unknown error decoding -Zmiri-seed as hex: {:?}", err),
                         });
                     if seed_raw.len() > 8 {
                         panic!(format!(

src/shims/posix/linux/foreign_items.rs

@@ -113,16 +113,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
             // Dynamically invoked syscalls
             "syscall" => {
-                // FIXME: The libc syscall() function is a variadic function.
-                // It's valid to call it with more arguments than a syscall
-                // needs, so none of these syscalls should use check_arg_count.
-                // It's even valid to call it with the wrong type of arguments,
-                // as long as they'd end up in the same place with the calling
-                // convention used. (E.g. using a `usize` instead of a pointer.)
-                // It's not directly clear which number, size, and type of arguments
-                // are acceptable in which cases and which aren't. (E.g. some
-                // types might take up the space of two registers.)
-                // So this needs to be researched first.
+                // 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);
+                    }
+                }
 
                 let sys_getrandom = this
                     .eval_libc("SYS_getrandom")?
@@ -144,22 +142,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     // is called if a `HashMap` is created the regular way (e.g. HashMap<K, V>).
                     id if id == sys_getrandom => {
                         // The first argument is the syscall id, so skip over it.
-                        let &[_, ptr, len, flags] = check_arg_count(args)?;
-                        getrandom(this, ptr, len, flags, dest)?;
+                        if args.len() < 4 {
+                            throw_ub_format!("incorrect number of arguments for `getrandom` syscall: got {}, expected at least 4", args.len());
+                        }
+                        getrandom(this, args[1], args[2], args[3], dest)?;
                     }
                     // `statx` is used by `libstd` to retrieve metadata information on `linux`
                     // instead of using `stat`,`lstat` or `fstat` as on `macos`.
                     id if id == sys_statx => {
                         // The first argument is the syscall id, so skip over it.
-                        let &[_, dirfd, pathname, flags, mask, statxbuf] = check_arg_count(args)?;
-                        let result = this.linux_statx(dirfd, pathname, flags, mask, statxbuf)?;
+                        if args.len() < 6 {
+                            throw_ub_format!("incorrect number of arguments for `statx` syscall: got {}, expected at least 6", args.len());
+                        }
+                        let result = this.linux_statx(args[1], args[2], args[3], args[4], args[5])?;
                         this.write_scalar(Scalar::from_machine_isize(result.into(), this), dest)?;
                     }
                     // `futex` is used by some synchonization primitives.
                     id if id == sys_futex => {
                         futex(this, args, dest)?;
                     }
-                    id => throw_unsup_format!("miri does not support syscall ID {}", id),
+                    id => throw_unsup_format!("Miri does not support syscall ID {}", id),
                 }
             }
 

src/shims/posix/linux/sync.rs

@@ -17,8 +17,8 @@ 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 !(4..=7).contains(&args.len()) {
-        throw_ub_format!("incorrect number of arguments for futex syscall: got {}, expected between 4 and 7 (inclusive)", args.len());
+    if args.len() < 4 {
+        throw_ub_format!("incorrect number of arguments for `futex` syscall: got {}, expected at least 4", args.len());
     }
 
     // The first three arguments (after the syscall number itself) are the same to all futex operations:
@@ -49,13 +49,13 @@ pub fn futex<'tcx>(
         // or *timeout expires. `timeout == null` for an infinite timeout.
         op if op & !futex_realtime == futex_wait => {
             if args.len() < 5 {
-                throw_ub_format!("incorrect number of arguments for FUTEX_WAIT syscall: got {}, expected at least 5", args.len());
+                throw_ub_format!("incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT`: got {}, expected at least 5", args.len());
             }
             let timeout = args[4];
             let timeout_time = if this.is_null(this.read_scalar(timeout)?.check_init()?)? {
                 None
             } else {
-                this.check_no_isolation("`syscall(SYS_FUTEX, op=FUTEX_WAIT)` with timeout")?;
+                this.check_no_isolation("`futex` syscall with `op=FUTEX_WAIT` and non-null timeout")?;
                 let duration = match this.read_timespec(timeout)? {
                     Some(duration) => duration,
                     None => {
@@ -126,7 +126,7 @@ pub fn futex<'tcx>(
             }
             this.write_scalar(Scalar::from_machine_isize(n, this), dest)?;
         }
-        op => throw_unsup_format!("miri does not support SYS_futex operation {}", op),
+        op => throw_unsup_format!("Miri does not support `futex` syscall with op={}", op),
     }
 
     Ok(())