Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Forbid inlining thread_local!'s __getit function on Windows #101368

Merged
merged 1 commit into from
Nov 23, 2022
Merged

Forbid inlining thread_local!'s __getit function on Windows #101368

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions library/std/src/thread/local.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ macro_rules! thread_local {
macro_rules! __thread_local_inner {
// used to generate the `LocalKey` value for const-initialized thread locals
(@key $t:ty, const $init:expr) => {{
#[cfg_attr(not(windows), inline)] // see comments below
#[cfg_attr(not(all(windows, target_thread_local)), inline)] // see comments below
#[cfg_attr(all(windows, target_thread_local), inline(never))]
#[deny(unsafe_op_in_unsafe_fn)]
unsafe fn __getit(
_init: $crate::option::Option<&mut $crate::option::Option<$t>>,
Expand Down Expand Up @@ -294,12 +295,17 @@ macro_rules! __thread_local_inner {
fn __init() -> $t { $init }

// When reading this function you might ask "why is this inlined
// everywhere other than Windows?", and that's a very reasonable
// question to ask. The short story is that it segfaults rustc if
// this function is inlined. The longer story is that Windows looks
// to not support `extern` references to thread locals across DLL
// boundaries. This appears to at least not be supported in the ABI
// that LLVM implements.
// everywhere other than Windows?", and "why must it not be inlined
// on Windows?" and these are very reasonable questions to ask.
//
// The short story is that Windows doesn't support referencing
// `#[thread_local]` across DLL boundaries. The slightly longer
// story is that each module (dll or exe) has its own separate set
// of static thread locals, so if you try and reference a
// `#[thread_local]` that comes from `crate1.dll` from within one of
// `crate2.dll`'s functions, then it might give you a completely
// different thread local than what you asked for (or it might just
// crash).
//
// Because of this we never inline on Windows, but we do inline on
// other platforms (where external references to thread locals
Expand All @@ -314,8 +320,9 @@ macro_rules! __thread_local_inner {
// Cargo question kinda). This means that, unfortunately, Windows
// gets the pessimistic path for now where it's never inlined.
//
// The issue of "should enable on Windows sometimes" is #84933
#[cfg_attr(not(windows), inline)]
// The issue of "should improve things on Windows" is #84933
#[cfg_attr(not(all(windows, target_thread_local)), inline)]
#[cfg_attr(all(windows, target_thread_local), inline(never))]
unsafe fn __getit(
init: $crate::option::Option<&mut $crate::option::Option<$t>>,
) -> $crate::option::Option<&'static $t> {
Expand Down