Skip to content

Remove -Zunique-is-unique #4307

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

Merged
merged 1 commit into from
May 5, 2025
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
3 changes: 0 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,6 @@ to Miri failing to detect cases of undefined behavior in a program.
casts are not supported in this mode, but that may change in the future.
* `-Zmiri-force-page-size=<num>` overrides the default page size for an architecture, in multiples of 1k.
`4` is default for most targets. This value should always be a power of 2 and nonzero.
* `-Zmiri-unique-is-unique` performs additional aliasing checks for `core::ptr::Unique` to ensure
that it could theoretically be considered `noalias`. This flag is experimental and has
an effect only when used with `-Zmiri-tree-borrows`.

[function ABI]: https://doc.rust-lang.org/reference/items/functions.html#extern-function-qualifier

Expand Down
10 changes: 0 additions & 10 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,6 @@ fn main() {
} else if arg == "-Zmiri-tree-borrows" {
miri_config.borrow_tracker = Some(BorrowTrackerMethod::TreeBorrows);
miri_config.provenance_mode = ProvenanceMode::Strict;
} else if arg == "-Zmiri-unique-is-unique" {
miri_config.unique_is_unique = true;
} else if arg == "-Zmiri-disable-data-race-detector" {
miri_config.data_race_detector = false;
miri_config.weak_memory_emulation = false;
Expand Down Expand Up @@ -722,14 +720,6 @@ fn main() {
rustc_args.push(arg);
}
}
// `-Zmiri-unique-is-unique` should only be used with `-Zmiri-tree-borrows`.
if miri_config.unique_is_unique
&& !matches!(miri_config.borrow_tracker, Some(BorrowTrackerMethod::TreeBorrows))
{
show_error!(
"-Zmiri-unique-is-unique only has an effect when -Zmiri-tree-borrows is also used"
);
}
// Tree Borrows implies strict provenance, and is not compatible with native calls.
if matches!(miri_config.borrow_tracker, Some(BorrowTrackerMethod::TreeBorrows)) {
if miri_config.provenance_mode != ProvenanceMode::Strict {
Expand Down
5 changes: 0 additions & 5 deletions src/borrow_tracker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ pub struct GlobalStateInner {
tracked_pointer_tags: FxHashSet<BorTag>,
/// Whether to recurse into datatypes when searching for pointers to retag.
retag_fields: RetagFields,
/// Whether `core::ptr::Unique` gets special (`Box`-like) handling.
unique_is_unique: bool,
}

impl VisitProvenance for GlobalStateInner {
Expand Down Expand Up @@ -164,7 +162,6 @@ impl GlobalStateInner {
borrow_tracker_method: BorrowTrackerMethod,
tracked_pointer_tags: FxHashSet<BorTag>,
retag_fields: RetagFields,
unique_is_unique: bool,
) -> Self {
GlobalStateInner {
borrow_tracker_method,
Expand All @@ -173,7 +170,6 @@ impl GlobalStateInner {
protected_tags: FxHashMap::default(),
tracked_pointer_tags,
retag_fields,
unique_is_unique,
}
}

Expand Down Expand Up @@ -239,7 +235,6 @@ impl BorrowTrackerMethod {
self,
config.tracked_pointer_tags.clone(),
config.retag_fields,
config.unique_is_unique,
))
}
}
Expand Down
71 changes: 10 additions & 61 deletions src/borrow_tracker/tree_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use rustc_abi::{BackendRepr, Size};
use rustc_middle::mir::{Mutability, RetagKind};
use rustc_middle::ty::layout::HasTypingEnv;
use rustc_middle::ty::{self, Ty};
use rustc_span::def_id::DefId;

use crate::borrow_tracker::{GlobalState, GlobalStateInner, ProtectorKind};
use crate::concurrency::data_race::NaReadType;
Expand Down Expand Up @@ -115,9 +114,6 @@ impl<'tcx> Tree {
/// Policy for a new borrow.
#[derive(Debug, Clone, Copy)]
struct NewPermission {
/// Optionally ignore the actual size to do a zero-size reborrow.
/// If this is set then `dereferenceable` is not enforced.
zero_size: bool,
/// Which permission should the pointer start with.
initial_state: Permission,
/// Whether this pointer is part of the arguments of a function call.
Expand Down Expand Up @@ -157,7 +153,7 @@ impl<'tcx> NewPermission {
};

let protector = is_protected.then_some(ProtectorKind::StrongProtector);
Some(Self { zero_size: false, initial_state, protector, initial_read })
Some(Self { initial_state, protector, initial_read })
}

