Auto merge of rust-lang#3345 - RalfJung:win-get-thread-name, r=RalfJung

Windows: support getting the thread name Also organize the thread name tests a bit.

Author: bors

src/tools/miri/src/concurrency/data_race.rs

@@ -812,6 +812,7 @@ impl VClockAlloc {
  | MiriMemoryKind::Miri
  | MiriMemoryKind::C
  | MiriMemoryKind::WinHeap
+ | MiriMemoryKind::WinLocal
  | MiriMemoryKind::Mmap,
  )
  | MemoryKind::Stack => {
@@ -820,7 +821,8 @@ impl VClockAlloc {
  alloc_timestamp.span = current_span;
  (alloc_timestamp, alloc_index)
  }
- // Other global memory should trace races but be allocated at the 0 timestamp.
+ // Other global memory should trace races but be allocated at the 0 timestamp
+ // (conceptually they are allocated before everything).
  MemoryKind::Machine(
  MiriMemoryKind::Global
  | MiriMemoryKind::Machine

src/tools/miri/src/concurrency/thread.rs

@@ -981,17 +981,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
  this.machine.threads.set_thread_name(thread, new_thread_name);
  }
 
- #[inline]
- fn set_thread_name_wide(&mut self, thread: ThreadId, new_thread_name: &[u16]) {
- let this = self.eval_context_mut();
-
- // The Windows `GetThreadDescription` shim to get the thread name isn't implemented, so being lossy is okay.
- // This is only read by diagnostics, which already use `from_utf8_lossy`.
- this.machine
- .threads
- .set_thread_name(thread, String::from_utf16_lossy(new_thread_name).into_bytes());
- }
-
  #[inline]
  fn get_thread_name<'c>(&'c self, thread: ThreadId) -> Option<&[u8]>
  where

src/tools/miri/src/machine.rs

