Skip to content

Commit a9feef6

Browse files
authored
Unrolled build for #144500
Rollup merge of #144500 - joboet:thread-name-stack-overflow, r=ChrisDenton thread name in stack overflow message Fixes #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 #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
2 parents 5529041 + 73751a0 commit a9feef6

File tree

14 files changed

+87
-32
lines changed

14 files changed

+87
-32
lines changed

library/std/src/sys/pal/hermit/thread.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@ impl Thread {
5858
}
5959
}
6060

61-
pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
61+
pub unsafe fn new(
62+
stack: usize,
63+
_name: Option<&str>,
64+
p: Box<dyn FnOnce()>,
65+
) -> io::Result<Thread> {
6266
unsafe {
6367
Thread::new_with_coreid(stack, p, -1 /* = no specific core */)
6468
}

library/std/src/sys/pal/itron/thread.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,11 @@ impl Thread {
8686
/// # Safety
8787
///
8888
/// See `thread::Builder::spawn_unchecked` for safety requirements.
89-
pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
89+
pub unsafe fn new(
90+
stack: usize,
91+
_name: Option<&str>,
92+
p: Box<dyn FnOnce()>,
93+
) -> io::Result<Thread> {
9094
let inner = Box::new(ThreadInner {
9195
start: UnsafeCell::new(ManuallyDrop::new(p)),
9296
lifecycle: AtomicUsize::new(LIFECYCLE_INIT),

library/std/src/sys/pal/sgx/thread.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,11 @@ pub mod wait_notify {
9696

9797
impl Thread {
9898
// unsafe: see thread::Builder::spawn_unchecked for safety requirements
99-
pub unsafe fn new(_stack: usize, p: Box<dyn FnOnce() + Send>) -> io::Result<Thread> {
99+
pub unsafe fn new(
100+
_stack: usize,
101+
_name: Option<&str>,
102+
p: Box<dyn FnOnce() + Send>,
103+
) -> io::Result<Thread> {
100104
let mut queue_lock = task_queue::lock();
101105
unsafe { usercalls::launch_thread()? };
102106
let (task, handle) = task_queue::Task::new(p);

library/std/src/sys/pal/teeos/thread.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@ unsafe extern "C" {
2222

2323
impl Thread {
2424
// unsafe: see thread::Builder::spawn_unchecked for safety requirements
25-
pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
25+
pub unsafe fn new(
26+
stack: usize,
27+
_name: Option<&str>,
28+
p: Box<dyn FnOnce()>,
29+
) -> io::Result<Thread> {
2630
let p = Box::into_raw(Box::new(p));
2731
let mut native: libc::pthread_t = unsafe { mem::zeroed() };
2832
let mut attr: libc::pthread_attr_t = unsafe { mem::zeroed() };

library/std/src/sys/pal/uefi/thread.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 64 * 1024;
1111

1212
impl Thread {
1313
// unsafe: see thread::Builder::spawn_unchecked for safety requirements
14-
pub unsafe fn new(_stack: usize, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
14+
pub unsafe fn new(
15+
_stack: usize,
16+
_name: Option<&str>,
17+
_p: Box<dyn FnOnce()>,
18+
) -> io::Result<Thread> {
1519
unsupported()
1620
}
1721

library/std/src/sys/pal/unix/stack_overflow.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ pub struct Handler {
88
}
99

1010
impl Handler {
11-
pub unsafe fn new() -> Handler {
12-
make_handler(false)
11+
pub unsafe fn new(thread_name: Option<Box<str>>) -> Handler {
12+
make_handler(false, thread_name)
1313
}
1414

1515
fn null() -> Handler {
@@ -72,7 +72,6 @@ mod imp {
7272
use crate::sync::OnceLock;
7373
use crate::sync::atomic::{Atomic, AtomicBool, AtomicPtr, AtomicUsize, Ordering};
7474
use crate::sys::pal::unix::os;
75-
use crate::thread::with_current_name;
7675
use crate::{io, mem, panic, ptr};
7776

7877
// Signal handler for the SIGSEGV and SIGBUS handlers. We've got guard pages
@@ -158,13 +157,12 @@ mod imp {
158157
if !NEED_ALTSTACK.load(Ordering::Relaxed) {
159158
// haven't set up our sigaltstack yet
160159
NEED_ALTSTACK.store(true, Ordering::Release);
161-
let handler = unsafe { make_handler(true) };
160+
let handler = unsafe { make_handler(true, None) };
162161
MAIN_ALTSTACK.store(handler.data, Ordering::Relaxed);
163162
mem::forget(handler);
164163

165164
if let Some(guard_page_range) = guard_page_range.take() {
166-
let thread_name = with_current_name(|name| name.map(Box::from));
167-
set_current_info(guard_page_range, thread_name);
165+
set_current_info(guard_page_range, Some(Box::from("main")));
168166
}
169167
}
170168

@@ -230,14 +228,13 @@ mod imp {
230228
/// # Safety
231229
/// Mutates the alternate signal stack
232230
#[forbid(unsafe_op_in_unsafe_fn)]
233-
pub unsafe fn make_handler(main_thread: bool) -> Handler {
231+
pub unsafe fn make_handler(main_thread: bool, thread_name: Option<Box<str>>) -> Handler {
234232
if !NEED_ALTSTACK.load(Ordering::Acquire) {
235233
return Handler::null();
236234
}
237235

238236
if !main_thread {
239237
if let Some(guard_page_range) = unsafe { current_guard() } {
240-
let thread_name = with_current_name(|name| name.map(Box::from));
241238
set_current_info(guard_page_range, thread_name);
242239
}
243240
}
@@ -634,7 +631,10 @@ mod imp {
634631

635632
pub unsafe fn cleanup() {}
636633

637-
pub unsafe fn make_handler(_main_thread: bool) -> super::Handler {
634+
pub unsafe fn make_handler(
635+
_main_thread: bool,
636+
_thread_name: Option<Box<str>>,
637+
) -> super::Handler {
638638
super::Handler::null()
639639
}
640640

@@ -717,7 +717,10 @@ mod imp {
717717

718718
pub unsafe fn cleanup() {}
719719

720-
pub unsafe fn make_handler(main_thread: bool) -> super::Handler {
720+
pub unsafe fn make_handler(
721+
main_thread: bool,
722+
_thread_name: Option<Box<str>>,
723+
) -> super::Handler {
721724
if !main_thread {
722725
reserve_stack();
723726
}

library/std/src/sys/pal/unix/thread.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 256 * 1024;
2222
#[cfg(any(target_os = "espidf", target_os = "nuttx"))]
2323
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
2424

25+
struct ThreadData {
26+
name: Option<Box<str>>,
27+
f: Box<dyn FnOnce()>,
28+
}
29+
2530
pub struct Thread {
2631
id: libc::pthread_t,
2732
}
@@ -34,8 +39,12 @@ unsafe impl Sync for Thread {}
3439
impl Thread {
3540
// unsafe: see thread::Builder::spawn_unchecked for safety requirements
3641
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
37-
pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
38-
let p = Box::into_raw(Box::new(p));
42+
pub unsafe fn new(
43+
stack: usize,
44+
name: Option<&str>,
45+
f: Box<dyn FnOnce()>,
46+
) -> io::Result<Thread> {
47+
let data = Box::into_raw(Box::new(ThreadData { name: name.map(Box::from), f }));
3948
let mut native: libc::pthread_t = mem::zeroed();
4049
let mut attr: mem::MaybeUninit<libc::pthread_attr_t> = mem::MaybeUninit::uninit();
4150
assert_eq!(libc::pthread_attr_init(attr.as_mut_ptr()), 0);
@@ -73,7 +82,7 @@ impl Thread {
7382
};
7483
}
7584

76-
let ret = libc::pthread_create(&mut native, attr.as_ptr(), thread_start, p as *mut _);
85+
let ret = libc::pthread_create(&mut native, attr.as_ptr(), thread_start, data as *mut _);
7786
// Note: if the thread creation fails and this assert fails, then p will
7887
// be leaked. However, an alternative design could cause double-free
7988
// which is clearly worse.
@@ -82,19 +91,20 @@ impl Thread {
8291
return if ret != 0 {
8392
// The thread failed to start and as a result p was not consumed. Therefore, it is
8493
// safe to reconstruct the box so that it gets deallocated.
85-
drop(Box::from_raw(p));
94+
drop(Box::from_raw(data));
8695
Err(io::Error::from_raw_os_error(ret))
8796
} else {
8897
Ok(Thread { id: native })
8998
};
9099

91-
extern "C" fn thread_start(main: *mut libc::c_void) -> *mut libc::c_void {
100+
extern "C" fn thread_start(data: *mut libc::c_void) -> *mut libc::c_void {
92101
unsafe {
102+
let data = Box::from_raw(data as *mut ThreadData);
93103
// Next, set up our stack overflow handler which may get triggered if we run
94104
// out of stack.
95-
let _handler = stack_overflow::Handler::new();
105+
let _handler = stack_overflow::Handler::new(data.name);
96106
// Finally, let's run some code.
97-
Box::from_raw(main as *mut Box<dyn FnOnce()>)();
107+
(data.f)();
98108
}
99109
ptr::null_mut()
100110
}

library/std/src/sys/pal/unsupported/thread.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 64 * 1024;
1010

1111
impl Thread {
1212
// unsafe: see thread::Builder::spawn_unchecked for safety requirements
13-
pub unsafe fn new(_stack: usize, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
13+
pub unsafe fn new(
14+
_stack: usize,
15+
_name: Option<&str>,
16+
_p: Box<dyn FnOnce()>,
17+
) -> io::Result<Thread> {
1418
unsupported()
1519
}
1620

library/std/src/sys/pal/wasi/thread.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl Thread {
7373
// unsafe: see thread::Builder::spawn_unchecked for safety requirements
7474
cfg_if::cfg_if! {
7575
if #[cfg(target_feature = "atomics")] {
76-
pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
76+
pub unsafe fn new(stack: usize, _name: Option<&str>, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
7777
let p = Box::into_raw(Box::new(p));
7878
let mut native: libc::pthread_t = unsafe { mem::zeroed() };
7979
let mut attr: libc::pthread_attr_t = unsafe { mem::zeroed() };
@@ -120,7 +120,7 @@ impl Thread {
120120
}
121121
}
122122
} else {
123-
pub unsafe fn new(_stack: usize, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
123+
pub unsafe fn new(_stack: usize, _name: Option<&str>, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
124124
crate::sys::unsupported()
125125
}
126126
}

library/std/src/sys/pal/wasm/atomics/thread.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 1024 * 1024;
1010

1111
impl Thread {
1212
// unsafe: see thread::Builder::spawn_unchecked for safety requirements
13-
pub unsafe fn new(_stack: usize, _p: Box<dyn FnOnce()>) -> io::Result<Thread> {
13+
pub unsafe fn new(
14+
_stack: usize,
15+
_name: Option<&str>,
16+
_p: Box<dyn FnOnce()>,
17+
) -> io::Result<Thread> {
1418
unsupported()
1519
}
1620

0 commit comments

Comments
 (0)