diff options
20 files changed, 346 insertions, 201 deletions
diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index ea7cba3f346..0351f586872 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -604,8 +604,7 @@ impl<'mir: 'ecx, 'tcx: 'mir, 'ecx> EvalContextPrivExt<'mir, 'tcx, 'ecx> { } trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'mir, 'tcx> { - /// Returns the `AllocId` the reborrow was done in, if some actual borrow stack manipulation - /// happened. + /// Returns the provenance that should be used henceforth. fn sb_reborrow( &mut self, place: &MPlaceTy<'tcx, Provenance>, @@ -613,7 +612,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' new_perm: NewPermission, new_tag: BorTag, retag_info: RetagInfo, // diagnostics info about this retag - ) -> InterpResult<'tcx, Option<AllocId>> { + ) -> InterpResult<'tcx, Option<Provenance>> { let this = self.eval_context_mut(); // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050). this.check_ptr_access_align(place.ptr, size, Align::ONE, CheckInAllocMsg::InboundsTest)?; @@ -695,11 +694,14 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // pointer tagging for example all calls to get_unchecked on them are invalid. if let Ok((alloc_id, base_offset, orig_tag)) = this.ptr_try_get_alloc_id(place.ptr) { log_creation(this, Some((alloc_id, base_offset, orig_tag)))?; - return Ok(Some(alloc_id)); + // Still give it the new provenance, it got retagged after all. + return Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })); + } else { + // This pointer doesn't come with an AllocId. :shrug: + log_creation(this, None)?; + // Provenance unchanged. + return Ok(place.ptr.provenance); } - // This pointer doesn't come with an AllocId. :shrug: - log_creation(this, None)?; - return Ok(None); } let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr)?; @@ -804,7 +806,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' } } - Ok(Some(alloc_id)) + Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })) } /// Retags an individual pointer, returning the retagged version. @@ -831,25 +833,10 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let new_tag = this.machine.borrow_tracker.as_mut().unwrap().get_mut().new_ptr(); // Reborrow. - let alloc_id = this.sb_reborrow(&place, size, new_perm, new_tag, info)?; + let new_prov = this.sb_reborrow(&place, size, new_perm, new_tag, info)?; // Adjust pointer. - let new_place = place.map_provenance(|p| { - p.map(|prov| { - match alloc_id { - Some(alloc_id) => { - // If `reborrow` could figure out the AllocId of this ptr, hard-code it into the new one. - // Even if we started out with a wildcard, this newly retagged pointer is tied to that allocation. - Provenance::Concrete { alloc_id, tag: new_tag } - } - None => { - // Looks like this has to stay a wildcard pointer. - assert!(matches!(prov, Provenance::Wildcard)); - Provenance::Wildcard - } - } - }) - }); + let new_place = place.map_provenance(|_| new_prov); // Return new pointer. Ok(ImmTy::from_immediate(new_place.to_ref(this), val.layout)) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs index 7723f06f296..fd45671ba29 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -218,7 +218,7 @@ impl<'tcx> Tree { } } -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy)] pub(super) enum TransitionError { /// This access is not allowed because some parent tag has insufficient permissions. /// For example, if a tag is `Frozen` and encounters a child write this will diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 283bc94d3cf..0fbe66360b2 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -117,7 +117,7 @@ impl<'tcx> NewPermission { let ty_is_freeze = pointee.is_freeze(*cx.tcx, cx.param_env()); let ty_is_unpin = pointee.is_unpin(*cx.tcx, cx.param_env()); let initial_state = match mutability { - Mutability::Mut if ty_is_unpin => Permission::new_unique_2phase(ty_is_freeze), + Mutability::Mut if ty_is_unpin => Permission::new_reserved(ty_is_freeze), Mutability::Not if ty_is_freeze => Permission::new_frozen(), // Raw pointers never enter this function so they are not handled. // However raw pointers are not the only pointers that take the parent @@ -146,7 +146,7 @@ impl<'tcx> NewPermission { let ty_is_freeze = ty.is_freeze(*cx.tcx, cx.param_env()); Self { zero_size, - initial_state: Permission::new_unique_2phase(ty_is_freeze), + initial_state: Permission::new_reserved(ty_is_freeze), protector: (kind == RetagKind::FnEntry).then_some(ProtectorKind::WeakProtector), } }) @@ -161,22 +161,14 @@ impl<'mir: 'ecx, 'tcx: 'mir, 'ecx> EvalContextPrivExt<'mir, 'tcx, 'ecx> { } trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'mir, 'tcx> { - /// Returns the `AllocId` the reborrow was done in, if there is some actual - /// memory associated with this pointer. Returns `None` if there is no actual - /// memory allocated. Also checks that the reborrow of size `ptr_size` is - /// within bounds of the allocation. - /// - /// Also returns the tag that the pointer should get, which is essentially - /// `if new_perm.is_some() { new_tag } else { parent_tag }` along with - /// some logging (always) and fake reads (if `new_perm` is - /// `Some(NewPermission { perform_read_access: true }`). + /// Returns the provenance that should be used henceforth. fn tb_reborrow( &mut self, place: &MPlaceTy<'tcx, Provenance>, // parent tag extracted from here ptr_size: Size, new_perm: NewPermission, new_tag: BorTag, - ) -> InterpResult<'tcx, Option<(AllocId, BorTag)>> { + ) -> InterpResult<'tcx, Option<Provenance>> { let this = self.eval_context_mut(); // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050). this.check_ptr_access_align( @@ -222,13 +214,14 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' place.layout.ty, ); log_creation(this, None)?; - return Ok(None); + // Keep original provenance. + return Ok(place.ptr.provenance); } }; log_creation(this, Some((alloc_id, base_offset, parent_prov)))?; let orig_tag = match parent_prov { - ProvenanceExtra::Wildcard => return Ok(None), // TODO: handle wildcard pointers + ProvenanceExtra::Wildcard => return Ok(place.ptr.provenance), // TODO: handle wildcard pointers ProvenanceExtra::Concrete(tag) => tag, }; @@ -255,31 +248,54 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' .insert(new_tag, protect); } + let alloc_kind = this.get_alloc_info(alloc_id).2; + if !matches!(alloc_kind, AllocKind::LiveData) { + assert_eq!(ptr_size, Size::ZERO); // we did the deref check above, size has to be 0 here + // There's not actually any bytes here where accesses could even be tracked. + // Just produce the new provenance, nothing else to do. + return Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })); + } + let span = this.machine.current_span(); let alloc_extra = this.get_alloc_extra(alloc_id)?; let range = alloc_range(base_offset, ptr_size); let mut tree_borrows = alloc_extra.borrow_tracker_tb().borrow_mut(); // 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( - AccessKind::Read, - orig_tag, - range, - global, - span, - diagnostics::AccessCause::Reborrow, - )?; + tree_borrows.perform_access( + AccessKind::Read, + orig_tag, + range, + this.machine.borrow_tracker.as_ref().unwrap(), + this.machine.current_span(), + diagnostics::AccessCause::Reborrow, + )?; + // Record the parent-child pair in the tree. + tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?; + drop(tree_borrows); + + // Also inform the data race model (but only if any bytes are actually affected). + if range.size.bytes() > 0 { if let Some(data_race) = alloc_extra.data_race.as_ref() { - data_race.read(alloc_id, range, &this.machine)?; + // We sometimes need to make it a write, since not all retags commute with reads! + // FIXME: Is that truly the semantics we want? Some optimizations are likely to be + // very unhappy without this. We'd tsill ge some UB just by picking a suitable + // interleaving, but wether UB happens can depend on whether a write occurs in the + // future... + let is_write = new_perm.initial_state.is_active() + || (new_perm.initial_state.is_resrved() && new_perm.protector.is_some()); + if is_write { + // Need to get mutable access to alloc_extra. + // (Cannot always do this as we can do read-only reborrowing on read-only allocations.) + let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?; + alloc_extra.data_race.as_mut().unwrap().write(alloc_id, range, machine)?; + } else { + data_race.read(alloc_id, range, &this.machine)?; + } } } - // Record the parent-child pair in the tree. - tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?; - Ok(Some((alloc_id, new_tag))) + Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })) } /// Retags an individual pointer, returning the retagged version. @@ -315,25 +331,10 @@ 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, reborrow_size, new_perm, new_tag)?; + let new_prov = this.tb_reborrow(&place, reborrow_size, new_perm, new_tag)?; // Adjust pointer. - let new_place = place.map_provenance(|p| { - p.map(|prov| { - match reborrowed { - Some((alloc_id, actual_tag)) => { - // If `reborrow` could figure out the AllocId of this ptr, hard-code it into the new one. - // Even if we started out with a wildcard, this newly retagged pointer is tied to that allocation. - Provenance::Concrete { alloc_id, tag: actual_tag } - } - None => { - // Looks like this has to stay a wildcard pointer. - assert!(matches!(prov, Provenance::Wildcard)); - Provenance::Wildcard - } - } - }) - }); + let new_place = place.map_provenance(|_| new_prov); // Return new pointer. Ok(ImmTy::from_immediate(new_place.to_ref(this), val.layout)) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index 051b209da17..b4a9a768e27 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -134,25 +134,32 @@ pub struct PermTransition { impl Permission { /// Default initial permission of the root of a new tree. - pub fn new_root() -> Self { + pub fn new_active() -> Self { Self { inner: Active } } /// Default initial permission of a reborrowed mutable reference. - pub fn new_unique_2phase(ty_is_freeze: bool) -> Self { + pub fn new_reserved(ty_is_freeze: bool) -> Self { Self { inner: Reserved { ty_is_freeze } } } - /// Default initial permission for return place. - pub fn new_active() -> Self { - Self { inner: Active } - } - /// Default initial permission of a reborrowed shared reference pub fn new_frozen() -> Self { Self { inner: Frozen } } + pub fn is_active(self) -> bool { + matches!(self.inner, Active) + } + + pub fn is_resrved(self) -> bool { + matches!(self.inner, Reserved { .. }) + } + + pub fn is_frozen(self) -> bool { + matches!(self.inner, Frozen) + } + /// Apply the transition to the inner PermissionPriv. pub fn perform_access( kind: AccessKind, @@ -438,7 +445,7 @@ mod propagation_optimization_checks { } #[test] - fn foreign_read_is_noop_after_write() { + fn foreign_read_is_noop_after_foreign_write() { use transition::*; let old_access = AccessKind::Write; let new_access = AccessKind::Read; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 355356b743a..5abf13229bb 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -110,7 +110,7 @@ impl LocationState { // Helper to optimize the tree traversal. // The optimization here consists of observing thanks to the tests - // `foreign_read_is_noop_after_write` and `all_transitions_idempotent`, + // `foreign_read_is_noop_after_foreign_write` and `all_transitions_idempotent`, // that there are actually just three possible sequences of events that can occur // in between two child accesses that produce different results. // @@ -139,7 +139,7 @@ impl LocationState { let new_access_noop = match (self.latest_foreign_access, access_kind) { // Previously applied transition makes the new one a guaranteed // noop in the two following cases: - // (1) justified by `foreign_read_is_noop_after_write` + // (1) justified by `foreign_read_is_noop_after_foreign_write` (Some(AccessKind::Write), AccessKind::Read) => true, // (2) justified by `all_transitions_idempotent` (Some(old), new) if old == new => true, @@ -376,7 +376,7 @@ where { impl Tree { /// Create a new tree, with only a root pointer. pub fn new(root_tag: BorTag, size: Size, span: Span) -> Self { - let root_perm = Permission::new_root(); + let root_perm = Permission::new_active(); let mut tag_mapping = UniKeyMap::default(); let root_idx = tag_mapping.insert(root_tag); let nodes = { @@ -670,7 +670,8 @@ impl AccessRelatedness { mod commutation_tests { use super::*; impl LocationState { - pub fn all_without_access() -> impl Iterator<Item = Self> { + pub fn all() -> impl Iterator<Item = Self> { + // We keep `latest_foreign_access` at `None` as that's just a cache. Permission::all().flat_map(|permission| { [false, true].into_iter().map(move |initialized| { Self { permission, initialized, latest_foreign_access: None } @@ -695,12 +696,12 @@ mod commutation_tests { // Any protector state works, but we can't move reads across function boundaries // so the two read accesses occur under the same protector. for &protected in &[true, false] { - for loc in LocationState::all_without_access() { + for loc in LocationState::all() { // Apply 1 then 2. Failure here means that there is UB in the source // and we skip the check in the target. let mut loc12 = loc; - let Ok(_) = loc12.perform_access(kind, rel1, protected) else { continue; }; - let Ok(_) = loc12.perform_access(kind, rel2, protected) else { continue; }; + let Ok(_) = loc12.perform_access(kind, rel1, protected) else { continue }; + let Ok(_) = loc12.perform_access(kind, rel2, protected) else { continue }; // If 1 followed by 2 succeeded, then 2 followed by 1 must also succeed... let mut loc21 = loc; @@ -718,4 +719,33 @@ mod commutation_tests { } } } + + #[test] + #[rustfmt::skip] + // Ensure that of 2 accesses happen, one foreign and one a child, and we are protected, that we + // get UB unless they are both reads. + fn protected_enforces_noalias() { + for rel1 in AccessRelatedness::all() { + for rel2 in AccessRelatedness::all() { + if rel1.is_foreign() == rel2.is_foreign() { + // We want to check pairs of accesses where one is foreign and one is not. + continue; + } + for kind1 in AccessKind::all() { + for kind2 in AccessKind::all() { + for mut state in LocationState::all() { + let protected = true; + let Ok(_) = state.perform_access(kind1, rel1, protected) else { continue }; + let Ok(_) = state.perform_access(kind2, rel2, protected) else { continue }; + // If these were both allowed, it must have been two reads. + assert!( + kind1 == AccessKind::Read && kind2 == AccessKind::Read, + "failed to enforce noalias between two accesses that are not both reads" + ); + } + } + } + } + } + } } diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 73035b01a1b..e19be417b22 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1219,7 +1219,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { // If we have a borrow tracker, we also have it set up protection so that all reads *and // writes* during this call are insta-UB. if ecx.machine.borrow_tracker.is_some() { - if let Either::Left(place) = place.as_mplace_or_local() { + // Have to do `to_op` first because a `Place::Local` doesn't imply the local doesn't have an address. + if let Either::Left(place) = ecx.place_to_op(place)?.as_mplace_or_imm() { ecx.protect_place(&place)?; } else { // Locals that don't have their address taken are as protected as they can ever be. diff --git a/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.rs b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.rs new file mode 100644 index 00000000000..f192e76de13 --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.rs @@ -0,0 +1,29 @@ +//@revisions: stack tree +//@compile-flags: -Zmiri-preemption-rate=0 +//@[tree]compile-flags: -Zmiri-tree-borrows +use std::thread; + +#[derive(Copy, Clone)] +struct SendPtr(*mut i32); +unsafe impl Send for SendPtr {} + +fn main() { + let mut mem = 0; + let ptr = SendPtr(&mut mem as *mut _); + + let t = thread::spawn(move || { + let ptr = ptr; + // We do a protected 2phase retag (but no write!) in this thread. + fn retag(_x: &mut i32) {} //~[tree]ERROR: Data race detected between (1) Read on thread `main` and (2) Write on thread `<unnamed>` + retag(unsafe { &mut *ptr.0 }); //~[stack]ERROR: Data race detected between (1) Read on thread `main` and (2) Write on thread `<unnamed>` + }); + + // We do a read in the main thread. + unsafe { ptr.0.read() }; + + // These two operations do not commute -- if the read happens after the retag, the retagged pointer + // gets frozen! So we want this to be considered UB so that we can still freely move the read around + // in this thread without worrying about reordering with retags in other threads. + + t.join().unwrap(); +} diff --git a/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.stack.stderr b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.stack.stderr new file mode 100644 index 00000000000..10fb1dece2a --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.stack.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: Data race detected between (1) Read on thread `main` and (2) Write on thread `<unnamed>` at ALLOC. (2) just happened here + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | retag(unsafe { &mut *ptr.0 }); + | ^^^^^^^^^^^ Data race detected between (1) Read on thread `main` and (2) Write on thread `<unnamed>` at ALLOC. (2) just happened here + | +help: and (1) occurred earlier here + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | unsafe { ptr.0.read() }; + | ^^^^^^^^^^^^ + = 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 (of the first span): + = note: inside closure at $DIR/retag_data_race_protected_read.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/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.tree.stderr b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.tree.stderr new file mode 100644 index 00000000000..173acf4b96c --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.tree.stderr @@ -0,0 +1,25 @@ +error: Undefined Behavior: Data race detected between (1) Read on thread `main` and (2) Write on thread `<unnamed>` at ALLOC. (2) just happened here + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | fn retag(_x: &mut i32) {} + | ^^ Data race detected between (1) Read on thread `main` and (2) Write on thread `<unnamed>` at ALLOC. (2) just happened here + | +help: and (1) occurred earlier here + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | unsafe { ptr.0.read() }; + | ^^^^^^^^^^^^ + = 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 (of the first span): + = note: inside `main::{closure#0}::retag` at $DIR/retag_data_race_protected_read.rs:LL:CC +note: inside closure + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | ... retag(unsafe { &mut *ptr.0 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.rs b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.rs index c1dded40d3c..868b3beb53b 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.rs +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.rs @@ -1,5 +1,7 @@ //! Make sure that a retag acts like a write for the data race model. +//@revisions: stack tree //@compile-flags: -Zmiri-preemption-rate=0 +//@[tree]compile-flags: -Zmiri-tree-borrows #[derive(Copy, Clone)] struct SendPtr(*mut u8); @@ -15,7 +17,7 @@ fn thread_1(p: SendPtr) { fn thread_2(p: SendPtr) { let p = p.0; unsafe { - *p = 5; //~ ERROR: Data race detected between (1) Write on thread `<unnamed>` and (2) Write on thread `<unnamed>` + *p = 5; //~ ERROR: /Data race detected between \(1\) (Read|Write) on thread `<unnamed>` and \(2\) Write on thread `<unnamed>`/ } } diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.stderr b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.stack.stderr index da5af600675..da5af600675 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.stderr +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.stack.stderr diff --git a/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.tree.stderr b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.tree.stderr new file mode 100644 index 00000000000..37d216b9877 --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.tree.stderr @@ -0,0 +1,25 @@ +error: Undefined Behavior: Data race detected between (1) Read on thread `<unnamed>` and (2) Write on thread `<unnamed>` at ALLOC. (2) just happened here + --> $DIR/retag_data_race_write.rs:LL:CC + | +LL | *p = 5; + | ^^^^^^ Data race detected between (1) Read on thread `<unnamed>` and (2) Write on thread `<unnamed>` at ALLOC. (2) just happened here + | +help: and (1) occurred earlier here + --> $DIR/retag_data_race_write.rs:LL:CC + | +LL | let _r = &mut *p; + | ^^^^^^^ + = 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 (of the first span): + = note: inside `thread_2` at $DIR/retag_data_race_write.rs:LL:CC +note: inside closure + --> $DIR/retag_data_race_write.rs:LL:CC + | +LL | let t2 = std::thread::spawn(move || thread_2(p)); + | ^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs new file mode 100644 index 00000000000..7e9a6320026 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs @@ -0,0 +1,30 @@ +//@compile-flags: -Zmiri-tree-borrows +#![feature(raw_ref_op)] +#![feature(core_intrinsics)] +#![feature(custom_mir)] + +use std::intrinsics::mir::*; + +#[custom_mir(dialect = "runtime", phase = "optimized")] +pub fn main() { + mir! { + { + let x = 0; + let ptr = &raw mut x; + // We arrange for `myfun` to have a pointer that aliases + // its return place. Even just reading from that pointer is UB. + Call(x, after_call, myfun(ptr)) + } + + after_call = { + Return() + } + } +} + +fn myfun(ptr: *mut i32) -> i32 { + // This overwrites the return place, which shouldn't be possible through another pointer. + unsafe { ptr.write(0) }; + //~^ ERROR: /write access .* forbidden/ + 13 +} diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr new file mode 100644 index 00000000000..33a8a4b46bd --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr @@ -0,0 +1,39 @@ +error: Undefined Behavior: write access through <TAG> (root of the allocation) is forbidden + --> $DIR/return_pointer_aliasing2.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^ write access through <TAG> (root of the allocation) 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 <TAG> (root of the allocation) is foreign to the protected tag <TAG> (i.e., it is not a child) + = help: this foreign write access would cause the protected tag <TAG> (currently Active) to become Disabled + = help: protected tags must never be Disabled +help: the accessed tag <TAG> was created here + --> $DIR/return_pointer_aliasing2.rs:LL:CC + | +LL | / mir! { +LL | | { +LL | | let x = 0; +LL | | let ptr = &raw mut x; +... | +LL | | } +LL | | } + | |_____^ +help: the protected tag <TAG> was created here, in the initial state Active + --> $DIR/return_pointer_aliasing2.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^^^^^^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `myfun` at $DIR/return_pointer_aliasing2.rs:LL:CC +note: inside `main` + --> $DIR/return_pointer_aliasing2.rs:LL:CC + | +LL | Call(x, after_call, myfun(ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/tree_borrows/retag-data-race.stderr b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stack.stderr index f2cdfe7c314..c53a495b5e1 100644 --- a/src/tools/miri/tests/fail/tree_borrows/retag-data-race.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stack.stderr @@ -1,23 +1,23 @@ error: Undefined Behavior: Data race detected between (1) Read on thread `<unnamed>` and (2) Write on thread `<unnamed>` at ALLOC. (2) just happened here - --> $DIR/retag-data-race.rs:LL:CC + --> $DIR/retag_data_race_read.rs:LL:CC | -LL | *p = 5; - | ^^^^^^ Data race detected between (1) Read on thread `<unnamed>` and (2) Write on thread `<unnamed>` at ALLOC. (2) just happened here +LL | *p = 5; + | ^^^^^^ Data race detected between (1) Read on thread `<unnamed>` and (2) Write on thread `<unnamed>` at ALLOC. (2) just happened here | help: and (1) occurred earlier here - --> $DIR/retag-data-race.rs:LL:CC + --> $DIR/retag_data_race_read.rs:LL:CC | -LL | let _r = &*p; - | ^^^ +LL | let _r = &*p; + | ^^^ = 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 (of the first span): - = note: inside `thread_2` at $DIR/retag-data-race.rs:LL:CC + = note: inside `thread_2` at $DIR/retag_data_race_read.rs:LL:CC note: inside closure - --> $DIR/retag-data-race.rs:LL:CC + --> $DIR/retag_data_race_read.rs:LL:CC | -LL | let t2 = std::thread::spawn(move || unsafe { thread_2(p) }); - | ^^^^^^^^^^^ +LL | let t2 = std::thread::spawn(move || thread_2(p)); + | ^^^^^^^^^^^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.tree.stderr b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.tree.stderr new file mode 100644 index 00000000000..1e154eb0564 --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.tree.stderr @@ -0,0 +1,26 @@ +error: Undefined Behavior: reborrow through <TAG> (root of the allocation) is forbidden + --> RUSTLIB/std/src/rt.rs:LL:CC + | +LL | panic::catch_unwind(move || unsafe { init(argc, argv, sigpipe) }).map_err(rt_abort)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reborrow through <TAG> (root of the allocation) 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 <TAG> (root of the allocation) is foreign to the protected tag <TAG> (i.e., it is not a child) + = help: this reborrow (acting as a foreign read access) would cause the protected tag <TAG> (currently Active) to become Disabled + = help: protected tags must never be Disabled +help: the accessed tag <TAG> was created here + --> RUSTLIB/std/src/rt.rs:LL:CC + | +LL | panic::catch_unwind(move || unsafe { init(argc, argv, sigpipe) }).map_err(rt_abort)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: the protected tag <TAG> was created here, in the initial state Active + --> RUSTLIB/std/src/panic.rs:LL:CC + | +LL | pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> { + | ^ + = note: BACKTRACE (of the first span): + = note: inside `std::rt::lang_start_internal` at RUSTLIB/std/src/rt.rs:LL:CC + = note: inside `std::rt::lang_start::<()>` at RUSTLIB/std/src/rt.rs:LL:CC + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.rs b/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.rs deleted file mode 100644 index 215100de0a1..00000000000 --- a/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.rs +++ /dev/null @@ -1,42 +0,0 @@ -//! Race-condition-like interaction between a read and a reborrow. -//! Even though no write or fake write occurs, reads have an effect on protected -//! Reserved. This is a protected-retag/read data race, but is not *detected* as -//! a data race violation because reborrows are not writes. -//! -//! This test is sensitive to the exact schedule so we disable preemption. -//@compile-flags: -Zmiri-tree-borrows -Zmiri-preemption-rate=0 -use std::ptr::addr_of_mut; -use std::thread; - -#[derive(Copy, Clone)] -struct SendPtr(*mut u8); - -unsafe impl Send for SendPtr {} - -// First thread is just a reborrow, but for an instant `x` is -// protected and thus vulnerable to foreign reads. -fn thread_1(x: &mut u8) -> SendPtr { - thread::yield_now(); // make the other thread go first - SendPtr(x as *mut u8) -} - -// Second thread simply performs a read. -fn thread_2(x: &u8) { - let _val = *x; -} - -fn main() { - let mut x = 0u8; - let x_1 = unsafe { &mut *addr_of_mut!(x) }; - let xg = unsafe { &*addr_of_mut!(x) }; - - // The two threads are executed in parallel on aliasing pointers. - // UB occurs if the read of thread_2 occurs while the protector of thread_1 - // is in place. - let hf = thread::spawn(move || thread_1(x_1)); - let hg = thread::spawn(move || thread_2(xg)); - let SendPtr(p) = hf.join().unwrap(); - let () = hg.join().unwrap(); - - unsafe { *p = 1 }; //~ ERROR: /write access through .* is forbidden/ -} diff --git a/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.stderr b/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.stderr deleted file mode 100644 index 910f51ba8a3..00000000000 --- a/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.stderr +++ /dev/null @@ -1,32 +0,0 @@ -error: Undefined Behavior: write access through <TAG> is forbidden - --> $DIR/fragile-data-race.rs:LL:CC - | -LL | unsafe { *p = 1 }; - | ^^^^^^ write access through <TAG> 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 <TAG> is a child of the conflicting tag <TAG> - = help: the conflicting tag <TAG> has state Frozen which forbids this child write access -help: the accessed tag <TAG> was created here - --> $DIR/fragile-data-race.rs:LL:CC - | -LL | fn thread_1(x: &mut u8) -> SendPtr { - | ^ -help: the conflicting tag <TAG> was created here, in the initial state Reserved - --> RUSTLIB/std/src/panic.rs:LL:CC - | -LL | pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> { - | ^ -help: the conflicting tag <TAG> later transitioned to Frozen due to a reborrow (acting as a foreign read access) at offsets [0x0..0x1] - --> RUSTLIB/core/src/ptr/mod.rs:LL:CC - | -LL | crate::intrinsics::read_via_copy(src) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: this transition corresponds to a loss of write permissions - = note: BACKTRACE (of the first span): - = note: inside `main` at $DIR/fragile-data-race.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/src/tools/miri/tests/fail/tree_borrows/retag-data-race.rs b/src/tools/miri/tests/fail/tree_borrows/retag-data-race.rs deleted file mode 100644 index 8ef3d23e804..00000000000 --- a/src/tools/miri/tests/fail/tree_borrows/retag-data-race.rs +++ /dev/null @@ -1,28 +0,0 @@ -//! Make sure that a retag acts like a read for the data race model. -//! This is a retag/write race condition. -//! -//! This test is sensitive to the exact schedule so we disable preemption. -//@compile-flags: -Zmiri-tree-borrows -Zmiri-preemption-rate=0 -#[derive(Copy, Clone)] -struct SendPtr(*mut u8); - -unsafe impl Send for SendPtr {} - -unsafe fn thread_1(SendPtr(p): SendPtr) { - let _r = &*p; -} - -unsafe fn thread_2(SendPtr(p): SendPtr) { - *p = 5; //~ ERROR: Data race detected between (1) Read on thread `<unnamed>` and (2) Write on thread `<unnamed>` -} - -fn main() { - let mut x = 0; - let p = std::ptr::addr_of_mut!(x); - let p = SendPtr(p); - - let t1 = std::thread::spawn(move || unsafe { thread_1(p) }); - let t2 = std::thread::spawn(move || unsafe { thread_2(p) }); - let _ = t1.join(); - let _ = t2.join(); -} diff --git a/src/tools/miri/tests/pass/strange_references.rs b/src/tools/miri/tests/pass/strange_references.rs new file mode 100644 index 00000000000..fe5ff93a9ca --- /dev/null +++ b/src/tools/miri/tests/pass/strange_references.rs @@ -0,0 +1,25 @@ +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows + +// Create zero-sized references to vtables and function data. +// Just make sure nothing explodes. + +use std::{mem, ptr}; + +fn check_ref(x: &()) { + let _ptr = ptr::addr_of!(*x); +} + +fn main() { + check_ref({ + // Create reference to a function. + let fnptr: fn(&()) = check_ref; + unsafe { mem::transmute(fnptr) } + }); + check_ref({ + // Create reference to a vtable. + let wideptr: &dyn Send = &0; + let fields: (&i32, &()) = unsafe { mem::transmute(wideptr) }; + fields.1 + }) +} |