Skip to content

mitigate MSVC alignment issue on x86-32 #139261

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

Merged
merged 2 commits into from
Apr 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
fn load(&mut self, ty: &'ll Type, ptr: &'ll Value, align: Align) -> &'ll Value {
unsafe {
let load = llvm::LLVMBuildLoad2(self.llbuilder, ty, ptr, UNNAMED);
let align = align.min(self.cx().tcx.sess.target.max_reliable_alignment());
llvm::LLVMSetAlignment(load, align.bytes() as c_uint);
load
}
Expand Down Expand Up @@ -807,6 +808,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
assert_eq!(self.cx.type_kind(self.cx.val_ty(ptr)), TypeKind::Pointer);
unsafe {
let store = llvm::LLVMBuildStore(self.llbuilder, val, ptr);
let align = align.min(self.cx().tcx.sess.target.max_reliable_alignment());
let align =
if flags.contains(MemFlags::UNALIGNED) { 1 } else { align.bytes() as c_uint };
llvm::LLVMSetAlignment(store, align);
Expand Down
32 changes: 28 additions & 4 deletions compiler/rustc_mir_transform/src/check_alignment.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use rustc_abi::Align;
use rustc_index::IndexVec;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::visit::PlaceContext;
Expand All @@ -11,10 +12,6 @@ pub(super) struct CheckAlignment;

impl<'tcx> crate::MirPass<'tcx> for CheckAlignment {
fn is_enabled(&self, sess: &Session) -> bool {
// FIXME(#112480) MSVC and rustc disagree on minimum stack alignment on x86 Windows
if sess.target.llvm_target == "i686-pc-windows-msvc" {
return false;
}
sess.ub_checks()
}

Expand Down Expand Up @@ -87,6 +84,33 @@ fn insert_alignment_check<'tcx>(
))),
});

// If this target does not have reliable alignment, further limit the mask by anding it with
// the mask for the highest reliable alignment.
#[allow(irrefutable_let_patterns)]
if let max_align = tcx.sess.target.max_reliable_alignment()
&& max_align < Align::MAX
{
let max_mask = max_align.bytes() - 1;
let max_mask = Operand::Constant(Box::new(ConstOperand {
span: source_info.span,
user_ty: None,
const_: Const::Val(
ConstValue::Scalar(Scalar::from_target_usize(max_mask, &tcx)),
tcx.types.usize,
),
}));
stmts.push(Statement {
source_info,
kind: StatementKind::Assign(Box::new((
alignment_mask,
Rvalue::BinaryOp(
BinOp::BitAnd,
Box::new((Operand::Copy(alignment_mask), max_mask)),
),
))),
});
}

// BitAnd the alignment mask with the pointer
let alignment_bits =
local_decls.push(LocalDecl::with_source_info(tcx.types.usize, source_info)).into();
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/callconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ pub struct ArgAttributes {
/// (corresponding to LLVM's dereferenceable_or_null attributes, i.e., it is okay for this to be
/// set on a null pointer, but all non-null pointers must be dereferenceable).
pub pointee_size: Size,
/// The minimum alignment of the pointee, if any.
pub pointee_align: Option<Align>,
}

Expand Down
23 changes: 22 additions & 1 deletion compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::{fmt, io};

use rustc_abi::{Endian, ExternAbi, Integer, Size, TargetDataLayout, TargetDataLayoutErrors};
use rustc_abi::{
Align, Endian, ExternAbi, Integer, Size, TargetDataLayout, TargetDataLayoutErrors,
};
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_fs_util::try_canonicalize;
use rustc_macros::{Decodable, Encodable, HashStable_Generic};
Expand Down Expand Up @@ -3598,6 +3600,25 @@ impl Target {
_ => return None,
})
}

/// Returns whether this target is known to have unreliable alignment:
/// native C code for the target fails to align some data to the degree
/// required by the C standard. We can't *really* do anything about that
/// since unsafe Rust code may assume alignment any time, but we can at least
/// inhibit some optimizations, and we suppress the alignment checks that
/// would detect this unsoundness.
///
/// Every target that returns less than `Align::MAX` here is still has a soundness bug.
pub fn max_reliable_alignment(&self) -> Align {
// FIXME(#112480) MSVC on x86-32 is unsound and fails to properly align many types with
// more-than-4-byte-alignment on the stack. This makes alignments larger than 4 generally
// unreliable on 32bit Windows.
if self.is_like_windows && self.arch == "x86" {
Align::from_bytes(4).unwrap()
} else {
Align::MAX
}
}
}

