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] alignment broken on Windows, GNU ABI #135719

Open
purplesyringa opened this issue Jan 19, 2025 · 16 comments
Open

#[thread_local] alignment broken on Windows, GNU ABI #135719

purplesyringa opened this issue Jan 19, 2025 · 16 comments
Labels
A-thread-locals Area: Thread local storage (TLS) C-bug Category: This is a bug. F-thread_local `#![feature(thread_local)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-windows-gnu Toolchain: GNU, Operating system: Windows requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@purplesyringa
Copy link
Contributor

purplesyringa commented Jan 19, 2025

I tried this code:

#![feature(thread_local)]

#[repr(align(256))]
struct HighAlignment(u8);

#[thread_local]
static EXAMPLE1: HighAlignment = const { HighAlignment(0) };
#[thread_local]
static EXAMPLE2: HighAlignment = const { HighAlignment(0) };
#[thread_local]
static EXAMPLE3: HighAlignment = const { HighAlignment(0) };

fn main() {
    println!(
        "{:?}, {:?}, {:?}",
        &EXAMPLE1 as *const HighAlignment,
        &EXAMPLE2 as *const HighAlignment,
        &EXAMPLE3 as *const HighAlignment
    );
}

I expected to see this happen: three 0x100-aligned pointers.

Instead, this happened: metal pipe falling

  • x86_64, GNU ABI, real Windows: 0x1f39e953850, 0x1f39e953950, 0x1f39e953a50 (unaligned)
  • x86_64, MSVC ABI, real Windows: 0x1c287893900, 0x1c287893a00, 0x1c287893b00 (aligned)
  • i686, GNU ABI, real Windows: 0xf9a5f8, 0xf9a6f8, 0xf9a7f8 (unaligned)
  • i686, MSVC ABI, real Windows: 0x6d2f00, 0x6d3000, 0x6d3100 (aligned)
  • x86_64, GNU ABI, Wine: 0x232c80, 0x232d80, 0x232e80 (unaligned)
  • x86_64, MSVC ABI, Wine: 0x1320e0, 0x1321e0, 0x1322e0 (unaligned)
  • i686, GNU ABI, Wine: 0x341f28, 0x342028, 0x342128 (unaligned)
  • i686, MSVC ABI, Wine: 0x34f010, 0x34f110, 0x34f210 (unaligned)

So either this affects all of Windows and I just got lucky twice, or this is two bugs in a trenchcoat, one affecting GNU ABI and another one in Wine. Not sure which.

Notably, checking mixed-alignment thread locals, like u8, u8, u128, u128, u128, shows that the variables are correctly aligned with respect to each other within TLS, but the TLS base itself seems to be unaligned.

Tentatively marking as I-unsound because of unaligned references bad, feel free to retag.

Meta

rustc --version --verbose:

rustc 1.86.0-nightly (419b3e2d3 2025-01-15)
binary: rustc
commit-hash: 419b3e2d3e350822550eee0e82eeded4d324d584
commit-date: 2025-01-15
host: x86_64-unknown-linux-gnu
release: 1.86.0-nightly
LLVM version: 19.1.6

wine --version: wine-9.22 (Staging)

Tracking issue for visibility: #29594

@purplesyringa purplesyringa added the C-bug Category: This is a bug. label Jan 19, 2025
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-thread-locals Area: Thread local storage (TLS) F-thread_local `#![feature(thread_local)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 19, 2025
@Noratrieb
Copy link
Member

Noratrieb commented Jan 19, 2025

Does this also reproduce with std::thread_local?

Also what exactly do you mean with Wine here? Building it normally on Windows and running it with Wine? Building it in Wine too?

@purplesyringa
Copy link
Contributor Author

Also what exactly do you mean with Wine here?

The real Windows vs Wine separation refers to the environment the code is run in.

I haven't considered building the code in different environments. All GNU targets were cross-compiled on Linux, all MSVC targets were compiled with cargo-xwin. I don't have access to a Windows machine right now, someone should probably try building directly on Windows, too, although I doubt there'd be a difference in the produced executables.

Does this also reproduce with std::thread_local?

Eugh, kind of.

#[repr(align(256))]
struct HighAlignment(u8);

thread_local! {
    static EXAMPLE1: HighAlignment = const { HighAlignment(0) };
}
thread_local! {
    static EXAMPLE2: HighAlignment = const { HighAlignment(0) };
}
thread_local! {
    static EXAMPLE3: HighAlignment = const { HighAlignment(0) };
}

