Skip to content

Commit

Permalink
miri: support 'promising' alignment for symbolic alignment check
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Nov 12, 2023
1 parent a04d56b commit 4668e3d
Show file tree
Hide file tree
Showing 11 changed files with 232 additions and 61 deletions.
29 changes: 15 additions & 14 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
27 changes: 27 additions & 0 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
55 changes: 51 additions & 4 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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),
)?;
Expand Down
37 changes: 35 additions & 2 deletions src/tools/miri/src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)?;
Expand Down
12 changes: 1 addition & 11 deletions src/tools/miri/src/shims/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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

54 changes: 54 additions & 0 deletions src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
Loading

0 comments on commit 4668e3d

Please sign in to comment.