Update comments, rearrange code

Author: divergentdave

src/machine.rs

@@ -217,6 +217,8 @@ pub struct Evaluator<'tcx> {
     pub(crate) time_anchor: Instant,
 
     /// Cached `TyLayout`s for primitive data types that are commonly used inside Miri.
+    /// FIXME: Search through the rest of the codebase for more layout_of() calls that
+    /// could be cached here.
     primitive_layouts: PrimitiveLayouts<'tcx>,
 }
 

src/shims/foreign_items/posix.rs

@@ -233,113 +233,101 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_null(dest)?;
             }
 
-            // Better error for attempts to create a thread
-            "pthread_create" => {
-                throw_unsup_format!("Miri does not support threading");
-            }
-
-            // Miscellaneous
-            "isatty" => {
-                let _fd = this.read_scalar(args[0])?.to_i32()?;
-                // "returns 1 if fd is an open file descriptor referring to a terminal; otherwise 0 is returned, and errno is set to indicate the error"
-                // FIXME: we just say nothing is a terminal.
-                let enotty = this.eval_libc("ENOTTY")?;
-                this.set_last_error(enotty)?;
-                this.write_null(dest)?;
-            }
-            "pthread_atfork" => {
-                let _prepare = this.read_scalar(args[0])?.not_undef()?;
-                let _parent = this.read_scalar(args[1])?.not_undef()?;
-                let _child = this.read_scalar(args[1])?.not_undef()?;
-                // We do not support forking, so there is nothing to do here.
-                this.write_null(dest)?;
-            }
-
-            // Incomplete shims that we "stub out" just to get pre-main initialization code to work.
-            // These shims are enabled only when the caller is in the standard library.
-            | "pthread_attr_init"
-            | "pthread_attr_destroy"
-            | "pthread_self"
-            | "pthread_attr_setstacksize"
-            | "pthread_condattr_init"
-            | "pthread_condattr_setclock"
-            | "pthread_cond_init"
-            | "pthread_condattr_destroy"
-            | "pthread_cond_destroy" if this.frame().instance.to_string().starts_with("std::sys::unix::")
-            => {
-                this.write_null(dest)?;
-            }
-
+            // Synchronization primitives
             "pthread_mutexattr_init" => {
                 let result = this.pthread_mutexattr_init(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_mutexattr_settype" => {
                 let result = this.pthread_mutexattr_settype(args[0], args[1])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_mutexattr_destroy" => {
                 let result = this.pthread_mutexattr_destroy(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_mutex_init" => {
                 let result = this.pthread_mutex_init(args[0], args[1])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_mutex_lock" => {
                 let result = this.pthread_mutex_lock(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_mutex_trylock" => {
                 let result = this.pthread_mutex_trylock(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_mutex_unlock" => {
                 let result = this.pthread_mutex_unlock(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_mutex_destroy" => {
                 let result = this.pthread_mutex_destroy(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_rwlock_rdlock" => {
                 let result = this.pthread_rwlock_rdlock(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_rwlock_tryrdlock" => {
                 let result = this.pthread_rwlock_tryrdlock(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_rwlock_wrlock" => {
                 let result = this.pthread_rwlock_wrlock(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_rwlock_trywrlock" => {
                 let result = this.pthread_rwlock_trywrlock(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_rwlock_unlock" => {
                 let result = this.pthread_rwlock_unlock(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
-
             "pthread_rwlock_destroy" => {
                 let result = this.pthread_rwlock_destroy(args[0])?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
 
+            // Better error for attempts to create a thread
+            "pthread_create" => {
+                throw_unsup_format!("Miri does not support threading");
+            }
+
+            // Miscellaneous
+            "isatty" => {
+                let _fd = this.read_scalar(args[0])?.to_i32()?;
+                // "returns 1 if fd is an open file descriptor referring to a terminal; otherwise 0 is returned, and errno is set to indicate the error"
+                // FIXME: we just say nothing is a terminal.
+                let enotty = this.eval_libc("ENOTTY")?;
+                this.set_last_error(enotty)?;
+                this.write_null(dest)?;
+            }
+            "pthread_atfork" => {
+                let _prepare = this.read_scalar(args[0])?.not_undef()?;
+                let _parent = this.read_scalar(args[1])?.not_undef()?;
+                let _child = this.read_scalar(args[1])?.not_undef()?;
+                // We do not support forking, so there is nothing to do here.
+                this.write_null(dest)?;
+            }
+
+            // Incomplete shims that we "stub out" just to get pre-main initialization code to work.
+            // These shims are enabled only when the caller is in the standard library.
+            | "pthread_attr_init"
+            | "pthread_attr_destroy"
+            | "pthread_self"
+            | "pthread_attr_setstacksize"
+            | "pthread_condattr_init"
+            | "pthread_condattr_setclock"
+            | "pthread_cond_init"
+            | "pthread_condattr_destroy"
+            | "pthread_cond_destroy" if this.frame().instance.to_string().starts_with("std::sys::unix::")
+            => {
+                this.write_null(dest)?;
+            }
+
             | "signal"
             | "sigaction"
             | "sigaltstack"

src/shims/sync.rs

@@ -20,8 +20,9 @@ fn assert_ptr_target_min_size<'mir, 'tcx: 'mir>(
 
 // pthread_mutexattr_t is either 4 or 8 bytes, depending on the platform.
 
-// Our chosen memory layout: store an i32 in the first four bytes equal to the
-// corresponding libc mutex kind constant (i.e. PTHREAD_MUTEX_NORMAL)
+// Our chosen memory layout for emulation (does not have to match the platform layout!):
+// store an i32 in the first four bytes equal to the corresponding libc mutex kind constant
+// (e.g. PTHREAD_MUTEX_NORMAL).
 
 fn mutexattr_get_kind<'mir, 'tcx: 'mir>(
     ecx: &MiriEvalContext<'mir, 'tcx>,
@@ -48,7 +49,7 @@ fn mutexattr_set_kind<'mir, 'tcx: 'mir>(
 
 // pthread_mutex_t is between 24 and 48 bytes, depending on the platform.
 
-// Our chosen memory layout:
+// Our chosen memory layout for the emulated mutex (does not have to match the platform layout!):
 // bytes 0-3: reserved for signature on macOS
 // (need to avoid this because it is set by static initializer macros)
 // bytes 4-7: count of how many times this mutex has been locked, as a u32
@@ -117,7 +118,7 @@ fn mutex_set_kind<'mir, 'tcx: 'mir>(
 
 // pthread_rwlock_t is between 32 and 56 bytes, depending on the platform.
 
-// Our chosen memory layout:
+// Our chosen memory layout for the emulated rwlock (does not have to match the platform layout!):
 // bytes 0-3: reserved for signature on macOS
 // (need to avoid this because it is set by static initializer macros)
 // bytes 4-7: reader count, as a u32

tests/run-pass/libc.rs

@@ -42,7 +42,92 @@ fn test_posix_fadvise() {
     assert_eq!(result, 0);
 }
 
+fn test_mutex_libc_init_recursive() {
+    unsafe {
+        let mut attr: libc::pthread_mutexattr_t = std::mem::zeroed();
+        assert_eq!(libc::pthread_mutexattr_init(&mut attr as *mut _), 0);
+        assert_eq!(libc::pthread_mutexattr_settype(&mut attr as *mut _, libc::PTHREAD_MUTEX_RECURSIVE), 0);
+        let mut mutex: libc::pthread_mutex_t = std::mem::zeroed();
+        assert_eq!(libc::pthread_mutex_init(&mut mutex as *mut _, &mut attr as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_lock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_trylock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_unlock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_unlock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_trylock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_lock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_unlock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_unlock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_unlock(&mut mutex as *mut _), libc::EPERM);
+        assert_eq!(libc::pthread_mutex_destroy(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutexattr_destroy(&mut attr as *mut _), 0);
+    }
+}
+
+fn test_mutex_libc_init_normal() {
+    unsafe {
+        let mut mutexattr: libc::pthread_mutexattr_t = std::mem::zeroed();
+        assert_eq!(libc::pthread_mutexattr_settype(&mut mutexattr as *mut _, libc::PTHREAD_MUTEX_NORMAL), 0);
+        let mut mutex: libc::pthread_mutex_t = std::mem::zeroed();
+        assert_eq!(libc::pthread_mutex_init(&mut mutex as *mut _, &mutexattr as *const _), 0);
+        assert_eq!(libc::pthread_mutex_lock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_trylock(&mut mutex as *mut _), libc::EBUSY);
+        assert_eq!(libc::pthread_mutex_unlock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_trylock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_unlock(&mut mutex as *mut _), 0);
+        assert_eq!(libc::pthread_mutex_destroy(&mut mutex as *mut _), 0);
+    }
+}
+
+// Only linux provides PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP,
+// libc for macOS just has the default PTHREAD_MUTEX_INITIALIZER.
+#[cfg(target_os = "linux")]
+fn test_mutex_libc_static_initializer_recursive() {
+    let mutex = std::cell::UnsafeCell::new(libc::PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP);
+    unsafe {
+        assert_eq!(libc::pthread_mutex_lock(mutex.get()), 0);
+        assert_eq!(libc::pthread_mutex_trylock(mutex.get()), 0);
+        assert_eq!(libc::pthread_mutex_unlock(mutex.get()), 0);
+        assert_eq!(libc::pthread_mutex_unlock(mutex.get()), 0);
+        assert_eq!(libc::pthread_mutex_trylock(mutex.get()), 0);
+        assert_eq!(libc::pthread_mutex_lock(mutex.get()), 0);
+        assert_eq!(libc::pthread_mutex_unlock(mutex.get()), 0);
+        assert_eq!(libc::pthread_mutex_unlock(mutex.get()), 0);
+        assert_eq!(libc::pthread_mutex_unlock(mutex.get()), libc::EPERM);
+        assert_eq!(libc::pthread_mutex_destroy(mutex.get()), 0);
+    }
+}
+
+// Testing the behavior of std::sync::RwLock does not fully exercise the pthread rwlock shims, we
+// need to go a layer deeper and test the behavior of the libc functions, because
+// std::sys::unix::rwlock::RWLock itself keeps track of write_locked and num_readers.
+fn test_rwlock_libc_static_initializer() {
+    let rw = std::cell::UnsafeCell::new(libc::PTHREAD_RWLOCK_INITIALIZER);
+    unsafe {
+        assert_eq!(libc::pthread_rwlock_rdlock(rw.get()), 0);
+        assert_eq!(libc::pthread_rwlock_rdlock(rw.get()), 0);
+        assert_eq!(libc::pthread_rwlock_unlock(rw.get()), 0);
+        assert_eq!(libc::pthread_rwlock_tryrdlock(rw.get()), 0);
+        assert_eq!(libc::pthread_rwlock_unlock(rw.get()), 0);
+        assert_eq!(libc::pthread_rwlock_trywrlock(rw.get()), libc::EBUSY);
+        assert_eq!(libc::pthread_rwlock_unlock(rw.get()), 0);
+
+        assert_eq!(libc::pthread_rwlock_wrlock(rw.get()), 0);
+        assert_eq!(libc::pthread_rwlock_tryrdlock(rw.get()), libc::EBUSY);
+        assert_eq!(libc::pthread_rwlock_trywrlock(rw.get()), libc::EBUSY);
+        assert_eq!(libc::pthread_rwlock_unlock(rw.get()), 0);
+
+        assert_eq!(libc::pthread_rwlock_destroy(rw.get()), 0);
+    }
+}
+
 fn main() {
     #[cfg(not(target_os = "macos"))]
     test_posix_fadvise();
+
+    test_mutex_libc_init_recursive();
+    test_mutex_libc_init_normal();
+    test_rwlock_libc_static_initializer();
+
+    #[cfg(target_os = "linux")]
+    test_mutex_libc_static_initializer_recursive();
 }

tests/run-pass/reentrant-println.rs

@@ -1,7 +1,7 @@
 use std::fmt::{Display, Error, Formatter};
 
 // This test case exercises std::sys_common::remutex::ReentrantMutex
-// by calling println!() from inside fmt
+// by calling println!() from inside fmt.
 
 struct InterruptingCow;