place evaluation: require the original pointer to be aligned if an access happens

Author: RalfJung

compiler/rustc_abi/src/lib.rs

@@ -681,6 +681,7 @@ impl fmt::Display for AlignFromBytesError {
 
 impl Align {
     pub const ONE: Align = Align { pow2: 0 };
+    // LLVM has a maximal supported alignment of 2^29, we inherit that.
     pub const MAX: Align = Align { pow2: 29 };
 
     #[inline]

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -1,3 +1,5 @@
+use std::mem;
+
 use either::{Left, Right};
 
 use rustc_hir::def::DefKind;
@@ -73,9 +75,9 @@ fn eval_body_using_ecx<'mir, 'tcx>(
             None => InternKind::Constant,
         }
     };
-    ecx.machine.check_alignment = CheckAlignment::No; // interning doesn't need to respect alignment
+    let check_alignment = mem::replace(&mut ecx.machine.check_alignment, CheckAlignment::No); // interning doesn't need to respect alignment
     intern_const_alloc_recursive(ecx, intern_kind, &ret)?;
-    // we leave alignment checks off, since this `ecx` will not be used for further evaluation anyway
+    ecx.machine.check_alignment = check_alignment;
 
     debug!("eval_body_using_ecx done: {:?}", ret);
     Ok(ret)

compiler/rustc_const_eval/src/errors.rs

@@ -5,8 +5,9 @@ use rustc_errors::{
 use rustc_hir::ConstContext;
 use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
 use rustc_middle::mir::interpret::{
-    CheckInAllocMsg, ExpectedKind, InterpError, InvalidMetaKind, InvalidProgramInfo, PointerKind,
-    ResourceExhaustionInfo, UndefinedBehaviorInfo, UnsupportedOpInfo, ValidationErrorInfo,
+    CheckInAllocMsg, ExpectedKind, InterpError, InvalidMetaKind, InvalidProgramInfo, Misalignment,
+    PointerKind, ResourceExhaustionInfo, UndefinedBehaviorInfo, UnsupportedOpInfo,
+    ValidationErrorInfo,
 };
 use rustc_middle::ty::{self, Ty};
 use rustc_span::Span;
@@ -567,7 +568,7 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
 
                 builder.set_arg("bad_pointer_message", bad_pointer_message(msg, handler));
             }
-            AlignmentCheckFailed { required, has } => {
+            AlignmentCheckFailed(Misalignment { required, has }) => {
                 builder.set_arg("required", required.bytes());
                 builder.set_arg("has", has.bytes());
             }

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -22,8 +22,8 @@ use crate::fluent_generated as fluent;
 
 use super::{
     alloc_range, AllocBytes, AllocId, AllocMap, AllocRange, Allocation, CheckInAllocMsg,
-    GlobalAlloc, InterpCx, InterpResult, Machine, MayLeak, Pointer, PointerArithmetic, Provenance,
-    Scalar,
+    GlobalAlloc, InterpCx, InterpResult, Machine, MayLeak, Misalignment, Pointer,
+    PointerArithmetic, Provenance, Scalar,
 };
 
 #[derive(Debug, PartialEq, Copy, Clone)]
@@ -372,7 +372,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         self.check_and_deref_ptr(
             ptr,
             size,
-            M::enforce_alignment(self).then_some(align),
+            align,
             CheckInAllocMsg::MemoryAccessTest,
             |alloc_id, offset, prov| {
                 let (size, align) = self
@@ -382,9 +382,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         )
     }
 
-    /// Check if the given pointer points to live memory of given `size` and `align`
-    /// (ignoring `M::enforce_alignment`). The caller can control the error message for the
-    /// out-of-bounds case.
+    /// Check if the given pointer points to live memory of given `size` and `align`.
+    /// The caller can control the error message for the out-of-bounds case.
     #[inline(always)]
     pub fn check_ptr_access_align(
         &self,
@@ -393,7 +392,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         align: Align,
         msg: CheckInAllocMsg,
     ) -> InterpResult<'tcx> {
-        self.check_and_deref_ptr(ptr, size, Some(align), msg, |alloc_id, _, _| {
+        self.check_and_deref_ptr(ptr, size, align, msg, |alloc_id, _, _| {
             let (size, align) = self.get_live_alloc_size_and_align(alloc_id, msg)?;
             Ok((size, align, ()))
         })?;
@@ -402,15 +401,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
     /// Low-level helper function to check if a ptr is in-bounds and potentially return a reference
     /// to the allocation it points to. Supports both shared and mutable references, as the actual
-    /// checking is offloaded to a helper closure. `align` defines whether and which alignment check
-    /// is done.
+    /// checking is offloaded to a helper closure.
     ///
     /// If this returns `None`, the size is 0; it can however return `Some` even for size 0.
     fn check_and_deref_ptr<T>(
         &self,
         ptr: Pointer<Option<M::Provenance>>,
         size: Size,
-        align: Option<Align>,
+        align: Align,
         msg: CheckInAllocMsg,
         alloc_size: impl FnOnce(
             AllocId,
@@ -426,9 +424,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     throw_ub!(DanglingIntPointer(addr, msg));
                 }
                 // Must be aligned.
-                if let Some(align) = align {
-                    self.check_offset_align(addr, align)?;
-                }
+                self.check_misalign(Self::offset_misalignment(addr, align))?;
                 None
             }
             Ok((alloc_id, offset, prov)) => {
@@ -450,18 +446,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 }
                 // Test align. Check this last; if both bounds and alignment are violated
                 // we want the error to be about the bounds.
-                if let Some(align) = align {
-                    if M::use_addr_for_alignment_check(self) {
-                        // `use_addr_for_alignment_check` can only be true if `OFFSET_IS_ADDR` is true.
-                        self.check_offset_align(ptr.addr().bytes(), align)?;
-                    } else {
-                        // Check allocation alignment and offset alignment.
-                        if alloc_align.bytes() < align.bytes() {
-                            throw_ub!(AlignmentCheckFailed { has: alloc_align, required: align });
-                        }
-                        self.check_offset_align(offset.bytes(), align)?;
-                    }
-                }
+                self.check_misalign(self.alloc_misalignment(ptr, offset, align, alloc_align))?;
 
                 // We can still be zero-sized in this branch, in which case we have to
                 // return `None`.
@@ -470,16 +455,59 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         })
     }
 
-    fn check_offset_align(&self, offset: u64, align: Align) -> InterpResult<'tcx> {
+    #[inline(always)]
+    pub(super) fn check_misalign(&self, misaligned: Option<Misalignment>) -> InterpResult<'tcx> {
+        if M::enforce_alignment(self) {
+            if let Some(misaligned) = misaligned {
+                throw_ub!(AlignmentCheckFailed(misaligned))
+            }
+        }
+        Ok(())
+    }
+
+    #[must_use]
+    fn offset_misalignment(offset: u64, align: Align) -> Option<Misalignment> {
         if offset % align.bytes() == 0 {
-            Ok(())
+            None
         } else {
             // The biggest power of two through which `offset` is divisible.
             let offset_pow2 = 1 << offset.trailing_zeros();
-            throw_ub!(AlignmentCheckFailed {
-                has: Align::from_bytes(offset_pow2).unwrap(),
-                required: align
-            });
+            Some(Misalignment { has: Align::from_bytes(offset_pow2).unwrap(), required: align })
+        }
+    }
+
+    #[must_use]
+    fn alloc_misalignment(
+        &self,
+        ptr: Pointer<Option<M::Provenance>>,
+        offset: Size,
+        align: Align,
+        alloc_align: Align,
+    ) -> Option<Misalignment> {
+        if M::use_addr_for_alignment_check(self) {
+            // `use_addr_for_alignment_check` can only be true if `OFFSET_IS_ADDR` is true.
+            Self::offset_misalignment(ptr.addr().bytes(), align)
+        } else {
+            // Check allocation alignment and offset alignment.
+            if alloc_align.bytes() < align.bytes() {
+                Some(Misalignment { has: alloc_align, required: align })
+            } else {
+                Self::offset_misalignment(offset.bytes(), align)
+            }
+        }
+    }
+
+    pub(super) fn is_ptr_misaligned(
+        &self,
+        ptr: Pointer<Option<M::Provenance>>,
+        align: Align,
+    ) -> Option<Misalignment> {
+        match self.ptr_try_get_alloc_id(ptr) {
+            Err(addr) => Self::offset_misalignment(addr, align),
+            Ok((alloc_id, offset, _prov)) => {
+                let (_size, alloc_align, _kind) = self.get_alloc_info(alloc_id);
+                self.alloc_misalignment(ptr, offset, align, alloc_align)
+            }
         }
     }
 }