/// Either a target tuple string or a path to a JSON file.
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_ty_utils/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,8 @@ fn adjust_for_rust_scalar<'tcx>(
None
};
if let Some(kind) = kind {
attrs.pointee_align = Some(pointee.align);
attrs.pointee_align =
Some(pointee.align.min(cx.tcx().sess.target.max_reliable_alignment()));

// `Box` are not necessarily dereferenceable for the entire duration of the function as
// they can be deallocated at any time. Same for non-frozen shared references (see
Expand Down
12 changes: 7 additions & 5 deletions src/doc/rustc/src/platform-support.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ target | notes
-------|-------
[`aarch64-apple-darwin`](platform-support/apple-darwin.md) | ARM64 macOS (11.0+, Big Sur+)
`aarch64-unknown-linux-gnu` | ARM64 Linux (kernel 4.1, glibc 2.17+)
`i686-pc-windows-msvc` | 32-bit MSVC (Windows 10+, Windows Server 2016+, Pentium 4) [^x86_32-floats-return-ABI]
`i686-pc-windows-msvc` | 32-bit MSVC (Windows 10+, Windows Server 2016+, Pentium 4) [^x86_32-floats-return-ABI] [^win32-msvc-alignment]
`i686-unknown-linux-gnu` | 32-bit Linux (kernel 3.2+, glibc 2.17+, Pentium 4) [^x86_32-floats-return-ABI]
[`x86_64-apple-darwin`](platform-support/apple-darwin.md) | 64-bit macOS (10.12+, Sierra+)
[`x86_64-pc-windows-gnu`](platform-support/windows-gnu.md) | 64-bit MinGW (Windows 10+, Windows Server 2016+)
Expand All @@ -43,6 +43,8 @@ target | notes

[^x86_32-floats-return-ABI]: Due to limitations of the C ABI, floating-point support on `i686` targets is non-compliant: floating-point return values are passed via an x87 register, so NaN payload bits can be lost. Functions with the default Rust ABI are not affected. See [issue #115567][x86-32-float-return-issue].

[^win32-msvc-alignment]: Due to non-standard behavior of MSVC, native C code on this target can cause types with an alignment of more than 4 bytes to be incorrectly aligned to only 4 bytes (this affects, e.g., `u64` and `i64`). Rust applies some mitigations to reduce the impact of this issue, but this can still cause unsoundness due to unsafe code that (correctly) assumes that references are always properly aligned. See [issue #112480](https://github.com/rust-lang/rust/issues/112480).

[77071]: https://github.com/rust-lang/rust/issues/77071
[x86-32-float-return-issue]: https://github.com/rust-lang/rust/issues/115567

Expand Down Expand Up @@ -95,7 +97,7 @@ target | notes
[`armv7-unknown-linux-ohos`](platform-support/openharmony.md) | Armv7-A OpenHarmony
[`loongarch64-unknown-linux-gnu`](platform-support/loongarch-linux.md) | LoongArch64 Linux, LP64D ABI (kernel 5.19, glibc 2.36)
[`loongarch64-unknown-linux-musl`](platform-support/loongarch-linux.md) | LoongArch64 Linux, LP64D ABI (kernel 5.19, musl 1.2.5)
[`i686-pc-windows-gnu`](platform-support/windows-gnu.md) | 32-bit MinGW (Windows 10+, Windows Server 2016+, Pentium 4) [^x86_32-floats-return-ABI]
[`i686-pc-windows-gnu`](platform-support/windows-gnu.md) | 32-bit MinGW (Windows 10+, Windows Server 2016+, Pentium 4) [^x86_32-floats-return-ABI] [^win32-msvc-alignment]
`powerpc-unknown-linux-gnu` | PowerPC Linux (kernel 3.2, glibc 2.17)
`powerpc64-unknown-linux-gnu` | PPC64 Linux (kernel 3.2, glibc 2.17)
[`powerpc64le-unknown-linux-gnu`](platform-support/powerpc64le-unknown-linux-gnu.md) | PPC64LE Linux (kernel 3.10, glibc 2.17)
Expand Down Expand Up @@ -169,7 +171,7 @@ target | std | notes
[`i686-pc-windows-gnullvm`](platform-support/windows-gnullvm.md) | ✓ | 32-bit x86 MinGW (Windows 10+, Pentium 4), LLVM ABI [^x86_32-floats-return-ABI]
[`i686-unknown-freebsd`](platform-support/freebsd.md) | ✓ | 32-bit x86 FreeBSD (Pentium 4) [^x86_32-floats-return-ABI]
`i686-unknown-linux-musl` | ✓ | 32-bit Linux with musl 1.2.3 (Pentium 4) [^x86_32-floats-return-ABI]
[`i686-unknown-uefi`](platform-support/unknown-uefi.md) | ? | 32-bit UEFI (Pentium 4, softfloat)
[`i686-unknown-uefi`](platform-support/unknown-uefi.md) | ? | 32-bit UEFI (Pentium 4, softfloat) [^win32-msvc-alignment]
[`loongarch64-unknown-none`](platform-support/loongarch-none.md) | * | LoongArch64 Bare-metal (LP64D ABI)
[`loongarch64-unknown-none-softfloat`](platform-support/loongarch-none.md) | * | LoongArch64 Bare-metal (LP64S ABI)
[`nvptx64-nvidia-cuda`](platform-support/nvptx64-nvidia-cuda.md) | * | --emit=asm generates PTX code that [runs on NVIDIA GPUs]
Expand Down Expand Up @@ -317,9 +319,9 @@ target | std | host | notes
[`i686-unknown-netbsd`](platform-support/netbsd.md) | ✓ | ✓ | NetBSD/i386 (Pentium 4) [^x86_32-floats-return-ABI]
[`i686-unknown-openbsd`](platform-support/openbsd.md) | ✓ | ✓ | 32-bit OpenBSD (Pentium 4) [^x86_32-floats-return-ABI]
`i686-uwp-windows-gnu` | ✓ | | [^x86_32-floats-return-ABI]
[`i686-uwp-windows-msvc`](platform-support/uwp-windows-msvc.md) | ✓ | | [^x86_32-floats-return-ABI]
[`i686-uwp-windows-msvc`](platform-support/uwp-windows-msvc.md) | ✓ | | [^x86_32-floats-return-ABI] [^win32-msvc-alignment]
[`i686-win7-windows-gnu`](platform-support/win7-windows-gnu.md) | ✓ | | 32-bit Windows 7 support [^x86_32-floats-return-ABI]
[`i686-win7-windows-msvc`](platform-support/win7-windows-msvc.md) | ✓ | | 32-bit Windows 7 support [^x86_32-floats-return-ABI]
[`i686-win7-windows-msvc`](platform-support/win7-windows-msvc.md) | ✓ | | 32-bit Windows 7 support [^x86_32-floats-return-ABI] [^win32-msvc-alignment]
[`i686-wrs-vxworks`](platform-support/vxworks.md) | ✓ | | [^x86_32-floats-return-ABI]
[`loongarch64-unknown-linux-ohos`](platform-support/openharmony.md) | ✓ | | LoongArch64 OpenHarmony
[`m68k-unknown-linux-gnu`](platform-support/m68k-unknown-linux-gnu.md) | ? | | Motorola 680x0 Linux
Expand Down
4 changes: 3 additions & 1 deletion tests/codegen/align-struct.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//@ compile-flags: -C no-prepopulate-passes -Z mir-opt-level=0
//
// 32bit MSVC does not align things properly so we suppress high alignment annotations (#112480)
//@ ignore-i686-pc-windows-msvc
//@ ignore-i686-pc-windows-gnu

#![crate_type = "lib"]

Expand Down
3 changes: 3 additions & 0 deletions tests/codegen/issues/issue-56927.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//@ compile-flags: -C no-prepopulate-passes
// 32bit MSVC does not align things properly so we suppress high alignment annotations (#112480)
//@ ignore-i686-pc-windows-msvc
//@ ignore-i686-pc-windows-gnu

#![crate_type = "rlib"]

Expand Down
Loading