Skip to content

Commit

Permalink
Auto merge of #123265 - joboet:guardians_of_the_unix, r=ChrisDenton
Browse files Browse the repository at this point in the history
Refactor stack overflow handling

Currently, every platform must implement a `Guard` that protects a thread from stack overflow. However, UNIX is the only platform that actually does so. Windows has a different mechanism for detecting stack overflow, while the other platforms don't detect it at all. Also, the UNIX stack overflow handling is split between `sys::pal::unix::stack_overflow`, which implements the signal handler, and `sys::pal::unix::thread`, which detects/installs guard pages.

This PR cleans this by getting rid of `Guard` and unifying UNIX stack overflow handling inside `stack_overflow` (commit 1). Therefore we can get rid of `sys_common::thread_info`, which stores `Guard` and the current `Thread` handle and move the `thread::current` TLS variable into `thread` (commit 2).

The second commit is not strictly speaking necessary. To keep the implementation clean, I've included it here, but if it causes too much noise, I can split it out without any trouble.
  • Loading branch information
bors committed Apr 1, 2024
2 parents 6bb6b81 + d7b55e4 commit c518e5a
Show file tree
Hide file tree
Showing 19 changed files with 327 additions and 500 deletions.
3 changes: 1 addition & 2 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use crate::sync::atomic::{AtomicBool, Ordering};
use crate::sync::{PoisonError, RwLock};
use crate::sys::stdio::panic_output;
use crate::sys_common::backtrace;
use crate::sys_common::thread_info;
use crate::thread;

#[cfg(not(test))]
Expand Down Expand Up @@ -256,7 +255,7 @@ fn default_hook(info: &PanicInfo<'_>) {
None => "Box<dyn Any>",
},
};
let thread = thread_info::current_thread();
let thread = thread::try_current();
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");

let write = |err: &mut dyn crate::io::Write| {
Expand Down
11 changes: 3 additions & 8 deletions library/std/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ pub use core::panicking::{panic_display, panic_fmt};

use crate::sync::Once;
use crate::sys;
use crate::sys_common::thread_info;
use crate::thread::Thread;
use crate::thread::{self, Thread};

// Prints to the "panic output", depending on the platform this may be:
// - the standard error output
Expand Down Expand Up @@ -96,13 +95,9 @@ unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
unsafe {
sys::init(argc, argv, sigpipe);

let main_guard = sys::thread::guard::init();
// Next, set up the current Thread with the guard information we just
// created. Note that this isn't necessary in general for new threads,
// but we just do this to name the main thread and to give it correct
// info about the stack bounds.
// Set up the current thread to give it the right name.
let thread = Thread::new(Some(rtunwrap!(Ok, CString::new("main"))));
thread_info::set(main_guard, thread);
thread::set_current(thread);
}
}

Expand Down
10 changes: 0 additions & 10 deletions library/std/src/sys/pal/hermit/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,3 @@ impl Thread {
pub fn available_parallelism() -> io::Result<NonZero<usize>> {
unsafe { Ok(NonZero::new_unchecked(abi::get_processor_count())) }
}

pub mod guard {
pub type Guard = !;
pub unsafe fn current() -> Option<Guard> {
None
}
pub unsafe fn init() -> Option<Guard> {
None
}
}
10 changes: 0 additions & 10 deletions library/std/src/sys/pal/itron/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,16 +312,6 @@ impl Drop for Thread {
}
}

pub mod guard {
pub type Guard = !;
pub unsafe fn current() -> Option<Guard> {
None
}
pub unsafe fn init() -> Option<Guard> {
None
}
}

/// Terminate and delete the specified task.
///
/// This function will abort if `deleted_task` refers to the calling task.
Expand Down
10 changes: 0 additions & 10 deletions library/std/src/sys/pal/sgx/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,3 @@ impl Thread {
pub fn available_parallelism() -> io::Result<NonZero<usize>> {
unsupported()
}

pub mod guard {
pub type Guard = !;
pub unsafe fn current() -> Option<Guard> {
None
}
pub unsafe fn init() -> Option<Guard> {
None
}
}
12 changes: 0 additions & 12 deletions library/std/src/sys/pal/teeos/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,18 +151,6 @@ pub fn available_parallelism() -> io::Result<NonZero<usize>> {
))
}

// stub
pub mod guard {
use crate::ops::Range;
pub type Guard = Range<usize>;
pub unsafe fn current() -> Option<Guard> {
None
}
pub unsafe fn init() -> Option<Guard> {
None
}
}

fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
libc::PTHREAD_STACK_MIN.try_into().expect("Infallible")
}
10 changes: 0 additions & 10 deletions library/std/src/sys/pal/uefi/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,3 @@ pub fn available_parallelism() -> io::Result<NonZero<usize>> {
// UEFI is single threaded
Ok(NonZero::new(1).unwrap())
}

pub mod guard {
pub type Guard = !;
pub unsafe fn current() -> Option<Guard> {
None
}
pub unsafe fn init() -> Option<Guard> {
None
}
}

0 comments on commit c518e5a

Please sign in to comment.