@@ -597,7 +625,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let ptr_and_alloc = self.check_and_deref_ptr(
             ptr,
             size,
-            M::enforce_alignment(self).then_some(align),
+            align,
             CheckInAllocMsg::MemoryAccessTest,
             |alloc_id, offset, prov| {
                 let alloc = self.get_alloc_raw(alloc_id)?;

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -10,7 +10,7 @@ use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
 use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter};
 use rustc_middle::ty::{ConstInt, Ty, TyCtxt};
 use rustc_middle::{mir, ty};
-use rustc_target::abi::{self, Abi, Align, HasDataLayout, Size};
+use rustc_target::abi::{self, Abi, HasDataLayout, Size};
 
 use super::{
     alloc_range, from_known_layout, mir_assign_valid_types, AllocId, Frame, InterpCx, InterpResult,
@@ -44,12 +44,16 @@ impl<Prov: Provenance> From<Scalar<Prov>> for Immediate<Prov> {
 }
 
 impl<Prov: Provenance> Immediate<Prov> {
-    pub fn from_pointer(ptr: Pointer<Prov>, cx: &impl HasDataLayout) -> Self {
-        Immediate::Scalar(Scalar::from_pointer(ptr, cx))
-    }
-
-    pub fn from_maybe_pointer(ptr: Pointer<Option<Prov>>, cx: &impl HasDataLayout) -> Self {
-        Immediate::Scalar(Scalar::from_maybe_pointer(ptr, cx))
+    pub fn new_pointer_with_meta(
+        ptr: Pointer<Option<Prov>>,
+        meta: MemPlaceMeta<Prov>,
+        cx: &impl HasDataLayout,
+    ) -> Self {
+        let ptr = Scalar::from_maybe_pointer(ptr, cx);
+        match meta {
+            MemPlaceMeta::None => Immediate::from(ptr),
+            MemPlaceMeta::Meta(meta) => Immediate::ScalarPair(ptr, meta),
+        }
     }
 
     pub fn new_slice(ptr: Pointer<Option<Prov>>, len: u64, cx: &impl HasDataLayout) -> Self {
@@ -328,14 +332,6 @@ pub(super) enum Operand<Prov: Provenance = AllocId> {
 pub struct OpTy<'tcx, Prov: Provenance = AllocId> {
     op: Operand<Prov>, // Keep this private; it helps enforce invariants.
     pub layout: TyAndLayout<'tcx>,
-    /// rustc does not have a proper way to represent the type of a field of a `repr(packed)` struct:
-    /// it needs to have a different alignment than the field type would usually have.
-    /// So we represent this here with a separate field that "overwrites" `layout.align`.
-    /// This means `layout.align` should never be used for an `OpTy`!
-    /// `None` means "alignment does not matter since this is a by-value operand"
-    /// (`Operand::Immediate`); this field is only relevant for `Operand::Indirect`.
-    /// Also CTFE ignores alignment anyway, so this is for Miri only.
-    pub align: Option<Align>,
 }
 
 impl<Prov: Provenance> std::fmt::Debug for OpTy<'_, Prov> {
@@ -351,18 +347,14 @@ impl<Prov: Provenance> std::fmt::Debug for OpTy<'_, Prov> {
 impl<'tcx, Prov: Provenance> From<ImmTy<'tcx, Prov>> for OpTy<'tcx, Prov> {
     #[inline(always)]
     fn from(val: ImmTy<'tcx, Prov>) -> Self {
-        OpTy { op: Operand::Immediate(val.imm), layout: val.layout, align: None }
+        OpTy { op: Operand::Immediate(val.imm), layout: val.layout }
     }
 }
 
 impl<'tcx, Prov: Provenance> From<MPlaceTy<'tcx, Prov>> for OpTy<'tcx, Prov> {
     #[inline(always)]
     fn from(mplace: MPlaceTy<'tcx, Prov>) -> Self {
-        OpTy {
-            op: Operand::Indirect(*mplace.mplace()),
-            layout: mplace.layout,
-            align: Some(mplace.align),
-        }
+        OpTy { op: Operand::Indirect(*mplace.mplace()), layout: mplace.layout }
     }
 }
 
@@ -635,7 +627,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 throw_inval!(ConstPropNonsense);
             }
         }
-        Ok(OpTy { op, layout, align: Some(layout.align.abi) })
+        Ok(OpTy { op, layout })
     }
 
     /// Every place can be read from, so we can turn them into an operand.
