Auto merge of #1390 - RalfJung:conc, r=RalfJung

Make spin_loop yield, and a panicking thread test

@vakaras agreed it would make sense to make `spin_loop` yield, so this implements that.

Also add a test for a panic being raised in one thread, caught by the panic machinery, and then expose through the join handle. Miraculously, that just works. :)

Author: bors

src/shims/foreign_items.rs

@@ -434,7 +434,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             }
 
             // Architecture-specific shims
-            "llvm.x86.sse2.pause" if this.tcx.sess.target.target.arch == "x86" || this.tcx.sess.target.target.arch == "x86_64" => {}
+            "llvm.x86.sse2.pause" if this.tcx.sess.target.target.arch == "x86" || this.tcx.sess.target.target.arch == "x86_64" => {
+                this.sched_yield()?;
+            }
 
             // Platform-specific shims
             _ => match this.tcx.sess.target.target.target_os.as_str() {

src/shims/foreign_items/posix/macos.rs

@@ -98,6 +98,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_scalar(stack_size, dest)?;
             }
 
+            // Threading
+            "pthread_setname_np" => {
+                let ptr = this.read_scalar(args[0])?.not_undef()?;
+                this.pthread_setname_np(ptr)?;
+            }
+
             // 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.
             "mmap" if this.frame().instance.to_string().starts_with("std::sys::unix::") => {

src/shims/thread.rs

@@ -95,6 +95,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         _arg5: OpTy<'tcx, Tag>,
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
+        this.assert_target_os("linux", "prctl");
 
         let option = this.read_scalar(option)?.to_i32()?;
         if option == this.eval_libc_i32("PR_SET_NAME")? {
@@ -118,6 +119,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         Ok(0)
     }
 
+    fn pthread_setname_np(
+        &mut self,
+        ptr: Scalar<Tag>,
+    ) -> InterpResult<'tcx> {
+        let this = self.eval_context_mut();
+        this.assert_target_os("macos", "pthread_setname_np");
+
+        let name = this.memory.read_c_str(ptr)?.to_owned();
+        this.set_active_thread_name(name)?;
+
+        Ok(())
+    }
+
     fn sched_yield(&mut self) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 

src/thread.rs

@@ -134,16 +134,20 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> {
         }
         false
     }
+
+    /// Get the name of the current thread, or `<unnamed>` if it was not set.
+    fn thread_name(&self) -> &[u8] {
+        if let Some(ref thread_name) = self.thread_name {
+            thread_name
+        } else {
+            b"<unnamed>"
+        }
+    }
 }
 
 impl<'mir, 'tcx> std::fmt::Debug for Thread<'mir, 'tcx> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        if let Some(ref name) = self.thread_name {
-            write!(f, "{}", String::from_utf8_lossy(name))?;
-        } else {
-            write!(f, "<unnamed>")?;
-        }
-        write!(f, "({:?}, {:?})", self.state, self.join_status)
+        write!(f, "{}({:?}, {:?})", String::from_utf8_lossy(self.thread_name()), self.state, self.join_status)
     }
 }
 
@@ -314,11 +318,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
 
     /// Get the name of the active thread.
     fn get_thread_name(&self) -> &[u8] {
-        if let Some(ref thread_name) = self.active_thread_ref().thread_name {
-            thread_name
-        } else {
-            b"<unnamed>"
-        }
+        self.active_thread_ref().thread_name()
     }
 
     /// Allocate a new blockset id.

tests/run-pass/concurrency/simple.rs

@@ -49,6 +49,25 @@ fn create_move_out() {
     assert_eq!(result.len(), 6);
 }
 
+fn panic() {
+    let result = thread::spawn(|| {
+        panic!("Hello!")
+    })
+    .join()
+    .unwrap_err();
+    let msg = result.downcast_ref::<&'static str>().unwrap();
+    assert_eq!(*msg, "Hello!");
+}
+
+fn panic_named() {
+    thread::Builder::new().name("childthread".to_string()).spawn(move || {
+        panic!("Hello, world!");
+    })
+    .unwrap()
+    .join()
+    .unwrap_err();
+}
+
 fn main() {
     create_and_detach();
     create_and_join();
@@ -58,4 +77,6 @@ fn main() {
     create_nested_and_join();
     create_move_in();
     create_move_out();
+    panic();
+    panic_named();
 }

tests/run-pass/concurrency/simple.stderr

@@ -1,2 +1,4 @@
 warning: thread support is experimental. For example, Miri does not detect data races yet.
 
+thread '<unnamed>' panicked at 'Hello!', $DIR/simple.rs:54:9
+thread 'childthread' panicked at 'Hello, world!', $DIR/simple.rs:64:9

tests/run-pass/sync.rs

@@ -1,7 +1,8 @@
-#![feature(rustc_private)]
+#![feature(rustc_private, renamed_spin_loop)]
 
 use std::sync::{Mutex, TryLockError};
 use std::sync::atomic;
+use std::hint;
 
 fn main() {
     test_mutex_stdlib();
@@ -56,6 +57,7 @@ impl<T> TryLockErrorExt<T> for TryLockError<T> {
 
 fn test_spin_loop_hint() {
     atomic::spin_loop_hint();
+    hint::spin_loop();
 }
 
 fn test_thread_yield_now() {

tests/run-pass/transmute_fat2.rs

@@ -8,5 +8,6 @@ fn main() {
     let bad = unsafe {
         std::mem::transmute::<u64, &[u8]>(42)
     };
+    // This created a slice with length 0, so the following will fail the bounds check.
     bad[0];
 }

tests/run-pass/transmute_fat2.stderr

@@ -1 +1 @@
-thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 0', $DIR/transmute_fat2.rs:11:5
+thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 0', $DIR/transmute_fat2.rs:12:5