Skip to content

Commit

Permalink
Rollup merge of #122647 - RalfJung:box-to-raw-retag, r=oli-obk
Browse files Browse the repository at this point in the history
add_retag: ensure box-to-raw-ptr casts are preserved for Miri

In rust-lang/rust#122233 I added `retag_box_to_raw` not realizing that we can already do `addr_of_mut!(*bx)` to turn a box into a raw pointer without an intermediate reference. We just need to ensure this information is preserved past the ElaborateBoxDerefs pass.

r? ``@oli-obk``
  • Loading branch information
matthiaskrgr authored Mar 18, 2024
2 parents 9f47db3 + 324b928 commit ec88165
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 65 deletions.
15 changes: 1 addition & 14 deletions 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, ty::Ty};
use rustc_middle::mir::RetagKind;
use rustc_target::abi::Size;

use crate::*;
Expand Down Expand Up @@ -291,19 +291,6 @@ 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
38 changes: 10 additions & 28 deletions src/borrow_tracker/stacked_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -865,24 +865,6 @@ 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 All @@ -891,9 +873,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let this = self.eval_context_mut();
let retag_fields = this.machine.borrow_tracker.as_mut().unwrap().get_mut().retag_fields;
let retag_cause = match kind {
RetagKind::Raw | RetagKind::TwoPhase { .. } => unreachable!(), // these can only happen in `retag_ptr_value`
RetagKind::TwoPhase { .. } => unreachable!(), // can only happen in `retag_ptr_value`
RetagKind::FnEntry => RetagCause::FnEntry,
RetagKind::Default => RetagCause::Normal,
RetagKind::Default | RetagKind::Raw => RetagCause::Normal,
};
let mut visitor =
RetagVisitor { ecx: this, kind, retag_cause, retag_fields, in_field: false };
Expand Down Expand Up @@ -959,14 +941,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

// Check the type of this value to see what to do with it (retag, or recurse).
match place.layout.ty.kind() {
ty::Ref(..) => {
let new_perm =
NewPermission::from_ref_ty(place.layout.ty, self.kind, self.ecx);
self.retag_ptr_inplace(place, new_perm)?;
}
ty::RawPtr(..) => {
// We do *not* want to recurse into raw pointers -- wide raw pointers have
// fields, and for dyn Trait pointees those can have reference type!
ty::Ref(..) | ty::RawPtr(..) => {
if matches!(place.layout.ty.kind(), ty::Ref(..))
|| self.kind == RetagKind::Raw
{
let new_perm =
NewPermission::from_ref_ty(place.layout.ty, self.kind, self.ecx);
self.retag_ptr_inplace(place, new_perm)?;
}
}
ty::Adt(adt, _) if adt.is_box() => {
// Recurse for boxes, they require some tricky handling and will end up in `visit_box` above.
Expand Down
9 changes: 0 additions & 9 deletions src/borrow_tracker/tree_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,15 +392,6 @@ 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
12 changes: 0 additions & 12 deletions src/shims/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,18 +140,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc)
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
--> $DIR/newtype_pair_retagging.rs:LL:CC
|
LL | let ptr = Box::into_raw(Box::new(0i32));
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/both_borrows/newtype_retagging.stack.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc)
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
--> $DIR/newtype_retagging.rs:LL:CC
|
LL | let ptr = Box::into_raw(Box::new(0i32));
Expand Down

0 comments on commit ec88165

Please sign in to comment.