add and update futex comments

Author: joboet

src/concurrency/sync.rs

@@ -734,11 +734,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         interp_ok(true)
     }
 
-    /// Wait for the futex to be signaled, or a timeout.
-    /// * On a signal, `retval_succ` is called with the number of waiters on the
-    /// futex and its result is written to `dest`.
-    /// * On a timeout, `retval_timeout` is written to `dest` and `errno_timeout`
-    /// is set as the last error.
+    /// Wait for the futex to be signaled, or a timeout. Once the thread is
+    /// unblocked, `callback` is called with the unblock reason.
     fn futex_wait(
         &mut self,
         futex_ref: FutexRef,

src/shims/unix/macos/foreign_items.rs

@@ -222,6 +222,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 this.write_scalar(res, dest)?;
             }
 
+            // Futex primitives
             "os_sync_wait_on_address" => {
                 let [addr_op, value_op, size_op, flags_op] =
                     this.check_shim(abi, Conv::C, link_name, args)?;
@@ -261,12 +262,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             "os_sync_wake_by_address_any" => {
                 let [addr_op, size_op, flags_op] =
                     this.check_shim(abi, Conv::C, link_name, args)?;
-                this.os_sync_wake_by_address(addr_op, size_op, flags_op, false, dest)?;
+                this.os_sync_wake_by_address(
+                    addr_op, size_op, flags_op, /* all */ false, dest,
+                )?;
             }
             "os_sync_wake_by_address_all" => {
                 let [addr_op, size_op, flags_op] =
                     this.check_shim(abi, Conv::C, link_name, args)?;
-                this.os_sync_wake_by_address(addr_op, size_op, flags_op, true, dest)?;
+                this.os_sync_wake_by_address(
+                    addr_op, size_op, flags_op, /* all */ true, dest,
+                )?;
             }
 
             "os_unfair_lock_lock" => {

src/shims/unix/macos/sync.rs

@@ -38,7 +38,9 @@ pub enum MacOsFutexTimeout<'a, 'tcx> {
 /// This structure keeps track of both the futex queue and these values.
 struct MacOsFutex {
     futex: FutexRef,
+    /// The size in bytes of the atomic primitive underlying this futex.
     size: Cell<u64>,
+    /// Whether the futex is shared across process boundaries.
     shared: Cell<bool>,
 }
 
@@ -153,7 +155,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         };
 
         // Detect mismatches between the flags and sizes used on this address
-        // by storing the parameters of the first waiter in a batch of waiters.
+        // by comparing with the parameters stored with the first waiter.
         if futex.futex.waiters() == 0 {
             futex.size.set(size);
             futex.shared.set(is_shared);
@@ -233,6 +235,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 shared: Cell::new(is_shared),
             }
         }) else {
+            // No AllocId, or no live allocation at that AllocId. Return an
+            // error code. (That seems nicer than silently doing something
+            // non-intuitive.) This means that if an address gets reused by a
+            // new allocation, we'll use an independent futex queue for this...
+            // that seems acceptable.
             this.set_last_error_and_return(LibcError("ENOENT"), dest)?;
             return interp_ok(());
         };

tests/pass-dep/concurrency/apple-futex.rs

@@ -6,7 +6,7 @@ use std::{io, ptr, thread};
 fn wake_nobody() {
     let futex = 0;
 
-    // Wake 1 waiter. Expect zero waiters woken up, as nobody is waiting.
+    // Wake 1 waiter. Expect ENOENT woken up, as nobody is waiting.
     unsafe {
         assert_eq!(
             libc::os_sync_wake_by_address_any(
@@ -181,6 +181,7 @@ fn param_mismatch() {
                     ),
                     -1,
                 );
+                // This call fails because it uses the shared flag whereas the first waiter didn't.
                 assert_eq!(io::Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL);
             }
         });
@@ -195,6 +196,7 @@ fn param_mismatch() {
                 ),
                 -1,
             );
+            // This call fails because it uses the shared flag whereas the waiter didn't.
             assert_eq!(io::Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL);
         }
     });