diff --git a/README.md b/README.md index 88ac8a135304..f6b2413c65f5 100644 --- a/README.md +++ b/README.md @@ -358,9 +358,7 @@ to Miri failing to detect cases of undefined behavior in a program. for pointer-to-int and int-to-pointer casts, respectively. This will necessarily miss some bugs as those semantics are not efficiently implementable in a sanitizer, but it will only miss bugs that concerns - memory/pointers which is subject to these operations. Also note that this flag - is currently incompatible with Stacked Borrows, so you will have to also pass - `-Zmiri-disable-stacked-borrows` to use this. + memory/pointers which is subject to these operations. * `-Zmiri-symbolic-alignment-check` makes the alignment check more strict. By default, alignment is checked by casting the pointer to an integer, and making sure that is a multiple of the alignment. This can lead to cases where a diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 0e3e693e33f9..230f46c569db 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -8,7 +8,7 @@ use rustc_middle::ty; use rustc_span::{source_map::DUMMY_SP, Span, SpanData, Symbol}; use crate::helpers::HexRange; -use crate::stacked_borrows::{diagnostics::TagHistory, AccessKind, SbTag}; +use crate::stacked_borrows::{diagnostics::TagHistory, AccessKind}; use crate::*; /// Details of premature program termination. @@ -61,9 +61,9 @@ impl MachineStopType for TerminationInfo {} /// Miri specific diagnostics pub enum NonHaltingDiagnostic { CreatedPointerTag(NonZeroU64), - /// This `Item` was popped from the borrow stack, either due to a grant of - /// `AccessKind` to `SbTag` or a deallocation when the second argument is `None`. - PoppedPointerTag(Item, Option<(SbTag, AccessKind)>), + /// This `Item` was popped from the borrow stack, either due to an access with the given tag or + /// a deallocation when the second argument is `None`. + PoppedPointerTag(Item, Option<(SbTagExtra, AccessKind)>), CreatedCallId(CallId), CreatedAlloc(AllocId), FreedAlloc(AllocId), diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 4a86490ed09a..cfaf61f9d5c8 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -101,14 +101,16 @@ impl<'mir, 'tcx> GlobalStateInner { } } - pub fn expose_addr(ecx: &MiriEvalContext<'mir, 'tcx>, alloc_id: AllocId) { - trace!("Exposing allocation id {:?}", alloc_id); - - let mut global_state = ecx.machine.intptrcast.borrow_mut(); + pub fn expose_ptr(ecx: &mut MiriEvalContext<'mir, 'tcx>, alloc_id: AllocId, sb: SbTag) { + let global_state = ecx.machine.intptrcast.get_mut(); // In legacy and strict mode, we don't need this, so we can save some cycles // by not tracking it. if global_state.provenance_mode == ProvenanceMode::Permissive { + trace!("Exposing allocation id {alloc_id:?}"); global_state.exposed.insert(alloc_id); + if ecx.machine.stacked_borrows.is_some() { + ecx.expose_tag(alloc_id, sb); + } } } @@ -140,9 +142,7 @@ impl<'mir, 'tcx> GlobalStateInner { // Determine the allocation this points to at cast time. let alloc_id = Self::alloc_id_from_addr(ecx, addr); Pointer::new( - alloc_id.map(|alloc_id| { - Tag::Concrete(ConcreteTag { alloc_id, sb: SbTag::Untagged }) - }), + alloc_id.map(|alloc_id| Tag::Concrete { alloc_id, sb: SbTag::Untagged }), Size::from_bytes(addr), ) } @@ -220,8 +220,8 @@ impl<'mir, 'tcx> GlobalStateInner { ) -> Option<(AllocId, Size)> { let (tag, addr) = ptr.into_parts(); // addr is absolute (Tag provenance) - let alloc_id = if let Tag::Concrete(concrete) = tag { - concrete.alloc_id + let alloc_id = if let Tag::Concrete { alloc_id, .. } = tag { + alloc_id } else { // A wildcard pointer. assert_eq!(ecx.machine.intptrcast.borrow().provenance_mode, ProvenanceMode::Permissive); diff --git a/src/lib.rs b/src/lib.rs index dc6c0b8dc603..68489c9b47b9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,6 +6,8 @@ #![feature(let_else)] #![feature(io_error_more)] #![feature(yeet_expr)] +#![feature(is_some_with)] +#![feature(nonzero_ops)] #![warn(rust_2018_idioms)] #![allow( clippy::collapsible_else_if, @@ -80,15 +82,15 @@ pub use crate::eval::{ pub use crate::helpers::{CurrentSpan, EvalContextExt as HelpersEvalContextExt}; pub use crate::intptrcast::ProvenanceMode; pub use crate::machine::{ - AllocExtra, ConcreteTag, Evaluator, FrameData, MiriEvalContext, MiriEvalContextExt, - MiriMemoryKind, Tag, NUM_CPUS, PAGE_SIZE, STACK_ADDR, STACK_SIZE, + AllocExtra, Evaluator, FrameData, MiriEvalContext, MiriEvalContextExt, MiriMemoryKind, Tag, + NUM_CPUS, PAGE_SIZE, STACK_ADDR, STACK_SIZE, }; pub use crate::mono_hash_map::MonoHashMap; pub use crate::operator::EvalContextExt as OperatorEvalContextExt; pub use crate::range_map::RangeMap; pub use crate::stacked_borrows::{ - CallId, EvalContextExt as StackedBorEvalContextExt, Item, Permission, PtrId, SbTag, Stack, - Stacks, + CallId, EvalContextExt as StackedBorEvalContextExt, Item, Permission, PtrId, SbTag, SbTagExtra, + Stack, Stacks, }; pub use crate::sync::{CondvarId, EvalContextExt as SyncEvalContextExt, MutexId, RwLockId}; pub use crate::thread::{ diff --git a/src/machine.rs b/src/machine.rs index d14ddaa1a6bb..79414ada5eac 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -130,17 +130,14 @@ impl fmt::Display for MiriMemoryKind { /// Pointer provenance (tag). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum Tag { - Concrete(ConcreteTag), + Concrete { + alloc_id: AllocId, + /// Stacked Borrows tag. + sb: SbTag, + }, Wildcard, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct ConcreteTag { - pub alloc_id: AllocId, - /// Stacked Borrows tag. - pub sb: SbTag, -} - #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(Pointer, 24); // #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] @@ -160,15 +157,15 @@ impl Provenance for Tag { write!(f, "0x{:x}", addr.bytes())?; match tag { - Tag::Concrete(tag) => { + Tag::Concrete { alloc_id, sb } => { // Forward `alternate` flag to `alloc_id` printing. if f.alternate() { - write!(f, "[{:#?}]", tag.alloc_id)?; + write!(f, "[{:#?}]", alloc_id)?; } else { - write!(f, "[{:?}]", tag.alloc_id)?; + write!(f, "[{:?}]", alloc_id)?; } // Print Stacked Borrows tag. - write!(f, "{:?}", tag.sb)?; + write!(f, "{:?}", sb)?; } Tag::Wildcard => { write!(f, "[Wildcard]")?; @@ -180,7 +177,7 @@ impl Provenance for Tag { fn get_alloc_id(self) -> Option { match self { - Tag::Concrete(concrete) => Some(concrete.alloc_id), + Tag::Concrete { alloc_id, .. } => Some(alloc_id), Tag::Wildcard => None, } } @@ -489,7 +486,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { type AllocExtra = AllocExtra; type PointerTag = Tag; - type TagExtra = SbTag; + type TagExtra = SbTagExtra; type MemoryMap = MonoHashMap, Allocation)>; @@ -649,7 +646,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { let alloc: Allocation = alloc.convert_tag_add_extra( &ecx.tcx, AllocExtra { - stacked_borrows: stacks, + stacked_borrows: stacks.map(RefCell::new), data_race: race_alloc, weak_memory: buffer_alloc, }, @@ -682,7 +679,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { SbTag::Untagged }; Pointer::new( - Tag::Concrete(ConcreteTag { alloc_id: ptr.provenance, sb: sb_tag }), + Tag::Concrete { alloc_id: ptr.provenance, sb: sb_tag }, Size::from_bytes(absolute_addr), ) } @@ -708,8 +705,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { ptr: Pointer, ) -> InterpResult<'tcx> { match ptr.provenance { - Tag::Concrete(concrete) => - intptrcast::GlobalStateInner::expose_addr(ecx, concrete.alloc_id), + Tag::Concrete { alloc_id, sb } => { + intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, sb); + } Tag::Wildcard => { // No need to do anything for wildcard pointers as // their provenances have already been previously exposed. @@ -728,8 +726,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { rel.map(|(alloc_id, size)| { let sb = match ptr.provenance { - Tag::Concrete(ConcreteTag { sb, .. }) => sb, - Tag::Wildcard => SbTag::Untagged, + Tag::Concrete { sb, .. } => SbTagExtra::Concrete(sb), + Tag::Wildcard => SbTagExtra::Wildcard, }; (alloc_id, size, sb) }) @@ -747,7 +745,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { data_race.read(alloc_id, range, machine.data_race.as_ref().unwrap())?; } if let Some(stacked_borrows) = &alloc_extra.stacked_borrows { - stacked_borrows.memory_read( + stacked_borrows.borrow_mut().memory_read( alloc_id, tag, range, @@ -773,7 +771,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { data_race.write(alloc_id, range, machine.data_race.as_mut().unwrap())?; } if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows { - stacked_borrows.memory_written( + stacked_borrows.get_mut().memory_written( alloc_id, tag, range, @@ -802,7 +800,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { data_race.deallocate(alloc_id, range, machine.data_race.as_mut().unwrap())?; } if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows { - stacked_borrows.memory_deallocated( + stacked_borrows.get_mut().memory_deallocated( alloc_id, tag, range, diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 0c537e0d7a5c..6fa70ddfc5d4 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -3,6 +3,7 @@ use log::trace; use std::cell::RefCell; +use std::cmp; use std::fmt; use std::num::NonZeroU64; @@ -24,7 +25,8 @@ use diagnostics::{AllocHistory, TagHistory}; pub type PtrId = NonZeroU64; pub type CallId = NonZeroU64; -pub type AllocExtra = Stacks; +// Even reading memory can have effects on the stack, so we need a `RefCell` here. +pub type AllocExtra = RefCell; /// Tracking pointer provenance #[derive(Copy, Clone, Hash, Eq)] @@ -60,6 +62,32 @@ impl fmt::Debug for SbTag { } } +/// The "extra" information an SB pointer has over a regular AllocId. +/// Newtype for `Option`. +#[derive(Copy, Clone)] +pub enum SbTagExtra { + Concrete(SbTag), + Wildcard, +} + +impl fmt::Debug for SbTagExtra { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + SbTagExtra::Concrete(tag) => write!(f, "{tag:?}"), + SbTagExtra::Wildcard => write!(f, ""), + } + } +} + +impl SbTagExtra { + fn and_then(self, f: impl FnOnce(SbTag) -> Option) -> Option { + match self { + SbTagExtra::Concrete(tag) => f(tag), + SbTagExtra::Wildcard => None, + } + } +} + /// Indicates which permission is granted (by this item to some pointers) #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum Permission { @@ -104,15 +132,24 @@ pub struct Stack { /// * Above a `SharedReadOnly` there can only be more `SharedReadOnly`. /// * Except for `Untagged`, no tag occurs in the stack more than once. borrows: Vec, + /// If this is `Some(id)`, then the actual current stack is unknown. This can happen when + /// wildcard pointers are used to access this location. What we do know is that `borrows` are at + /// the top of the stack, and below it are arbitrarily many items whose `tag` is either + /// `Untagged` or strictly less than `id`. + /// When the bottom is unknown, `borrows` always has a `SharedReadOnly` or `Unique` at the bottom; + /// we never have the unknown-to-known boundary in an SRW group. + unknown_bottom: Option, } /// Extra per-allocation state. #[derive(Clone, Debug)] pub struct Stacks { // Even reading memory can have effects on the stack, so we need a `RefCell` here. - stacks: RefCell>, + stacks: RangeMap, /// Stores past operations on this allocation - history: RefCell, + history: AllocHistory, + /// The set of tags that have been exposed inside this allocation. + exposed_tags: FxHashSet, } /// Extra global state, available to the memory access hooks. @@ -282,19 +319,61 @@ impl Permission { /// Core per-location operations: access, dealloc, reborrow. impl<'tcx> Stack { /// Find the item granting the given kind of access to the given tag, and return where - /// it is on the stack. - fn find_granting(&self, access: AccessKind, tag: SbTag) -> Option { - self.borrows - .iter() - .enumerate() // we also need to know *where* in the stack - .rev() // search top-to-bottom - // Return permission of first item that grants access. - // We require a permission with the right tag, ensuring U3 and F3. - .find_map( - |(idx, item)| { + /// it is on the stack. For wildcard tags, the given index is approximate, but if *no* + /// index is given it means the match was *not* in the known part of the stack. + /// `Ok(None)` indicates it matched the "unknown" part of the stack. + /// `Err` indicates it was not found. + fn find_granting( + &self, + access: AccessKind, + tag: SbTagExtra, + exposed_tags: &FxHashSet, + ) -> Result, ()> { + let SbTagExtra::Concrete(tag) = tag else { + // Handle the wildcard case. + // Go search the stack for an exposed tag. + if let Some(idx) = + self.borrows + .iter() + .enumerate() // we also need to know *where* in the stack + .rev() // search top-to-bottom + .find_map(|(idx, item)| { + // If the item fits and *might* be this wildcard, use it. + if item.perm.grants(access) && exposed_tags.contains(&item.tag) { + Some(idx) + } else { + None + } + }) + { + return Ok(Some(idx)); + } + // If we couldn't find it in the stack, check the unknown bottom. + return if self.unknown_bottom.is_some() { Ok(None) } else { Err(()) }; + }; + + if let Some(idx) = + self.borrows + .iter() + .enumerate() // we also need to know *where* in the stack + .rev() // search top-to-bottom + // Return permission of first item that grants access. + // We require a permission with the right tag, ensuring U3 and F3. + .find_map(|(idx, item)| { if tag == item.tag && item.perm.grants(access) { Some(idx) } else { None } - }, - ) + }) + { + return Ok(Some(idx)); + } + + // Couldn't find it in the stack; but if there is an unknown bottom it might be there. + let found = self.unknown_bottom.is_some_and(|&unknown_limit| { + match tag { + SbTag::Tagged(tag_id) => tag_id < unknown_limit, // unknown_limit is an upper bound for what can be in the unknown bottom. + SbTag::Untagged => true, // yeah whatever + } + }); + if found { Ok(None) } else { Err(()) } } /// Find the first write-incompatible item above the given one -- @@ -304,8 +383,10 @@ impl<'tcx> Stack { match perm { Permission::SharedReadOnly => bug!("Cannot use SharedReadOnly for writing"), Permission::Disabled => bug!("Cannot use Disabled for anything"), - // On a write, everything above us is incompatible. - Permission::Unique => granting + 1, + Permission::Unique => { + // On a write, everything above us is incompatible. + granting + 1 + } Permission::SharedReadWrite => { // The SharedReadWrite *just* above us are compatible, to skip those. let mut idx = granting + 1; @@ -333,7 +414,7 @@ impl<'tcx> Stack { /// currently checking. fn check_protector( item: &Item, - provoking_access: Option<(SbTag, AllocRange, Size, AccessKind)>, // just for debug printing and error messages + provoking_access: Option<(SbTagExtra, AllocRange, Size, AccessKind)>, // just for debug printing and error messages global: &GlobalStateInner, alloc_history: &mut AllocHistory, ) -> InterpResult<'tcx> { @@ -354,12 +435,14 @@ impl<'tcx> Stack { tag, item ), None, - alloc_history.get_logs_relevant_to( - tag, - alloc_range, - offset, - Some(item.tag), - ), + tag.and_then(|tag| { + alloc_history.get_logs_relevant_to( + tag, + alloc_range, + offset, + Some(item.tag), + ) + }), ))? } else { Err(err_sb_ub( @@ -380,25 +463,36 @@ impl<'tcx> Stack { fn access( &mut self, access: AccessKind, - tag: SbTag, + tag: SbTagExtra, (alloc_id, alloc_range, offset): (AllocId, AllocRange, Size), // just for debug printing and error messages global: &mut GlobalStateInner, current_span: &mut CurrentSpan<'_, '_, 'tcx>, alloc_history: &mut AllocHistory, + exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { // Two main steps: Find granting item, remove incompatible items above. // Step 1: Find granting item. - let granting_idx = self.find_granting(access, tag).ok_or_else(|| { + let granting_idx = self.find_granting(access, tag, exposed_tags).map_err(|_| { alloc_history.access_error(access, tag, alloc_id, alloc_range, offset, self) })?; // Step 2: Remove incompatible items above them. Make sure we do not remove protected // items. Behavior differs for reads and writes. + // In case of wildcards/unknown matches, we remove everything that is *definitely* gone. if access == AccessKind::Write { // Remove everything above the write-compatible items, like a proper stack. This makes sure read-only and unique // pointers become invalid on write accesses (ensures F2a, and ensures U2 for write accesses). - let first_incompatible_idx = self.find_first_write_incompatible(granting_idx); + let first_incompatible_idx = if let Some(granting_idx) = granting_idx { + // The granting_idx *might* be approximate, but any lower idx would remove more + // things. Even if this is a Unique and the lower idx is an SRW (which removes + // less), there is an SRW group boundary here so strictly more would get removed. + self.find_first_write_incompatible(granting_idx) + } else { + // We are writing to something in the unknown part. + // There is a SRW group boundary between the unknown and the known, so everything is incompatible. + 0 + }; for item in self.borrows.drain(first_incompatible_idx..).rev() { trace!("access: popping item {:?}", item); Stack::check_protector( @@ -418,8 +512,16 @@ impl<'tcx> Stack { // This pattern occurs a lot in the standard library: create a raw pointer, then also create a shared // reference and use that. // We *disable* instead of removing `Unique` to avoid "connecting" two neighbouring blocks of SRWs. - for idx in ((granting_idx + 1)..self.borrows.len()).rev() { + let first_incompatible_idx = if let Some(granting_idx) = granting_idx { + // The granting_idx *might* be approximate, but any lower idx would disable more things. + granting_idx + 1 + } else { + // We are reading from something in the unknown part. That means *all* `Unique` we know about are dead now. + 0 + }; + for idx in (first_incompatible_idx..self.borrows.len()).rev() { let item = &mut self.borrows[idx]; + if item.perm == Permission::Unique { trace!("access: disabling item {:?}", item); Stack::check_protector( @@ -434,6 +536,31 @@ impl<'tcx> Stack { } } + // If this was an approximate action, we now collapse everything into an unknown. + if granting_idx.is_none() || matches!(tag, SbTagExtra::Wildcard) { + // Compute the upper bound of the items that remain. + // (This is why we did all the work above: to reduce the items we have to consider here.) + let mut max = NonZeroU64::new(1).unwrap(); + for item in &self.borrows { + // Skip disabled items, they cannot be matched anyway. + if !matches!(item.perm, Permission::Disabled) { + if let SbTag::Tagged(tag) = item.tag { + // We are looking for a strict upper bound, so add 1 to this tag. + max = cmp::max(tag.checked_add(1).unwrap(), max); + } + } + } + if let Some(unk) = self.unknown_bottom { + max = cmp::max(unk, max); + } + // Use `max` as new strict upper bound for everything. + trace!( + "access: forgetting stack to upper bound {max} due to wildcard or unknown access" + ); + self.borrows.clear(); + self.unknown_bottom = Some(max); + } + // Done. Ok(()) } @@ -442,19 +569,20 @@ impl<'tcx> Stack { /// active protectors at all because we will remove all items. fn dealloc( &mut self, - tag: SbTag, + tag: SbTagExtra, (alloc_id, alloc_range, offset): (AllocId, AllocRange, Size), // just for debug printing and error messages global: &GlobalStateInner, alloc_history: &mut AllocHistory, + exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { - // Step 1: Find granting item. - self.find_granting(AccessKind::Write, tag).ok_or_else(|| { + // Step 1: Make sure there is a granting item. + self.find_granting(AccessKind::Write, tag, exposed_tags).map_err(|_| { err_sb_ub(format!( "no item granting write access for deallocation to tag {:?} at {:?} found in borrow stack", tag, alloc_id, ), None, - alloc_history.get_logs_relevant_to(tag, alloc_range, offset, None), + tag.and_then(|tag| alloc_history.get_logs_relevant_to(tag, alloc_range, offset, None)), ) })?; @@ -462,7 +590,6 @@ impl<'tcx> Stack { for item in self.borrows.drain(..).rev() { Stack::check_protector(&item, None, global, alloc_history)?; } - Ok(()) } @@ -474,21 +601,24 @@ impl<'tcx> Stack { /// `range` that we are currently checking. fn grant( &mut self, - derived_from: SbTag, + derived_from: SbTagExtra, new: Item, (alloc_id, alloc_range, offset): (AllocId, AllocRange, Size), // just for debug printing and error messages global: &mut GlobalStateInner, current_span: &mut CurrentSpan<'_, '_, 'tcx>, alloc_history: &mut AllocHistory, + exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { // Figure out which access `perm` corresponds to. let access = if new.perm.grants(AccessKind::Write) { AccessKind::Write } else { AccessKind::Read }; + // Now we figure out which item grants our parent (`derived_from`) this kind of access. // We use that to determine where to put the new item. - let granting_idx = self.find_granting(access, derived_from).ok_or_else(|| { - alloc_history.grant_error(derived_from, new, alloc_id, alloc_range, offset, self) - })?; + let granting_idx = + self.find_granting(access, derived_from, exposed_tags).map_err(|_| { + alloc_history.grant_error(derived_from, new, alloc_id, alloc_range, offset, self) + })?; // Compute where to put the new item. // Either way, we ensure that we insert the new item in a way such that between @@ -498,6 +628,18 @@ impl<'tcx> Stack { access == AccessKind::Write, "this case only makes sense for stack-like accesses" ); + + let (Some(granting_idx), SbTagExtra::Concrete(_)) = (granting_idx, derived_from) else { + // The parent is a wildcard pointer or matched the unknown bottom. + // This is approximate. Nobody knows what happened, so forget everything. + // The new thing is SRW anyway, so we cannot push it "on top of the unkown part" + // (for all we know, it might join an SRW group inside the unknown). + trace!("reborrow: forgetting stack entirely due to SharedReadWrite reborrow from wildcard or unknown"); + self.borrows.clear(); + self.unknown_bottom = Some(global.next_ptr_id); + return Ok(()); + }; + // SharedReadWrite can coexist with "existing loans", meaning they don't act like a write // access. Instead of popping the stack, we insert the item at the place the stack would // be popped to (i.e., we insert it above all the write-compatible items). @@ -514,6 +656,7 @@ impl<'tcx> Stack { global, current_span, alloc_history, + exposed_tags, )?; // We insert "as far up as possible": We know only compatible items are remaining @@ -524,14 +667,16 @@ impl<'tcx> Stack { }; // Put the new item there. As an optimization, deduplicate if it is equal to one of its new neighbors. - if self.borrows[new_idx - 1] == new || self.borrows.get(new_idx) == Some(&new) { + // `new_idx` might be 0 if we just cleared the entire stack. + if self.borrows.get(new_idx) == Some(&new) + || (new_idx > 0 && self.borrows[new_idx - 1] == new) + { // Optimization applies, done. trace!("reborrow: avoiding adding redundant item {:?}", new); } else { trace!("reborrow: adding item {:?}", new); self.borrows.insert(new_idx, new); } - Ok(()) } } @@ -542,38 +687,28 @@ impl<'tcx> Stacks { /// Creates new stack with initial tag. fn new(size: Size, perm: Permission, tag: SbTag) -> Self { let item = Item { perm, tag, protector: None }; - let stack = Stack { borrows: vec![item] }; + let stack = Stack { borrows: vec![item], unknown_bottom: None }; Stacks { - stacks: RefCell::new(RangeMap::new(size, stack)), - history: RefCell::new(AllocHistory::new()), + stacks: RangeMap::new(size, stack), + history: AllocHistory::new(), + exposed_tags: FxHashSet::default(), } } /// Call `f` on every stack in the range. fn for_each( - &self, - range: AllocRange, - mut f: impl FnMut(Size, &mut Stack, &mut AllocHistory) -> InterpResult<'tcx>, - ) -> InterpResult<'tcx> { - let mut stacks = self.stacks.borrow_mut(); - let history = &mut *self.history.borrow_mut(); - for (offset, stack) in stacks.iter_mut(range.start, range.size) { - f(offset, stack, history)?; - } - Ok(()) - } - - /// Call `f` on every stack in the range. - fn for_each_mut( &mut self, range: AllocRange, - mut f: impl FnMut(Size, &mut Stack, &mut AllocHistory) -> InterpResult<'tcx>, + mut f: impl FnMut( + Size, + &mut Stack, + &mut AllocHistory, + &mut FxHashSet, + ) -> InterpResult<'tcx>, ) -> InterpResult<'tcx> { - let stacks = self.stacks.get_mut(); - let history = &mut *self.history.borrow_mut(); - for (offset, stack) in stacks.iter_mut(range.start, range.size) { - f(offset, stack, history)?; + for (offset, stack) in self.stacks.iter_mut(range.start, range.size) { + f(offset, stack, &mut self.history, &mut self.exposed_tags)?; } Ok(()) } @@ -620,8 +755,8 @@ impl Stacks { (tag, Permission::SharedReadWrite) } }; - let stacks = Stacks::new(size, perm, base_tag); - stacks.history.borrow_mut().log_creation( + let mut stacks = Stacks::new(size, perm, base_tag); + stacks.history.log_creation( None, base_tag, alloc_range(Size::ZERO, size), @@ -632,9 +767,9 @@ impl Stacks { #[inline(always)] pub fn memory_read<'tcx>( - &self, + &mut self, alloc_id: AllocId, - tag: SbTag, + tag: SbTagExtra, range: AllocRange, state: &GlobalState, mut current_span: CurrentSpan<'_, '_, 'tcx>, @@ -646,7 +781,7 @@ impl Stacks { range.size.bytes() ); let mut state = state.borrow_mut(); - self.for_each(range, |offset, stack, history| { + self.for_each(range, |offset, stack, history, exposed_tags| { stack.access( AccessKind::Read, tag, @@ -654,6 +789,7 @@ impl Stacks { &mut state, &mut current_span, history, + exposed_tags, ) }) } @@ -662,7 +798,7 @@ impl Stacks { pub fn memory_written<'tcx>( &mut self, alloc_id: AllocId, - tag: SbTag, + tag: SbTagExtra, range: AllocRange, state: &GlobalState, mut current_span: CurrentSpan<'_, '_, 'tcx>, @@ -674,7 +810,7 @@ impl Stacks { range.size.bytes() ); let mut state = state.borrow_mut(); - self.for_each_mut(range, |offset, stack, history| { + self.for_each(range, |offset, stack, history, exposed_tags| { stack.access( AccessKind::Write, tag, @@ -682,6 +818,7 @@ impl Stacks { &mut state, &mut current_span, history, + exposed_tags, ) }) } @@ -690,14 +827,14 @@ impl Stacks { pub fn memory_deallocated<'tcx>( &mut self, alloc_id: AllocId, - tag: SbTag, + tag: SbTagExtra, range: AllocRange, state: &GlobalState, ) -> InterpResult<'tcx> { trace!("deallocation with tag {:?}: {:?}, size {}", tag, alloc_id, range.size.bytes()); let state = state.borrow(); - self.for_each_mut(range, |offset, stack, history| { - stack.dealloc(tag, (alloc_id, range, offset), &state, history) + self.for_each(range, |offset, stack, history, exposed_tags| { + stack.dealloc(tag, (alloc_id, range, offset), &state, history, exposed_tags) })?; Ok(()) } @@ -707,6 +844,8 @@ impl Stacks { /// to grant for which references, and when to add protectors. impl<'mir, 'tcx: 'mir> EvalContextPrivExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { + /// Returns the `AllocId` the reborrow was done in, if some actual borrow stack manipulation + /// happened. fn reborrow( &mut self, place: &MPlaceTy<'tcx, Tag>, @@ -714,7 +853,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx kind: RefKind, new_tag: SbTag, protect: bool, - ) -> InterpResult<'tcx> { + ) -> InterpResult<'tcx, Option> { let this = self.eval_context_mut(); let current_span = &mut this.machine.current_span(); @@ -724,23 +863,36 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx base_offset, orig_tag| -> InterpResult<'tcx> { + let SbTagExtra::Concrete(orig_tag) = orig_tag else { + // FIXME: should we log this? + return Ok(()) + }; let extra = this.get_alloc_extra(alloc_id)?; - let stacked_borrows = - extra.stacked_borrows.as_ref().expect("we should have Stacked Borrows data"); - let mut alloc_history = stacked_borrows.history.borrow_mut(); - alloc_history.log_creation( + let mut stacked_borrows = extra + .stacked_borrows + .as_ref() + .expect("we should have Stacked Borrows data") + .borrow_mut(); + stacked_borrows.history.log_creation( Some(orig_tag), new_tag, alloc_range(base_offset, size), current_span, ); if protect { - alloc_history.log_protector(orig_tag, new_tag, current_span); + stacked_borrows.history.log_protector(orig_tag, new_tag, current_span); } Ok(()) }; if size == Size::ZERO { + trace!( + "reborrow of size 0: {} reference {:?} derived from {:?} (pointee {})", + kind, + new_tag, + place.ptr, + place.layout.ty, + ); // Don't update any stacks for a zero-sized access; borrow stacks are per-byte and this // touches no bytes so there is no stack to put this tag in. // However, if the pointer for this operation points at a real allocation we still @@ -750,16 +902,10 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // 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, current_span, alloc_id, base_offset, orig_tag)?; + return Ok(Some(alloc_id)); } - - trace!( - "reborrow of size 0: {} reference {:?} derived from {:?} (pointee {})", - kind, - new_tag, - place.ptr, - place.layout.ty, - ); - return Ok(()); + // This pointer doesn't come with an AllocId. :shrug: + return Ok(None); } let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr)?; log_creation(this, current_span, alloc_id, base_offset, orig_tag)?; @@ -810,8 +956,11 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // We have to use shared references to alloc/memory_extra here since // `visit_freeze_sensitive` needs to access the global state. let extra = this.get_alloc_extra(alloc_id)?; - let stacked_borrows = - extra.stacked_borrows.as_ref().expect("we should have Stacked Borrows data"); + let mut stacked_borrows = extra + .stacked_borrows + .as_ref() + .expect("we should have Stacked Borrows data") + .borrow_mut(); this.visit_freeze_sensitive(place, size, |mut range, frozen| { // Adjust range. range.start += base_offset; @@ -830,7 +979,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx }; let item = Item { perm, tag: new_tag, protector }; let mut global = this.machine.stacked_borrows.as_ref().unwrap().borrow_mut(); - stacked_borrows.for_each(range, |offset, stack, history| { + stacked_borrows.for_each(range, |offset, stack, history, exposed_tags| { stack.grant( orig_tag, item, @@ -838,23 +987,27 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx &mut *global, current_span, history, + exposed_tags, ) }) })?; - return Ok(()); + return Ok(Some(alloc_id)); } }; // Here we can avoid `borrow()` calls because we have mutable references. // Note that this asserts that the allocation is mutable -- but since we are creating a // mutable pointer, that seems reasonable. let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?; - let stacked_borrows = - alloc_extra.stacked_borrows.as_mut().expect("we should have Stacked Borrows data"); + let mut stacked_borrows = alloc_extra + .stacked_borrows + .as_mut() + .expect("we should have Stacked Borrows data") + .borrow_mut(); let item = Item { perm, tag: new_tag, protector }; let range = alloc_range(base_offset, size); let mut global = machine.stacked_borrows.as_ref().unwrap().borrow_mut(); let current_span = &mut machine.current_span(); // `get_alloc_extra_mut` invalidated our old `current_span` - stacked_borrows.for_each_mut(range, |offset, stack, history| { + stacked_borrows.for_each(range, |offset, stack, history, exposed_tags| { stack.grant( orig_tag, item, @@ -862,10 +1015,11 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx &mut global, current_span, history, + exposed_tags, ) })?; - Ok(()) + Ok(Some(alloc_id)) } /// Retags an indidual pointer, returning the retagged version. @@ -900,16 +1054,22 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx }; // Reborrow. - this.reborrow(&place, size, kind, new_tag, protect)?; + let alloc_id = this.reborrow(&place, size, kind, new_tag, protect)?; // Adjust pointer. let new_place = place.map_provenance(|p| { - p.map(|t| { - // TODO: Fix this eventually - if let Tag::Concrete(t) = t { - Tag::Concrete(ConcreteTag { sb: new_tag, ..t }) - } else { - t + 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. + Tag::Concrete { alloc_id, sb: new_tag } + } + None => { + // Looks like this has to stay a wildcard pointer. + assert!(matches!(prov, Tag::Wildcard)); + Tag::Wildcard + } } }) }); @@ -990,4 +1150,27 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(()) } + + /// Mark the given tag as exposed. It was found on a pointer with the given AllocId. + fn expose_tag(&mut self, alloc_id: AllocId, tag: SbTag) { + let this = self.eval_context_mut(); + + // Function pointers and dead objects don't have an alloc_extra so we ignore them. + // This is okay because accessing them is UB anyway, no need for any Stacked Borrows checks. + // NOT using `get_alloc_extra_mut` since this might be a read-only allocation! + // FIXME: this catches `InterpError`, which we should not usually do. + // We might need a proper fallible API from `memory.rs` to avoid this though. + match this.get_alloc_extra(alloc_id) { + Ok(alloc_extra) => { + trace!("Stacked Borrows tag {tag:?} exposed in {alloc_id}"); + alloc_extra.stacked_borrows.as_ref().unwrap().borrow_mut().exposed_tags.insert(tag); + } + Err(err) => { + trace!( + "Not exposing Stacked Borrows tag {tag:?} due to error \ + when accessing {alloc_id}: {err}" + ); + } + } + } } diff --git a/src/stacked_borrows/diagnostics.rs b/src/stacked_borrows/diagnostics.rs index 5400e9abe503..a4a7f5e7a1e3 100644 --- a/src/stacked_borrows/diagnostics.rs +++ b/src/stacked_borrows/diagnostics.rs @@ -8,6 +8,7 @@ use crate::helpers::{CurrentSpan, HexRange}; use crate::stacked_borrows::{err_sb_ub, AccessKind, Permission}; use crate::Item; use crate::SbTag; +use crate::SbTagExtra; use crate::Stack; use rustc_middle::mir::interpret::InterpError; @@ -197,7 +198,7 @@ impl AllocHistory { /// Report a descriptive error when `new` could not be granted from `derived_from`. pub fn grant_error<'tcx>( &self, - derived_from: SbTag, + derived_from: SbTagExtra, new: Item, alloc_id: AllocId, alloc_range: AllocRange, @@ -205,8 +206,7 @@ impl AllocHistory { stack: &Stack, ) -> InterpError<'tcx> { let action = format!( - "trying to reborrow {:?} for {:?} permission at {}[{:#x}]", - derived_from, + "trying to reborrow {derived_from:?} for {:?} permission at {}[{:#x}]", new.perm, alloc_id, error_offset.bytes(), @@ -214,7 +214,9 @@ impl AllocHistory { err_sb_ub( format!("{}{}", action, error_cause(stack, derived_from)), Some(operation_summary("a reborrow", alloc_id, alloc_range)), - self.get_logs_relevant_to(derived_from, alloc_range, error_offset, None), + derived_from.and_then(|derived_from| { + self.get_logs_relevant_to(derived_from, alloc_range, error_offset, None) + }), ) } @@ -222,23 +224,21 @@ impl AllocHistory { pub fn access_error<'tcx>( &self, access: AccessKind, - tag: SbTag, + tag: SbTagExtra, alloc_id: AllocId, alloc_range: AllocRange, error_offset: Size, stack: &Stack, ) -> InterpError<'tcx> { let action = format!( - "attempting a {} using {:?} at {}[{:#x}]", - access, - tag, + "attempting a {access} using {tag:?} at {}[{:#x}]", alloc_id, error_offset.bytes(), ); err_sb_ub( format!("{}{}", action, error_cause(stack, tag)), Some(operation_summary("an access", alloc_id, alloc_range)), - self.get_logs_relevant_to(tag, alloc_range, error_offset, None), + tag.and_then(|tag| self.get_logs_relevant_to(tag, alloc_range, error_offset, None)), ) } } @@ -251,10 +251,14 @@ fn operation_summary( format!("this error occurs as part of {} at {:?}{}", operation, alloc_id, HexRange(alloc_range)) } -fn error_cause(stack: &Stack, tag: SbTag) -> &'static str { - if stack.borrows.iter().any(|item| item.tag == tag && item.perm != Permission::Disabled) { - ", but that tag only grants SharedReadOnly permission for this location" +fn error_cause(stack: &Stack, tag: SbTagExtra) -> &'static str { + if let SbTagExtra::Concrete(tag) = tag { + if stack.borrows.iter().any(|item| item.tag == tag && item.perm != Permission::Disabled) { + ", but that tag only grants SharedReadOnly permission for this location" + } else { + ", but that tag does not exist in the borrow stack for this location" + } } else { - ", but that tag does not exist in the borrow stack for this location" + ", but no exposed tags have suitable permission in the borrow stack for this location" } } diff --git a/tests/fail/provenance/permissive_provenance_transmute.rs b/tests/fail/provenance/permissive_provenance_transmute.rs index dbfc5732ed3b..28e6ba623080 100644 --- a/tests/fail/provenance/permissive_provenance_transmute.rs +++ b/tests/fail/provenance/permissive_provenance_transmute.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-permissive-provenance -Zmiri-disable-stacked-borrows +// compile-flags: -Zmiri-permissive-provenance #![feature(strict_provenance)] use std::mem; diff --git a/tests/fail/provenance/ptr_int_unexposed.rs b/tests/fail/provenance/ptr_int_unexposed.rs index 310024c1efc7..ad29d38dc3f7 100644 --- a/tests/fail/provenance/ptr_int_unexposed.rs +++ b/tests/fail/provenance/ptr_int_unexposed.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-permissive-provenance -Zmiri-disable-stacked-borrows +// compile-flags: -Zmiri-permissive-provenance #![feature(strict_provenance)] fn main() { diff --git a/tests/fail/stacked_borrows/exposed_only_ro.rs b/tests/fail/stacked_borrows/exposed_only_ro.rs new file mode 100644 index 000000000000..9b4234499df0 --- /dev/null +++ b/tests/fail/stacked_borrows/exposed_only_ro.rs @@ -0,0 +1,12 @@ +// compile-flags: -Zmiri-permissive-provenance +#![feature(strict_provenance)] + +// If we have only exposed read-only pointers, doing a write through a wildcard ptr should fail. + +fn main() { + let mut x = 0; + let _fool = &mut x as *mut i32; // this would have fooled the old untagged pointer logic + let addr = (&x as *const i32).expose_addr(); + let ptr = std::ptr::from_exposed_addr_mut::(addr); + unsafe { *ptr = 0 }; //~ ERROR: borrow stack +} diff --git a/tests/fail/stacked_borrows/exposed_only_ro.stderr b/tests/fail/stacked_borrows/exposed_only_ro.stderr new file mode 100644 index 000000000000..28fa98b6020c --- /dev/null +++ b/tests/fail/stacked_borrows/exposed_only_ro.stderr @@ -0,0 +1,18 @@ +error: Undefined Behavior: attempting a write access using at ALLOC[0x0], but no exposed tags have suitable permission in the borrow stack for this location + --> $DIR/exposed_only_ro.rs:LL:CC + | +LL | unsafe { *ptr = 0 }; + | ^^^^^^^^ + | | + | attempting a write access using at ALLOC[0x0], but no exposed tags have suitable permission in the borrow stack for this location + | this error occurs as part of an access at ALLOC[0x0..0x4] + | + = 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 + + = note: inside `main` at $DIR/exposed_only_ro.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/stacked_borrows/illegal_read_despite_exposed1.rs b/tests/fail/stacked_borrows/illegal_read_despite_exposed1.rs new file mode 100644 index 000000000000..61a5e05d34cd --- /dev/null +++ b/tests/fail/stacked_borrows/illegal_read_despite_exposed1.rs @@ -0,0 +1,17 @@ +// compile-flags: -Zmiri-permissive-provenance + +fn main() { + unsafe { + let root = &mut 42; + let addr = root as *mut i32 as usize; + let exposed_ptr = addr as *mut i32; + // From the exposed ptr, we get a new unique ptr. + let root2 = &mut *exposed_ptr; + let _fool = root2 as *mut _; // this would have fooled the old untagged pointer logic + // Stack: Unknown( at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + --> $DIR/illegal_read_despite_exposed1.rs:LL:CC + | +LL | let _val = *root2; + | ^^^^^^ + | | + | attempting a read access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + | this error occurs as part of an access at ALLOC[0x0..0x4] + | + = 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 + + = note: inside `main` at $DIR/illegal_read_despite_exposed1.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/stacked_borrows/illegal_read_despite_exposed2.rs b/tests/fail/stacked_borrows/illegal_read_despite_exposed2.rs new file mode 100644 index 000000000000..19d0784591e4 --- /dev/null +++ b/tests/fail/stacked_borrows/illegal_read_despite_exposed2.rs @@ -0,0 +1,20 @@ +// compile-flags: -Zmiri-permissive-provenance + +fn main() { + unsafe { + let root = &mut 42; + let addr = root as *mut i32 as usize; + let exposed_ptr = addr as *mut i32; + // From the exposed ptr, we get a new unique ptr. + let root2 = &mut *exposed_ptr; + // let _fool = root2 as *mut _; // this would fool us, since SRW(N+1) remains on the stack + // Stack: Unknown( at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + --> $DIR/illegal_read_despite_exposed2.rs:LL:CC + | +LL | let _val = *root2; + | ^^^^^^ + | | + | attempting a read access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + | this error occurs as part of an access at ALLOC[0x0..0x4] + | + = 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 + + = note: inside `main` at $DIR/illegal_read_despite_exposed2.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/stacked_borrows/illegal_write_despite_exposed1.rs b/tests/fail/stacked_borrows/illegal_write_despite_exposed1.rs new file mode 100644 index 000000000000..b50399b9df52 --- /dev/null +++ b/tests/fail/stacked_borrows/illegal_write_despite_exposed1.rs @@ -0,0 +1,16 @@ +// compile-flags: -Zmiri-permissive-provenance + +fn main() { + unsafe { + let root = &mut 42; + let addr = root as *mut i32 as usize; + let exposed_ptr = addr as *mut i32; + // From the exposed ptr, we get a new SRO ptr. + let root2 = &*exposed_ptr; + // Stack: Unknown( at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + --> $DIR/illegal_write_despite_exposed1.rs:LL:CC + | +LL | let _val = *root2; + | ^^^^^^ + | | + | attempting a read access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + | this error occurs as part of an access at ALLOC[0x0..0x4] + | + = 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 + + = note: inside `main` at $DIR/illegal_write_despite_exposed1.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/ptr_int_from_exposed.rs b/tests/pass/ptr_int_from_exposed.rs index e025cf921313..dc9cb393b781 100644 --- a/tests/pass/ptr_int_from_exposed.rs +++ b/tests/pass/ptr_int_from_exposed.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-permissive-provenance -Zmiri-disable-stacked-borrows +// compile-flags: -Zmiri-permissive-provenance #![feature(strict_provenance)] use std::ptr; diff --git a/tests/pass/stacked-borrows/int-to-ptr.rs b/tests/pass/stacked-borrows/int-to-ptr.rs index efba0da1b935..dc3675406276 100644 --- a/tests/pass/stacked-borrows/int-to-ptr.rs +++ b/tests/pass/stacked-borrows/int-to-ptr.rs @@ -1,6 +1,6 @@ -fn main() { - ref_raw_int_raw(); -} +// compile-flags: -Zmiri-permissive-provenance +#![feature(strict_provenance)] +use std::ptr; // Just to make sure that casting a ref to raw, to int and back to raw // and only then using it works. This rules out ideas like "do escape-to-raw lazily"; @@ -11,3 +11,73 @@ fn ref_raw_int_raw() { let xraw = xref as *mut i32 as usize as *mut i32; assert_eq!(unsafe { *xraw }, 3); } + +/// Ensure that we do not just pick the topmost possible item on int2ptr casts. +fn example(variant: bool) { + unsafe { + fn not_so_innocent(x: &mut u32) -> usize { + let x_raw4 = x as *mut u32; + x_raw4.expose_addr() + } + + let mut c = 42u32; + + let x_unique1 = &mut c; + // [..., Unique(1)] + + let x_raw2 = x_unique1 as *mut u32; + let x_raw2_addr = x_raw2.expose_addr(); + // [..., Unique(1), SharedRW(2)] + + let x_unique3 = &mut *x_raw2; + // [.., Unique(1), SharedRW(2), Unique(3)] + + assert_eq!(not_so_innocent(x_unique3), x_raw2_addr); + // [.., Unique(1), SharedRW(2), Unique(3), ..., SharedRW(4)] + + // Do an int2ptr cast. This can pick tag 2 or 4 (the two previously exposed tags). + // 4 is the "obvious" choice (topmost tag, what we used to do with untagged pointers). + // And indeed if `variant == true` it is the only possible choice. + // But if `variant == false` then 2 is the only possible choice! + let x_wildcard = ptr::from_exposed_addr_mut::(x_raw2_addr); + + if variant { + // If we picked 2, this will invalidate 3. + *x_wildcard = 10; + // Now we use 3. Only possible if above we picked 4. + *x_unique3 = 12; + } else { + // This invalidates tag 4. + *x_unique3 = 10; + // Now try to write with the "guessed" tag; it must be 2. + *x_wildcard = 12; + } + } +} + +fn test() { + unsafe { + let root = &mut 42; + let root_raw = root as *mut i32; + let addr1 = root_raw as usize; + let child = &mut *root_raw; + let child_raw = child as *mut i32; + let addr2 = child_raw as usize; + assert_eq!(addr1, addr2); + // First use child. + *(addr2 as *mut i32) -= 2; // picks child_raw + *child -= 2; + // Then use root. + *(addr1 as *mut i32) += 2; // picks root_raw + *root += 2; + // Value should be unchanged. + assert_eq!(*root, 42); + } +} + +fn main() { + ref_raw_int_raw(); + example(false); + example(true); + test(); +}