Skip to content

Commit bebba4f

Browse files
committed
miri: support 'promising' alignment for symbolic alignment check
1 parent 7ceaf19 commit bebba4f

File tree

14 files changed

+298
-118
lines changed

14 files changed

+298
-118
lines changed

compiler/rustc_const_eval/src/interpret/machine.rs

+15-14
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ use rustc_middle::mir;
1212
use rustc_middle::ty::layout::TyAndLayout;
1313
use rustc_middle::ty::{self, TyCtxt};
1414
use rustc_span::def_id::DefId;
15-
use rustc_target::abi::Size;
15+
use rustc_target::abi::{Align, Size};
1616
use rustc_target::spec::abi::Abi as CallAbi;
1717

1818
use super::{
19-
AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
20-
InterpResult, MPlaceTy, MemoryKind, OpTy, PlaceTy, Pointer, Provenance,
19+
AllocBytes, AllocId, AllocKind, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy,
20+
InterpCx, InterpResult, MPlaceTy, MemoryKind, Misalignment, OpTy, PlaceTy, Pointer, Provenance,
2121
};
2222

2323
/// Data returned by Machine::stack_pop,
@@ -143,11 +143,18 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
143143
/// Whether memory accesses should be alignment-checked.
144144
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
145145

146-
/// Whether, when checking alignment, we should look at the actual address and thus support
147-
/// custom alignment logic based on whatever the integer address happens to be.
148-
///
149-
/// If this returns true, Provenance::OFFSET_IS_ADDR must be true.
150-
fn use_addr_for_alignment_check(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
146+
/// Gives the machine a chance to detect more misalignment than the built-in checks would catch.
147+
#[inline(always)]
148+
fn alignment_check(
149+
_ecx: &InterpCx<'mir, 'tcx, Self>,
150+
_alloc_id: AllocId,
151+
_alloc_align: Align,
152+
_alloc_kind: AllocKind,
153+
_offset: Size,
154+
_align: Align,
155+
) -> Option<Misalignment> {
156+
None
157+
}
151158

152159
/// Whether to enforce the validity invariant for a specific layout.
153160
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>) {
519526
type FrameExtra = ();
520527
type Bytes = Box<[u8]>;
521528

522-
#[inline(always)]
523-
fn use_addr_for_alignment_check(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
524-
// We do not support `use_addr`.
525-
false
526-
}
527-
528529
#[inline(always)]
529530
fn ignore_optional_overflow_checks(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
530531
false

compiler/rustc_const_eval/src/interpret/memory.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ impl<T: fmt::Display> fmt::Display for MemoryKind<T> {
5858
}
5959

6060
/// The return value of `get_alloc_info` indicates the "kind" of the allocation.
61+
#[derive(Copy, Clone, PartialEq, Debug)]
6162
pub enum AllocKind {
6263
/// A regular live data allocation.
6364
LiveData,
@@ -473,8 +474,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
473474
match self.ptr_try_get_alloc_id(ptr) {
474475
Err(addr) => offset_misalignment(addr, align),
475476
Ok((alloc_id, offset, _prov)) => {
476-
let (_size, alloc_align, _kind) = self.get_alloc_info(alloc_id);
477-
if M::use_addr_for_alignment_check(self) {
477+
let (_size, alloc_align, kind) = self.get_alloc_info(alloc_id);
478+
if let Some(misalign) =
479+
M::alignment_check(self, alloc_id, alloc_align, kind, offset, align)
480+
{
481+
Some(misalign)
482+
} else if M::Provenance::OFFSET_IS_ADDR {
478483
// `use_addr_for_alignment_check` can only be true if `OFFSET_IS_ADDR` is true.
479484
offset_misalignment(ptr.addr().bytes(), align)
480485
} else {

library/core/src/ptr/const_ptr.rs

+29-3
Original file line numberDiff line numberDiff line change
@@ -1368,10 +1368,36 @@ impl<T: ?Sized> *const T {
13681368
panic!("align_offset: align is not a power-of-two");
13691369
}
13701370

1371-
{
1372-
// SAFETY: `align` has been checked to be a power of 2 above
1373-
unsafe { align_offset(self, align) }
1371+
// SAFETY: `align` has been checked to be a power of 2 above
1372+
let ret = unsafe { align_offset(self, align) };
1373+
1374+
// Inform Miri that we want to consider the resulting pointer to be suitably aligned.
1375+
#[cfg(miri)]
1376+
if ret != usize::MAX {
1377+
fn runtime(ptr: *const (), align: usize) {
1378+
extern "Rust" {
1379+
pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize);
1380+
}
1381+
1382+
// SAFETY: this call is always safe.
1383+
unsafe {
1384+
miri_promise_symbolic_alignment(ptr, align);
1385+
}
1386+
}
1387+
1388+
const fn compiletime(_ptr: *const (), _align: usize) {}
1389+
1390+
// SAFETY: the extra behavior at runtime is for UB checks only.
1391+
unsafe {
1392+
intrinsics::const_eval_select(
1393+
(self.wrapping_add(ret).cast(), align),
1394+
compiletime,
1395+
runtime,
1396+
);
1397+
}
13741398
}
1399+
1400+
ret
13751401
}
13761402

13771403
/// Returns whether the pointer is properly aligned for `T`.

library/core/src/ptr/mut_ptr.rs

+29-3
Original file line numberDiff line numberDiff line change
@@ -1635,10 +1635,36 @@ impl<T: ?Sized> *mut T {
16351635
panic!("align_offset: align is not a power-of-two");
16361636
}
16371637

1638-
{
1639-
// SAFETY: `align` has been checked to be a power of 2 above
1640-
unsafe { align_offset(self, align) }
1638+
// SAFETY: `align` has been checked to be a power of 2 above
1639+
let ret = unsafe { align_offset(self, align) };
1640+
1641+
// Inform Miri that we want to consider the resulting pointer to be suitably aligned.
1642+
#[cfg(miri)]
1643+
if ret != usize::MAX {
1644+
fn runtime(ptr: *const (), align: usize) {
1645+
extern "Rust" {
1646+
pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize);
1647+
}
1648+
1649+
// SAFETY: this call is always safe.
1650+
unsafe {
1651+
miri_promise_symbolic_alignment(ptr, align);
1652+
}
1653+
}
1654+
1655+
const fn compiletime(_ptr: *const (), _align: usize) {}
1656+
1657+
// SAFETY: the extra behavior at runtime is for UB checks only.
1658+
unsafe {
1659+
intrinsics::const_eval_select(
1660+
(self.wrapping_add(ret).cast_const().cast(), align),
1661+
compiletime,
1662+
runtime,
1663+
);
1664+
}
16411665
}
1666+
1667+
ret
16421668
}
16431669

16441670
/// Returns whether the pointer is properly aligned for `T`.

library/core/src/slice/mod.rs

+27
Original file line numberDiff line numberDiff line change
@@ -3868,6 +3868,18 @@ impl<T> [T] {
38683868
} else {
38693869
let (left, rest) = self.split_at(offset);
38703870
let (us_len, ts_len) = rest.align_to_offsets::<U>();
3871+
// Inform Miri that we want to consider the "middle" pointer to be suitably aligned.
3872+
#[cfg(miri)]
3873+
{
3874+
extern "Rust" {
3875+
pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize);
3876+
}
3877+
3878+
// SAFETY: this call is always safe.
3879+
unsafe {
3880+
miri_promise_symbolic_alignment(rest.as_ptr().cast(), mem::align_of::<U>());
3881+
}
3882+
}
38713883
// SAFETY: now `rest` is definitely aligned, so `from_raw_parts` below is okay,
38723884
// since the caller guarantees that we can transmute `T` to `U` safely.
38733885
unsafe {
@@ -3938,6 +3950,21 @@ impl<T> [T] {
39383950
let (us_len, ts_len) = rest.align_to_offsets::<U>();
39393951
let rest_len = rest.len();
39403952
let mut_ptr = rest.as_mut_ptr();
3953+
// Inform Miri that we want to consider the "middle" pointer to be suitably aligned.
3954+
#[cfg(miri)]
3955+
{
3956+
extern "Rust" {
3957+
pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize);
3958+
}
3959+
3960+
// SAFETY: this call is always safe.
3961+
unsafe {
3962+
miri_promise_symbolic_alignment(
3963+
mut_ptr.cast() as *const (),
3964+
mem::align_of::<U>(),
3965+
);
3966+
}
3967+
}
39413968
// We can't use `rest` again after this, that would invalidate its alias `mut_ptr`!
39423969
// SAFETY: see comments for `align_to`.
39433970
unsafe {

src/tools/miri/src/machine.rs

+51-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//! `Machine` trait.
33
44
use std::borrow::Cow;
5-
use std::cell::RefCell;
5+
use std::cell::{Cell, RefCell};
66
use std::fmt;
77
use std::path::Path;
88
use std::process;
@@ -309,11 +309,20 @@ pub struct AllocExtra<'tcx> {
309309
/// if this allocation is leakable. The backtrace is not
310310
/// pruned yet; that should be done before printing it.
311311
pub backtrace: Option<Vec<FrameInfo<'tcx>>>,
312+
/// An offset inside this allocation that was deemed aligned even for symbolic alignment checks.
313+
/// Invariant: the promised alignment will never be less than the native alignment of this allocation.
314+
pub symbolic_alignment: Cell<Option<(Size, Align)>>,
312315
}
313316

314317
impl VisitProvenance for AllocExtra<'_> {
315318
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
316-
let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self;
319+
let AllocExtra {
320+
borrow_tracker,
321+
data_race,
322+
weak_memory,
323+
backtrace: _,
324+
symbolic_alignment: _,
325+
} = self;
317326

318327
borrow_tracker.visit_provenance(visit);
319328
data_race.visit_provenance(visit);
@@ -907,8 +916,45 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
907916
}
908917

909918
#[inline(always)]
910-
fn use_addr_for_alignment_check(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool {
911-
ecx.machine.check_alignment == AlignmentCheck::Int
919+
fn alignment_check(
920+
ecx: &MiriInterpCx<'mir, 'tcx>,
921+
alloc_id: AllocId,
922+
alloc_align: Align,
923+
alloc_kind: AllocKind,
924+
offset: Size,
925+
align: Align,
926+
) -> Option<Misalignment> {
927+
if ecx.machine.check_alignment != AlignmentCheck::Symbolic {
928+
// Just use the built-in check.
929+
return None;
930+
}
931+
if alloc_kind != AllocKind::LiveData {
932+
// Can't have any extra info here.
933+
return None;
934+
}
935+
// Let's see which alignment we have been promised for this allocation.
936+
let alloc_info = ecx.get_alloc_extra(alloc_id).unwrap(); // cannot fail since the allocation is live
937+
let (promised_offset, promised_align) =
938+
alloc_info.symbolic_alignment.get().unwrap_or((Size::ZERO, alloc_align));
939+
if promised_align < align {
940+
// Definitely not enough.
941+
Some(Misalignment { has: promised_align, required: align })
942+
} else {
943+
// What's the offset between us and the promised alignment?
944+
let distance = offset.bytes().wrapping_sub(promised_offset.bytes());
945+
// That must also be aligned.
946+
if distance % align.bytes() == 0 {
947+
// All looking good!
948+
None
949+
} else {
950+
// The biggest power of two through which `distance` is divisible.
951+
let distance_pow2 = 1 << distance.trailing_zeros();
952+
Some(Misalignment {
953+
has: Align::from_bytes(distance_pow2).unwrap(),
954+
required: align,
955+
})
956+
}
957+
}
912958
}
913959

914960
#[inline(always)]
@@ -1112,6 +1158,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
11121158
data_race: race_alloc,
11131159
weak_memory: buffer_alloc,
11141160
backtrace,
1161+
symbolic_alignment: Cell::new(None),
11151162
},
11161163
|ptr| ecx.global_base_pointer(ptr),
11171164
)?;

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

+35-2
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
467467
let ptr = this.read_pointer(ptr)?;
468468
let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr).map_err(|_e| {
469469
err_machine_stop!(TerminationInfo::Abort(format!(
470-
"pointer passed to miri_get_alloc_id must not be dangling, got {ptr:?}"
470+
"pointer passed to `miri_get_alloc_id` must not be dangling, got {ptr:?}"
471471
)))
472472
})?;
473473
this.write_scalar(Scalar::from_u64(alloc_id.0.get()), dest)?;
@@ -499,7 +499,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
499499
let (alloc_id, offset, _) = this.ptr_get_alloc_id(ptr)?;
500500
if offset != Size::ZERO {
501501
throw_unsup_format!(
502-
"pointer passed to miri_static_root must point to beginning of an allocated block"
502+
"pointer passed to `miri_static_root` must point to beginning of an allocated block"
503503
);
504504
}
505505
this.machine.static_roots.push(alloc_id);
@@ -556,6 +556,39 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
556556
};
557557
}
558558

559+
// Promises that a pointer has a given symbolic alignment.
560+
"miri_promise_symbolic_alignment" => {
561+
let [ptr, align] = this.check_shim(abi, Abi::Rust, link_name, args)?;
562+
let ptr = this.read_pointer(ptr)?;
563+
let align = this.read_target_usize(align)?;
564+
let Ok(align) = Align::from_bytes(align) else {
565+
throw_unsup_format!(
566+
"`miri_promise_symbolic_alignment`: alignment must be a power of 2"
567+
);
568+
};
569+
let (_, addr) = ptr.into_parts(); // we know the offset is absolute
570+
if addr.bytes() % align.bytes() != 0 {
571+
throw_unsup_format!(
572+
"`miri_promise_symbolic_alignment`: pointer is not actually aligned"
573+
);
574+
}
575+
if let Ok((alloc_id, offset, ..)) = this.ptr_try_get_alloc_id(ptr) {
576+
let (_size, alloc_align, _kind) = this.get_alloc_info(alloc_id);
577+
// Not `get_alloc_extra_mut`, need to handle read-only allocations!
578+
let alloc_extra = this.get_alloc_extra(alloc_id)?;
579+
// If the newly promised alignment is bigger than the native alignment of this
580+
// allocation, and bigger than the previously promised alignment, then set it.
581+
if align > alloc_align
582+
&& !alloc_extra
583+
.symbolic_alignment
584+
.get()
585+
.is_some_and(|(_, old_align)| align <= old_align)
586+
{
587+
alloc_extra.symbolic_alignment.set(Some((offset, align)));
588+
}
589+
}
590+
}
591+
559592
// Standard C allocation
560593
"malloc" => {
561594
let [size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;

0 commit comments

Comments
 (0)