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: do not apply aliasing restrictions to Box with custom allocator #122233

Merged
merged 1 commit into from
Mar 9, 2024
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
8 changes: 6 additions & 2 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use rustc_middle::mir::interpret::{
ExpectedKind, InterpError, InvalidMetaKind, Misalignment, PointerKind, Provenance,
ValidationErrorInfo, ValidationErrorKind, ValidationErrorKind::*,
};
use rustc_middle::ty;
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
use rustc_middle::ty::{self, Ty};
use rustc_span::symbol::{sym, Symbol};
use rustc_target::abi::{
Abi, FieldIdx, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange,
Expand Down Expand Up @@ -783,7 +783,11 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
}

#[inline]
fn visit_box(&mut self, op: &OpTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
fn visit_box(
&mut self,
_box_ty: Ty<'tcx>,
op: &OpTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx> {
self.check_safe_pointer(op, PointerKind::Box)?;
Ok(())
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_const_eval/src/interpret/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use rustc_index::IndexVec;
use rustc_middle::mir::interpret::InterpResult;
use rustc_middle::ty;
use rustc_middle::ty::{self, Ty};
use rustc_target::abi::FieldIdx;
use rustc_target::abi::{FieldsShape, VariantIdx, Variants};

Expand Down Expand Up @@ -47,10 +47,10 @@ pub trait ValueVisitor<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized {
Ok(())
}
/// Visits the given value as the pointer of a `Box`. There is nothing to recurse into.
/// The type of `v` will be a raw pointer, but this is a field of `Box<T>` and the
/// pointee type is the actual `T`.
/// The type of `v` will be a raw pointer to `T`, but this is a field of `Box<T>` and the
/// pointee type is the actual `T`. `box_ty` provides the full type of the `Box` itself.
#[inline(always)]
fn visit_box(&mut self, _v: &Self::V) -> InterpResult<'tcx> {
fn visit_box(&mut self, _box_ty: Ty<'tcx>, _v: &Self::V) -> InterpResult<'tcx> {
Ok(())
}

Expand Down Expand Up @@ -144,7 +144,7 @@ pub trait ValueVisitor<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized {
assert_eq!(nonnull_ptr.layout().fields.count(), 1);
let raw_ptr = self.ecx().project_field(&nonnull_ptr, 0)?; // the actual raw ptr
// ... whose only field finally is a raw ptr we can dereference.
self.visit_box(&raw_ptr)?;
self.visit_box(ty, &raw_ptr)?;

// The second `Box` field is the allocator, which we recursively check for validity
// like in regular structs.
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,10 @@ pub fn check_intrinsic_type(
sym::simd_shuffle => (3, 0, vec![param(0), param(0), param(1)], param(2)),
sym::simd_shuffle_generic => (2, 1, vec![param(0), param(0)], param(1)),

sym::retag_box_to_raw => {
(2, 0, vec![Ty::new_mut_ptr(tcx, param(0))], Ty::new_mut_ptr(tcx, param(0)))
}

other => {
tcx.dcx().emit_err(UnrecognizedIntrinsicFunction { span, name: other });
return;
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2008,13 +2008,10 @@ impl<'tcx> Ty<'tcx> {
// Single-argument Box is always global. (for "minicore" tests)
return true;
};
if let Some(alloc_adt) = alloc.expect_ty().ty_adt_def() {
alloc.expect_ty().ty_adt_def().is_some_and(|alloc_adt| {
let global_alloc = tcx.require_lang_item(LangItem::GlobalAlloc, None);
alloc_adt.did() == global_alloc
} else {
// Allocator is not an ADT...
false
}
})
}
_ => false,
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,7 @@ symbols! {
residual,
result,
resume,
retag_box_to_raw,
return_position_impl_trait_in_trait,
return_type_notation,
rhs,
Expand Down
24 changes: 14 additions & 10 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ use core::error::Error;
use core::fmt;
use core::future::Future;
use core::hash::{Hash, Hasher};
use core::intrinsics::retag_box_to_raw;
use core::iter::FusedIterator;
use core::marker::Tuple;
use core::marker::Unsize;
Expand Down Expand Up @@ -1110,8 +1111,16 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
#[unstable(feature = "allocator_api", issue = "32838")]
#[inline]
pub fn into_raw_with_allocator(b: Self) -> (*mut T, A) {
let (leaked, alloc) = Box::into_unique(b);
(leaked.as_ptr(), alloc)
// This is the transition point from `Box` to raw pointers. For Stacked Borrows, these casts
// are relevant -- if this is a global allocator Box and we just get the pointer from `b.0`,
// it will have `Unique` permission, which is not what we want from a raw pointer. We could
// fix that by going through `&mut`, but then if this is *not* a global allocator Box, we'd
// be adding uniqueness assertions that we do not want. So for Miri's sake we pass this
// pointer through an intrinsic for box-to-raw casts, which can do the right thing wrt the
// aliasing model.
let b = mem::ManuallyDrop::new(b);
let alloc = unsafe { ptr::read(&b.1) };
(unsafe { retag_box_to_raw::<T, A>(b.0.as_ptr()) }, alloc)
}

#[unstable(
Expand All @@ -1122,13 +1131,8 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
#[inline]
#[doc(hidden)]
pub fn into_unique(b: Self) -> (Unique<T>, A) {
// Box is recognized as a "unique pointer" by Stacked Borrows, but internally it is a
// raw pointer for the type system. Turning it directly into a raw pointer would not be
// recognized as "releasing" the unique pointer to permit aliased raw accesses,
// so all raw pointer methods have to go through `Box::leak`. Turning *that* to a raw pointer
// behaves correctly.
let alloc = unsafe { ptr::read(&b.1) };
(Unique::from(Box::leak(b)), alloc)
let (ptr, alloc) = Box::into_raw_with_allocator(b);
unsafe { (Unique::from(&mut *ptr), alloc) }
}

/// Returns a reference to the underlying allocator.
Expand Down Expand Up @@ -1184,7 +1188,7 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
where
A: 'a,
{
unsafe { &mut *mem::ManuallyDrop::new(b).0.as_ptr() }
unsafe { &mut *Box::into_raw(b) }
}

/// Converts a `Box<T>` into a `Pin<Box<T>>`. If `T` does not implement [`Unpin`], then
Expand Down
13 changes: 13 additions & 0 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2695,6 +2695,19 @@ pub unsafe fn vtable_size(_ptr: *const ()) -> usize {
unreachable!()
}

/// Retag a box pointer as part of casting it to a raw pointer. This is the `Box` equivalent of
/// `(x: &mut T) as *mut T`. The input pointer must be the pointer of a `Box` (passed as raw pointer
/// to avoid all questions around move semantics and custom allocators), and `A` must be the `Box`'s
/// allocator.
#[unstable(feature = "core_intrinsics", issue = "none")]
#[rustc_nounwind]
#[cfg_attr(not(bootstrap), rustc_intrinsic)]
#[cfg_attr(bootstrap, inline)]
pub unsafe fn retag_box_to_raw<T: ?Sized, A>(ptr: *mut T) -> *mut T {
// Miri needs to adjust the provenance, but for regular codegen this is not needed
ptr
}

// Some functions are defined here because they accidentally got made
// available in this module on stable. See <https://github.com/rust-lang/rust/issues/15702>.
// (`transmute` also falls into this category, but it cannot be wrapped due to the
Expand Down
15 changes: 14 additions & 1 deletion src/tools/miri/src/borrow_tracker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::num::NonZero;
use smallvec::SmallVec;

use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_middle::mir::RetagKind;
use rustc_middle::{mir::RetagKind, ty::Ty};
use rustc_target::abi::Size;

use crate::*;
Expand Down Expand Up @@ -291,6 +291,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
}

fn retag_box_to_raw(
&mut self,
val: &ImmTy<'tcx, Provenance>,
alloc_ty: Ty<'tcx>,
) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> {
let this = self.eval_context_mut();
let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method;
match method {
BorrowTrackerMethod::StackedBorrows => this.sb_retag_box_to_raw(val, alloc_ty),
BorrowTrackerMethod::TreeBorrows => this.tb_retag_box_to_raw(val, alloc_ty),
}
}

fn retag_place_contents(
&mut self,
kind: RetagKind,
Expand Down
34 changes: 30 additions & 4 deletions src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.sb_retag_reference(val, new_perm, RetagInfo { cause, in_field: false })
}

fn sb_retag_box_to_raw(
&mut self,
val: &ImmTy<'tcx, Provenance>,
alloc_ty: Ty<'tcx>,
) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> {
let this = self.eval_context_mut();
let is_global_alloc = alloc_ty.ty_adt_def().is_some_and(|adt| {
let global_alloc = this.tcx.require_lang_item(rustc_hir::LangItem::GlobalAlloc, None);
adt.did() == global_alloc
});
if is_global_alloc {
// Retag this as-if it was a mutable reference.
this.sb_retag_ptr_value(RetagKind::Raw, val)
} else {
Ok(val.clone())
}
}

fn sb_retag_place_contents(
&mut self,
kind: RetagKind,
Expand Down Expand Up @@ -916,10 +934,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
self.ecx
}

fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
// Boxes get a weak protectors, since they may be deallocated.
let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx);
self.retag_ptr_inplace(place, new_perm)
fn visit_box(
&mut self,
box_ty: Ty<'tcx>,
place: &PlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
// Only boxes for the global allocator get any special treatment.
if box_ty.is_box_global(*self.ecx.tcx) {
// Boxes get a weak protectors, since they may be deallocated.
let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx);
self.retag_ptr_inplace(place, new_perm)?;
}
Ok(())
}

fn visit_value(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
Expand Down
33 changes: 25 additions & 8 deletions src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
}

fn tb_retag_box_to_raw(
&mut self,
val: &ImmTy<'tcx, Provenance>,
_alloc_ty: Ty<'tcx>,
) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> {
// Casts to raw pointers are NOPs in Tree Borrows.
Ok(val.clone())
}

/// Retag all pointers that are stored in this place.
fn tb_retag_place_contents(
&mut self,
Expand Down Expand Up @@ -441,14 +450,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// Regardless of how `Unique` is handled, Boxes are always reborrowed.
/// When `Unique` is also reborrowed, then it behaves exactly like `Box`
/// except for the fact that `Box` has a non-zero-sized reborrow.
fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
let new_perm = NewPermission::from_unique_ty(
place.layout.ty,
self.kind,
self.ecx,
/* zero_size */ false,
);
self.retag_ptr_inplace(place, new_perm)
fn visit_box(
&mut self,
box_ty: Ty<'tcx>,
place: &PlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
// Only boxes for the global allocator get any special treatment.
if box_ty.is_box_global(*self.ecx.tcx) {
let new_perm = NewPermission::from_unique_ty(
place.layout.ty,
self.kind,
self.ecx,
/* zero_size */ false,
);
self.retag_ptr_inplace(place, new_perm)?;
}
Ok(())
}

fn visit_value(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
Expand Down
13 changes: 13 additions & 0 deletions src/tools/miri/src/shims/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.write_bytes_ptr(ptr, iter::repeat(val_byte).take(byte_count.bytes_usize()))?;
}

// Memory model / provenance manipulation
"ptr_mask" => {
let [ptr, mask] = check_arg_count(args)?;

Expand All @@ -139,6 +140,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

this.write_pointer(Pointer::new(ptr.provenance, masked_addr), dest)?;
}
"retag_box_to_raw" => {
let [ptr] = check_arg_count(args)?;
let alloc_ty = generic_args[1].expect_ty();

let val = this.read_immediate(ptr)?;
let new_val = if this.machine.borrow_tracker.is_some() {
this.retag_box_to_raw(&val, alloc_ty)?
} else {
val
};
this.write_immediate(*new_val, dest)?;
}

// We want to return either `true` or `false` at random, or else something like
// ```
Expand Down
Loading
Loading