Skip to content

Commit 21a86ec

Browse files
committed
Auto merge of #139261 - RalfJung:msvc-align-mitigation, r=<try>
mitigate MSVC alignment issue on x86-32 This implements mitigation for #112480 by stopping to emit `align` attributes on loads and function arguments when building for a win32 MSVC target. MSVC is known to not properly align `u64` and similar types, and claiming to LLVM that everything is properly aligned increases the chance that this will cause problems. Of course, the misalignment is still a bug, but we can't fix that bug, only MSVC can. Also add an errata note to the platform support page warning users about this known problem. try-job: `i686-msvc*`
2 parents ae9173d + ce44d17 commit 21a86ec

File tree

6 files changed

+34
-8
lines changed

6 files changed

+34
-8
lines changed

compiler/rustc_codegen_llvm/src/builder.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,9 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
594594
fn load(&mut self, ty: &'ll Type, ptr: &'ll Value, align: Align) -> &'ll Value {
595595
unsafe {
596596
let load = llvm::LLVMBuildLoad2(self.llbuilder, ty, ptr, UNNAMED);
597-
llvm::LLVMSetAlignment(load, align.bytes() as c_uint);
597+
if !self.cx().tcx.sess.target.has_unreliable_alignment() {
598+
llvm::LLVMSetAlignment(load, align.bytes() as c_uint);
599+
}
598600
load
599601
}
600602
}
@@ -809,7 +811,9 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
809811
let store = llvm::LLVMBuildStore(self.llbuilder, val, ptr);
810812
let align =
811813
if flags.contains(MemFlags::UNALIGNED) { 1 } else { align.bytes() as c_uint };
812-
llvm::LLVMSetAlignment(store, align);
814+
if !self.cx().tcx.sess.target.has_unreliable_alignment() {
815+
llvm::LLVMSetAlignment(store, align);
816+
}
813817
if flags.contains(MemFlags::VOLATILE) {
814818
llvm::LLVMSetVolatile(store, llvm::True);
815819
}

compiler/rustc_mir_transform/src/check_alignment.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ pub(super) struct CheckAlignment;
1111

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

compiler/rustc_target/src/callconv/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ pub struct ArgAttributes {
144144
/// (corresponding to LLVM's dereferenceable_or_null attributes, i.e., it is okay for this to be
145145
/// set on a null pointer, but all non-null pointers must be dereferenceable).
146146
pub pointee_size: Size,
147+
/// The minimum alignment of the pointee, if any.
147148
pub pointee_align: Option<Align>,
148149
}
149150

compiler/rustc_target/src/spec/mod.rs

+18
Original file line numberDiff line numberDiff line change
@@ -3602,6 +3602,24 @@ impl Target {
36023602
_ => return None,
36033603
})
36043604
}
3605+
3606+
/// Returns whether this target is known to have unreliable alignment:
3607+
/// native C code for the target fails to align some data to the degree
3608+
/// required by the C standard. We can't *really* do anything about that
3609+
/// since unsafe Rust code may assume alignment any time, but we can at least
3610+
/// inhibit some optimizations, and we suppress the alignment checks that
3611+
/// would detect this unsoundness.
3612+
///
3613+
/// Every `return true` here is still a soundness bug for that target.
3614+
pub fn has_unreliable_alignment(&self) -> bool {
3615+
// FIXME(#112480) MSVC on x86-32 is unsound and fails to properly align many types with
3616+
// more-than-4-byte-alignment on the stack.
3617+
if self.is_like_msvc && self.arch == "x86" {
3618+
return true;
3619+
}
3620+
3621+
false
3622+
}
36053623
}
36063624

36073625
/// Either a target tuple string or a path to a JSON file.

