Auto merge of #3943 - RalfJung:pthread-mutex-reentrant, r=RalfJung

pthread mutex: better error in reentrant-locking-UB

Also test reentrant locking of PTHREAD_MUTEX_INITIALIZER

src/tools/miri/src/shims/unix/sync.rs

@@ -483,7 +483,9 @@ fn pthread_mutex_lock(
                 // Trying to acquire the same mutex again.
                 match kind {
                     MutexKind::Default =>
-                        throw_ub_format!("trying to acquire already locked default mutex"),
+                        throw_ub_format!(
+                            "trying to acquire default mutex already locked by the current thread"
+                        ),
                     MutexKind::Normal => throw_machine_stop!(TerminationInfo::Deadlock),
                     MutexKind::ErrorCheck => this.eval_libc_i32("EDEADLK"),
                     MutexKind::Recursive => {

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_NULL_reentrant.rs

@@ -1,12 +1,12 @@
 //@ignore-target: windows # No pthreads on Windows
 //
-// Check that if we pass NULL attribute, then we get the default mutex type.
+// Check that if we pass NULL attribute, then reentrant locking is UB.
 
 fn main() {
     unsafe {
         let mut mutex: libc::pthread_mutex_t = std::mem::zeroed();
         assert_eq!(libc::pthread_mutex_init(&mut mutex as *mut _, std::ptr::null() as *const _), 0);
         assert_eq!(libc::pthread_mutex_lock(&mut mutex as *mut _), 0);
-        libc::pthread_mutex_lock(&mut mutex as *mut _); //~ ERROR: Undefined Behavior: trying to acquire already locked default mutex
+        libc::pthread_mutex_lock(&mut mutex as *mut _); //~ ERROR: already locked by the current thread
     }
 }

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_NULL_reentrant.stderr

@@ -1,13 +1,13 @@
-error: Undefined Behavior: trying to acquire already locked default mutex
+error: Undefined Behavior: trying to acquire default mutex already locked by the current thread
-  --> tests/fail-dep/concurrency/libc_pthread_mutex_NULL_deadlock.rs:LL:CC
+  --> tests/fail-dep/concurrency/libc_pthread_mutex_NULL_reentrant.rs:LL:CC
    |
 LL |         libc::pthread_mutex_lock(&mut mutex as *mut _);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to acquire already locked default mutex
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to acquire default mutex already locked by the current thread
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE:
-   = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_mutex_NULL_deadlock.rs:LL:CC
+   = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_mutex_NULL_reentrant.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_default_reentrant.rs

@@ -1,6 +1,11 @@
 //@ignore-target: windows # No pthreads on Windows
 //
-// Check that if we do not set the mutex type, it is the default.
+// Check that if we do not set the mutex type, it is UB to do reentrant locking. glibc apparently
+// actually exploits this, see
+// <https://github.molgen.mpg.de/git-mirror/glibc/blob/master/nptl/pthread_mutexattr_settype.c#L31>:
+// one must actively call pthread_mutexattr_settype to disable lock elision. This means a call to
+// pthread_mutexattr_settype(PTHREAD_MUTEX_NORMAL) makes a difference even if
+// PTHREAD_MUTEX_NORMAL == PTHREAD_MUTEX_DEFAULT!
 
 fn main() {
     unsafe {
@@ -9,6 +14,6 @@ fn main() {
         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);
-        libc::pthread_mutex_lock(&mut mutex as *mut _); //~ ERROR: Undefined Behavior: trying to acquire already locked default mutex
+        libc::pthread_mutex_lock(&mut mutex as *mut _); //~ ERROR: already locked by the current thread
     }
 }

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_default_reentrant.stderr

@@ -1,13 +1,13 @@
-error: Undefined Behavior: trying to acquire already locked default mutex
+error: Undefined Behavior: trying to acquire default mutex already locked by the current thread
-  --> tests/fail-dep/concurrency/libc_pthread_mutex_default_deadlock.rs:LL:CC
+  --> tests/fail-dep/concurrency/libc_pthread_mutex_default_reentrant.rs:LL:CC
    |
 LL |         libc::pthread_mutex_lock(&mut mutex as *mut _);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to acquire already locked default mutex
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to acquire default mutex already locked by the current thread
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE:
-   = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_mutex_default_deadlock.rs:LL:CC
+   = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_mutex_default_reentrant.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_normal_reentrant.rs

@@ -10,6 +10,8 @@ fn main() {
         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);
+        // A "normal" mutex properly tries to acquire the lock even if its is already held
+        // by the current thread -- and then we deadlock.
         libc::pthread_mutex_lock(&mut mutex as *mut _); //~ ERROR: deadlock: the evaluated program deadlocked
     }
 }

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_normal_reentrant.stderr

@@ -1,11 +1,11 @@
 error: deadlock: the evaluated program deadlocked
-  --> tests/fail-dep/concurrency/libc_pthread_mutex_normal_deadlock.rs:LL:CC
+  --> tests/fail-dep/concurrency/libc_pthread_mutex_normal_reentrant.rs:LL:CC
    |
 LL |         libc::pthread_mutex_lock(&mut mutex as *mut _);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program deadlocked
    |
    = note: BACKTRACE:
-   = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_mutex_normal_deadlock.rs:LL:CC
+   = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_mutex_normal_reentrant.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
 

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_staticinit_reentrant.rs

@@ -0,0 +1,12 @@
+//@ignore-target: windows # No pthreads on Windows
+//
+// Check that if we use PTHREAD_MUTEX_INITIALIZER, then reentrant locking is UB.
+// glibc apparently actually exploits this so we better catch it!
+
+fn main() {
+    unsafe {
+        let mut mutex: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER;
+        assert_eq!(libc::pthread_mutex_lock(&mut mutex as *mut _), 0);
+        libc::pthread_mutex_lock(&mut mutex as *mut _); //~ ERROR: already locked by the current thread
+    }
+}

src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_staticinit_reentrant.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: trying to acquire default mutex already locked by the current thread
+  --> tests/fail-dep/concurrency/libc_pthread_mutex_staticinit_reentrant.rs:LL:CC
+   |
+LL |         libc::pthread_mutex_lock(&mut mutex as *mut _);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to acquire default mutex already locked by the current thread
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at tests/fail-dep/concurrency/libc_pthread_mutex_staticinit_reentrant.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+