miri: support 'promising' alignment for symbolic alignment check

Author: RalfJung

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -12,12 +12,12 @@ use rustc_middle::mir;
 use rustc_middle::ty::layout::TyAndLayout;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::def_id::DefId;
-use rustc_target::abi::Size;
+use rustc_target::abi::{Align, Size};
 use rustc_target::spec::abi::Abi as CallAbi;
 
 use super::{
-    AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
-    InterpResult, MPlaceTy, MemoryKind, OpTy, PlaceTy, Pointer, Provenance,
+    AllocBytes, AllocId, AllocKind, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy,
+    InterpCx, InterpResult, MPlaceTy, MemoryKind, Misalignment, OpTy, PlaceTy, Pointer, Provenance,
 };
 
 /// Data returned by Machine::stack_pop,
@@ -135,11 +135,18 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
     /// Whether memory accesses should be alignment-checked.
     fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
 
-    /// Whether, when checking alignment, we should look at the actual address and thus support
-    /// custom alignment logic based on whatever the integer address happens to be.
-    ///
-    /// If this returns true, Provenance::OFFSET_IS_ADDR must be true.
-    fn use_addr_for_alignment_check(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
+    /// Gives the machine a chance to detect more misalignment than the built-in checks would catch.
+    #[inline(always)]
+    fn alignment_check(
+        _ecx: &InterpCx<'mir, 'tcx, Self>,
+        _alloc_id: AllocId,
+        _alloc_align: Align,
+        _alloc_kind: AllocKind,
+        _offset: Size,
+        _align: Align,
+    ) -> Option<Misalignment> {
+        None
+    }
 
     /// Whether to enforce the validity invariant for a specific layout.
     fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool;
@@ -511,12 +518,6 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
     type FrameExtra = ();
     type Bytes = Box<[u8]>;
 
-    #[inline(always)]
-    fn use_addr_for_alignment_check(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
-        // We do not support `use_addr`.
-        false
-    }
-
     #[inline(always)]
     fn ignore_optional_overflow_checks(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
         false

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -58,6 +58,7 @@ impl<T: fmt::Display> fmt::Display for MemoryKind<T> {
 }
 
 /// The return value of `get_alloc_info` indicates the "kind" of the allocation.
+#[derive(Copy, Clone, PartialEq, Debug)]
 pub enum AllocKind {
     /// A regular live data allocation.
     LiveData,
@@ -473,8 +474,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         match self.ptr_try_get_alloc_id(ptr) {
             Err(addr) => offset_misalignment(addr, align),
             Ok((alloc_id, offset, _prov)) => {
-                let (_size, alloc_align, _kind) = self.get_alloc_info(alloc_id);
-                if M::use_addr_for_alignment_check(self) {
+                let (_size, alloc_align, kind) = self.get_alloc_info(alloc_id);
+                if let Some(misalign) =
+                    M::alignment_check(self, alloc_id, alloc_align, kind, offset, align)
+                {
+                    Some(misalign)
+                } else if M::Provenance::OFFSET_IS_ADDR {
                     // `use_addr_for_alignment_check` can only be true if `OFFSET_IS_ADDR` is true.
                     offset_misalignment(ptr.addr().bytes(), align)
                 } else {

library/core/src/slice/mod.rs

@@ -3876,6 +3876,18 @@ impl<T> [T] {
         } else {
             let (left, rest) = self.split_at(offset);
             let (us_len, ts_len) = rest.align_to_offsets::<U>();
+            // Inform Miri that we want to consider the "middle" pointer to be suitably aligned.
+            #[cfg(miri)]
+            {
+                extern "Rust" {
+                    pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize);
+                }
+
+                // SAFETY: the pointer is dereferenceable and the alignment a power of 2.
+                unsafe {
+                    miri_promise_symbolic_alignment(rest.as_ptr().cast(), mem::align_of::<U>());
+                }
+            }
             // SAFETY: now `rest` is definitely aligned, so `from_raw_parts` below is okay,
             // since the caller guarantees that we can transmute `T` to `U` safely.
             unsafe {
@@ -3946,6 +3958,21 @@ impl<T> [T] {
             let (us_len, ts_len) = rest.align_to_offsets::<U>();
             let rest_len = rest.len();
             let mut_ptr = rest.as_mut_ptr();
+            // Inform Miri that we want to consider the "middle" pointer to be suitably aligned.
+            #[cfg(miri)]
+            {
+                extern "Rust" {
+                    pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize);
+                }
+
+                // SAFETY: the pointer is dereferenceable and the alignment a power of 2.
+                unsafe {
+                    miri_promise_symbolic_alignment(
+                        mut_ptr.cast() as *const (),
+                        mem::align_of::<U>(),
+                    );
+                }
+            }
             // We can't use `rest` again after this, that would invalidate its alias `mut_ptr`!
             // SAFETY: see comments for `align_to`.
             unsafe {

src/tools/miri/src/machine.rs

@@ -2,7 +2,7 @@
 //! `Machine` trait.
 
 use std::borrow::Cow;
-use std::cell::RefCell;
+use std::cell::{Cell, RefCell};
 use std::fmt;
 use std::path::Path;
 use std::process;
@@ -309,11 +309,20 @@ pub struct AllocExtra<'tcx> {
     /// if this allocation is leakable. The backtrace is not
     /// pruned yet; that should be done before printing it.
     pub backtrace: Option<Vec<FrameInfo<'tcx>>>,
+    /// An offset inside this allocation that was deemed aligned even for symbolic alignment checks.
+    /// Invariant: the promised alignment will never be less than the native alignment of this allocation.
+    pub symbolic_alignment: Cell<Option<(Size, Align)>>,
 }
 
 impl VisitTags for AllocExtra<'_> {
     fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
-        let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self;
+        let AllocExtra {
+            borrow_tracker,
+            data_race,
+            weak_memory,
+            backtrace: _,
+            symbolic_alignment: _,
+        } = self;
 
         borrow_tracker.visit_tags(visit);
         data_race.visit_tags(visit);
@@ -907,8 +916,45 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
     }
 
     #[inline(always)]
-    fn use_addr_for_alignment_check(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool {
-        ecx.machine.check_alignment == AlignmentCheck::Int
+    fn alignment_check(
+        ecx: &MiriInterpCx<'mir, 'tcx>,
+        alloc_id: AllocId,
+        alloc_align: Align,
+        alloc_kind: AllocKind,
+        offset: Size,
+        align: Align,
+    ) -> Option<Misalignment> {
+        if ecx.machine.check_alignment != AlignmentCheck::Symbolic {
+            // Just use the built-in check.
+            return None;
+        }
+        if alloc_kind != AllocKind::LiveData {
+            // Can't have any extra info here.
+            return None;
+        }
+        // Let's see which alignment we have been promised for this allocation.
+        let alloc_info = ecx.get_alloc_extra(alloc_id).unwrap(); // cannot fail since the allocation is live
+        let (promised_offset, promised_align) =
+            alloc_info.symbolic_alignment.get().unwrap_or((Size::ZERO, alloc_align));
+        if promised_align < align {
+            // Definitely not enough.
+            Some(Misalignment { has: promised_align, required: align })
+        } else {
+            // What's the offset between us and the promised alignment?
+            let distance = offset.bytes().wrapping_sub(promised_offset.bytes());
+            // That must also be aligned.
+            if distance % align.bytes() == 0 {
+                // All looking good!
+                None
+            } else {
+                // The biggest power of two through which `distance` is divisible.
+                let distance_pow2 = 1 << distance.trailing_zeros();
+                Some(Misalignment {
+                    has: Align::from_bytes(distance_pow2).unwrap(),
+                    required: align,
+                })
+            }
+        }
     }
 
     #[inline(always)]
@@ -1112,6 +1158,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
                 data_race: race_alloc,
                 weak_memory: buffer_alloc,
                 backtrace,
+                symbolic_alignment: Cell::new(None),
             },
             |ptr| ecx.global_base_pointer(ptr),
         )?;

src/tools/miri/src/shims/foreign_items.rs

@@ -464,7 +464,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let ptr = this.read_pointer(ptr)?;
                 let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr).map_err(|_e| {
                     err_machine_stop!(TerminationInfo::Abort(format!(
-                        "pointer passed to miri_get_alloc_id must not be dangling, got {ptr:?}"
+                        "pointer passed to `miri_get_alloc_id` must not be dangling, got {ptr:?}"
                     )))
                 })?;
                 this.write_scalar(Scalar::from_u64(alloc_id.0.get()), dest)?;
@@ -496,7 +496,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let (alloc_id, offset, _) = this.ptr_get_alloc_id(ptr)?;
                 if offset != Size::ZERO {
                     throw_unsup_format!(
-                        "pointer passed to miri_static_root must point to beginning of an allocated block"
+                        "pointer passed to `miri_static_root` must point to beginning of an allocated block"
                     );
                 }
                 this.machine.static_roots.push(alloc_id);
@@ -553,6 +553,39 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 };
             }
 
+            // Promises that a pointer has a given symbolic alignment.
+            "miri_promise_symbolic_alignment" => {
+                let [ptr, align] = this.check_shim(abi, Abi::Rust, link_name, args)?;
+                let ptr = this.read_pointer(ptr)?;
+                let align = this.read_target_usize(align)?;
+                let Ok(align) = Align::from_bytes(align) else {
+                    throw_unsup_format!(
+                        "`miri_promise_symbolic_alignment`: alignment must be a power of 2"
+                    );
+                };
+                let (_, addr) = ptr.into_parts(); // we know the offset is absolute
+                if addr.bytes() % align.bytes() != 0 {
+                    throw_unsup_format!(
+                        "`miri_promise_symbolic_alignment`: pointer is not actually aligned"
+                    );
+                }
+                if let Ok((alloc_id, offset, ..)) = this.ptr_try_get_alloc_id(ptr) {
+                    let (_size, alloc_align, _kind) = this.get_alloc_info(alloc_id);
+                    // Not `get_alloc_extra_mut`, need to handle read-only allocations!
+                    let alloc_extra = this.get_alloc_extra(alloc_id)?;
+                    // If the newly promised alignment is bigger than the native alignment of this
+                    // allocation, and bigger than the previously promised alignment, then set it.
+                    if align > alloc_align
+                        && !alloc_extra
+                            .symbolic_alignment
+                            .get()
+                            .is_some_and(|(_, old_align)| align <= old_align)
+                    {
+                        alloc_extra.symbolic_alignment.set(Some((offset, align)));
+                    }
+                }
+            }
+
             // Standard C allocation
             "malloc" => {
                 let [size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;

src/tools/miri/src/shims/mod.rs

@@ -23,7 +23,6 @@ use rustc_middle::{mir, ty};
 use rustc_target::spec::abi::Abi;
 
 use crate::*;
-use helpers::check_arg_count;
 
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
@@ -39,16 +38,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let this = self.eval_context_mut();
         trace!("eval_fn_call: {:#?}, {:?}", instance, dest);
 
-        // There are some more lang items we want to hook that CTFE does not hook (yet).
-        if this.tcx.lang_items().align_offset_fn() == Some(instance.def.def_id()) {
-            let args = this.copy_fn_args(args)?;
-            let [ptr, align] = check_arg_count(&args)?;
-            if this.align_offset(ptr, align, dest, ret, unwind)? {
-                return Ok(None);
-            }
-        }
-
-        // Try to see if we can do something about foreign items.
+        // For foreign items, try to see if we can emulate them.
         if this.tcx.is_foreign_item(instance.def_id()) {
             // An external function call that does not have a MIR body. We either find MIR elsewhere
             // or emulate its effect.
@@ -64,53 +54,4 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         // Otherwise, load the MIR.
         Ok(Some((this.load_mir(instance.def, None)?, instance)))
     }
-
-    /// Returns `true` if the computation was performed, and `false` if we should just evaluate
-    /// the actual MIR of `align_offset`.
-    fn align_offset(
-        &mut self,
-        ptr_op: &OpTy<'tcx, Provenance>,
-        align_op: &OpTy<'tcx, Provenance>,
-        dest: &PlaceTy<'tcx, Provenance>,
-        ret: Option<mir::BasicBlock>,
-        unwind: mir::UnwindAction,
-    ) -> InterpResult<'tcx, bool> {
-        let this = self.eval_context_mut();
-        let ret = ret.unwrap();
-
-        if this.machine.check_alignment != AlignmentCheck::Symbolic {
-            // Just use actual implementation.
-            return Ok(false);
-        }
-
-        let req_align = this.read_target_usize(align_op)?;
-
-        // Stop if the alignment is not a power of two.
-        if !req_align.is_power_of_two() {
-            this.start_panic("align_offset: align is not a power-of-two", unwind)?;
-            return Ok(true); // nothing left to do
-        }
-
-        let ptr = this.read_pointer(ptr_op)?;
-        // If this carries no provenance, treat it like an integer.
-        if ptr.provenance.is_none() {
-            // Use actual implementation.
-            return Ok(false);
-        }
-
-        if let Ok((alloc_id, _offset, _)) = this.ptr_try_get_alloc_id(ptr) {
-            // Only do anything if we can identify the allocation this goes to.
-            let (_size, cur_align, _kind) = this.get_alloc_info(alloc_id);
-            if cur_align.bytes() >= req_align {
-                // If the allocation alignment is at least the required alignment we use the
-                // real implementation.
-                return Ok(false);
-            }
-        }
-
-        // Return error result (usize::MAX), and jump to caller.
-        this.write_scalar(Scalar::from_target_usize(this.target_usize_max(), this), dest)?;
-        this.go_to_block(ret);
-        Ok(true)
-    }
 }

src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.call_unaligned_ptr.stderr

@@ -0,0 +1,14 @@
+error: unsupported operation: `miri_promise_symbolic_alignment`: pointer is not actually aligned
+  --> $DIR/promise_alignment.rs:LL:CC
+   |
+LL |         unsafe { utils::miri_promise_symbolic_alignment(align8.add(1).cast(), 8) };
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `miri_promise_symbolic_alignment`: pointer is not actually aligned
+   |
+   = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
+   = note: BACKTRACE:
+   = note: inside `main` at $DIR/promise_alignment.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+

src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.read_unaligned_ptr.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: accessing memory based on pointer with alignment ALIGN, but alignment ALIGN is required
+  --> $DIR/promise_alignment.rs:LL:CC
+   |
+LL |         let _val = unsafe { align8.cast::<Align16>().read() };
+   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ accessing memory based on pointer with alignment ALIGN, but alignment ALIGN is required
+   |
+   = help: this usually indicates that your program performed an invalid operation and caused Undefined Behavior
+   = help: but due to `-Zmiri-symbolic-alignment-check`, alignment errors can also be false positives
+   = note: BACKTRACE:
+   = note: inside `main` at $DIR/promise_alignment.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+

src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs

@@ -0,0 +1,54 @@
+//@compile-flags: -Zmiri-symbolic-alignment-check
+//@revisions: call_unaligned_ptr read_unaligned_ptr
+#![feature(strict_provenance)]
+
+#[path = "../../utils/mod.rs"]
+mod utils;
+
+#[repr(align(8))]
+#[derive(Copy, Clone)]
+struct Align8(u64);
+
+fn main() {
+    let buffer = [0u32; 128]; // get some 4-aligned memory
+    let buffer = buffer.as_ptr();
+    // "Promising" the alignment down to 1 must not hurt.
+    unsafe { utils::miri_promise_symbolic_alignment(buffer.cast(), 1) };
+    let _val = unsafe { buffer.read() };
+
+    // Let's find a place to promise alignment 8.
+    let align8 = if buffer.addr() % 8 == 0 {
+        buffer
+    } else {
+        buffer.wrapping_add(1)
+    };
+    assert!(align8.addr() % 8 == 0);
+    unsafe { utils::miri_promise_symbolic_alignment(align8.cast(), 8) };
+    // Promising the alignment down to 1 *again* still must not hurt.
+    unsafe { utils::miri_promise_symbolic_alignment(buffer.cast(), 1) };
+    // Now we can do 8-aligned reads here.
+    let _val = unsafe { align8.cast::<Align8>().read() };
+
+    // Make sure we error if the pointer is not actually aligned.
+    if cfg!(call_unaligned_ptr) {
+        unsafe { utils::miri_promise_symbolic_alignment(align8.add(1).cast(), 8) };
+        //~[call_unaligned_ptr]^ ERROR: pointer is not actually aligned
+    }
+
+    // Also don't accept even higher-aligned reads.
+    if cfg!(read_unaligned_ptr) {
+        #[repr(align(16))]
+        #[derive(Copy, Clone)]
+        struct Align16(u128);
+
+        let align16 = if align8.addr() % 16 == 0 {
+            align8
+        } else {
+            align8.wrapping_add(2)
+        };
+        assert!(align16.addr() % 16 == 0);
+
+        let _val = unsafe { align8.cast::<Align16>().read() };
+        //~[read_unaligned_ptr]^ ERROR: accessing memory based on pointer with alignment 8, but alignment 16 is required
+    }
+}

src/tools/miri/tests/pass/align_offset_symbolic.rs

@@ -1,36 +1,15 @@
 //@compile-flags: -Zmiri-symbolic-alignment-check
 #![feature(strict_provenance)]
 
-use std::ptr;
-
-fn test_align_offset() {
-    let d = Box::new([0u32; 4]);
-    // Get u8 pointer to base
-    let raw = d.as_ptr() as *const u8;
-
-    assert_eq!(raw.align_offset(2), 0);
-    assert_eq!(raw.align_offset(4), 0);
-    assert_eq!(raw.align_offset(8), usize::MAX); // requested alignment higher than allocation alignment
-
-    assert_eq!(raw.wrapping_offset(1).align_offset(2), 1);
-    assert_eq!(raw.wrapping_offset(1).align_offset(4), 3);
-    assert_eq!(raw.wrapping_offset(1).align_offset(8), usize::MAX); // requested alignment higher than allocation alignment
-
-    assert_eq!(raw.wrapping_offset(2).align_offset(2), 0);
-    assert_eq!(raw.wrapping_offset(2).align_offset(4), 2);
-    assert_eq!(raw.wrapping_offset(2).align_offset(8), usize::MAX); // requested alignment higher than allocation alignment
-
-    let p = ptr::invalid::<()>(1);
-    assert_eq!(p.align_offset(1), 0);
-}
-
 fn test_align_to() {
     const N: usize = 4;
     let d = Box::new([0u32; N]);
     // Get u8 slice covering the entire thing
     let s = unsafe { std::slice::from_raw_parts(d.as_ptr() as *const u8, 4 * N) };
     let raw = s.as_ptr();
 
+    // Cases where we get the expected "middle" part without any fuzz, since the allocation is
+    // 4-aligned.
     {
         let (l, m, r) = unsafe { s.align_to::<u32>() };
         assert_eq!(l.len(), 0);
@@ -63,14 +42,16 @@ fn test_align_to() {
         assert_eq!(raw.wrapping_offset(4), m.as_ptr() as *const u8);
     }
 
+    // Cases where we request more alignment than the allocation has.
     {
         #[repr(align(8))]
+        #[derive(Copy, Clone)]
         struct Align8(u64);
 
-        let (l, m, r) = unsafe { s.align_to::<Align8>() }; // requested alignment higher than allocation alignment
-        assert_eq!(l.len(), 4 * N);
-        assert_eq!(r.len(), 0);
-        assert_eq!(m.len(), 0);
+        let (_l, m, _r) = unsafe { s.align_to::<Align8>() };
+        assert!(m.len() > 0);
+        // Ensure the symbolic alignment check has been informed that this is okay now.
+        let _val = m[0];
     }
 }
 
@@ -104,7 +85,6 @@ fn test_u64_array() {
 }
 
 fn main() {
-    test_align_offset();
     test_align_to();
     test_from_utf8();
     test_u64_array();

src/tools/miri/tests/utils/miri_extern.rs

@@ -137,4 +137,9 @@ extern "Rust" {
         out: *mut std::ffi::c_char,
         out_size: usize,
     ) -> usize;
+
+    /// Miri-provided extern function to promise that a given pointer is properly aligned for
+    /// "symbolic" alignment checks. Will fail if the pointer is dangling or not actually aligned.
+    /// Has no effect when alignment checks are concrete (which is the default).
+    pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize);
 }