fix/add comments for better explanation + some formatting

Author: LorrensP-2158466

src/shims/unix/freebsd/sync.rs

@@ -24,9 +24,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     /// `obj`: a pointer to the futex object (can be a lot of things, mostly *AtomicU32)
     /// `op`: the futex operation to run
     /// `val`: the current value of the object as a `c_long` (for wait/wake)
-    /// `uaddr`: pointer to optional parameter (mostly timeouts)
-    /// `uaddr2`: pointer to optional parameter (mostly timeouts)
+    /// `uaddr`: `op`-specific optional parameter, pointer-sized integer or pointer to an `op`-specific struct
+    /// `uaddr2`: `op`-specific optional parameter, pointer-sized integer or pointer to an `op`-specific struct
     /// `dest`: the place this syscall returns to, 0 for success, -1 for failure
+    ///
+    /// # Note
+    /// Curently only the WAIT and WAKE operations are implemented.
     fn _umtx_op(
         &mut self,
         obj: &OpTy<'tcx>,
@@ -83,7 +86,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
                     // From the manual:
                     // The timeout is specified by passing either the address of `struct timespec`, or its
-                    // extended  variant, `struct _umtx_time`, as the `uaddr2` argument of _umtx_op().
+                    // extended variant, `struct _umtx_time`, as the `uaddr2` argument of _umtx_op().
                     // They are distinguished by the `uaddr` value, which must be equal
                     // to the size of the structure pointed to by `uaddr2`, casted to uintptr_t.
                     let timeout = if this.ptr_is_null(uaddr2)? {
@@ -113,7 +116,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                             // RealTime clock can't be used in isolation mode.
                             this.check_no_isolation("`_umtx_op` with `timespec` timeout")?;
 
-                            // `uaddr2` points to a `struct timespec`
+                            // `uaddr2` points to a `struct timespec`.
                             let timespec = this.ptr_to_mplace(uaddr2, timespec_layout);
                             let duration = match this.read_timespec(&timespec)? {
                                 Some(duration) => duration,
@@ -123,9 +126,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                                 }
                             };
 
-                            // From the FreeBSD source code:
-                            // In the function `umtx_copyin_umtx_time` in kern_umtx.c,
-                            // _clock_id is set to CLOCK_REALTIME when using a `timespec` timeout.
+                            // FreeBSD does not seem to document which clock is used when the timeout
+                            // is passed as a `struct timespec*`. Based on discussions online and the source
+                            // code (umtx_copyin_umtx_time() in kern_umtx.c), it seems to default to CLOCK_REALTIME,
+                            // so that's what we also do.
+                            // Discussion in golang: https://github.com/golang/go/issues/17168#issuecomment-250235271
                             Some((TimeoutClock::RealTime, TimeoutAnchor::Relative, duration))
                         } else {
                             return this.set_last_error_and_return(LibcError("EINVAL"), dest);
@@ -144,7 +149,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                             |ecx, unblock: UnblockKind| match unblock {
                                 UnblockKind::Ready => {
                                     // From the manual:
-                                    // If successful, all requests, except UMTX_SHM_CREAT and  UMTX_SHM_LOOKUP
+                                    // If successful, all requests, except UMTX_SHM_CREAT and UMTX_SHM_LOOKUP
                                     // sub-requests of the UMTX_OP_SHM request, will return zero.
                                     ecx.write_int(0, &dest)
                                 }
@@ -191,8 +196,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 )?;
 
                 // From the manual:
-                // If successful, all requests, except UMTX_SHM_CREAT and  UMTX_SHM_LOOKUP
-                // sub-requests  of	 the  UMTX_OP_SHM  request,  will  return  zero.
+                // If successful, all requests, except UMTX_SHM_CREAT and UMTX_SHM_LOOKUP
+                // sub-requests of the UMTX_OP_SHM request, will return zero.
                 this.write_int(0, dest)?;
                 interp_ok(())
             }