fn main() {
    EXAMPLE1.with(|a| {
        EXAMPLE2.with(|b| {
            EXAMPLE3.with(|c| {
                println!(
                    "{:?}, {:?}, {:?}",
                    a as *const HighAlignment, b as *const HighAlignment, c as *const HighAlignment
                );
            })
        })
    });
}

Gives unaligned addresses on Wine on i686-pc-windows-msvc and only that target. I'm going offline now, but it's worth trying to run this on real Windows. If it turns out it applies there too, congratulations, we have a stable unsoundness.

@oli-obk oli-obk added the E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status label Jan 19, 2025
@Fulgen301
Copy link
Contributor

Running that sample on real Windows yields

0xdcdf00, 0xdce000, 0xdce100

, so it's fine.

@oli-obk oli-obk removed the E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status label Jan 19, 2025
@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 19, 2025
@purplesyringa
Copy link
Contributor Author

Turns out x86_64-pc-windows-msvc on Wine also gives unaligned access on the snippet above if you run the code in another thread (i.e.: wrap main in spawn+join). I haven't managed to reproduce this thread_local! bug on GNU ABI (still under Wine) and I don't see a clear reason why -- I hold that there's a possibility that aligned allocations are just a coincidence. I'll see if I can get my hands on Windows to play around with this more.

@purplesyringa
Copy link
Contributor Author

purplesyringa commented Jan 19, 2025

I haven't managed to reproduce the bug with std::thread_local! on real Windows. So we currently only have a bug on Wine without the feature and a bug on real Windows with the feature.

@ProgramCrafter
Copy link

@purplesyringa

I hold that there's a possibility that aligned allocations are just a coincidence.

Have you tried to put a smaller thread local between these over-aligned ones? like

thread_local! {
    static EXAMPLE1: HighAlignment = const { HighAlignment(0) };
    static BREAK_ALIGNMENT: u16 = 135;
    static EXAMPLE2: HighAlignment = const { HighAlignment(0) };
    static EXAMPLE3: HighAlignment = const { HighAlignment(0) };
}

@purplesyringa
Copy link
Contributor Author

@ProgramCrafter This is a bit more complicated. The thread locals themselves are aligned just fine within the global TLS storage. You can think of it as a typical Rust structure containing individual variables, and it's layouted correctly. However, the TLS storage itself is underaligned when allocated. So adding more fields won't help, as the goal is to analyze the allocation properties of the TLS storage itself.

@mati865
Copy link
Contributor

mati865 commented Jan 20, 2025

Also note windows-gnu targets do not enable has_thread_local in the spec, unlike windows-gnullvm and windows-msvc targets. It might also affect this issue.

@purplesyringa
Copy link
Contributor Author

So!

First things first: I forgot about the -gnullvm ABI, so I tested it too. Results are included below.

A bit of an introduction:

#[thread_local] variables are stored in so-called implicit module storage. A PE file contains a .tls section, which stores the initial contents of the thread-local storage. Upon loading an image, ntdll allocates a chunk of memory on the runtime heap, copies the .tls section into it, and then uses the pointer returned by the allocator as a TLS base.

As sections are purely descriptive and don't affect the behavior of the PE loader, the information about the .tls section is also stored in the so-called TLS directory, and the .tls section itself is ignored and may be removed altogether. Among basic information like the base address and the section on-disk and in-memory size, the TLS directory contains a field called Characteristics. Citing MSDN:

The four bits [23:20] describe alignment info. Possible values are those defined as IMAGE_SCN_ALIGN_*, which are also used to describe alignment of section in object files. The other 28 bits are reserved for future use.

objdump from binutils doesn't let us explore the TLS directory, but dumpbin from MS build tools does. Running dumpbin /tls on the code from the original post, when built for x86_64-pc-windows-msvc, shows:

File Type: EXECUTABLE IMAGE

  Section contains the following TLS directory:

    000000014001F300 Start of raw data
    000000014001F750 End of raw data
    0000000140023260 Address of index
    000000014001A338 Address of callbacks
                   0 Size of zero fill
            00900000 Characteristics
                       256 byte align

<some other irrelevant info>

"256 byte align" was parsed from the Characteristics field. Building for i686-pc-windows-msvc produces approximately the same result. This holds for both binaries produced on Windows and binaries cross-compiled with xwin. *-gnullvm targets also work fine.

