]> git.lizzy.rs Git - rust.git/commitdiff
Improve handling of the `addr` argument in SYS_futex.
authorMara Bos <m-ou.se@m-ou.se>
Fri, 2 Oct 2020 08:47:36 +0000 (10:47 +0200)
committerMara Bos <m-ou.se@m-ou.se>
Fri, 2 Oct 2020 08:47:36 +0000 (10:47 +0200)
src/shims/posix/linux/sync.rs
src/sync.rs

index a891a7dd9946fe7215eadfda1fab0d268beba5d7..2b31961559d1440e71e6e888ea47e25fea8bbcde 100644 (file)
@@ -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);
index 986857221b62ba7c000c1a737364120678a1a18d..f8b6f99f1e0332324b3ce2cdf228791107a6a52d 100644 (file)
@@ -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<stacked_borrows::Tag>, 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<ThreadId> {
+    fn futex_wake(&mut self, addr: Pointer<stacked_borrows::Tag>) -> Option<ThreadId> {
         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)
     }
 }