Rollup merge of #123879 - beetrees:missing-unsafe, r=Mark-Simulacrum
Add missing `unsafe` to some internal `std` functions Adds `unsafe` to a few internal functions that have safety requirements but were previously not marked as `unsafe`. Specifically: - `std::sys::pal::unix:🧵:min_stack_size` needs to be `unsafe` as `__pthread_get_minstack` might dereference the passed pointer. All callers currently pass a valid initialised `libc::pthread_attr_t`. - `std:🧵:Thread::new` (and `new_inner`) need to be `unsafe` as it requires the passed thread name to be valid UTF-8, otherwise `Thread::name` will trigger undefined behaviour. I've taken the opportunity to split out the unnamed thread case into a separate `new_unnamed` function to make the safety requirement clearer. All callers meet the safety requirement now that #123505 has been merged.
This commit is contained in:
commit
2ba0c627de
@ -709,7 +709,7 @@ fn find_mountpoint(group_path: &Path) -> Option<(Cow<'static, str>, &Path)> {
|
||||
// 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
|
||||
}
|
||||
|
@ -202,7 +202,7 @@ fn new(write: bool) -> 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);
|
||||
}
|
||||
|
||||
|
@ -487,9 +487,11 @@ unsafe fn spawn_unchecked_<'a, 'scope, F, T>(
|
||||
amt
|
||||
});
|
||||
|
||||
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<Packet<'scope, T>> = Arc::new(Packet {
|
||||
@ -711,7 +713,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<Thread> {
|
||||
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.
|
||||
@ -1307,21 +1309,26 @@ pub struct Thread {
|
||||
}
|
||||
|
||||
impl Thread {
|
||||
// Used only internally to construct a thread object without spawning
|
||||
pub(crate) fn new(name: Option<CString>) -> 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.
|
||||
//
|
||||
|
Loading…
Reference in New Issue
Block a user