From f46e1624cbee1558cccf10f0b392ea22c4165e45 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 4 Oct 2024 20:11:26 +0200 Subject: [PATCH] Prefer refutable slice patterns over len check + index op --- .../src/borrow_tracker/tree_borrows/tree.rs | 5 ++-- src/tools/miri/src/shims/unix/fd.rs | 14 ++++----- src/tools/miri/src/shims/unix/fs.rs | 8 ++--- .../src/shims/unix/linux/foreign_items.rs | 10 +++---- src/tools/miri/src/shims/unix/linux/sync.rs | 30 +++++++++---------- 5 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 410f4a58ac5..15cefab1a68 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -859,14 +859,15 @@ fn can_be_replaced_by_single_child( ) -> Option { let node = self.nodes.get(idx).unwrap(); + let [child_idx] = node.children[..] else { return None }; + // We never want to replace the root node, as it is also kept in `root_ptr_tags`. - if node.children.len() != 1 || live.contains(&node.tag) || node.parent.is_none() { + if live.contains(&node.tag) || node.parent.is_none() { return None; } // Since protected nodes are never GC'd (see `borrow_tracker::FrameExtra::visit_provenance`), // we know that `node` is not protected because otherwise `live` would // have contained `node.tag`. - let child_idx = node.children[0]; let child = self.nodes.get(child_idx).unwrap(); // Check that for that one child, `can_be_replaced_by_child` holds for the permission // on all locations. diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index f2d3115f985..34e29760da7 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -481,14 +481,14 @@ fn flock(&mut self, fd_num: i32, op: i32) -> InterpResult<'tcx, Scalar> { fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - if args.len() < 2 { + let [fd_num, cmd, ..] = args else { throw_ub_format!( "incorrect number of arguments for fcntl: got {}, expected at least 2", args.len() ); - } - let fd_num = this.read_scalar(&args[0])?.to_i32()?; - let cmd = this.read_scalar(&args[1])?.to_i32()?; + }; + let fd_num = this.read_scalar(fd_num)?.to_i32()?; + let cmd = this.read_scalar(cmd)?.to_i32()?; // We only support getting the flags for a descriptor. if cmd == this.eval_libc_i32("F_GETFD") { @@ -508,13 +508,13 @@ fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> { // 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. - if args.len() < 3 { + let [_, _, start, ..] = args else { 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 start = this.read_scalar(start)?.to_i32()?; match this.machine.fds.get(fd_num) { Some(fd) => diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 2df178c4385..6c9a2beac2d 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -433,18 +433,18 @@ fn maybe_sync_file( impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn open(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, Scalar> { - if args.len() < 2 { + let [path_raw, flag, ..] = args else { throw_ub_format!( "incorrect number of arguments for `open`: got {}, expected at least 2", args.len() ); - } + }; let this = self.eval_context_mut(); - let path_raw = this.read_pointer(&args[0])?; + let path_raw = this.read_pointer(path_raw)?; let path = this.read_path_from_c_str(path_raw)?; - let flag = this.read_scalar(&args[1])?.to_i32()?; + let flag = this.read_scalar(flag)?.to_i32()?; let mut options = OpenOptions::new(); diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 9726dac7e51..4b5f3b6c81b 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -122,19 +122,19 @@ fn emulate_foreign_item_inner( id if id == sys_getrandom => { // Used by getrandom 0.1 // The first argument is the syscall id, so skip over it. - if args.len() < 4 { + let [_, ptr, len, flags, ..] = args else { throw_ub_format!( "incorrect number of arguments for `getrandom` syscall: got {}, expected at least 4", args.len() ); - } + }; - let ptr = this.read_pointer(&args[1])?; - let len = this.read_target_usize(&args[2])?; + let ptr = this.read_pointer(ptr)?; + let len = this.read_target_usize(len)?; // The only supported flags are GRND_RANDOM and GRND_NONBLOCK, // neither of which have any effect on our current PRNG. // See for a discussion of argument sizes. - let _flags = this.read_scalar(&args[3])?.to_i32()?; + let _flags = this.read_scalar(flags)?.to_i32()?; this.gen_random(ptr, len)?; this.write_scalar(Scalar::from_target_usize(len, this), dest)?; diff --git a/src/tools/miri/src/shims/unix/linux/sync.rs b/src/tools/miri/src/shims/unix/linux/sync.rs index 404479e76e3..5833ec64fc6 100644 --- a/src/tools/miri/src/shims/unix/linux/sync.rs +++ b/src/tools/miri/src/shims/unix/linux/sync.rs @@ -15,19 +15,19 @@ 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() < 3 { + let [addr, op, val, ..] = args else { throw_ub_format!( "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_pointer(&args[0])?; - let op = this.read_scalar(&args[1])?.to_i32()?; - let val = this.read_scalar(&args[2])?.to_i32()?; + let addr = this.read_pointer(addr)?; + let op = this.read_scalar(op)?.to_i32()?; + let val = this.read_scalar(val)?.to_i32()?; // This is a vararg function so we have to bring our own type for this pointer. let addr = this.ptr_to_mplace(addr, this.machine.layouts.i32); @@ -55,15 +55,15 @@ pub fn futex<'tcx>( let wait_bitset = op & !futex_realtime == futex_wait_bitset; let bitset = if wait_bitset { - if args.len() < 6 { + let [_, _, _, timeout, uaddr2, bitset, ..] = args else { throw_ub_format!( "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT_BITSET`: got {}, expected at least 6", args.len() ); - } - let _timeout = this.read_pointer(&args[3])?; - let _uaddr2 = this.read_pointer(&args[4])?; - this.read_scalar(&args[5])?.to_u32()? + }; + let _timeout = this.read_pointer(timeout)?; + let _uaddr2 = this.read_pointer(uaddr2)?; + this.read_scalar(bitset)?.to_u32()? } else { if args.len() < 4 { throw_ub_format!( @@ -183,15 +183,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() < 6 { + let [_, _, _, timeout, uaddr2, bitset, ..] = args else { throw_ub_format!( "incorrect number of arguments for `futex` syscall with `op=FUTEX_WAKE_BITSET`: got {}, expected at least 6", args.len() ); - } - let _timeout = this.read_pointer(&args[3])?; - let _uaddr2 = this.read_pointer(&args[4])?; - this.read_scalar(&args[5])?.to_u32()? + }; + let _timeout = this.read_pointer(timeout)?; + let _uaddr2 = this.read_pointer(uaddr2)?; + this.read_scalar(bitset)?.to_u32()? } else { u32::MAX };