@@ -650,16 +642,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             Right((frame, local, offset)) => {
                 debug_assert!(place.layout.is_sized()); // only sized locals can ever be `Place::Local`.
                 let base = self.local_to_op(&self.stack()[frame], local, None)?;
-                let mut field = match offset {
+                Ok(match offset {
                     Some(offset) => base.offset(offset, place.layout, self)?,
                     None => {
                         // In the common case this hasn't been projected.
                         debug_assert_eq!(place.layout, base.layout);
                         base
                     }
-                };
-                field.align = Some(place.align);
-                Ok(field)
+                })
             }
         }
     }
@@ -747,27 +737,23 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             })
         };
         let layout = from_known_layout(self.tcx, self.param_env, layout, || self.layout_of(ty))?;
-        let op = match val_val {
+        let imm = match val_val {
             mir::ConstValue::Indirect { alloc_id, offset } => {
                 // We rely on mutability being set correctly in that allocation to prevent writes
                 // where none should happen.
                 let ptr = self.global_base_pointer(Pointer::new(alloc_id, offset))?;
-                Operand::Indirect(MemPlace::from_ptr(ptr.into()))
+                return Ok(self.ptr_to_mplace(ptr.into(), layout).into());
             }
-            mir::ConstValue::Scalar(x) => Operand::Immediate(adjust_scalar(x)?.into()),
-            mir::ConstValue::ZeroSized => Operand::Immediate(Immediate::Uninit),
+            mir::ConstValue::Scalar(x) => adjust_scalar(x)?.into(),
+            mir::ConstValue::ZeroSized => Immediate::Uninit,
             mir::ConstValue::Slice { data, meta } => {
                 // We rely on mutability being set correctly in `data` to prevent writes
                 // where none should happen.
                 let ptr = Pointer::new(self.tcx.reserve_and_set_memory_alloc(data), Size::ZERO);
-                Operand::Immediate(Immediate::new_slice(
-                    self.global_base_pointer(ptr)?.into(),
-                    meta,
-                    self,
-                ))
+                Immediate::new_slice(self.global_base_pointer(ptr)?.into(), meta, self)
             }
         };
-        Ok(OpTy { op, layout, align: Some(layout.align.abi) })
+        Ok(OpTy { op: Operand::Immediate(imm), layout })
     }
 }
 
@@ -780,6 +766,6 @@ mod size_asserts {
     static_assert_size!(Immediate, 48);
     static_assert_size!(ImmTy<'_>, 64);
     static_assert_size!(Operand, 56);
-    static_assert_size!(OpTy<'_>, 80);
+    static_assert_size!(OpTy<'_>, 72);
     // tidy-alphabetical-end
 }