/// Compute permission for `Box`-like type (`Box` always, and also `Unique` if enabled).
Expand All @@ -167,7 +163,6 @@ impl<'tcx> NewPermission {
ty: Ty<'tcx>,
kind: RetagKind,
cx: &crate::MiriInterpCx<'tcx>,
zero_size: bool,
) -> Option<Self> {
let pointee = ty.builtin_deref(true).unwrap();
pointee.is_unpin(*cx.tcx, cx.typing_env()).then_some(()).map(|()| {
Expand All @@ -177,7 +172,6 @@ impl<'tcx> NewPermission {
let protected = kind == RetagKind::FnEntry;
let initial_state = Permission::new_reserved(ty_is_freeze, protected);
Self {
zero_size,
initial_state,
protector: protected.then_some(ProtectorKind::WeakProtector),
initial_read: true,
Expand Down Expand Up @@ -341,15 +335,12 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Determine the size of the reborrow.
// For most types this is the entire size of the place, however
// - when `extern type` is involved we use the size of the known prefix,
// - if the pointer is not reborrowed (raw pointer) or if `zero_size` is set
// then we override the size to do a zero-length reborrow.
let reborrow_size = match new_perm {
NewPermission { zero_size: false, .. } =>
this.size_and_align_of_mplace(place)?
.map(|(size, _)| size)
.unwrap_or(place.layout.size),
_ => Size::from_bytes(0),
};
// - if the pointer is not reborrowed (raw pointer) then we override the size
// to do a zero-length reborrow.
let reborrow_size = this
.size_and_align_of_mplace(place)?
.map(|(size, _)| size)
.unwrap_or(place.layout.size);
trace!("Creating new permission: {:?} with size {:?}", new_perm, reborrow_size);

// This new tag is not guaranteed to actually be used.
Expand Down Expand Up @@ -413,17 +404,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let this = self.eval_context_mut();
let options = this.machine.borrow_tracker.as_mut().unwrap().get_mut();
let retag_fields = options.retag_fields;
let unique_did =
options.unique_is_unique.then(|| this.tcx.lang_items().ptr_unique()).flatten();
let mut visitor = RetagVisitor { ecx: this, kind, retag_fields, unique_did };
let mut visitor = RetagVisitor { ecx: this, kind, retag_fields };
return visitor.visit_value(place);

// The actual visitor.
struct RetagVisitor<'ecx, 'tcx> {
ecx: &'ecx mut MiriInterpCx<'tcx>,
kind: RetagKind,
retag_fields: RetagFields,
unique_did: Option<DefId>,
}
impl<'ecx, 'tcx> RetagVisitor<'ecx, 'tcx> {
#[inline(always)] // yes this helps in our benchmarks
Expand Down Expand Up @@ -454,12 +442,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
fn visit_box(&mut self, box_ty: Ty<'tcx>, place: &PlaceTy<'tcx>) -> 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,
);
let new_perm =
NewPermission::from_unique_ty(place.layout.ty, self.kind, self.ecx);
self.retag_ptr_inplace(place, new_perm)?;
}
interp_ok(())
Expand Down Expand Up @@ -493,16 +477,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// even if field retagging is not enabled. *shrug*)
self.walk_value(place)?;
}
ty::Adt(adt, _) if self.unique_did == Some(adt.did()) => {
let place = inner_ptr_of_unique(self.ecx, place)?;
let new_perm = NewPermission::from_unique_ty(
place.layout.ty,
self.kind,
self.ecx,
/* zero_size */ true,
);
self.retag_ptr_inplace(&place, new_perm)?;
}
_ => {
// Not a reference/pointer/box. Only recurse if configured appropriately.
let recurse = match self.retag_fields {
Expand Down Expand Up @@ -541,7 +515,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Retag it. With protection! That is the entire point.
let new_perm = NewPermission {
initial_state: Permission::new_reserved(ty_is_freeze, /* protected */ true),
zero_size: false,
protector: Some(ProtectorKind::StrongProtector),
initial_read: true,
};
Expand Down Expand Up @@ -603,27 +576,3 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
tree_borrows.give_pointer_debug_name(tag, nth_parent, name)
}
}

/// Takes a place for a `Unique` and turns it into a place with the inner raw pointer.
/// I.e. input is what you get from the visitor upon encountering an `adt` that is `Unique`,
/// and output can be used by `retag_ptr_inplace`.
fn inner_ptr_of_unique<'tcx>(
ecx: &MiriInterpCx<'tcx>,
place: &PlaceTy<'tcx>,
) -> InterpResult<'tcx, PlaceTy<'tcx>> {
// Follows the same layout as `interpret/visitor.rs:walk_value` for `Box` in
// `rustc_const_eval`, just with one fewer layer.
// Here we have a `Unique(NonNull(*mut), PhantomData)`
assert_eq!(place.layout.fields.count(), 2, "Unique must have exactly 2 fields");
let (nonnull, phantom) = (ecx.project_field(place, 0)?, ecx.project_field(place, 1)?);
assert!(
phantom.layout.ty.ty_adt_def().is_some_and(|adt| adt.is_phantom_data()),
"2nd field of `Unique` should be `PhantomData` but is `{:?}`",
phantom.layout.ty,
);
// Now down to `NonNull(*mut)`
assert_eq!(nonnull.layout.fields.count(), 1, "NonNull must have exactly 1 field");
let ptr = ecx.project_field(&nonnull, 0)?;
// Finally a plain `*mut`
interp_ok(ptr)
}
5 changes: 0 additions & 5 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ pub struct MiriConfig {
pub validation: ValidationMode,
/// Determines if Stacked Borrows or Tree Borrows is enabled.
pub borrow_tracker: Option<BorrowTrackerMethod>,
/// Whether `core::ptr::Unique` receives special treatment.
/// If `true` then `Unique` is reborrowed with its own new tag and permission,
/// otherwise `Unique` is just another raw pointer.
pub unique_is_unique: bool,
/// Controls alignment checking.
pub check_alignment: AlignmentCheck,
/// Action for an op requiring communication with the host.
Expand Down Expand Up @@ -177,7 +173,6 @@ impl Default for MiriConfig {
env: vec![],
validation: ValidationMode::Shallow,
borrow_tracker: Some(BorrowTrackerMethod::StackedBorrows),
unique_is_unique: false,
check_alignment: AlignmentCheck::Int,
isolated_op: IsolatedOp::Reject(RejectOpWith::Abort),
ignore_leaks: false,
Expand Down
15 changes: 0 additions & 15 deletions tests/fail/tree_borrows/children-can-alias.default.stderr

This file was deleted.

58 changes: 0 additions & 58 deletions tests/fail/tree_borrows/children-can-alias.rs

This file was deleted.

31 changes: 0 additions & 31 deletions tests/fail/tree_borrows/children-can-alias.uniq.stderr

This file was deleted.

27 changes: 0 additions & 27 deletions tests/fail/tree_borrows/unique.rs

This file was deleted.

Loading