Skip to content

Commit 654acda

Browse files
authored
Unrolled build for rust-lang#122233
Rollup merge of rust-lang#122233 - RalfJung:custom-alloc-box, r=oli-obk miri: do not apply aliasing restrictions to Box with custom allocator This is the Miri side of rust-lang#122018. The "intrinsics with body" made this much more pleasant. :) Fixes rust-lang/miri#3341. r? `@oli-obk`
2 parents b054da8 + e632e3f commit 654acda

File tree

12 files changed

+250
-35
lines changed

12 files changed

+250
-35
lines changed

compiler/rustc_const_eval/src/interpret/validity.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use rustc_middle::mir::interpret::{
1717
ExpectedKind, InterpError, InvalidMetaKind, Misalignment, PointerKind, Provenance,
1818
ValidationErrorInfo, ValidationErrorKind, ValidationErrorKind::*,
1919
};
20-
use rustc_middle::ty;
2120
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
21+
use rustc_middle::ty::{self, Ty};
2222
use rustc_span::symbol::{sym, Symbol};
2323
use rustc_target::abi::{
2424
Abi, FieldIdx, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange,
@@ -783,7 +783,11 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
783783
}
784784

785785
#[inline]
786-
fn visit_box(&mut self, op: &OpTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
786+
fn visit_box(
787+
&mut self,
788+
_box_ty: Ty<'tcx>,
789+
op: &OpTy<'tcx, M::Provenance>,
790+
) -> InterpResult<'tcx> {
787791
self.check_safe_pointer(op, PointerKind::Box)?;
788792
Ok(())
789793
}

compiler/rustc_const_eval/src/interpret/visitor.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
44
use rustc_index::IndexVec;
55
use rustc_middle::mir::interpret::InterpResult;
6-
use rustc_middle::ty;
6+
use rustc_middle::ty::{self, Ty};
77
use rustc_target::abi::FieldIdx;
88
use rustc_target::abi::{FieldsShape, VariantIdx, Variants};
99

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

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

149149
// The second `Box` field is the allocator, which we recursively check for validity
150150
// like in regular structs.

compiler/rustc_hir_analysis/src/check/intrinsic.rs

+4
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,10 @@ pub fn check_intrinsic_type(
657657
sym::simd_shuffle => (3, 0, vec![param(0), param(0), param(1)], param(2)),
658658
sym::simd_shuffle_generic => (2, 1, vec![param(0), param(0)], param(1)),
659659

660+
sym::retag_box_to_raw => {
661+
(2, 0, vec![Ty::new_mut_ptr(tcx, param(0))], Ty::new_mut_ptr(tcx, param(0)))
662+
}
663+
660664
other => {
661665
tcx.dcx().emit_err(UnrecognizedIntrinsicFunction { span, name: other });
662666
return;

compiler/rustc_middle/src/ty/sty.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -2008,13 +2008,10 @@ impl<'tcx> Ty<'tcx> {
20082008
// Single-argument Box is always global. (for "minicore" tests)
20092009
return true;
20102010
};
2011-
if let Some(alloc_adt) = alloc.expect_ty().ty_adt_def() {
2011+
alloc.expect_ty().ty_adt_def().is_some_and(|alloc_adt| {
20122012
let global_alloc = tcx.require_lang_item(LangItem::GlobalAlloc, None);
20132013
alloc_adt.did() == global_alloc
2014-
} else {
2015-
// Allocator is not an ADT...
2016-
false
2017-
}
2014+
})
20182015
}
20192016
_ => false,
20202017
}

compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1450,6 +1450,7 @@ symbols! {
14501450
residual,
14511451
result,
14521452
resume,
1453+
retag_box_to_raw,
14531454
return_position_impl_trait_in_trait,
14541455
return_type_notation,
14551456
rhs,

library/alloc/src/boxed.rs

+14-10
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ use core::error::Error;
155155
use core::fmt;
156156
use core::future::Future;
157157
use core::hash::{Hash, Hasher};
158+
use core::intrinsics::retag_box_to_raw;
158159
use core::iter::FusedIterator;
159160
use core::marker::Tuple;
160161
use core::marker::Unsize;
@@ -1110,8 +1111,16 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
11101111
#[unstable(feature = "allocator_api", issue = "32838")]
11111112
#[inline]
11121113
pub fn into_raw_with_allocator(b: Self) -> (*mut T, A) {
1113-
let (leaked, alloc) = Box::into_unique(b);
1114-
(leaked.as_ptr(), alloc)
1114+
// This is the transition point from `Box` to raw pointers. For Stacked Borrows, these casts
1115+
// are relevant -- if this is a global allocator Box and we just get the pointer from `b.0`,
1116+
// it will have `Unique` permission, which is not what we want from a raw pointer. We could
1117+
// fix that by going through `&mut`, but then if this is *not* a global allocator Box, we'd
1118+
// be adding uniqueness assertions that we do not want. So for Miri's sake we pass this
1119+
// pointer through an intrinsic for box-to-raw casts, which can do the right thing wrt the
1120+
// aliasing model.
1121+
let b = mem::ManuallyDrop::new(b);
1122+
let alloc = unsafe { ptr::read(&b.1) };
1123+
(unsafe { retag_box_to_raw::<T, A>(b.0.as_ptr()) }, alloc)
11151124
}
11161125

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

11341138
/// Returns a reference to the underlying allocator.
@@ -1184,7 +1188,7 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
11841188
where
11851189
A: 'a,
11861190
{
1187-
unsafe { &mut *mem::ManuallyDrop::new(b).0.as_ptr() }
1191+
unsafe { &mut *Box::into_raw(b) }
11881192
}
11891193

11901194
/// Converts a `Box<T>` into a `Pin<Box<T>>`. If `T` does not implement [`Unpin`], then

library/core/src/intrinsics.rs

+13
Original file line numberDiff line numberDiff line change
@@ -2695,6 +2695,19 @@ pub unsafe fn vtable_size(_ptr: *const ()) -> usize {
26952695
unreachable!()
26962696
}
26972697

2698+
/// Retag a box pointer as part of casting it to a raw pointer. This is the `Box` equivalent of
2699+
/// `(x: &mut T) as *mut T`. The input pointer must be the pointer of a `Box` (passed as raw pointer
2700+
/// to avoid all questions around move semantics and custom allocators), and `A` must be the `Box`'s
2701+
/// allocator.
2702+
#[unstable(feature = "core_intrinsics", issue = "none")]
2703+
#[rustc_nounwind]
2704+
#[cfg_attr(not(bootstrap), rustc_intrinsic)]
2705+
#[cfg_attr(bootstrap, inline)]
2706+
pub unsafe fn retag_box_to_raw<T: ?Sized, A>(ptr: *mut T) -> *mut T {
2707+
// Miri needs to adjust the provenance, but for regular codegen this is not needed
2708+
ptr
2709+
}
2710+
26982711
// Some functions are defined here because they accidentally got made
26992712
// available in this module on stable. See <https://github.com/rust-lang/rust/issues/15702>.
27002713
// (`transmute` also falls into this category, but it cannot be wrapped due to the

src/tools/miri/src/borrow_tracker/mod.rs

+14-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::num::NonZero;
55
use smallvec::SmallVec;
66

77
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
8-
use rustc_middle::mir::RetagKind;
8+
use rustc_middle::{mir::RetagKind, ty::Ty};
99
use rustc_target::abi::Size;
1010

1111
use crate::*;
@@ -291,6 +291,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
291291
}
292292
}
293293

294+
fn retag_box_to_raw(
295+
&mut self,
296+
val: &ImmTy<'tcx, Provenance>,
297+
alloc_ty: Ty<'tcx>,
298+
) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> {
299+
let this = self.eval_context_mut();
300+
let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method;
301+
match method {
302+
BorrowTrackerMethod::StackedBorrows => this.sb_retag_box_to_raw(val, alloc_ty),
303+
BorrowTrackerMethod::TreeBorrows => this.tb_retag_box_to_raw(val, alloc_ty),
304+
}
305+
}
306+
294307
fn retag_place_contents(
295308
&mut self,
296309
kind: RetagKind,

src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

+30-4
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
865865
this.sb_retag_reference(val, new_perm, RetagInfo { cause, in_field: false })
866866
}
867867