Now, if you compile for the *-gnu targets, the result is different:

File Type: EXECUTABLE IMAGE

  Section contains the following TLS directory:

    00000001400DB000 Start of raw data
    00000001400DB400 End of raw data
    00000001400D720C Address of index
    00000001400DA038 Address of callbacks
                   0 Size of zero fill
            00000000 Characteristics
                       (no align specified)

<some other irrelevant info>

The Characteristics field is left as zero, leading ntdll to allocate the TLS at an unaligned address. For reasons unclear to me, the MinGW toolchain does not seem to populate it at all.

If we check the target specifications, we'll see that MSVC and GNU LLVM enable cfg(target_thread_local), but GNU doesn't, perhaps for this reason among others. However, this does not prevent #[thread_local] from compiling and then failing disgracefully in runtime like in this issue. We really need to turn this into a compile error until GCC is fixed.

The above explains the behavior of the first four bullet points from the original post. What about Wine? It turns out Wine allocates TLS on the heap without a particular alignment, ignoring the Characteristics field altogether:

https://github.com/wine-mirror/wine/blob/5b3306a0d0aff221195ecc29872f25ed082eedd0/dlls/ntdll/loader.c#L1351

As expected, Wine fails with GNU LLVM and MSVC binaries, because it never respects the alignment requirements.

That was all about #[thread_local]. For std::thread_local!, the combination of Wine and MSVC or GNU LLVM fails the test. It's interesting that *-gnu vs *-msvc/*-gnullvm and Windows vs Wine are flipped here compared to #[thread_local], but this can be easily explained. std::thread_local! uses #[thread_local] whenever cfg(target_thread_local) is set, and otherwise falls back to a slower, but more robust method. cfg(target_thread_local) is enabled but broken for precisely this combination.

I don't think we can work around this Wine bug. Fixing it on the Wine side sounds simpler and shouldn't be difficult. I'll see if I can do that within the next week.

@purplesyringa
Copy link
Contributor Author

@purplesyringa
Copy link
Contributor Author

To summarize, there are two bugs:

  1. #[thread_local] uses ELF-style TLS and thus produces unsound code on Windows, GNU ABI due to a MinGW bug, despite the target's has_thread_local being set to false. I think we should either abort compilation or use pthread-style TLS instead, if possible.
  2. Wine handles TLS alignment incorrectly when ELF-style TLS is used even if we sidestep MinGW.

Let us track the former bug here and leave the latter to Wine folks.

@rustbot label -O-windows +O-windows-gnu

@rustbot rustbot added O-windows-gnu Toolchain: GNU, Operating system: Windows and removed O-windows Operating system: Windows labels Jan 20, 2025
@purplesyringa purplesyringa changed the title #[thread_local] alignment broken on Windows #[thread_local] alignment broken on Windows, GNU ABI Jan 20, 2025
@rustbot rustbot added the O-windows Operating system: Windows label Jan 20, 2025
@purplesyringa

This comment has been minimized.

@jieyouxu

This comment has been minimized.

@jieyouxu jieyouxu removed the O-windows-gnu Toolchain: GNU, Operating system: Windows label Jan 20, 2025
@purplesyringa

This comment has been minimized.

@rustbot rustbot added O-windows-gnu Toolchain: GNU, Operating system: Windows and removed O-windows Operating system: Windows labels Jan 20, 2025
@mati865
Copy link
Contributor

mati865 commented Jan 20, 2025

If we check the target specifications, we'll see that MSVC and GNU LLVM enable cfg(target_thread_local), but GNU doesn't, perhaps for this reason among others.

I haven't dug deeply, but I think I've narrowed it down to another binutils bug. Once you enable it in the spec, you will get a few thousand crashes in the test suite. Enabling emulated TLS could possibly help (this is how GCC handles TLS for Windows), but it might need some fixes first.

@apiraino
Copy link
Contributor

apiraino commented Jan 21, 2025

WG-prioritization assigning priority (Zulip discussion).

Nightly only issue

@rustbot label -P-medium +requires-nightly

@rustbot rustbot added P-medium Medium priority requires-nightly This issue requires a nightly compiler in some way. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. P-medium Medium priority labels Jan 21, 2025
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. F-thread_local `#![feature(thread_local)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-windows-gnu Toolchain: GNU, Operating system: Windows requires-nightly This issue requires a nightly compiler in some way. 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

9 participants