@@ -113,6 +113,8 @@ pub enum MiriMemoryKind {
  C,
  /// Windows `HeapAlloc` memory.
  WinHeap,
+ /// Windows "local" memory (to be freed with `LocalFree`)
+ WinLocal,
  /// Memory for args, errno, and other parts of the machine-managed environment.
  /// This memory may leak.
  Machine,
@@ -144,7 +146,7 @@ impl MayLeak for MiriMemoryKind {
  fn may_leak(self) -> bool {
  use self::MiriMemoryKind::*;
  match self {
- Rust | Miri | C | WinHeap | Runtime => false,
+ Rust | Miri | C | WinHeap | WinLocal | Runtime => false,
  Machine | Global | ExternStatic | Tls | Mmap => true,
  }
  }
@@ -156,7 +158,7 @@ impl MiriMemoryKind {
  use self::MiriMemoryKind::*;
  match self {
  // Heap allocations are fine since the `Allocation` is created immediately.
- Rust | Miri | C | WinHeap | Mmap => true,
+ Rust | Miri | C | WinHeap | WinLocal | Mmap => true,
  // Everything else is unclear, let's not show potentially confusing spans.
  Machine | Global | ExternStatic | Tls | Runtime => false,
  }
@@ -171,6 +173,7 @@ impl fmt::Display for MiriMemoryKind {
  Miri => write!(f, "Miri bare-metal heap"),
  C => write!(f, "C heap"),
  WinHeap => write!(f, "Windows heap"),
+ WinLocal => write!(f, "Windows local memory"),
  Machine => write!(f, "machine-managed memory"),
  Runtime => write!(f, "language runtime memory"),
  Global => write!(f, "global (static or const)"),

src/tools/miri/src/shims/windows/foreign_items.rs

@@ -5,14 +5,19 @@ use rustc_span::Symbol;
 use rustc_target::abi::Size;
 use rustc_target::spec::abi::Abi;
 
+use crate::shims::os_str::bytes_to_os_str;
 use crate::*;
 use shims::foreign_items::EmulateForeignItemResult;
 use shims::windows::handle::{EvalContextExt as _, Handle, PseudoHandle};
 use shims::windows::sync::EvalContextExt as _;
 use shims::windows::thread::EvalContextExt as _;
 
 fn is_dyn_sym(name: &str) -> bool {
- matches!(name, "SetThreadDescription" | "WaitOnAddress" | "WakeByAddressSingle")
+ // std does dynamic detection for these symbols
+ matches!(
+ name,
+ "SetThreadDescription" | "GetThreadDescription" | "WaitOnAddress" | "WakeByAddressSingle"
+ )
 }
 
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
@@ -172,6 +177,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
  let res = this.realloc(ptr, size, MiriMemoryKind::WinHeap)?;
  this.write_pointer(res, dest)?;
  }
+ "LocalFree" => {
+ let [ptr] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
+ let ptr = this.read_pointer(ptr)?;
+ this.free(ptr, MiriMemoryKind::WinLocal)?;
+ this.write_null(dest)?;
+ }
 
  // errno
  "SetLastError" => {
@@ -403,7 +414,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
  this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
 
  let handle = this.read_scalar(handle)?;
-
  let name = this.read_wide_str(this.read_pointer(name)?)?;
 
  let thread = match Handle::from_scalar(handle, this)? {
@@ -412,7 +422,31 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
  _ => this.invalid_handle("SetThreadDescription")?,
  };
 
- this.set_thread_name_wide(thread, &name);
+ // FIXME: use non-lossy conversion
+ this.set_thread_name(thread, String::from_utf16_lossy(&name).into_bytes());
+
+ this.write_null(dest)?;
+ }
+ "GetThreadDescription" => {
+ let [handle, name_ptr] =
+ this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
+
+ let handle = this.read_scalar(handle)?;
+ let name_ptr = this.deref_pointer(name_ptr)?; // the pointer where we should store the ptr to the name
+
+ let thread = match Handle::from_scalar(handle, this)? {
+ Some(Handle::Thread(thread)) => thread,
+ Some(Handle::Pseudo(PseudoHandle::CurrentThread)) => this.get_active_thread(),
+ _ => this.invalid_handle("SetThreadDescription")?,
+ };
+ // Looks like the default thread name is empty.
+ let name = this.get_thread_name(thread).unwrap_or(b"").to_owned();
+ let name = this.alloc_os_str_as_wide_str(
+ bytes_to_os_str(&name)?,
+ MiriMemoryKind::WinLocal.into(),
+ )?;
+
+ this.write_scalar(Scalar::from_maybe_pointer(name, this), &name_ptr)?;
 
  this.write_null(dest)?;
  }

src/tools/miri/tests/pass/concurrency/simple.rs

@@ -45,23 +45,6 @@ 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();
-}
-
 // This is not a data race!
 fn shared_readonly() {
  use std::sync::Arc;
@@ -89,6 +72,4 @@ fn main() {
  create_move_in();
  create_move_out();
  shared_readonly();
- panic();
- panic_named();
 }

src/tools/miri/tests/pass/concurrency/simple.stderr

@@ -0,0 +1,21 @@
+use std::thread;
+
+fn main() {
+ // When we have not set the name...
+ thread::spawn(|| {
+ assert!(thread::current().name().is_none());
+ });
+
+ // ... and when we have set it.
+ thread::Builder::new()
+ .name("childthread".to_string())
+ .spawn(move || {
+ assert_eq!(thread::current().name().unwrap(), "childthread");
+ })
+ .unwrap()
+ .join()
+ .unwrap();
+
+ // Also check main thread name.
+ assert_eq!(thread::current().name().unwrap(), "main");
+}

src/tools/miri/tests/pass/concurrency/thread_name.rs

@@ -0,0 +1,25 @@
+//! Panicking in other threads.
+
+use std::thread;
+
+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() {
+ panic();
+ panic_named();
+}

src/tools/miri/tests/pass/panic/thread_panic.rs

@@ -0,0 +1,5 @@
+thread '<unnamed>' panicked at $DIR/thread_panic.rs:LL:CC:
+Hello!
+note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
+thread 'childthread' panicked at $DIR/thread_panic.rs:LL:CC:
+Hello, world!

src/tools/miri/tests/pass/panic/thread_panic.stderr

@@ -1,4 +1,4 @@
-//@only-target-windows: this directly tests windows only random functions
+//@only-target-windows: this directly tests windows-only functions
 use core::ffi::c_void;
 use core::mem::size_of_val;
 use core::ptr::null_mut;
@@ -26,12 +26,12 @@ extern "system" {
 #[cfg(target_arch = "x86")]
 #[link(name = "bcryptprimitives", kind = "raw-dylib", import_name_type = "undecorated")]
 extern "system" {
- pub fn ProcessPrng(pbdata: *mut u8, cbdata: usize) -> BOOL;
+ fn ProcessPrng(pbdata: *mut u8, cbdata: usize) -> BOOL;
 }
 #[cfg(not(target_arch = "x86"))]
 #[link(name = "bcryptprimitives", kind = "raw-dylib")]
 extern "system" {
- pub fn ProcessPrng(pbdata: *mut u8, cbdata: usize) -> BOOL;
+ fn ProcessPrng(pbdata: *mut u8, cbdata: usize) -> BOOL;
 }
 
 fn main() {