From 6f6bb16718ffd4b143762be576b237efbedd38b4 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 16 Sep 2021 14:28:21 +0200 Subject: [PATCH 1/8] Merge sys_common::rt into rt --- library/std/src/lib.rs | 10 ++-- library/std/src/process.rs | 2 +- library/std/src/rt.rs | 86 +++++++++++++++++++++++++++++-- library/std/src/sys_common/mod.rs | 2 - library/std/src/sys_common/rt.rs | 81 ----------------------------- 5 files changed, 89 insertions(+), 92 deletions(-) delete mode 100644 library/std/src/sys_common/rt.rs diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 83c6ba0e6ea45..6380631cf0d8b 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -518,8 +518,12 @@ pub mod task { pub use alloc::task::*; } -// Platform-abstraction modules +// The runtime entry point and a few unstable public functions used by the +// compiler #[macro_use] +pub mod rt; + +// Platform-abstraction modules mod sys_common; mod sys; @@ -528,10 +532,6 @@ pub mod alloc; // Private support modules mod panicking; -// The runtime entry point and a few unstable public functions used by the -// compiler -pub mod rt; - #[path = "../../backtrace/src/lib.rs"] #[allow(dead_code, unused_attributes)] mod backtrace_rs; diff --git a/library/std/src/process.rs b/library/std/src/process.rs index c9b21fcf9c6d2..5c68400114d49 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -1907,7 +1907,7 @@ impl Child { /// [platform-specific behavior]: #platform-specific-behavior #[stable(feature = "rust1", since = "1.0.0")] pub fn exit(code: i32) -> ! { - crate::sys_common::rt::cleanup(); + crate::rt::cleanup(); crate::sys::os::exit(code) } diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index 72e6c23ee4990..8e38f98105b1f 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -13,10 +13,90 @@ issue = "none" )] #![doc(hidden)] +#![deny(unsafe_op_in_unsafe_fn)] +#![allow(unused_macros)] // Re-export some of our utilities which are expected by other crates. pub use crate::panicking::{begin_panic, begin_panic_fmt, panic_count}; +use crate::sync::Once; +use crate::sys; +use crate::sys_common::thread_info; +use crate::thread::Thread; + +// One-time runtime initialization. +// Runs before `main`. +// SAFETY: must be called only once during runtime initialization. +// NOTE: this is not guaranteed to run, for example when Rust code is called externally. +#[cfg_attr(test, allow(dead_code))] +unsafe fn init(argc: isize, argv: *const *const u8) { + unsafe { + sys::init(argc, argv); + + 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. + let thread = Thread::new(Some("main".to_owned())); + thread_info::set(main_guard, thread); + } +} + +// One-time runtime cleanup. +// Runs after `main` or at program exit. +// NOTE: this is not guaranteed to run, for example when the program aborts. +pub(crate) fn cleanup() { + static CLEANUP: Once = Once::new(); + CLEANUP.call_once(|| unsafe { + // Flush stdout and disable buffering. + crate::io::cleanup(); + // SAFETY: Only called once during runtime cleanup. + sys::cleanup(); + }); +} + +// Prints to the "panic output", depending on the platform this may be: +// - the standard error output +// - some dedicated platform specific output +// - nothing (so this macro is a no-op) +macro_rules! rtprintpanic { + ($($t:tt)*) => { + if let Some(mut out) = crate::sys::stdio::panic_output() { + let _ = crate::io::Write::write_fmt(&mut out, format_args!($($t)*)); + } + } +} + +macro_rules! rtabort { + ($($t:tt)*) => { + { + rtprintpanic!("fatal runtime error: {}\n", format_args!($($t)*)); + crate::sys::abort_internal(); + } + } +} + +macro_rules! rtassert { + ($e:expr) => { + if !$e { + rtabort!(concat!("assertion failed: ", stringify!($e))); + } + }; +} + +macro_rules! rtunwrap { + ($ok:ident, $e:expr) => { + match $e { + $ok(v) => v, + ref err => { + let err = err.as_ref().map(drop); // map Ok/Some which might not be Debug + rtabort!(concat!("unwrap failed: ", stringify!($e), " = {:?}"), err) + } + } + }; +} + // To reduce the generated code of the new `lang_start`, this function is doing // the real work. #[cfg(not(test))] @@ -25,7 +105,7 @@ fn lang_start_internal( argc: isize, argv: *const *const u8, ) -> Result { - use crate::{mem, panic, sys, sys_common}; + use crate::{mem, panic}; let rt_abort = move |e| { mem::forget(e); rtabort!("initialization or cleanup bug"); @@ -41,14 +121,14 @@ fn lang_start_internal( // prevent libstd from accidentally introducing a panic to these functions. Another is from // user code from `main` or, more nefariously, as described in e.g. issue #86030. // SAFETY: Only called once during runtime initialization. - panic::catch_unwind(move || unsafe { sys_common::rt::init(argc, argv) }).map_err(rt_abort)?; + panic::catch_unwind(move || unsafe { init(argc, argv) }).map_err(rt_abort)?; let ret_code = panic::catch_unwind(move || panic::catch_unwind(main).unwrap_or(101) as isize) .map_err(move |e| { mem::forget(e); rtprintpanic!("drop of the panic payload panicked"); sys::abort_internal() }); - panic::catch_unwind(sys_common::rt::cleanup).map_err(rt_abort)?; + panic::catch_unwind(cleanup).map_err(rt_abort)?; ret_code } diff --git a/library/std/src/sys_common/mod.rs b/library/std/src/sys_common/mod.rs index 894440564b738..5a5913ebd79a3 100644 --- a/library/std/src/sys_common/mod.rs +++ b/library/std/src/sys_common/mod.rs @@ -28,8 +28,6 @@ pub mod memchr; pub mod mutex; pub mod process; pub mod remutex; -#[macro_use] -pub mod rt; pub mod rwlock; pub mod thread; pub mod thread_info; diff --git a/library/std/src/sys_common/rt.rs b/library/std/src/sys_common/rt.rs deleted file mode 100644 index 02013ecc4ced6..0000000000000 --- a/library/std/src/sys_common/rt.rs +++ /dev/null @@ -1,81 +0,0 @@ -#![deny(unsafe_op_in_unsafe_fn)] -#![allow(unused_macros)] - -use crate::sync::Once; -use crate::sys; -use crate::sys_common::thread_info; -use crate::thread::Thread; - -// One-time runtime initialization. -// Runs before `main`. -// SAFETY: must be called only once during runtime initialization. -// NOTE: this is not guaranteed to run, for example when Rust code is called externally. -#[cfg_attr(test, allow(dead_code))] -pub unsafe fn init(argc: isize, argv: *const *const u8) { - unsafe { - sys::init(argc, argv); - - 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. - let thread = Thread::new(Some("main".to_owned())); - thread_info::set(main_guard, thread); - } -} - -// One-time runtime cleanup. -// Runs after `main` or at program exit. -// NOTE: this is not guaranteed to run, for example when the program aborts. -#[cfg_attr(test, allow(dead_code))] -pub fn cleanup() { - static CLEANUP: Once = Once::new(); - CLEANUP.call_once(|| unsafe { - // Flush stdout and disable buffering. - crate::io::cleanup(); - // SAFETY: Only called once during runtime cleanup. - sys::cleanup(); - }); -} - -// Prints to the "panic output", depending on the platform this may be: -// - the standard error output -// - some dedicated platform specific output -// - nothing (so this macro is a no-op) -macro_rules! rtprintpanic { - ($($t:tt)*) => { - if let Some(mut out) = crate::sys::stdio::panic_output() { - let _ = crate::io::Write::write_fmt(&mut out, format_args!($($t)*)); - } - } -} - -macro_rules! rtabort { - ($($t:tt)*) => { - { - rtprintpanic!("fatal runtime error: {}\n", format_args!($($t)*)); - crate::sys::abort_internal(); - } - } -} - -macro_rules! rtassert { - ($e:expr) => { - if !$e { - rtabort!(concat!("assertion failed: ", stringify!($e))); - } - }; -} - -macro_rules! rtunwrap { - ($ok:ident, $e:expr) => { - match $e { - $ok(v) => v, - ref err => { - let err = err.as_ref().map(drop); // map Ok/Some which might not be Debug - rtabort!(concat!("unwrap failed: ", stringify!($e), " = {:?}"), err) - } - } - }; -} From af7eededaa2a239c1e23c40024cf557a4b83fffa Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 16 Sep 2021 14:41:09 +0200 Subject: [PATCH 2/8] Remove an allocation from rt::init Previously the thread name would first be heap allocated and then re-allocated to add a nul terminator. Now it will be heap allocated only once with nul terminator added form the start. --- library/std/src/rt.rs | 4 +++- library/std/src/thread/mod.rs | 12 +++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index 8e38f98105b1f..8465dfc0ab2fb 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -16,6 +16,8 @@ #![deny(unsafe_op_in_unsafe_fn)] #![allow(unused_macros)] +use crate::ffi::CString; + // Re-export some of our utilities which are expected by other crates. pub use crate::panicking::{begin_panic, begin_panic_fmt, panic_count}; @@ -38,7 +40,7 @@ unsafe fn init(argc: isize, argv: *const *const u8) { // 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. - let thread = Thread::new(Some("main".to_owned())); + let thread = Thread::new(Some(CString::new("main").unwrap())); thread_info::set(main_guard, thread); } } diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index f44df845bf4dd..9d659102b0320 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -457,7 +457,9 @@ impl Builder { let stack_size = stack_size.unwrap_or_else(thread::min_stack); - let my_thread = Thread::new(name); + let my_thread = Thread::new(name.map(|name| { + CString::new(name).expect("thread name may not contain interior null bytes") + })); let their_thread = my_thread.clone(); let my_packet: Arc>>> = Arc::new(UnsafeCell::new(None)); @@ -1073,12 +1075,8 @@ pub struct Thread { impl Thread { // Used only internally to construct a thread object without spawning // Panics if the name contains nuls. - pub(crate) fn new(name: Option) -> Thread { - let cname = - name.map(|n| CString::new(n).expect("thread name may not contain interior null bytes")); - Thread { - inner: Arc::new(Inner { name: cname, id: ThreadId::new(), parker: Parker::new() }), - } + pub(crate) fn new(name: Option) -> Thread { + Thread { inner: Arc::new(Inner { name, id: ThreadId::new(), parker: Parker::new() }) } } /// Atomically makes the handle's token available if it is not already. From f78cd44602fd8360657b9a205061ca7bf0592140 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 16 Sep 2021 14:48:33 +0200 Subject: [PATCH 3/8] Optimize ThreadInfo::with The RefCell is now borrowed exactly once. In addition a code sequence that contains an unwrap that is guaranteed to never panic at runtime is replaced with get_or_insert_with, which makes the intended behavior clearer and will not emit code to panic even without optimizations. --- library/std/src/sys_common/thread_info.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/library/std/src/sys_common/thread_info.rs b/library/std/src/sys_common/thread_info.rs index f09d16c33e6d6..a0dbdcd979add 100644 --- a/library/std/src/sys_common/thread_info.rs +++ b/library/std/src/sys_common/thread_info.rs @@ -17,12 +17,13 @@ impl ThreadInfo { F: FnOnce(&mut ThreadInfo) -> R, { THREAD_INFO - .try_with(move |c| { - if c.borrow().is_none() { - *c.borrow_mut() = - Some(ThreadInfo { stack_guard: None, thread: Thread::new(None) }) - } - f(c.borrow_mut().as_mut().unwrap()) + .try_with(move |thread_info| { + let mut thread_info = thread_info.borrow_mut(); + let thread_info = thread_info.get_or_insert_with(|| ThreadInfo { + stack_guard: None, + thread: Thread::new(None), + }); + f(thread_info) }) .ok() } From a8bb3bcd38502e63c3b729ca215fdb18a0c70e77 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 16 Sep 2021 14:55:15 +0200 Subject: [PATCH 4/8] Use const {} for the THREAD_INFO thread local This makes accesses to it cheaper --- library/std/src/sys_common/thread_info.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys_common/thread_info.rs b/library/std/src/sys_common/thread_info.rs index a0dbdcd979add..1d2e2dd4361df 100644 --- a/library/std/src/sys_common/thread_info.rs +++ b/library/std/src/sys_common/thread_info.rs @@ -1,4 +1,5 @@ #![allow(dead_code)] // stack_guard isn't used right now on all platforms +#![allow(unused_unsafe)] // thread_local with `const {}` triggers this liny use crate::cell::RefCell; use crate::sys::thread::guard::Guard; @@ -9,7 +10,7 @@ struct ThreadInfo { thread: Thread, } -thread_local! { static THREAD_INFO: RefCell> = RefCell::new(None) } +thread_local! { static THREAD_INFO: RefCell> = const { RefCell::new(None) } } impl ThreadInfo { fn with(f: F) -> Option From 1ad44b23d1e6d0d5f52d3ed689f35abbe797ef11 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 16 Sep 2021 14:58:36 +0200 Subject: [PATCH 5/8] Remove unused function --- library/std/src/sys_common/thread_info.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/library/std/src/sys_common/thread_info.rs b/library/std/src/sys_common/thread_info.rs index 1d2e2dd4361df..f18d83b53b45c 100644 --- a/library/std/src/sys_common/thread_info.rs +++ b/library/std/src/sys_common/thread_info.rs @@ -42,7 +42,3 @@ pub fn set(stack_guard: Option, thread: Thread) { THREAD_INFO.with(|c| assert!(c.borrow().is_none())); THREAD_INFO.with(move |c| *c.borrow_mut() = Some(ThreadInfo { stack_guard, thread })); } - -pub fn reset_guard(stack_guard: Option) { - THREAD_INFO.with(move |c| c.borrow_mut().as_mut().unwrap().stack_guard = stack_guard); -} From cb14269145abef952861b4f73beb78a7ca79e8f6 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 16 Sep 2021 15:20:44 +0200 Subject: [PATCH 6/8] Replace a couple of asserts with rtassert! in rt code This replaces a couple of panic locations with hard aborts. The panics can't be catched by the user anyway in these locations. --- library/std/src/rt.rs | 64 +++++++++++------------ library/std/src/sys/unix/mod.rs | 2 +- library/std/src/sys_common/thread_info.rs | 2 +- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index 8465dfc0ab2fb..9adf9a20747d5 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -26,38 +26,6 @@ use crate::sys; use crate::sys_common::thread_info; use crate::thread::Thread; -// One-time runtime initialization. -// Runs before `main`. -// SAFETY: must be called only once during runtime initialization. -// NOTE: this is not guaranteed to run, for example when Rust code is called externally. -#[cfg_attr(test, allow(dead_code))] -unsafe fn init(argc: isize, argv: *const *const u8) { - unsafe { - sys::init(argc, argv); - - 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. - let thread = Thread::new(Some(CString::new("main").unwrap())); - thread_info::set(main_guard, thread); - } -} - -// One-time runtime cleanup. -// Runs after `main` or at program exit. -// NOTE: this is not guaranteed to run, for example when the program aborts. -pub(crate) fn cleanup() { - static CLEANUP: Once = Once::new(); - CLEANUP.call_once(|| unsafe { - // Flush stdout and disable buffering. - crate::io::cleanup(); - // SAFETY: Only called once during runtime cleanup. - sys::cleanup(); - }); -} - // Prints to the "panic output", depending on the platform this may be: // - the standard error output // - some dedicated platform specific output @@ -99,6 +67,38 @@ macro_rules! rtunwrap { }; } +// One-time runtime initialization. +// Runs before `main`. +// SAFETY: must be called only once during runtime initialization. +// NOTE: this is not guaranteed to run, for example when Rust code is called externally. +#[cfg_attr(test, allow(dead_code))] +unsafe fn init(argc: isize, argv: *const *const u8) { + unsafe { + sys::init(argc, argv); + + 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. + let thread = Thread::new(Some(rtunwrap!(Ok, CString::new("main")))); + thread_info::set(main_guard, thread); + } +} + +// One-time runtime cleanup. +// Runs after `main` or at program exit. +// NOTE: this is not guaranteed to run, for example when the program aborts. +pub(crate) fn cleanup() { + static CLEANUP: Once = Once::new(); + CLEANUP.call_once(|| unsafe { + // Flush stdout and disable buffering. + crate::io::cleanup(); + // SAFETY: Only called once during runtime cleanup. + sys::cleanup(); + }); +} + // To reduce the generated code of the new `lang_start`, this function is doing // the real work. #[cfg(not(test))] diff --git a/library/std/src/sys/unix/mod.rs b/library/std/src/sys/unix/mod.rs index f5424e3d28214..1c37f4ee4981e 100644 --- a/library/std/src/sys/unix/mod.rs +++ b/library/std/src/sys/unix/mod.rs @@ -120,7 +120,7 @@ pub unsafe fn init(argc: isize, argv: *const *const u8) { unsafe fn reset_sigpipe() { #[cfg(not(any(target_os = "emscripten", target_os = "fuchsia")))] - assert!(signal(libc::SIGPIPE, libc::SIG_IGN) != libc::SIG_ERR); + rtassert!(signal(libc::SIGPIPE, libc::SIG_IGN) != libc::SIG_ERR); } } diff --git a/library/std/src/sys_common/thread_info.rs b/library/std/src/sys_common/thread_info.rs index f18d83b53b45c..c8c0ecceb48cb 100644 --- a/library/std/src/sys_common/thread_info.rs +++ b/library/std/src/sys_common/thread_info.rs @@ -39,6 +39,6 @@ pub fn stack_guard() -> Option { } pub fn set(stack_guard: Option, thread: Thread) { - THREAD_INFO.with(|c| assert!(c.borrow().is_none())); + THREAD_INFO.with(|c| rtassert!(c.borrow().is_none())); THREAD_INFO.with(move |c| *c.borrow_mut() = Some(ThreadInfo { stack_guard, thread })); } From 5e7f641a32a01c1cf2b7544020331c1a010e25e0 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 16 Sep 2021 15:24:53 +0200 Subject: [PATCH 7/8] Merge two THREAD_INFO.with and following RefCell borrow This is a bit faster --- library/std/src/sys_common/thread_info.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys_common/thread_info.rs b/library/std/src/sys_common/thread_info.rs index c8c0ecceb48cb..38c9e50009af5 100644 --- a/library/std/src/sys_common/thread_info.rs +++ b/library/std/src/sys_common/thread_info.rs @@ -39,6 +39,9 @@ pub fn stack_guard() -> Option { } pub fn set(stack_guard: Option, thread: Thread) { - THREAD_INFO.with(|c| rtassert!(c.borrow().is_none())); - THREAD_INFO.with(move |c| *c.borrow_mut() = Some(ThreadInfo { stack_guard, thread })); + THREAD_INFO.with(move |thread_info| { + let mut thread_info = thread_info.borrow_mut(); + rtassert!(thread_info.is_none()); + *thread_info = Some(ThreadInfo { stack_guard, thread }); + }); } From 37608c7c501b12b634834363827e314f6750946f Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 16 Sep 2021 15:54:12 +0200 Subject: [PATCH 8/8] Rustfmt --- library/std/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 6380631cf0d8b..1f089bbc0e62e 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -524,8 +524,8 @@ pub mod task { pub mod rt; // Platform-abstraction modules -mod sys_common; mod sys; +mod sys_common; pub mod alloc;