From 53f55c6635d746613eb1ceb879d58f8e3da97057 Mon Sep 17 00:00:00 2001 From: beetrees Date: Fri, 12 Apr 2024 22:44:25 +0100 Subject: [PATCH 1/2] Add missing `unsafe` to internal function `std::sys::pal::unix::thread::min_stack_size` --- library/std/src/sys/pal/unix/thread.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/pal/unix/thread.rs b/library/std/src/sys/pal/unix/thread.rs index f56e64c350588..c550cc04a3944 100644 --- a/library/std/src/sys/pal/unix/thread.rs +++ b/library/std/src/sys/pal/unix/thread.rs @@ -709,7 +709,7 @@ mod cgroups { // is created in an application with big thread-local storage requirements. // See #6233 for rationale and details. #[cfg(all(target_os = "linux", target_env = "gnu"))] -fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { +unsafe fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { // We use dlsym to avoid an ELF version dependency on GLIBC_PRIVATE. (#23628) // We shouldn't really be using such an internal symbol, but there's currently // no other way to account for the TLS size. @@ -723,11 +723,11 @@ fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { // No point in looking up __pthread_get_minstack() on non-glibc platforms. #[cfg(all(not(all(target_os = "linux", target_env = "gnu")), not(target_os = "netbsd")))] -fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { +unsafe fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { libc::PTHREAD_STACK_MIN } #[cfg(target_os = "netbsd")] -fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { +unsafe fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { 2048 // just a guess } From 126c762b8577dc65ccba1e0105d5c153bcc1ad56 Mon Sep 17 00:00:00 2001 From: beetrees Date: Fri, 12 Apr 2024 22:44:36 +0100 Subject: [PATCH 2/2] Add missing `unsafe` to internal `std::thread::Thread` creation functions --- library/std/src/sys/sync/rwlock/queue.rs | 2 +- library/std/src/thread/mod.rs | 33 ++++++++++++++---------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/library/std/src/sys/sync/rwlock/queue.rs b/library/std/src/sys/sync/rwlock/queue.rs index d1918855797ad..337cc6c2ca094 100644 --- a/library/std/src/sys/sync/rwlock/queue.rs +++ b/library/std/src/sys/sync/rwlock/queue.rs @@ -202,7 +202,7 @@ impl Node { fn prepare(&mut self) { // Fall back to creating an unnamed `Thread` handle to allow locking in // TLS destructors. - self.thread.get_or_init(|| thread::try_current().unwrap_or_else(|| Thread::new(None))); + self.thread.get_or_init(|| thread::try_current().unwrap_or_else(Thread::new_unnamed)); self.completed = AtomicBool::new(false); } diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index f792a27dd694d..99ca770aacf19 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -470,9 +470,11 @@ impl Builder { let stack_size = stack_size.unwrap_or_else(thread::min_stack); - let my_thread = Thread::new(name.map(|name| { - CString::new(name).expect("thread name may not contain interior null bytes") - })); + let my_thread = name.map_or_else(Thread::new_unnamed, |name| unsafe { + Thread::new( + CString::new(name).expect("thread name may not contain interior null bytes"), + ) + }); let their_thread = my_thread.clone(); let my_packet: Arc> = Arc::new(Packet { @@ -694,7 +696,7 @@ pub(crate) fn set_current(thread: Thread) { /// In contrast to the public `current` function, this will not panic if called /// from inside a TLS destructor. pub(crate) fn try_current() -> Option { - CURRENT.try_with(|current| current.get_or_init(|| Thread::new(None)).clone()).ok() + CURRENT.try_with(|current| current.get_or_init(|| Thread::new_unnamed()).clone()).ok() } /// Gets a handle to the thread that invokes it. @@ -1290,21 +1292,26 @@ pub struct Thread { } impl Thread { - // Used only internally to construct a thread object without spawning - pub(crate) fn new(name: Option) -> Thread { - if let Some(name) = name { - Self::new_inner(ThreadName::Other(name)) - } else { - Self::new_inner(ThreadName::Unnamed) - } + /// Used only internally to construct a thread object without spawning. + /// + /// # Safety + /// `name` must be valid UTF-8. + pub(crate) unsafe fn new(name: CString) -> Thread { + unsafe { Self::new_inner(ThreadName::Other(name)) } + } + + pub(crate) fn new_unnamed() -> Thread { + unsafe { Self::new_inner(ThreadName::Unnamed) } } // Used in runtime to construct main thread pub(crate) fn new_main() -> Thread { - Self::new_inner(ThreadName::Main) + unsafe { Self::new_inner(ThreadName::Main) } } - fn new_inner(name: ThreadName) -> Thread { + /// # Safety + /// If `name` is `ThreadName::Other(_)`, the contained string must be valid UTF-8. + unsafe fn new_inner(name: ThreadName) -> Thread { // We have to use `unsafe` here to construct the `Parker` in-place, // which is required for the UNIX implementation. //