compiler/rustc_ty_utils/src/abi.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,9 @@ fn adjust_for_rust_scalar<'tcx>(
347347
None
348348
};
349349
if let Some(kind) = kind {
350-
attrs.pointee_align = Some(pointee.align);
350+
if !cx.tcx().sess.target.has_unreliable_alignment() {
351+
attrs.pointee_align = Some(pointee.align);
352+
}
351353

352354
// `Box` are not necessarily dereferenceable for the entire duration of the function as
353355
// they can be deallocated at any time. Same for non-frozen shared references (see

src/doc/rustc/src/platform-support.md

+5-3
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ target | notes
3535
[`aarch64-apple-darwin`](platform-support/apple-darwin.md) | ARM64 macOS (11.0+, Big Sur+)
3636
`aarch64-unknown-linux-gnu` | ARM64 Linux (kernel 4.1, glibc 2.17+)
3737
`i686-pc-windows-gnu` | 32-bit MinGW (Windows 10+, Windows Server 2016+, Pentium 4) [^x86_32-floats-return-ABI]
38-
`i686-pc-windows-msvc` | 32-bit MSVC (Windows 10+, Windows Server 2016+, Pentium 4) [^x86_32-floats-return-ABI]
38+
`i686-pc-windows-msvc` | 32-bit MSVC (Windows 10+, Windows Server 2016+, Pentium 4) [^x86_32-floats-return-ABI] [^win32-msvc-alignment]
3939
`i686-unknown-linux-gnu` | 32-bit Linux (kernel 3.2+, glibc 2.17+, Pentium 4) [^x86_32-floats-return-ABI]
4040
[`x86_64-apple-darwin`](platform-support/apple-darwin.md) | 64-bit macOS (10.12+, Sierra+)
4141
`x86_64-pc-windows-gnu` | 64-bit MinGW (Windows 10+, Windows Server 2016+)
@@ -44,6 +44,8 @@ target | notes
4444

4545
[^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].
4646

47+
[^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).
48+
4749
[77071]: https://github.com/rust-lang/rust/issues/77071
4850
[x86-32-float-return-issue]: https://github.com/rust-lang/rust/issues/115567
4951

@@ -317,9 +319,9 @@ target | std | host | notes
317319
[`i686-unknown-netbsd`](platform-support/netbsd.md) | ✓ | ✓ | NetBSD/i386 (Pentium 4) [^x86_32-floats-return-ABI]
318320
[`i686-unknown-openbsd`](platform-support/openbsd.md) | ✓ | ✓ | 32-bit OpenBSD (Pentium 4) [^x86_32-floats-return-ABI]
319321
`i686-uwp-windows-gnu` | ✓ | | [^x86_32-floats-return-ABI]
320-
[`i686-uwp-windows-msvc`](platform-support/uwp-windows-msvc.md) | ✓ | | [^x86_32-floats-return-ABI]
322+
[`i686-uwp-windows-msvc`](platform-support/uwp-windows-msvc.md) | ✓ | | [^x86_32-floats-return-ABI] [^win32-msvc-alignment]
321323
[`i686-win7-windows-gnu`](platform-support/win7-windows-gnu.md) | ✓ | | 32-bit Windows 7 support [^x86_32-floats-return-ABI]
322-
[`i686-win7-windows-msvc`](platform-support/win7-windows-msvc.md) | ✓ | | 32-bit Windows 7 support [^x86_32-floats-return-ABI]
324+
[`i686-win7-windows-msvc`](platform-support/win7-windows-msvc.md) | ✓ | | 32-bit Windows 7 support [^x86_32-floats-return-ABI] [^win32-msvc-alignment]
323325
[`i686-wrs-vxworks`](platform-support/vxworks.md) | ✓ | | [^x86_32-floats-return-ABI]
324326
[`loongarch64-unknown-linux-ohos`](platform-support/openharmony.md) | ✓ | | LoongArch64 OpenHarmony
325327
[`m68k-unknown-linux-gnu`](platform-support/m68k-unknown-linux-gnu.md) | ? | | Motorola 680x0 Linux

0 commit comments

Comments
 (0)