diff --git a/README.md b/README.md index 79b0daf9e8..daf13984fb 100644 --- a/README.md +++ b/README.md @@ -435,6 +435,9 @@ to Miri failing to detect cases of undefined behavior in a program. so with this flag. * `-Zmiri-force-page-size=` 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 diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 083dd4800d..8a51a7e663 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -340,6 +340,8 @@ fn main() { miri_config.borrow_tracker = None; } else if arg == "-Zmiri-tree-borrows" { miri_config.borrow_tracker = Some(BorrowTrackerMethod::TreeBorrows); + } 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; @@ -557,6 +559,14 @@ 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" + ); + } debug!("rustc arguments: {:?}", rustc_args); debug!("crate arguments: {:?}", miri_config.args); diff --git a/src/borrow_tracker/mod.rs b/src/borrow_tracker/mod.rs index ffc49eedb5..faa23fd262 100644 --- a/src/borrow_tracker/mod.rs +++ b/src/borrow_tracker/mod.rs @@ -103,6 +103,8 @@ pub struct GlobalStateInner { pub tracked_call_ids: FxHashSet, /// Whether to recurse into datatypes when searching for pointers to retag. pub retag_fields: RetagFields, + /// Whether `core::ptr::Unique` gets special (`Box`-like) handling. + pub unique_is_unique: bool, } impl VisitTags for GlobalStateInner { @@ -170,6 +172,7 @@ impl GlobalStateInner { tracked_pointer_tags: FxHashSet, tracked_call_ids: FxHashSet, retag_fields: RetagFields, + unique_is_unique: bool, ) -> Self { GlobalStateInner { borrow_tracker_method, @@ -180,6 +183,7 @@ impl GlobalStateInner { tracked_pointer_tags, tracked_call_ids, retag_fields, + unique_is_unique, } } @@ -244,6 +248,7 @@ impl BorrowTrackerMethod { config.tracked_pointer_tags.clone(), config.tracked_call_ids.clone(), config.retag_fields, + config.unique_is_unique, )) } } diff --git a/src/borrow_tracker/tree_borrows/mod.rs b/src/borrow_tracker/tree_borrows/mod.rs index 1c7a735779..c4fc2fea74 100644 --- a/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/borrow_tracker/tree_borrows/mod.rs @@ -11,6 +11,7 @@ use rustc_middle::{ Ty, }, }; +use rustc_span::def_id::DefId; use crate::*; @@ -99,9 +100,9 @@ impl<'tcx> Tree { /// Policy for a new borrow. #[derive(Debug, Clone, Copy)] struct NewPermission { - /// Whether this borrow requires a read access on its parent. - /// `perform_read_access` is `true` for all pointers marked `dereferenceable`. - perform_read_access: bool, + /// 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. @@ -128,20 +129,19 @@ impl<'tcx> NewPermission { // `&`s, which are excluded above. _ => return None, }; - // This field happens to be redundant since right now we always do a read, - // but it could be useful in the future. - let perform_read_access = true; let protector = (kind == RetagKind::FnEntry).then_some(ProtectorKind::StrongProtector); - Some(Self { perform_read_access, initial_state, protector }) + Some(Self { zero_size: false, initial_state, protector }) } - // Boxes are not handled by `from_ref_ty`, they need special behavior - // implemented here. - fn from_box_ty( + /// Compute permission for `Box`-like type (`Box` always, and also `Unique` if enabled). + /// These pointers allow deallocation so need a different kind of protector not handled + /// by `from_ref_ty`. + fn from_unique_ty( ty: Ty<'tcx>, kind: RetagKind, cx: &crate::MiriInterpCx<'_, 'tcx>, + zero_size: bool, ) -> Option { let pointee = ty.builtin_deref(true).unwrap().ty; pointee.is_unpin(*cx.tcx, cx.param_env()).then_some(()).map(|()| { @@ -149,7 +149,7 @@ impl<'tcx> NewPermission { // because it is valid to deallocate it within the function. let ty_is_freeze = ty.is_freeze(*cx.tcx, cx.param_env()); Self { - perform_read_access: true, + zero_size, initial_state: Permission::new_unique_2phase(ty_is_freeze), protector: (kind == RetagKind::FnEntry).then_some(ProtectorKind::WeakProtector), } @@ -201,6 +201,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' Ok(()) }; + trace!("Reborrow of size {:?}", ptr_size); let (alloc_id, base_offset, parent_prov) = if ptr_size > Size::ZERO { this.ptr_get_alloc_id(place.ptr)? } else { @@ -276,8 +277,8 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let range = alloc_range(base_offset, ptr_size); let mut tree_borrows = alloc_extra.borrow_tracker_tb().borrow_mut(); - if new_perm.perform_read_access { - // Count this reborrow as a read access + // All reborrows incur a (possibly zero-sized) read access to the parent + { let global = &this.machine.borrow_tracker.as_ref().unwrap(); let span = this.machine.current_span(); tree_borrows.perform_access( @@ -308,12 +309,19 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // We want a place for where the ptr *points to*, so we get one. let place = this.ref_to_mplace(val)?; - // Get a lower bound of the size of this place. - // (When `extern type` are involved, use the size of the known prefix.) - let size = this - .size_and_align_of_mplace(&place)? - .map(|(size, _)| size) - .unwrap_or(place.layout.size); + // 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 { + Some(NewPermission { zero_size: false, .. }) => + this.size_and_align_of_mplace(&place)? + .map(|(size, _)| size) + .unwrap_or(place.layout.size), + _ => Size::from_bytes(0), + }; + trace!("Creating new permission: {:?} with size {:?}", new_perm, reborrow_size); // This new tag is not guaranteed to actually be used. // @@ -324,7 +332,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let new_tag = this.machine.borrow_tracker.as_mut().unwrap().get_mut().new_ptr(); // Compute the actual reborrow. - let reborrowed = this.tb_reborrow(&place, size, new_perm, new_tag)?; + let reborrowed = this.tb_reborrow(&place, reborrow_size, new_perm, new_tag)?; // Adjust pointer. let new_place = place.map_provenance(|p| { @@ -359,10 +367,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { val: &ImmTy<'tcx, Provenance>, ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); - let new_perm = if let &ty::Ref(_, pointee, mutability) = val.layout.ty.kind() { - NewPermission::from_ref_ty(pointee, mutability, kind, this) - } else { - None + let new_perm = match val.layout.ty.kind() { + &ty::Ref(_, pointee, mutability) => + NewPermission::from_ref_ty(pointee, mutability, kind, this), + _ => None, }; this.tb_retag_reference(val, new_perm) } @@ -374,8 +382,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { place: &PlaceTy<'tcx, Provenance>, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let retag_fields = this.machine.borrow_tracker.as_mut().unwrap().get_mut().retag_fields; - let mut visitor = RetagVisitor { ecx: this, kind, retag_fields }; + 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 }; return visitor.visit_value(place); // The actual visitor. @@ -383,6 +394,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ecx: &'ecx mut MiriInterpCx<'mir, 'tcx>, kind: RetagKind, retag_fields: RetagFields, + unique_did: Option, } impl<'ecx, 'mir, 'tcx> RetagVisitor<'ecx, 'mir, 'tcx> { #[inline(always)] // yes this helps in our benchmarks @@ -407,8 +419,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { self.ecx } + /// 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_box_ty(place.layout.ty, self.kind, self.ecx); + let new_perm = NewPermission::from_unique_ty( + place.layout.ty, + self.kind, + self.ecx, + /* zero_size */ false, + ); self.retag_ptr_inplace(place, new_perm) } @@ -440,6 +460,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, '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 { @@ -456,7 +486,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } } - Ok(()) } } @@ -486,7 +515,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // FIXME: do we truly want a 2phase borrow here? let new_perm = Some(NewPermission { initial_state: Permission::new_unique_2phase(/*freeze*/ false), - perform_read_access: true, + zero_size: false, protector: Some(ProtectorKind::StrongProtector), }); let val = this.tb_retag_reference(&val, new_perm)?; @@ -552,3 +581,27 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, '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: &mut MiriInterpCx<'_, 'tcx>, + place: &PlaceTy<'tcx, Provenance>, +) -> InterpResult<'tcx, PlaceTy<'tcx, Provenance>> { + // 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.place_field(place, 0)?, ecx.place_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.place_field(&nonnull, 0)?; + // Finally a plain `*mut` + Ok(ptr) +} diff --git a/src/eval.rs b/src/eval.rs index 1e9d48be65..91e004e010 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -90,6 +90,10 @@ pub struct MiriConfig { pub validate: bool, /// Determines if Stacked Borrows or Tree Borrows is enabled. pub borrow_tracker: Option, + /// 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, /// Controls function [ABI](Abi) checking. @@ -156,6 +160,7 @@ impl Default for MiriConfig { env: vec![], validate: true, borrow_tracker: Some(BorrowTrackerMethod::StackedBorrows), + unique_is_unique: false, check_alignment: AlignmentCheck::Int, check_abi: true, isolated_op: IsolatedOp::Reject(RejectOpWith::Abort), diff --git a/tests/fail/tree_borrows/children-can-alias.default.stderr b/tests/fail/tree_borrows/children-can-alias.default.stderr new file mode 100644 index 0000000000..1b05be9c54 --- /dev/null +++ b/tests/fail/tree_borrows/children-can-alias.default.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: entering unreachable code + --> $DIR/children-can-alias.rs:LL:CC + | +LL | std::hint::unreachable_unchecked(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ entering unreachable code + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `main` at $DIR/children-can-alias.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/tests/fail/tree_borrows/children-can-alias.rs b/tests/fail/tree_borrows/children-can-alias.rs new file mode 100644 index 0000000000..b5a01cd432 --- /dev/null +++ b/tests/fail/tree_borrows/children-can-alias.rs @@ -0,0 +1,59 @@ +//@revisions: default uniq +//@compile-flags: -Zmiri-tree-borrows +//@[uniq]compile-flags: -Zmiri-unique-is-unique + +//! This is NOT intended behavior. +//! We should eventually find a solution so that the version with `Unique` passes too, +//! otherwise `Unique` is more strict than `&mut`! + +#![feature(ptr_internals)] + +use core::ptr::addr_of_mut; +use core::ptr::Unique; + +fn main() { + let mut data = 0u8; + let raw = addr_of_mut!(data); + unsafe { + raw_children_of_refmut_can_alias(&mut *raw); + raw_children_of_unique_can_alias(Unique::new_unchecked(raw)); + + // Ultimately the intended behavior is that both above tests would + // succeed. + std::hint::unreachable_unchecked(); + //~[default]^ ERROR: entering unreachable code + } +} + +unsafe fn raw_children_of_refmut_can_alias(x: &mut u8) { + let child1 = addr_of_mut!(*x); + let child2 = addr_of_mut!(*x); + // We create two raw aliases of `x`: they have the exact same + // tag and can be used interchangeably. + child1.write(1); + child2.write(2); + child1.write(1); + child2.write(2); +} + +unsafe fn raw_children_of_unique_can_alias(x: Unique) { + let child1 = x.as_ptr(); + let child2 = x.as_ptr(); + // Under `-Zmiri-unique-is-unique`, `Unique` accidentally offers more guarantees + // than `&mut`. Not because it responds differently to accesses but because + // there is no easy way to obtain a copy with the same tag. + // + // The closest (non-hack) attempt is two calls to `as_ptr`. + // - Without `-Zmiri-unique-is-unique`, independent `as_ptr` calls return pointers + // with the same tag that can thus be used interchangeably. + // - With the current implementation of `-Zmiri-unique-is-unique`, they return cousin + // tags with permissions that do not tolerate aliasing. + // Eventually we should make such aliasing allowed in some situations + // (e.g. when there is no protector), which will probably involve + // introducing a new kind of permission. + child1.write(1); + child2.write(2); + //~[uniq]^ ERROR: /write access through .* is forbidden/ + child1.write(1); + child2.write(2); +} diff --git a/tests/fail/tree_borrows/children-can-alias.uniq.stderr b/tests/fail/tree_borrows/children-can-alias.uniq.stderr new file mode 100644 index 0000000000..2f77b27729 --- /dev/null +++ b/tests/fail/tree_borrows/children-can-alias.uniq.stderr @@ -0,0 +1,37 @@ +error: Undefined Behavior: write access through is forbidden + --> $DIR/children-can-alias.rs:LL:CC + | +LL | child2.write(2); + | ^^^^^^^^^^^^^^^ write access through is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag is a child of the conflicting tag + = help: the conflicting tag has state Disabled which forbids this child write access +help: the accessed tag was created here + --> $DIR/children-can-alias.rs:LL:CC + | +LL | let child2 = x.as_ptr(); + | ^^^^^^^^^^ +help: the conflicting tag was created here, in the initial state Reserved + --> $DIR/children-can-alias.rs:LL:CC + | +LL | let child2 = x.as_ptr(); + | ^ +help: the conflicting tag later transitioned to Disabled due to a foreign write access at offsets [0x0..0x1] + --> $DIR/children-can-alias.rs:LL:CC + | +LL | child1.write(1); + | ^^^^^^^^^^^^^^^ + = help: this transition corresponds to a loss of read and write permissions + = note: BACKTRACE (of the first span): + = note: inside `raw_children_of_unique_can_alias` at $DIR/children-can-alias.rs:LL:CC +note: inside `main` + --> $DIR/children-can-alias.rs:LL:CC + | +LL | raw_children_of_unique_can_alias(Unique::new_unchecked(raw)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/tests/fail/tree_borrows/unique.default.stderr b/tests/fail/tree_borrows/unique.default.stderr new file mode 100644 index 0000000000..eb4d5c10c2 --- /dev/null +++ b/tests/fail/tree_borrows/unique.default.stderr @@ -0,0 +1,32 @@ +error: Undefined Behavior: write access through is forbidden + --> $DIR/unique.rs:LL:CC + | +LL | *uniq.as_ptr() = 3; + | ^^^^^^^^^^^^^^^^^^ write access through is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag has state Frozen which forbids this child write access +help: the accessed tag was created here, in the initial state Reserved + --> $DIR/unique.rs:LL:CC + | +LL | let refmut = &mut data; + | ^^^^^^^^^ +help: the accessed tag later transitioned to Active due to a child write access at offsets [0x0..0x1] + --> $DIR/unique.rs:LL:CC + | +LL | *uniq.as_ptr() = 1; // activation + | ^^^^^^^^^^^^^^^^^^ + = help: this transition corresponds to the first write to a 2-phase borrowed mutable reference +help: the accessed tag later transitioned to Frozen due to a foreign read access at offsets [0x0..0x1] + --> $DIR/unique.rs:LL:CC + | +LL | let _definitely_parent = data; // definitely Frozen by now + | ^^^^ + = help: this transition corresponds to a loss of write permissions + = note: BACKTRACE (of the first span): + = note: inside `main` at $DIR/unique.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/tests/fail/tree_borrows/unique.rs b/tests/fail/tree_borrows/unique.rs new file mode 100644 index 0000000000..0844dd21a5 --- /dev/null +++ b/tests/fail/tree_borrows/unique.rs @@ -0,0 +1,27 @@ +//@revisions: default uniq +//@compile-flags: -Zmiri-tree-borrows +//@[uniq]compile-flags: -Zmiri-unique-is-unique + +// A pattern that detects if `Unique` is treated as exclusive or not: +// activate the pointer behind a `Unique` then do a read that is parent +// iff `Unique` was specially reborrowed. + +#![feature(ptr_internals)] +use core::ptr::Unique; + +fn main() { + let mut data = 0u8; + let refmut = &mut data; + let rawptr = refmut as *mut u8; + + unsafe { + let uniq = Unique::new_unchecked(rawptr); + *uniq.as_ptr() = 1; // activation + let _maybe_parent = *rawptr; // maybe becomes Frozen + *uniq.as_ptr() = 2; + //~[uniq]^ ERROR: /write access through .* is forbidden/ + let _definitely_parent = data; // definitely Frozen by now + *uniq.as_ptr() = 3; + //~[default]^ ERROR: /write access through .* is forbidden/ + } +} diff --git a/tests/fail/tree_borrows/unique.uniq.stderr b/tests/fail/tree_borrows/unique.uniq.stderr new file mode 100644 index 0000000000..2d8936f273 --- /dev/null +++ b/tests/fail/tree_borrows/unique.uniq.stderr @@ -0,0 +1,38 @@ +error: Undefined Behavior: write access through is forbidden + --> $DIR/unique.rs:LL:CC + | +LL | *uniq.as_ptr() = 2; + | ^^^^^^^^^^^^^^^^^^ write access through is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag is a child of the conflicting tag + = help: the conflicting tag has state Frozen which forbids this child write access +help: the accessed tag was created here + --> $DIR/unique.rs:LL:CC + | +LL | *uniq.as_ptr() = 2; + | ^^^^^^^^^^^^^ +help: the conflicting tag was created here, in the initial state Reserved + --> $DIR/unique.rs:LL:CC + | +LL | let uniq = Unique::new_unchecked(rawptr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: the conflicting tag later transitioned to Active due to a child write access at offsets [0x0..0x1] + --> $DIR/unique.rs:LL:CC + | +LL | *uniq.as_ptr() = 1; // activation + | ^^^^^^^^^^^^^^^^^^ + = help: this transition corresponds to the first write to a 2-phase borrowed mutable reference +help: the conflicting tag later transitioned to Frozen due to a foreign read access at offsets [0x0..0x1] + --> $DIR/unique.rs:LL:CC + | +LL | let _maybe_parent = *rawptr; // maybe becomes Frozen + | ^^^^^^^ + = help: this transition corresponds to a loss of write permissions + = note: BACKTRACE (of the first span): + = note: inside `main` at $DIR/unique.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/tests/pass/tree_borrows/formatting.rs b/tests/pass/tree_borrows/formatting.rs index 9021c41763..64697cac26 100644 --- a/tests/pass/tree_borrows/formatting.rs +++ b/tests/pass/tree_borrows/formatting.rs @@ -17,6 +17,7 @@ fn main() { unsafe fn alignment_check() { let data: &mut [u8] = &mut [0; 1024]; name!(data.as_ptr()=>2, "data"); + name!(data.as_ptr()=>2, "data"); let alloc_id = alloc_id!(data.as_ptr()); let x = &mut data[1]; name!(x as *mut _, "data[1]"); diff --git a/tests/pass/tree_borrows/formatting.stderr b/tests/pass/tree_borrows/formatting.stderr index 8c3779fe1f..effd0d9f96 100644 --- a/tests/pass/tree_borrows/formatting.stderr +++ b/tests/pass/tree_borrows/formatting.stderr @@ -2,7 +2,7 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1.. 2.. 10.. 11..100..101..1000..1001..1024 | Act| Act| Act| Act| Act| Act| Act| Act| Act| └─┬── -| Res| Act| Res| Act| Res| Act| Res| Act| Res| └─┬── +| Res| Act| Res| Act| Res| Act| Res| Act| Res| └─┬── |----| Act|----|?Dis|----|?Dis| ----| ?Dis| ----| ├──── |----|----|----| Act|----|?Dis| ----| ?Dis| ----| ├──── |----|----|----|----|----| Frz| ----| ?Dis| ----| ├──── diff --git a/tests/pass/tree_borrows/tree-borrows.rs b/tests/pass/tree_borrows/tree-borrows.rs index aa6f707889..6bdad69596 100644 --- a/tests/pass/tree_borrows/tree-borrows.rs +++ b/tests/pass/tree_borrows/tree-borrows.rs @@ -1,4 +1,6 @@ +//@revisions: default uniq //@compile-flags: -Zmiri-tree-borrows +//@[uniq]compile-flags: -Zmiri-unique-is-unique #![feature(allocator_api)] use std::mem; diff --git a/tests/pass/tree_borrows/unique.default.stderr b/tests/pass/tree_borrows/unique.default.stderr new file mode 100644 index 0000000000..11e05d50f2 --- /dev/null +++ b/tests/pass/tree_borrows/unique.default.stderr @@ -0,0 +1,21 @@ +────────────────────────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 1 +| Act| └─┬── +| Res| └─┬── +| Res| └──── +────────────────────────────────────────────────────────────────────── +────────────────────────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 1 +| Act| └─┬── +| Act| └─┬── +| Act| └──── +────────────────────────────────────────────────────────────────────── +────────────────────────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 1 +| Act| └─┬── +| Act| └─┬── +| Act| └──── +────────────────────────────────────────────────────────────────────── diff --git a/tests/pass/tree_borrows/unique.rs b/tests/pass/tree_borrows/unique.rs new file mode 100644 index 0000000000..d0c3d133da --- /dev/null +++ b/tests/pass/tree_borrows/unique.rs @@ -0,0 +1,66 @@ +//@revisions: default uniq +//@compile-flags: -Zmiri-tree-borrows -Zmiri-tag-gc=0 +//@[uniq]compile-flags: -Zmiri-unique-is-unique + +#![feature(ptr_internals)] + +#[path = "../../utils/mod.rs"] +mod utils; +use utils::macros::*; + +use core::ptr::Unique; + +// Check general handling of Unique + +fn main() { + unsafe { + let base = &mut 5u8; + let alloc_id = alloc_id!(base); + name!(base); + + let raw = &mut *base as *mut u8; + name!(raw); + + // We create a `Unique` and expect it to have a fresh tag + // and uninitialized permissions. + let uniq = Unique::new_unchecked(raw); + + // With `-Zmiri-unique-is-unique`, `Unique::as_ptr` (which is called by + // `Vec::as_ptr`) generates pointers with a fresh tag, so to name the actual + // `base` pointer we care about we have to walk up the tree a bit. + // + // We care about naming this specific parent tag because it is the one + // that stays `Active` during the entire execution, unlike the leaves + // that will be invalidated the next time `as_ptr` is called. + // + // (We name it twice so that we have an indicator in the output of + // whether we got the distance correct: + // If the output shows + // + // |- + // '- + // + // then `nth_parent` is not big enough. + // The correct value for `nth_parent` should be the minimum + // integer for which the output shows + // + // '- + // ) + // + // Ultimately we want pointers obtained through independent + // calls of `as_ptr` to be able to alias, which will probably involve + // a new permission that allows aliasing when there is no protector. + let nth_parent = if cfg!(uniq) { 2 } else { 0 }; + name!(uniq.as_ptr()=>nth_parent, "uniq"); + name!(uniq.as_ptr()=>nth_parent, "uniq"); + print_state!(alloc_id); + + // We can activate the Unique and use it mutably. + *uniq.as_ptr() = 42; + print_state!(alloc_id); + + // Write through the raw parent disables the Unique + *raw = 42; + print_state!(alloc_id); + } +} diff --git a/tests/pass/tree_borrows/unique.uniq.stderr b/tests/pass/tree_borrows/unique.uniq.stderr new file mode 100644 index 0000000000..5008b66741 --- /dev/null +++ b/tests/pass/tree_borrows/unique.uniq.stderr @@ -0,0 +1,24 @@ +────────────────────────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 1 +| Act| └─┬── +| Res| └─┬── +| Res| └─┬── +|----| └──── +────────────────────────────────────────────────────────────────────── +────────────────────────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 1 +| Act| └─┬── +| Act| └─┬── +| Act| └─┬── +| Act| └──── +────────────────────────────────────────────────────────────────────── +────────────────────────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 1 +| Act| └─┬── +| Act| └─┬── +| Act| └─┬── +| Dis| └──── +────────────────────────────────────────────────────────────────────── diff --git a/tests/pass/tree_borrows/vec_unique.default.stderr b/tests/pass/tree_borrows/vec_unique.default.stderr new file mode 100644 index 0000000000..f1af1ea3d8 --- /dev/null +++ b/tests/pass/tree_borrows/vec_unique.default.stderr @@ -0,0 +1,6 @@ +───────────────────────────────────────────────────────────────────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 2 +| Act| └─┬── +| Res| └──── +───────────────────────────────────────────────────────────────────────────────────────────────────────────────── diff --git a/tests/pass/tree_borrows/vec_unique.rs b/tests/pass/tree_borrows/vec_unique.rs new file mode 100644 index 0000000000..3516f8d2eb --- /dev/null +++ b/tests/pass/tree_borrows/vec_unique.rs @@ -0,0 +1,68 @@ +//@revisions: default uniq +//@compile-flags: -Zmiri-tree-borrows -Zmiri-tag-gc=0 +//@[uniq]compile-flags: -Zmiri-unique-is-unique + +#![feature(vec_into_raw_parts)] + +#[path = "../../utils/mod.rs"] +mod utils; +use utils::macros::*; + +// Check general handling of `Unique`: +// there is no *explicit* `Unique` being used here, but there is one +// hidden a few layers inside `Vec` that should be reflected in the tree structure. + +fn main() { + unsafe { + let base = vec![0u8, 1]; + let alloc_id = alloc_id!(base.as_ptr()); + + // With `-Zmiri-unique-is-unique`, `Unique::as_ptr` (which is called by + // `Vec::as_ptr`) generates pointers with a fresh tag, so to name the actual + // `base` pointer we care about we have to walk up the tree a bit. + // + // We care about naming this specific parent tag because it is the one + // that stays `Active` during the entire execution, unlike the leaves + // that will be invalidated the next time `as_ptr` is called. + // + // (We name it twice so that we have an indicator in the output of + // whether we got the distance correct: + // If the output shows + // + // |- + // '- + // + // then `nth_parent` is not big enough. + // The correct value for `nth_parent` should be the minimum + // integer for which the output shows + // + // '- + // ) + // + // Ultimately we want pointers obtained through independent + // calls of `as_ptr` to be able to alias, which will probably involve + // a new permission that allows aliasing when there is no protector. + let nth_parent = if cfg!(uniq) { 2 } else { 0 }; + name!(base.as_ptr()=>nth_parent); + name!(base.as_ptr()=>nth_parent); + + // Destruct the `Vec` + let (ptr, len, cap) = base.into_raw_parts(); + + // Expect this to be again the same pointer as the one obtained from `as_ptr`. + // Under `-Zmiri-unique-is-unique`, this will be a strict child. + name!(ptr, "raw_parts.0"); + + // This is where the presence of `Unique` has implications, + // because there will be a reborrow here iff the exclusivity of `Unique` + // is enforced. + let reconstructed = Vec::from_raw_parts(ptr, len, cap); + + // The `as_ptr` here (twice for the same reason as above) return either + // the same pointer once more (default) or a strict child (uniq). + name!(reconstructed.as_ptr()=>nth_parent); + name!(reconstructed.as_ptr()=>nth_parent); + + print_state!(alloc_id, false); + } +} diff --git a/tests/pass/tree_borrows/vec_unique.uniq.stderr b/tests/pass/tree_borrows/vec_unique.uniq.stderr new file mode 100644 index 0000000000..00ff1ee00e --- /dev/null +++ b/tests/pass/tree_borrows/vec_unique.uniq.stderr @@ -0,0 +1,8 @@ +────────────────────────────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 2 +| Act| └─┬── +|----| └─┬── +|----| └─┬── +|----| └──── +────────────────────────────────────────────────────────────────────────── diff --git a/tests/pass/vec.rs b/tests/pass/vec.rs index 06ec2f9917..048f7d1c35 100644 --- a/tests/pass/vec.rs +++ b/tests/pass/vec.rs @@ -1,6 +1,7 @@ -//@revisions: stack tree -//@[tree]compile-flags: -Zmiri-tree-borrows +//@revisions: stack tree tree_uniq //@compile-flags: -Zmiri-strict-provenance +//@[tree]compile-flags: -Zmiri-tree-borrows +//@[tree_uniq]compile-flags: -Zmiri-tree-borrows -Zmiri-unique-is-unique #![feature(iter_advance_by, iter_next_chunk)] // Gather all references from a mutable iterator and make sure Miri notices if diff --git a/tests/pass/vecdeque.rs b/tests/pass/vecdeque.rs index dfbf9bb83c..ccecf3d30a 100644 --- a/tests/pass/vecdeque.rs +++ b/tests/pass/vecdeque.rs @@ -1,6 +1,8 @@ -//@revisions: stack tree -//@[tree]compile-flags: -Zmiri-tree-borrows +//@revisions: stack tree tree_uniq //@compile-flags: -Zmiri-strict-provenance +//@[tree]compile-flags: -Zmiri-tree-borrows +//@[tree_uniq]compile-flags: -Zmiri-tree-borrows -Zmiri-unique-is-unique + use std::collections::VecDeque; fn test_all_refs<'a, T: 'a>(dummy: &mut T, iter: impl Iterator) { diff --git a/tests/pass/vecdeque.tree_uniq.stdout b/tests/pass/vecdeque.tree_uniq.stdout new file mode 100644 index 0000000000..63de960ee2 --- /dev/null +++ b/tests/pass/vecdeque.tree_uniq.stdout @@ -0,0 +1,2 @@ +[2, 2] Iter([2, 2], []) +Iter([], []) diff --git a/tests/utils/macros.rs b/tests/utils/macros.rs index de223410fb..28b4095430 100644 --- a/tests/utils/macros.rs +++ b/tests/utils/macros.rs @@ -47,12 +47,12 @@ macro_rules! name { ($ptr:expr) => { crate::utils::macros::name!($ptr => 0, stringify!($ptr)); }; - ($ptr:expr => $nb:expr) => { - crate::utils::macros::name!($ptr => $nb, stringify!($ptr)); + ($ptr:expr => $nth_parent:expr) => { + crate::utils::macros::name!($ptr => $nth_parent, stringify!($ptr)); }; - ($ptr:expr => $nb:expr, $name:expr) => { + ($ptr:expr => $nth_parent:expr, $name:expr) => { let name = $name.as_bytes(); - crate::utils::miri_extern::miri_pointer_name($ptr as *const u8 as *const (), $nb, name); + crate::utils::miri_extern::miri_pointer_name($ptr as *const u8 as *const (), $nth_parent, name); }; } diff --git a/tests/utils/miri_extern.rs b/tests/utils/miri_extern.rs index 6c4298c613..55f3c1cc33 100644 --- a/tests/utils/miri_extern.rs +++ b/tests/utils/miri_extern.rs @@ -65,7 +65,7 @@ extern "Rust" { /// /// This is only useful as an input to `miri_print_borrow_stacks`, and it is a separate call because /// getting a pointer to an allocation at runtime can change the borrow stacks in the allocation. - /// This function should be considered unstable. It exists only to support `miri_print_borrow_stacks` and so + /// This function should be considered unstable. It exists only to support `miri_print_borrow_state` and so /// inherits all of its instability. pub fn miri_get_alloc_id(ptr: *const ()) -> u64;