Rollup merge of #139261 - RalfJung:msvc-align-mitigation, r=oli-obk

mitigate MSVC alignment issue on x86-32

This implements mitigation for https://github.com/rust-lang/rust/issues/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*`

compiler/rustc_codegen_llvm/src/builder.rs

@@ -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
         }
@@ -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);

compiler/rustc_mir_transform/src/check_alignment.rs

@@ -1,3 +1,4 @@
+use rustc_abi::Align;
 use rustc_index::IndexVec;
 use rustc_middle::mir::interpret::Scalar;
 use rustc_middle::mir::visit::PlaceContext;
@@ -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()
     }
 
@@ -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();

compiler/rustc_target/src/callconv/mod.rs

@@ -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>,
 }
 

compiler/rustc_target/src/spec/mod.rs

@@ -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};
@@ -3599,6 +3601,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.

compiler/rustc_ty_utils/src/abi.rs

@@ -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