From bebba4f6e05fae60ce8dcddc5eb6b4e7686bd995 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 12 Nov 2023 16:06:50 +0100 Subject: [PATCH 1/2] miri: support 'promising' alignment for symbolic alignment check --- .../rustc_const_eval/src/interpret/machine.rs | 29 ++++----- .../rustc_const_eval/src/interpret/memory.rs | 9 ++- library/core/src/ptr/const_ptr.rs | 32 +++++++++- library/core/src/ptr/mut_ptr.rs | 32 +++++++++- library/core/src/slice/mod.rs | 27 ++++++++ src/tools/miri/src/machine.rs | 55 +++++++++++++++-- src/tools/miri/src/shims/foreign_items.rs | 37 ++++++++++- src/tools/miri/src/shims/mod.rs | 61 +------------------ ...romise_alignment.call_unaligned_ptr.stderr | 14 +++++ ...romise_alignment.read_unaligned_ptr.stderr | 15 +++++ .../unaligned_pointers/promise_alignment.rs | 54 ++++++++++++++++ .../miri/tests/pass/align_offset_symbolic.rs | 43 +++++-------- .../miri/tests/pass/panic/catch_panic.rs | 3 +- src/tools/miri/tests/utils/miri_extern.rs | 5 ++ 14 files changed, 298 insertions(+), 118 deletions(-) create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.call_unaligned_ptr.stderr create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.read_unaligned_ptr.stderr create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 6617f6f2ffb68..221ff2b60e9e1 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/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, @@ -143,11 +143,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 { + None + } /// Whether to enforce the validity invariant for a specific layout. fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool; @@ -519,12 +526,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 diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 1c1fd2a71ba1d..f865c0cc5fa6c 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -58,6 +58,7 @@ impl fmt::Display for MemoryKind { } /// 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 { diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 5b5284d8c4cd9..46e8676162a96 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -1368,10 +1368,36 @@ impl *const T { panic!("align_offset: align is not a power-of-two"); } - { - // SAFETY: `align` has been checked to be a power of 2 above - unsafe { align_offset(self, align) } + // SAFETY: `align` has been checked to be a power of 2 above + let ret = unsafe { align_offset(self, align) }; + + // Inform Miri that we want to consider the resulting pointer to be suitably aligned. + #[cfg(miri)] + if ret != usize::MAX { + fn runtime(ptr: *const (), align: usize) { + extern "Rust" { + pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); + } + + // SAFETY: this call is always safe. + unsafe { + miri_promise_symbolic_alignment(ptr, align); + } + } + + const fn compiletime(_ptr: *const (), _align: usize) {} + + // SAFETY: the extra behavior at runtime is for UB checks only. + unsafe { + intrinsics::const_eval_select( + (self.wrapping_add(ret).cast(), align), + compiletime, + runtime, + ); + } } + + ret } /// Returns whether the pointer is properly aligned for `T`. diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index a9208698a29e3..3f44d78795b6a 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -1635,10 +1635,36 @@ impl *mut T { panic!("align_offset: align is not a power-of-two"); } - { - // SAFETY: `align` has been checked to be a power of 2 above - unsafe { align_offset(self, align) } + // SAFETY: `align` has been checked to be a power of 2 above + let ret = unsafe { align_offset(self, align) }; + + // Inform Miri that we want to consider the resulting pointer to be suitably aligned. + #[cfg(miri)] + if ret != usize::MAX { + fn runtime(ptr: *const (), align: usize) { + extern "Rust" { + pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); + } + + // SAFETY: this call is always safe. + unsafe { + miri_promise_symbolic_alignment(ptr, align); + } + } + + const fn compiletime(_ptr: *const (), _align: usize) {} + + // SAFETY: the extra behavior at runtime is for UB checks only. + unsafe { + intrinsics::const_eval_select( + (self.wrapping_add(ret).cast_const().cast(), align), + compiletime, + runtime, + ); + } } + + ret } /// Returns whether the pointer is properly aligned for `T`. diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index a7b36fe7d2957..7d88f70efd172 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -3868,6 +3868,18 @@ impl [T] { } else { let (left, rest) = self.split_at(offset); let (us_len, ts_len) = rest.align_to_offsets::(); + // 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: this call is always safe. + unsafe { + miri_promise_symbolic_alignment(rest.as_ptr().cast(), mem::align_of::()); + } + } // 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 { @@ -3938,6 +3950,21 @@ impl [T] { let (us_len, ts_len) = rest.align_to_offsets::(); 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: this call is always safe. + unsafe { + miri_promise_symbolic_alignment( + mut_ptr.cast() as *const (), + mem::align_of::(), + ); + } + } // We can't use `rest` again after this, that would invalidate its alias `mut_ptr`! // SAFETY: see comments for `align_to`. unsafe { diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 66d7dfcf3a108..0826034d75bac 100644 --- a/src/tools/miri/src/machine.rs +++ b/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>>, + /// 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>, } impl VisitProvenance for AllocExtra<'_> { fn visit_provenance(&self, visit: &mut VisitWith<'_>) { - 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_provenance(visit); data_race.visit_provenance(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 { + 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), )?; diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 0957e72ee9104..3a0ff7a556724 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -467,7 +467,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)?; @@ -499,7 +499,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); @@ -556,6 +556,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)?; diff --git a/src/tools/miri/src/shims/mod.rs b/src/tools/miri/src/shims/mod.rs index a031a2a25c968..1e9d927e1a957 100644 --- a/src/tools/miri/src/shims/mod.rs +++ b/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, - 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) - } } diff --git a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.call_unaligned_ptr.stderr b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.call_unaligned_ptr.stderr new file mode 100644 index 0000000000000..e23ac5ac2fc5d --- /dev/null +++ b/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 1 previous error + diff --git a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.read_unaligned_ptr.stderr b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.read_unaligned_ptr.stderr new file mode 100644 index 0000000000000..0842ccd6d5bb6 --- /dev/null +++ b/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::().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 1 previous error + diff --git a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs new file mode 100644 index 0000000000000..d2d49c604afbb --- /dev/null +++ b/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::().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::().read() }; + //~[read_unaligned_ptr]^ ERROR: accessing memory based on pointer with alignment 8, but alignment 16 is required + } +} diff --git a/src/tools/miri/tests/pass/align_offset_symbolic.rs b/src/tools/miri/tests/pass/align_offset_symbolic.rs index 4e1584b838714..292858ebc2e53 100644 --- a/src/tools/miri/tests/pass/align_offset_symbolic.rs +++ b/src/tools/miri/tests/pass/align_offset_symbolic.rs @@ -1,29 +1,6 @@ //@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]); @@ -31,6 +8,8 @@ fn test_align_to() { 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::() }; assert_eq!(l.len(), 0); @@ -63,18 +42,21 @@ 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::() }; // 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::() }; + assert!(m.len() > 0); + // Ensure the symbolic alignment check has been informed that this is okay now. + let _val = m[0]; } } fn test_from_utf8() { + // uses `align_offset` internally const N: usize = 10; let vec = vec![0x4141414141414141u64; N]; let content = unsafe { std::slice::from_raw_parts(vec.as_ptr() as *const u8, 8 * N) }; @@ -103,9 +85,14 @@ fn test_u64_array() { example(&Data::default()); } +fn test_cstr() { + // uses `align_offset` internally + std::ffi::CStr::from_bytes_with_nul(b"this is a test that is longer than 16 bytes\0").unwrap(); +} + fn main() { - test_align_offset(); test_align_to(); test_from_utf8(); test_u64_array(); + test_cstr(); } diff --git a/src/tools/miri/tests/pass/panic/catch_panic.rs b/src/tools/miri/tests/pass/panic/catch_panic.rs index f5b4eaf685da8..b83902a8b19a4 100644 --- a/src/tools/miri/tests/pass/panic/catch_panic.rs +++ b/src/tools/miri/tests/pass/panic/catch_panic.rs @@ -1,5 +1,3 @@ -// We test the `align_offset` panic below, make sure we test the interpreter impl and not the "real" one. -//@compile-flags: -Zmiri-symbolic-alignment-check #![feature(never_type)] #![allow(unconditional_panic, non_fmt_panics)] @@ -70,6 +68,7 @@ fn main() { process::abort() }); + // Panic somewhere in the standard library. test(Some("align_offset: align is not a power-of-two"), |_old_val| { let _ = std::ptr::null::().align_offset(3); process::abort() diff --git a/src/tools/miri/tests/utils/miri_extern.rs b/src/tools/miri/tests/utils/miri_extern.rs index 7363c189840a8..ff7990561f2c9 100644 --- a/src/tools/miri/tests/utils/miri_extern.rs +++ b/src/tools/miri/tests/utils/miri_extern.rs @@ -142,4 +142,9 @@ extern "Rust" { /// but in tests we want to for sure run it at certain points to check /// that it doesn't break anything. pub fn miri_run_provenance_gc(); + + /// Miri-provided extern function to promise that a given pointer is properly aligned for + /// "symbolic" alignment checks. Will fail if the pointer is not actually aligned or `align` is + /// not a power of two. Has no effect when alignment checks are concrete (which is the default). + pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); } From 2a3fcc0a57d620f09207ab76671709eba8af8559 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 20 Nov 2023 08:22:56 +0100 Subject: [PATCH 2/2] move calling miri_promise_symbolic_alignment to a shared helper --- library/core/src/intrinsics.rs | 25 +++++++++++++++++++++++++ library/core/src/ptr/const_ptr.rs | 22 +--------------------- library/core/src/ptr/mut_ptr.rs | 25 ++++--------------------- library/core/src/slice/mod.rs | 31 ++++++++----------------------- 4 files changed, 38 insertions(+), 65 deletions(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 214c8e49a5adf..ab86e4e39ea54 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2859,3 +2859,28 @@ pub const unsafe fn write_bytes(dst: *mut T, val: u8, count: usize) { write_bytes(dst, val, count) } } + +/// Inform Miri that a given pointer definitely has a certain alignment. +#[cfg(miri)] +pub(crate) const fn miri_promise_symbolic_alignment(ptr: *const (), align: usize) { + extern "Rust" { + /// Miri-provided extern function to promise that a given pointer is properly aligned for + /// "symbolic" alignment checks. Will fail if the pointer is not actually aligned or `align` is + /// not a power of two. Has no effect when alignment checks are concrete (which is the default). + fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); + } + + fn runtime(ptr: *const (), align: usize) { + // SAFETY: this call is always safe. + unsafe { + miri_promise_symbolic_alignment(ptr, align); + } + } + + const fn compiletime(_ptr: *const (), _align: usize) {} + + // SAFETY: the extra behavior at runtime is for UB checks only. + unsafe { + const_eval_select((ptr, align), compiletime, runtime); + } +} diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 46e8676162a96..2f47ca29ec534 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -1374,27 +1374,7 @@ impl *const T { // Inform Miri that we want to consider the resulting pointer to be suitably aligned. #[cfg(miri)] if ret != usize::MAX { - fn runtime(ptr: *const (), align: usize) { - extern "Rust" { - pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); - } - - // SAFETY: this call is always safe. - unsafe { - miri_promise_symbolic_alignment(ptr, align); - } - } - - const fn compiletime(_ptr: *const (), _align: usize) {} - - // SAFETY: the extra behavior at runtime is for UB checks only. - unsafe { - intrinsics::const_eval_select( - (self.wrapping_add(ret).cast(), align), - compiletime, - runtime, - ); - } + intrinsics::miri_promise_symbolic_alignment(self.wrapping_add(ret).cast(), align); } ret diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 3f44d78795b6a..3aaae679a6f7c 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -1641,27 +1641,10 @@ impl *mut T { // Inform Miri that we want to consider the resulting pointer to be suitably aligned. #[cfg(miri)] if ret != usize::MAX { - fn runtime(ptr: *const (), align: usize) { - extern "Rust" { - pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); - } - - // SAFETY: this call is always safe. - unsafe { - miri_promise_symbolic_alignment(ptr, align); - } - } - - const fn compiletime(_ptr: *const (), _align: usize) {} - - // SAFETY: the extra behavior at runtime is for UB checks only. - unsafe { - intrinsics::const_eval_select( - (self.wrapping_add(ret).cast_const().cast(), align), - compiletime, - runtime, - ); - } + intrinsics::miri_promise_symbolic_alignment( + self.wrapping_add(ret).cast_const().cast(), + align, + ); } ret diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 7d88f70efd172..5957f9fd44395 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -3870,16 +3870,10 @@ impl [T] { let (us_len, ts_len) = rest.align_to_offsets::(); // 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: this call is always safe. - unsafe { - miri_promise_symbolic_alignment(rest.as_ptr().cast(), mem::align_of::()); - } - } + crate::intrinsics::miri_promise_symbolic_alignment( + rest.as_ptr().cast(), + mem::align_of::(), + ); // 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 { @@ -3952,19 +3946,10 @@ impl [T] { 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: this call is always safe. - unsafe { - miri_promise_symbolic_alignment( - mut_ptr.cast() as *const (), - mem::align_of::(), - ); - } - } + crate::intrinsics::miri_promise_symbolic_alignment( + mut_ptr.cast() as *const (), + mem::align_of::(), + ); // We can't use `rest` again after this, that would invalidate its alias `mut_ptr`! // SAFETY: see comments for `align_to`. unsafe {