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.

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

#[thread_local] from windows DLLs gets duplicated by inlining. #44391

Open
eddyb opened this issue Sep 7, 2017 · 7 comments
Open

#[thread_local] from windows DLLs gets duplicated by inlining. #44391

eddyb opened this issue Sep 7, 2017 · 7 comments
Labels
A-thread-locals Area: Thread local storage (TLS) C-bug Category: This is a bug. O-windows-msvc Toolchain: MSVC, Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Sep 7, 2017

This is most likely responsible for the windows failures from #43931 when the __getit function, which may return a reference to a #[thread_local] static, was #[inline] (cc @alexcrichton):

// a.rs
#![crate_type = "dylib"]
#![feature(thread_local)]

#[thread_local]
static FOO: u32 = 25;

pub fn addr() -> *const u32 {
    &FOO
}

#[inline]
pub fn addr_inline() -> *const u32 {
    &FOO
}
// b.rs
extern crate a;

fn main() {
    assert_eq!(a::addr(), a::addr_inline());
}

Running b, on x64 MSVC (thanks, @bcata6!) results in:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `0x134cb61ace4`,
 right: `0x134cb6139c0`', b.rs:4:4
@alexcrichton
Copy link
Member

I think this may be a dllexport/import problem, looks like b.rs is correctly annotated with dllimport but a.rs isn't annotated with dllexport!

@alexcrichton
Copy link
Member

Hm no I take that back, I forgot that we don't use dllexport but rather we use link.exe's /DEF argument. I've confirmed that the thread local symbol is indeed listed in that file, and I've also confirmed that it's exported from the DLL. Furthermore I've confirmed again dllimport is applied on the import.

I wonder if this is just not supposed to work? Or maybe this is how TLS works on Windows?

@TimNN TimNN added A-thread-locals Area: Thread local storage (TLS) C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 17, 2017
@Zoxc
Copy link
Contributor

Zoxc commented Dec 2, 2018

MSVC doesn't support this. Tried this on compiler explorer:

#include <stdint.h>

_declspec( dllimport ) thread_local size_t a;

size_t test() {
    return a;
}

I don't know if it's possible or not to do using the DLL loader, but LLVM probably doesn't support it even if it would work.

cc @retep998

@Zoxc
Copy link
Contributor

Zoxc commented Dec 2, 2018

This also means that the bug applies to thread_local! as there is nothing stopping it from being inlined into other crates.

@mati865
Copy link
Contributor

mati865 commented Dec 6, 2019

#44391 (comment) works fine on windows-gnu.

@rustbot modify labels: -O-windows +O-windows-msvc

@rustbot rustbot added O-windows-msvc Toolchain: MSVC, Operating system: Windows and removed O-windows Operating system: Windows labels Dec 6, 2019
@ChrisDenton
Copy link
Member

To be clear the TLS isn't duplicated, it simply doesn't exist in b.exe.

On Windows, native TLS is scoped to the module (where "module" means either a dll or exe file). So when the dylib is built, the TLS value is in that scope. This would be fine in a normal dll file but Rust's dylibs are special in that they're kind of a weird cross between an actual dll and a static lib. So when compiling an exe with the dylib, the value itself remains declared in the dylib but the (inlined) code for accessing it is moved to the exe, where it ends up referencing some arbitrary memory.

At best it's a STATUS_ACCESS_VIOLATION at worst it accesses something that looks legit but isn't.

@ChrisDenton
Copy link
Member

On further investigation it looks like the exe does get an entry embedded in the binary's TLS data array. But it's only one byte no matter the size of the value. And it is always zero initialized. So I guess it is legal to access the first byte of the TLS value. The dylib has the full value in its TLS data array, properly initialized.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Aug 11, 2022
…ichaelwoerister

Never inline Windows dtor access

Inlining can cause problem If used in a Rust dylib. See rust-lang#44391.

r? `@Mark-Simulacrum`
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 15, 2022
…haelwoerister

Never inline Windows dtor access

Inlining can cause problem If used in a Rust dylib. See rust-lang#44391.

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-thread-locals Area: Thread local storage (TLS) C-bug Category: This is a bug. O-windows-msvc Toolchain: MSVC, Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants