Rollup merge of #144500 - joboet:thread-name-stack-overflow, r=ChrisDenton

thread name in stack overflow message

Fixes rust-lang/rust#144481, which is caused by the thread name not being initialised yet when setting up the stack overflow information. Unfortunately, the stack overflow UI test did not test for the correct thread name being present, and testing this separately didn't occur to me when writing https://github.com/rust-lang/rust/pull/140628.

This PR contains the smallest possible fix I could think of: passing the thread name explicitly to the platform thread creation function. In the future I'd very much like to explore some possibilities around merging the thread packet and thread handle into one structure and using that in the platform code instead – but that's best left for another PR.

This PR also amends the stack overflow test to check for thread names, so we don't run into this again.

``@rustbot`` label +beta-nominated

13 files changed, 78 insertions, 29 deletions

diff --git a/library/std/src/sys/pal/hermit/thread.rs b/library/std/src/sys/pal/hermit/thread.rs
index 9bc5a16b800..95fe4f902d3 100644
--- a/library/std/src/sys/pal/hermit/thread.rs
+++ b/library/std/src/sys/pal/hermit/thread.rs
@@ -58,7 +58,11 @@ impl Thread {
         }
     }
 
-    pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+    pub unsafe fn new(
+        stack: usize,
+        _name: Option<&str>,
+        p: Box<dyn FnOnce()>,
+    ) -> io::Result<Thread> {
         unsafe {
             Thread::new_with_coreid(stack, p, -1 /* = no specific core */)
         }
diff --git a/library/std/src/sys/pal/itron/thread.rs b/library/std/src/sys/pal/itron/thread.rs
index 813e1cbcd58..0d28051fcc4 100644
--- a/library/std/src/sys/pal/itron/thread.rs
+++ b/library/std/src/sys/pal/itron/thread.rs
@@ -86,7 +86,11 @@ impl Thread {
     /// # Safety
     ///
     /// See `thread::Builder::spawn_unchecked` for safety requirements.
-    pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+    pub unsafe fn new(
+        stack: usize,
+        _name: Option<&str>,
+        p: Box<dyn FnOnce()>,
+    ) -> io::Result<Thread> {
         let inner = Box::new(ThreadInner {
             start: UnsafeCell::new(ManuallyDrop::new(p)),
             lifecycle: AtomicUsize::new(LIFECYCLE_INIT),
diff --git a/library/std/src/sys/pal/sgx/thread.rs b/library/std/src/sys/pal/sgx/thread.rs
index 85f6dcd96b4..a236c362706 100644
--- a/library/std/src/sys/pal/sgx/thread.rs
+++ b/library/std/src/sys/pal/sgx/thread.rs
@@ -96,7 +96,11 @@ pub mod wait_notify {
 
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
-    pub unsafe fn new(_stack: usize, p: Box<dyn FnOnce() + Send>) -> io::Result<Thread> {
+    pub unsafe fn new(
+        _stack: usize,
+        _name: Option<&str>,
+        p: Box<dyn FnOnce() + Send>,
+    ) -> io::Result<Thread> {
         let mut queue_lock = task_queue::lock();
         unsafe { usercalls::launch_thread()? };
         let (task, handle) = task_queue::Task::new(p);
diff --git a/library/std/src/sys/pal/teeos/thread.rs b/library/std/src/sys/pal/teeos/thread.rs
index b9cdc7a2a58..a91d95626e7 100644
--- a/library/std/src/sys/pal/teeos/thread.rs
+++ b/library/std/src/sys/pal/teeos/thread.rs
@@ -22,7 +22,11 @@ unsafe extern "C" {
 
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
-    pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+    pub unsafe fn new(
+        stack: usize,
+        _name: Option<&str>,
+        p: Box<dyn FnOnce()>,
+    ) -> io::Result<Thread> {
         let p = Box::into_raw(Box::new(p));
         let mut native: libc::pthread_t = unsafe { mem::zeroed() };
         let mut attr: libc::pthread_attr_t = unsafe { mem::zeroed() };
diff --git a/library/std/src/sys/pal/uefi/thread.rs b/library/std/src/sys/pal/uefi/thread.rs
index e4776ec42fb..75c364362b2 100644
--- a/library/std/src/sys/pal/uefi/thread.rs
+++ b/library/std/src/sys/pal/uefi/thread.rs
@@ -11,7 +11,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 64 * 1024;
 
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
-    pub unsafe fn new(_stack: usize, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+    pub unsafe fn new(
+        _stack: usize,
+        _name: Option<&str>,
+        _p: Box<dyn FnOnce()>,
+    ) -> io::Result<Thread> {
         unsupported()
     }
 
diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs
index a3be2cdf738..d89100e6919 100644
--- a/library/std/src/sys/pal/unix/stack_overflow.rs
+++ b/library/std/src/sys/pal/unix/stack_overflow.rs
@@ -8,8 +8,8 @@ pub struct Handler {
 }
 
 impl Handler {
-    pub unsafe fn new() -> Handler {
-        make_handler(false)
+    pub unsafe fn new(thread_name: Option<Box<str>>) -> Handler {
+        make_handler(false, thread_name)
     }
 
     fn null() -> Handler {
@@ -72,7 +72,6 @@ mod imp {
     use crate::sync::OnceLock;
     use crate::sync::atomic::{Atomic, AtomicBool, AtomicPtr, AtomicUsize, Ordering};
     use crate::sys::pal::unix::os;
-    use crate::thread::with_current_name;
     use crate::{io, mem, panic, ptr};
 
     // Signal handler for the SIGSEGV and SIGBUS handlers. We've got guard pages
@@ -158,13 +157,12 @@ mod imp {
                 if !NEED_ALTSTACK.load(Ordering::Relaxed) {
                     // haven't set up our sigaltstack yet
                     NEED_ALTSTACK.store(true, Ordering::Release);
-                    let handler = unsafe { make_handler(true) };
+                    let handler = unsafe { make_handler(true, None) };
                     MAIN_ALTSTACK.store(handler.data, Ordering::Relaxed);
                     mem::forget(handler);
 
                     if let Some(guard_page_range) = guard_page_range.take() {
-                        let thread_name = with_current_name(|name| name.map(Box::from));
-                        set_current_info(guard_page_range, thread_name);
+                        set_current_info(guard_page_range, Some(Box::from("main")));
                     }
                 }
 
@@ -230,14 +228,13 @@ mod imp {
     /// # Safety
     /// Mutates the alternate signal stack
     #[forbid(unsafe_op_in_unsafe_fn)]
-    pub unsafe fn make_handler(main_thread: bool) -> Handler {
+    pub unsafe fn make_handler(main_thread: bool, thread_name: Option<Box<str>>) -> Handler {
         if !NEED_ALTSTACK.load(Ordering::Acquire) {
             return Handler::null();
         }
 
         if !main_thread {
             if let Some(guard_page_range) = unsafe { current_guard() } {
-                let thread_name = with_current_name(|name| name.map(Box::from));
                 set_current_info(guard_page_range, thread_name);
             }
         }
@@ -634,7 +631,10 @@ mod imp {
 
     pub unsafe fn cleanup() {}
 
-    pub unsafe fn make_handler(_main_thread: bool) -> super::Handler {
+    pub unsafe fn make_handler(
+        _main_thread: bool,
+        _thread_name: Option<Box<str>>,
+    ) -> super::Handler {
         super::Handler::null()
     }
 
@@ -717,7 +717,10 @@ mod imp {
 
     pub unsafe fn cleanup() {}
 
-    pub unsafe fn make_handler(main_thread: bool) -> super::Handler {
+    pub unsafe fn make_handler(
+        main_thread: bool,
+        _thread_name: Option<Box<str>>,
+    ) -> super::Handler {
         if !main_thread {
             reserve_stack();
         }
diff --git a/library/std/src/sys/pal/unix/thread.rs b/library/std/src/sys/pal/unix/thread.rs
index e4f5520d8a3..7f6440152d4 100644
--- a/library/std/src/sys/pal/unix/thread.rs
+++ b/library/std/src/sys/pal/unix/thread.rs
@@ -22,6 +22,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 256 * 1024;
 #[cfg(any(target_os = "espidf", target_os = "nuttx"))]
 pub const DEFAULT_MIN_STACK_SIZE: usize = 0; // 0 indicates that the stack size configured in the ESP-IDF/NuttX menuconfig system should be used
 
+struct ThreadData {
+    name: Option<Box<str>>,
+    f: Box<dyn FnOnce()>,
+}
+
 pub struct Thread {
     id: libc::pthread_t,
 }
@@ -34,8 +39,12 @@ unsafe impl Sync for Thread {}
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
     #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
-    pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
-        let p = Box::into_raw(Box::new(p));
+    pub unsafe fn new(
+        stack: usize,
+        name: Option<&str>,
+        f: Box<dyn FnOnce()>,
+    ) -> io::Result<Thread> {
+        let data = Box::into_raw(Box::new(ThreadData { name: name.map(Box::from), f }));
         let mut native: libc::pthread_t = mem::zeroed();
         let mut attr: mem::MaybeUninit<libc::pthread_attr_t> = mem::MaybeUninit::uninit();
         assert_eq!(libc::pthread_attr_init(attr.as_mut_ptr()), 0);
@@ -73,7 +82,7 @@ impl Thread {
             };
         }
 
-        let ret = libc::pthread_create(&mut native, attr.as_ptr(), thread_start, p as *mut _);
+        let ret = libc::pthread_create(&mut native, attr.as_ptr(), thread_start, data as *mut _);
         // Note: if the thread creation fails and this assert fails, then p will
         // be leaked. However, an alternative design could cause double-free
         // which is clearly worse.
@@ -82,19 +91,20 @@ impl Thread {
         return if ret != 0 {
             // The thread failed to start and as a result p was not consumed. Therefore, it is
             // safe to reconstruct the box so that it gets deallocated.
-            drop(Box::from_raw(p));
+            drop(Box::from_raw(data));
             Err(io::Error::from_raw_os_error(ret))
         } else {
             Ok(Thread { id: native })
         };
 
-        extern "C" fn thread_start(main: *mut libc::c_void) -> *mut libc::c_void {
+        extern "C" fn thread_start(data: *mut libc::c_void) -> *mut libc::c_void {
             unsafe {
+                let data = Box::from_raw(data as *mut ThreadData);
                 // Next, set up our stack overflow handler which may get triggered if we run
                 // out of stack.
-                let _handler = stack_overflow::Handler::new();
+                let _handler = stack_overflow::Handler::new(data.name);
                 // Finally, let's run some code.
-                Box::from_raw(main as *mut Box<dyn FnOnce()>)();
+                (data.f)();
             }
             ptr::null_mut()
         }
diff --git a/library/std/src/sys/pal/unsupported/thread.rs b/library/std/src/sys/pal/unsupported/thread.rs
index 8a3119fa292..5a1e3fde986 100644
--- a/library/std/src/sys/pal/unsupported/thread.rs
+++ b/library/std/src/sys/pal/unsupported/thread.rs
@@ -10,7 +10,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 64 * 1024;
 
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
-    pub unsafe fn new(_stack: usize, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+    pub unsafe fn new(
+        _stack: usize,
+        _name: Option<&str>,
+        _p: Box<dyn FnOnce()>,
+    ) -> io::Result<Thread> {
         unsupported()
     }
 
diff --git a/library/std/src/sys/pal/wasi/thread.rs b/library/std/src/sys/pal/wasi/thread.rs
index 5f21a553673..a46c74630c9 100644
--- a/library/std/src/sys/pal/wasi/thread.rs
+++ b/library/std/src/sys/pal/wasi/thread.rs
@@ -73,7 +73,7 @@ impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
     cfg_if::cfg_if! {
         if #[cfg(target_feature = "atomics")] {
-            pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+            pub unsafe fn new(stack: usize, _name: Option<&str>, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
                 let p = Box::into_raw(Box::new(p));
                 let mut native: libc::pthread_t = unsafe { mem::zeroed() };
                 let mut attr: libc::pthread_attr_t = unsafe { mem::zeroed() };
@@ -120,7 +120,7 @@ impl Thread {
                 }
             }
         } else {
-            pub unsafe fn new(_stack: usize, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+            pub unsafe fn new(_stack: usize, _name: Option<&str>, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
                 crate::sys::unsupported()
             }
         }
diff --git a/library/std/src/sys/pal/wasm/atomics/thread.rs b/library/std/src/sys/pal/wasm/atomics/thread.rs
index 44ce3eab109..ebfabaafc79 100644
--- a/library/std/src/sys/pal/wasm/atomics/thread.rs
+++ b/library/std/src/sys/pal/wasm/atomics/thread.rs
@@ -10,7 +10,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 1024 * 1024;
 
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
-    pub unsafe fn new(_stack: usize, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+    pub unsafe fn new(
+        _stack: usize,
+        _name: Option<&str>,
+        _p: Box<dyn FnOnce()>,
+    ) -> io::Result<Thread> {
         unsupported()
     }
 
diff --git a/library/std/src/sys/pal/windows/thread.rs b/library/std/src/sys/pal/windows/thread.rs
index 14785171755..b45f76fb546 100644
--- a/library/std/src/sys/pal/windows/thread.rs
+++ b/library/std/src/sys/pal/windows/thread.rs
@@ -20,7 +20,11 @@ pub struct Thread {
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
     #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
-    pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+    pub unsafe fn new(
+        stack: usize,
+        _name: Option<&str>,
+        p: Box<dyn FnOnce()>,
+    ) -> io::Result<Thread> {
         let p = Box::into_raw(Box::new(p));
 
         // CreateThread rounds up values for the stack size to the nearest page size (at least 4kb).
diff --git a/library/std/src/sys/pal/xous/thread.rs b/library/std/src/sys/pal/xous/thread.rs
index 1b344e984dc..f2404a62abf 100644
--- a/library/std/src/sys/pal/xous/thread.rs
+++ b/library/std/src/sys/pal/xous/thread.rs
@@ -20,7 +20,11 @@ pub const GUARD_PAGE_SIZE: usize = 4096;
 
 impl Thread {
     // unsafe: see thread::Builder::spawn_unchecked for safety requirements
-    pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
+    pub unsafe fn new(
+        stack: usize,
+        _name: Option<&str>,
+        p: Box<dyn FnOnce()>,
+    ) -> io::Result<Thread> {
         let p = Box::into_raw(Box::new(p));
         let mut stack_size = crate::cmp::max(stack, MIN_STACK_SIZE);
 
diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs
index 8cd1e0163a1..dff981c900c 100644
--- a/library/std/src/thread/mod.rs
+++ b/library/std/src/thread/mod.rs
@@ -595,7 +595,7 @@ impl Builder {
             // Similarly, the `sys` implementation must guarantee that no references to the closure
             // exist after the thread has terminated, which is signaled by `Thread::join`
             // returning.
-            native: unsafe { imp::Thread::new(stack_size, main)? },
+            native: unsafe { imp::Thread::new(stack_size, my_thread.name(), main)? },
             thread: my_thread,
             packet: my_packet,
         })