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

Windows 7 i686 does not reliably align 16 byte aligned thread locals #138903

Open
drewkett opened this issue Mar 24, 2025 · 1 comment
Open

Windows 7 i686 does not reliably align 16 byte aligned thread locals #138903

drewkett opened this issue Mar 24, 2025 · 1 comment
Labels
A-align Area: alignment control (`repr(align(N))` and so on) A-thread-locals Area: Thread local storage (TLS) C-bug Category: This is a bug. O-windows-7 OS: Windows 7 or Windows Server 2008 R2 or etc. O-x86_32 Target: x86 processors, 32 bit (like i686-*) (IA-32) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@drewkett
Copy link

drewkett commented Mar 24, 2025

The following program will reliably crash using the i686-win7-windows-msvc target on a Windows 7 installation using latest nightly.

I tried this code:

#[repr(align(16))]
struct Align16(u8);

thread_local! {
    static ALIGN16: Align16 = Align16(0);
}

fn main() {
    let mut threads = Vec::new();
    for _ in 0..10 {
        threads.push(std::thread::spawn(|| {
            ALIGN16.with(|r| println!("ALIGN16 {:p}", r as *const _));
        }));
    }
    for thread in threads {
        thread.join().unwrap();
    }
}

This reliably produces the following panic.

unsafe precondition(s) violated: ptr::replace requires that the pointer argument is aligned and non-null

This indicates a bug in the program. This Undefined Behavior check is optional, and cannot be relied on for safety.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread caused a non-unwinding panic. aborting.

My experience with/understanding of thread locals is mostly limited to what i've researched due to this issue, but it does appear that an i686 targeted executable on Windows 7 does not reliably align thread locals when 16 byte alignment is requested. I didn't see issues with even 8 byte alignment. I was not able to reproduce this issue on Windows 10.

Given that i686 win7 is a likely pretty niche target, do you think it would be reasonable to partially undo #123257 by disabling has_thread_local for i686-win7-windows-msvc. It seems that i686 on Windows 7 can't be trusted for alignment of thread locals. I did test adjusting the i686-win7-windows-msvc target in a local build of the compiler and it does appear to work for the test program.

The 16 byte aligned thread local came from matrixmultiply which actually works around misalignment internally, but compiled this way I get the UB check panic anyway. I can certainly work around it by seeing if matrixmultiply can be patched, or just doing that locally. But it seems like this should be fixed in the compiler ideally.

@drewkett drewkett added the C-bug Category: This is a bug. label Mar 24, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 24, 2025
@moxian
Copy link
Contributor

moxian commented Mar 24, 2025

@rustbot label: +O-windows-7 +O-x86_32 +A-thread-locals +A-align -needs-triage +T-compiler
cc @roblabla @ChrisDenton

@rustbot rustbot added A-align Area: alignment control (`repr(align(N))` and so on) A-thread-locals Area: Thread local storage (TLS) O-windows-7 OS: Windows 7 or Windows Server 2008 R2 or etc. O-x86_32 Target: x86 processors, 32 bit (like i686-*) (IA-32) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 24, 2025
drewkett added a commit to drewkett/matrixmultiply that referenced this issue Mar 25, 2025
Windows 7 does not respect 16 byte alignment for thread locals on i686
builds. Rust 1.79 changed i686 windows builds to use native thread local
support. As a result, using `matrixmultiply` on i686 win7 builds leads
to a UB check panic nounwind when run on Windows 7. See
<rust-lang/rust#138903> for more info.

This change adds `i686-win7-windows-msvc` as an excluded target for the
alignment attribute on `MaskBuffer`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-align Area: alignment control (`repr(align(N))` and so on) A-thread-locals Area: Thread local storage (TLS) C-bug Category: This is a bug. O-windows-7 OS: Windows 7 or Windows Server 2008 R2 or etc. O-x86_32 Target: x86 processors, 32 bit (like i686-*) (IA-32) 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

3 participants