Description
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.