Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

miri: support 'promising' alignment for symbolic alignment check #117840

Merged
merged 2 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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<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 @@ -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
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
25 changes: 25 additions & 0 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2859,3 +2859,28 @@ pub const unsafe fn write_bytes<T>(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);
}
}
12 changes: 9 additions & 3 deletions library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1368,10 +1368,16 @@ impl<T: ?Sized> *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 {
intrinsics::miri_promise_symbolic_alignment(self.wrapping_add(ret).cast(), align);
}

ret
}

/// Returns whether the pointer is properly aligned for `T`.
Expand Down
15 changes: 12 additions & 3 deletions library/core/src/ptr/mut_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1635,10 +1635,19 @@ impl<T: ?Sized> *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 {
intrinsics::miri_promise_symbolic_alignment(
self.wrapping_add(ret).cast_const().cast(),
align,
);
}

ret
}

/// Returns whether the pointer is properly aligned for `T`.
Expand Down
12 changes: 12 additions & 0 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3868,6 +3868,12 @@ 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)]
crate::intrinsics::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 @@ -3938,6 +3944,12 @@ 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)]
crate::intrinsics::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 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);
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 @@ -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)?;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)?;
Expand Down
61 changes: 1 addition & 60 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 All @@ -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)
}
}
Loading
Loading