868+
fn sb_retag_box_to_raw(
869+
&mut self,
870+
val: &ImmTy<'tcx, Provenance>,
871+
alloc_ty: Ty<'tcx>,
872+
) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> {
873+
let this = self.eval_context_mut();
874+
let is_global_alloc = alloc_ty.ty_adt_def().is_some_and(|adt| {
875+
let global_alloc = this.tcx.require_lang_item(rustc_hir::LangItem::GlobalAlloc, None);
876+
adt.did() == global_alloc
877+
});
878+
if is_global_alloc {
879+
// Retag this as-if it was a mutable reference.
880+
this.sb_retag_ptr_value(RetagKind::Raw, val)
881+
} else {
882+
Ok(val.clone())
883+
}
884+
}
885+
868886
fn sb_retag_place_contents(
869887
&mut self,
870888
kind: RetagKind,
@@ -916,10 +934,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
916934
self.ecx
917935
}
918936

919-
fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
920-
// Boxes get a weak protectors, since they may be deallocated.
921-
let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx);
922-
self.retag_ptr_inplace(place, new_perm)
937+
fn visit_box(
938+
&mut self,
939+
box_ty: Ty<'tcx>,
940+
place: &PlaceTy<'tcx, Provenance>,
941+
) -> InterpResult<'tcx> {
942+
// Only boxes for the global allocator get any special treatment.
943+
if box_ty.is_box_global(*self.ecx.tcx) {
944+
// Boxes get a weak protectors, since they may be deallocated.
945+
let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx);
946+
self.retag_ptr_inplace(place, new_perm)?;
947+
}
948+
Ok(())
923949
}
924950

925951
fn visit_value(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

+25-8
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
392392
}
393393
}
394394

395+
fn tb_retag_box_to_raw(
396+
&mut self,
397+
val: &ImmTy<'tcx, Provenance>,
398+
_alloc_ty: Ty<'tcx>,
399+
) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> {
400+
// Casts to raw pointers are NOPs in Tree Borrows.
401+
Ok(val.clone())
402+
}
403+
395404
/// Retag all pointers that are stored in this place.
396405
fn tb_retag_place_contents(
397406
&mut self,
@@ -441,14 +450,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
441450
/// Regardless of how `Unique` is handled, Boxes are always reborrowed.
442451
/// When `Unique` is also reborrowed, then it behaves exactly like `Box`
443452
/// except for the fact that `Box` has a non-zero-sized reborrow.
444-
fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
445-
let new_perm = NewPermission::from_unique_ty(
446-
place.layout.ty,
447-
self.kind,
448-
self.ecx,
449-
/* zero_size */ false,
450-
);
451-
self.retag_ptr_inplace(place, new_perm)
453+
fn visit_box(
454+
&mut self,
455+
box_ty: Ty<'tcx>,
456+
place: &PlaceTy<'tcx, Provenance>,
457+
) -> InterpResult<'tcx> {
458+
// Only boxes for the global allocator get any special treatment.
459+
if box_ty.is_box_global(*self.ecx.tcx) {
460+
let new_perm = NewPermission::from_unique_ty(
461+
place.layout.ty,
462+
self.kind,
463+
self.ecx,
464+
/* zero_size */ false,
465+
);
466+
self.retag_ptr_inplace(place, new_perm)?;
467+
}
468+
Ok(())
452469
}
453470

454471
fn visit_value(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {

src/tools/miri/src/shims/intrinsics/mod.rs

+13
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
129129
this.write_bytes_ptr(ptr, iter::repeat(val_byte).take(byte_count.bytes_usize()))?;
130130
}
131131

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

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

140141
this.write_pointer(Pointer::new(ptr.provenance, masked_addr), dest)?;
141142
}
143+
"retag_box_to_raw" => {
144+
let [ptr] = check_arg_count(args)?;
145+
let alloc_ty = generic_args[1].expect_ty();
146+
147+
let val = this.read_immediate(ptr)?;
148+
let new_val = if this.machine.borrow_tracker.is_some() {
149+
this.retag_box_to_raw(&val, alloc_ty)?
150+
} else {
151+
val
152+
};
153+
this.write_immediate(*new_val, dest)?;
154+
}
142155

143156
// We want to return either `true` or `false` at random, or else something like
144157
// ```

0 commit comments

Comments
 (0)