Skip to content

Commit

Permalink
Unrolled build for rust-lang#127918
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#127918 - ChrisDenton:thread-name-string, r=joboet

Safely enforce thread name requirements

The requirements for the thread name to be both UTF-8 and null terminated are easily enforced by a wrapper type so lets do that. The fact this used to be just a bare `CString` has tripped me up before because it was entirely safe to use a non UTF-8 `CString`.
  • Loading branch information
rust-timer committed Jul 20, 2024
2 parents 41ff460 + 9432955 commit aeaddb0
Showing 1 changed file with 51 additions and 23 deletions.
74 changes: 51 additions & 23 deletions library/std/src/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ mod tests;
use crate::any::Any;
use crate::cell::{Cell, OnceCell, UnsafeCell};
use crate::env;
use crate::ffi::{CStr, CString};
use crate::ffi::CStr;
use crate::fmt;
use crate::io;
use crate::marker::PhantomData;
Expand Down Expand Up @@ -487,11 +487,7 @@ impl Builder {
amt
});

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 my_thread = name.map_or_else(Thread::new_unnamed, Thread::new);
let their_thread = my_thread.clone();

let my_packet: Arc<Packet<'scope, T>> = Arc::new(Packet {
Expand Down Expand Up @@ -1299,10 +1295,51 @@ impl ThreadId {
/// The internal representation of a `Thread`'s name.
enum ThreadName {
Main,
Other(CString),
Other(ThreadNameString),
Unnamed,
}

// This module ensures private fields are kept private, which is necessary to enforce the safety requirements.
mod thread_name_string {
use super::ThreadName;
use crate::ffi::{CStr, CString};
use core::str;

/// Like a `String` it's guaranteed UTF-8 and like a `CString` it's null terminated.
pub(crate) struct ThreadNameString {
inner: CString,
}
impl core::ops::Deref for ThreadNameString {
type Target = CStr;
fn deref(&self) -> &CStr {
&self.inner
}
}
impl From<String> for ThreadNameString {
fn from(s: String) -> Self {
Self {
inner: CString::new(s).expect("thread name may not contain interior null bytes"),
}
}
}
impl ThreadName {
pub fn as_cstr(&self) -> Option<&CStr> {
match self {
ThreadName::Main => Some(c"main"),
ThreadName::Other(other) => Some(other),
ThreadName::Unnamed => None,
}
}

pub fn as_str(&self) -> Option<&str> {
// SAFETY: `as_cstr` can only return `Some` for a fixed CStr or a `ThreadNameString`,
// which is guaranteed to be UTF-8.
self.as_cstr().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) })
}
}
}
pub(crate) use thread_name_string::ThreadNameString;

/// The internal representation of a `Thread` handle
struct Inner {
name: ThreadName, // Guaranteed to be UTF-8
Expand Down Expand Up @@ -1342,25 +1379,20 @@ pub struct Thread {

impl Thread {
/// 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(name: String) -> Thread {
Self::new_inner(ThreadName::Other(name.into()))
}

pub(crate) fn new_unnamed() -> Thread {
unsafe { Self::new_inner(ThreadName::Unnamed) }
Self::new_inner(ThreadName::Unnamed)
}

// Used in runtime to construct main thread
pub(crate) fn new_main() -> Thread {
unsafe { Self::new_inner(ThreadName::Main) }
Self::new_inner(ThreadName::Main)
}

/// # Safety
/// If `name` is `ThreadName::Other(_)`, the contained string must be valid UTF-8.
unsafe fn new_inner(name: ThreadName) -> Thread {
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.
//
Expand Down Expand Up @@ -1483,15 +1515,11 @@ impl Thread {
#[stable(feature = "rust1", since = "1.0.0")]
#[must_use]
pub fn name(&self) -> Option<&str> {
self.cname().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) })
self.inner.name.as_str()
}

fn cname(&self) -> Option<&CStr> {
match &self.inner.name {
ThreadName::Main => Some(c"main"),
ThreadName::Other(other) => Some(&other),
ThreadName::Unnamed => None,
}
self.inner.name.as_cstr()
}
}

Expand Down

0 comments on commit aeaddb0

Please sign in to comment.