From 712e8006b3c3a055b53326196081df11da123d38 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Fri, 2 Oct 2020 10:47:36 +0200 Subject: [PATCH] Improve handling of the `addr` argument in SYS_futex. --- src/shims/posix/linux/sync.rs | 18 +++++++++++++----- src/sync.rs | 8 ++++---- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/shims/posix/linux/sync.rs b/src/shims/posix/linux/sync.rs index a891a7dd994..2b31961559d 100644 --- a/src/shims/posix/linux/sync.rs +++ b/src/shims/posix/linux/sync.rs @@ -13,12 +13,18 @@ pub fn futex<'tcx>( 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()); } - let addr = args[1]; - let addr_scalar = this.read_scalar(addr)?.check_init()?; - let futex_ptr = this.force_ptr(addr_scalar)?.erase_tag(); + + // The first three arguments (after the syscall number itself) are the same to all futex operations: + // (int *addr, int op, int val). + // Although note that the first one is often passed as a different pointer type, e.g. `*const AtomicU32` or `*mut u32`. + let addr = this.deref_operand(args[1])?; let op = this.read_scalar(args[2])?.to_i32()?; let val = this.read_scalar(args[3])?.to_i32()?; + // The raw pointer value is used to identify the mutex. + // Not all mutex operations actually read from this address or even require this address to exist. + let futex_ptr = addr.ptr.assert_ptr(); + let thread = this.get_active_thread(); let futex_private = this.eval_libc_i32("FUTEX_PRIVATE_FLAG")?; @@ -34,8 +40,10 @@ pub fn futex<'tcx>( if !this.is_null(timeout)? { throw_ub_format!("miri does not support timeouts for futex operations"); } - this.memory.check_ptr_access(addr_scalar, Size::from_bytes(4), Align::from_bytes(4).unwrap())?; - let futex_val = this.read_scalar_at_offset(args[1], 0, this.machine.layouts.i32)?.to_i32()?; + // Check the pointer for alignment. Atomic operations are only available for fully aligned values. + this.memory.check_ptr_access(addr.ptr.into(), Size::from_bytes(4), Align::from_bytes(4).unwrap())?; + // Read an `i32` through the pointer, regardless of any wrapper types (e.g. `AtomicI32`). + let futex_val = this.read_scalar(addr.offset(Size::ZERO, MemPlaceMeta::None, this.machine.layouts.i32, this)?.into())?.to_i32()?; if val == futex_val { this.block_thread(thread); this.futex_wait(futex_ptr, thread); diff --git a/src/sync.rs b/src/sync.rs index 986857221b6..f8b6f99f1e0 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -418,16 +418,16 @@ fn condvar_remove_waiter(&mut self, id: CondvarId, thread: ThreadId) { this.machine.threads.sync.condvars[id].waiters.retain(|waiter| waiter.thread != thread); } - fn futex_wait(&mut self, addr: Pointer, thread: ThreadId) { + fn futex_wait(&mut self, addr: Pointer, thread: ThreadId) { let this = self.eval_context_mut(); - let waiters = &mut this.machine.threads.sync.futexes.entry(addr).or_default().waiters; + let waiters = &mut this.machine.threads.sync.futexes.entry(addr.erase_tag()).or_default().waiters; assert!(waiters.iter().all(|waiter| waiter.thread != thread), "thread is already waiting"); waiters.push_back(FutexWaiter { thread }); } - fn futex_wake(&mut self, addr: Pointer) -> Option { + fn futex_wake(&mut self, addr: Pointer) -> Option { let this = self.eval_context_mut(); - let waiters = &mut this.machine.threads.sync.futexes.get_mut(&addr)?.waiters; + let waiters = &mut this.machine.threads.sync.futexes.get_mut(&addr.erase_tag())?.waiters; waiters.pop_front().map(|waiter| waiter.thread) } } -- 2.44.0