Skip to content

Commit

Permalink
Forbid inlining thread_local!'s __getit function on Windows
Browse files Browse the repository at this point in the history
  • Loading branch information
thomcc committed Nov 22, 2022
1 parent 911cbf8 commit 3099dfd
Showing 1 changed file with 16 additions and 9 deletions.
25 changes: 16 additions & 9 deletions library/std/src/thread/local.rs
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

0 comments on commit 3099dfd

Please sign in to comment.