Auto merge of #117840 - RalfJung:miri-promise-align, r=cjgillot

miri: support 'promising' alignment for symbolic alignment check

Then use that ability in `slice::align_to`, so that even with `-Zmiri-symbolic-alignment-check`, it no longer has to return spuriously empty "middle" parts.

Fixes https://github.com/rust-lang/miri/issues/3068
This commit is contained in:
bors 2023-12-03 22:53:44 +00:00
commit c9808f8702
15 changed files with 271 additions and 118 deletions

View File

@ -12,12 +12,12 @@
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<Misalignment> {
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 @@ fn after_local_allocated(
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

View File

@ -58,6 +58,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
}
/// 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 @@ fn offset_misalignment(offset: u64, align: Align) -> Option<Misalignment> {
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 {

View File

@ -2859,3 +2859,28 @@ pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, 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);
}
}

View File

@ -1368,10 +1368,16 @@ pub const fn align_offset(self, align: usize) -> usize
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`.

View File

@ -1635,10 +1635,19 @@ pub const fn align_offset(self, align: usize) -> usize
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`.

View File

@ -3868,6 +3868,12 @@ pub unsafe fn align_to<U>(&self) -> (&[T], &[U], &[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 {
@ -3938,6 +3944,12 @@ pub unsafe fn align_to_mut<U>(&mut self) -> (&mut [T], &mut [U], &mut [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 {

View File

@ -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 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 @@ fn enforce_alignment(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool {
}
#[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 @@ fn adjust_allocation<'b>(
data_race: race_alloc,
weak_memory: buffer_alloc,
backtrace,
symbolic_alignment: Cell::new(None),
},
|ptr| ecx.global_base_pointer(ptr),
)?;

View File

@ -467,7 +467,7 @@ fn emulate_foreign_item_inner(
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 @@ fn emulate_foreign_item_inner(
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 @@ fn emulate_foreign_item_inner(
};
}
// 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)?;

View File

@ -23,7 +23,6 @@
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 @@ fn find_mir_or_eval_fn(
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 @@ fn find_mir_or_eval_fn(
// 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)
}
}

View File

@ -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

View File

@ -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 1 previous error

View File

@ -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
}
}

View File

@ -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::<u32>() };
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::<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];
}
}
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 example(data: &Data) {
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();
}

View File

@ -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::<u8>().align_offset(3);
process::abort()

View File

@ -142,4 +142,9 @@ pub fn miri_host_to_target_path(
/// 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);
}