diff options
Diffstat (limited to 'src')
36 files changed, 370 insertions, 444 deletions
diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index e1a948c92fa..740118bb4a0 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -117,6 +117,41 @@ jobs: - name: rustdoc run: RUSTDOCFLAGS="-Dwarnings" ./miri doc --document-private-items + bootstrap: + name: bootstrap build + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + # Deliberately skipping `./.github/workflows/setup` as we do our own setup + - name: Add cache for cargo + id: cache + uses: actions/cache@v4 + with: + path: | + # Taken from <https://doc.rust-lang.org/nightly/cargo/guide/cargo-home.html#caching-the-cargo-home-in-ci>. + # Cache package/registry information + ~/.cargo/registry/index + ~/.cargo/registry/cache + ~/.cargo/git/db + # Cache bootstrap downloads + ../rust/build/cache + key: cargo-bootstrap-${{ hashFiles('rust-version') }} + restore-keys: cargo-bootstrap + - name: prepare build environment + run: | + MIRIDIR=$(pwd) + cd .. + # Bootstrap needs at least depth 2 to function. + git clone https://github.com/rust-lang/rust/ rust --depth 2 --revision $(cat "$MIRIDIR/rust-version") + cd rust + # Replace the in-tree Miri with the current version. + rm src/tools/miri -rf + ln -s "$MIRIDIR" src/tools/miri + - name: check build + run: | + cd ../rust # ./x does not seem to like being invoked from elsewhere + ./x check miri + coverage: name: coverage report runs-on: ubuntu-latest @@ -130,7 +165,7 @@ jobs: # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! # And they should be added below in `cron-fail-notify` as well. conclusion: - needs: [test, style, coverage] + needs: [test, style, bootstrap, coverage] # We need to ensure this job does *not* get skipped if its dependencies fail, # because a skipped job is considered a success by GitHub. So we have to # overwrite `if:`. We use `!cancelled()` to ensure the job does still not get run @@ -211,7 +246,7 @@ jobs: cron-fail-notify: name: cronjob failure notification runs-on: ubuntu-latest - needs: [test, style, coverage] + needs: [test, style, bootstrap, coverage] if: ${{ github.event_name == 'schedule' && failure() }} steps: # Send a Zulip notification diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index ee09b9b4b73..f1b52293123 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -130,15 +130,15 @@ impl Command { let new_commit = sh.read_file("rust-version")?.trim().to_owned(); let current_commit = { let rustc_info = cmd!(sh, "rustc +miri --version -v").read(); - if rustc_info.is_err() { - None - } else { - let metadata = rustc_version::version_meta_for(&rustc_info.unwrap())?; + if let Ok(rustc_info) = rustc_info { + let metadata = rustc_version::version_meta_for(&rustc_info)?; Some( metadata .commit_hash .ok_or_else(|| anyhow!("rustc metadata did not contain commit hash"))?, ) + } else { + None } }; // Check if we already are at that commit. diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 0eb16a943d6..388e88fe43e 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -3f1552a273e43e15f6ed240d00e1efdd6a53e65e +f6092f224d2b1774b31033f12d0bee626943b02f 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 7b4c533cfae..00f921b0f8a 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -244,8 +244,8 @@ pub(super) enum TransitionError { ChildAccessForbidden(Permission), /// A protector was triggered due to an invalid transition that loses /// too much permissions. - /// For example, if a protected tag goes from `Active` to `Disabled` due - /// to a foreign write this will produce a `ProtectedDisabled(Active)`. + /// For example, if a protected tag goes from `Unique` to `Disabled` due + /// to a foreign write this will produce a `ProtectedDisabled(Unique)`. /// This kind of error can only occur on foreign accesses. ProtectedDisabled(Permission), /// Cannot deallocate because some tag in the allocation is strongly protected. @@ -504,7 +504,7 @@ impl DisplayFmt { if let Some(perm) = perm { format!( "{ac}{st}", - ac = if perm.is_accessed() { self.accessed.yes } else { self.accessed.no }, + ac = if perm.accessed() { self.accessed.yes } else { self.accessed.no }, st = perm.permission().short_name(), ) } else { diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/foreign_access_skipping.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/foreign_access_skipping.rs index 928b3e6baef..90df05d36d9 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/foreign_access_skipping.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/foreign_access_skipping.rs @@ -24,7 +24,7 @@ use super::tree::AccessRelatedness; /// "manually" reset the parent's SIFA to be at least as strong as the new child's. This is accomplished with the `ensure_no_stronger_than` method. /// /// Note that we derive Ord and PartialOrd, so the order in which variants are listed below matters: -/// None < Read < Write. Do not change that order. See the `test_order` test. +/// None < Read < Write (weaker to stronger). Do not change that order. See the `test_order` test. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Default)] pub enum IdempotentForeignAccess { #[default] 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 bed65440dc9..6e5b5c807aa 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -294,24 +294,6 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return interp_ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })); } - let span = this.machine.current_span(); - - // When adding a new node, the SIFA of its parents needs to be updated, potentially across - // the entire memory range. For the parts that are being accessed below, the access itself - // trivially takes care of that. However, we have to do some more work to also deal with the - // parts that are not being accessed. Specifically what we do is that we call - // `update_last_accessed_after_retag` on the SIFA of the permission set for the part of - // memory outside `perm_map` -- so that part is definitely taken care of. The remaining - // concern is the part of memory that is in the range of `perms_map`, but not accessed - // below. There we have two cases: - // * If the type is `!Freeze`, then the non-accessed part uses `nonfreeze_perm`, so the - // `nonfreeze_perm` initialized parts are also fine. We enforce the `freeze_perm` parts to - // be accessed via the assert below, and thus everything is taken care of. - // * If the type is `Freeze`, then `freeze_perm` is used everywhere (both inside and outside - // the initial range), and we update everything to have the `freeze_perm`'s SIFA, so there - // are no issues. (And this assert below is not actually needed in this case). - assert!(new_perm.freeze_access); - let protected = new_perm.protector.is_some(); let precise_interior_mut = this .machine @@ -337,7 +319,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { LocationState::new_non_accessed(perm, sifa) } }; - let perms_map = if !precise_interior_mut { + let inside_perms = if !precise_interior_mut { // For `!Freeze` types, just pretend the entire thing is an `UnsafeCell`. let ty_is_freeze = place.layout.ty.is_freeze(*this.tcx, this.typing_env()); let state = loc_state(ty_is_freeze); @@ -364,8 +346,8 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let alloc_extra = this.get_alloc_extra(alloc_id)?; let mut tree_borrows = alloc_extra.borrow_tracker_tb().borrow_mut(); - for (perm_range, perm) in perms_map.iter_all() { - if perm.is_accessed() { + for (perm_range, perm) in inside_perms.iter_all() { + if perm.accessed() { // Some reborrows incur a read access to the parent. // Adjust range to be relative to allocation start (rather than to `place`). let range_in_alloc = AllocRange { @@ -401,10 +383,10 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { base_offset, orig_tag, new_tag, - perms_map, + inside_perms, new_perm.outside_perm, protected, - span, + this.machine.current_span(), )?; drop(tree_borrows); 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 390435e58d1..e21775c9f23 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -14,7 +14,7 @@ enum PermissionPriv { Cell, /// represents: a local mutable reference that has not yet been written to; /// allows: child reads, foreign reads; - /// affected by: child writes (becomes Active), + /// affected by: child writes (becomes Unique), /// rejects: foreign writes (Disabled). /// /// `ReservedFrz` is mostly for types that are `Freeze` (no interior mutability). @@ -31,17 +31,17 @@ enum PermissionPriv { /// This is so that the behavior of `Reserved` adheres to the rules of `noalias`: /// - foreign-read then child-write is UB due to `conflicted`, /// - child-write then foreign-read is UB since child-write will activate and then - /// foreign-read disables a protected `Active`, which is UB. + /// foreign-read disables a protected `Unique`, which is UB. ReservedFrz { conflicted: bool }, /// Alternative version of `ReservedFrz` made for types with interior mutability. /// allows: child reads, foreign reads, foreign writes (extra); - /// affected by: child writes (becomes Active); + /// affected by: child writes (becomes Unique); /// rejects: nothing. ReservedIM, /// represents: a unique pointer; /// allows: child reads, child writes; /// rejects: foreign reads (Frozen), foreign writes (Disabled). - Active, + Unique, /// represents: a shared pointer; /// allows: all read accesses; /// rejects child writes (UB), foreign writes (Disabled). @@ -56,7 +56,7 @@ use super::foreign_access_skipping::IdempotentForeignAccess; impl PartialOrd for PermissionPriv { /// PermissionPriv is ordered by the reflexive transitive closure of - /// `Reserved(conflicted=false) < Reserved(conflicted=true) < Active < Frozen < Disabled`. + /// `Reserved(conflicted=false) < Reserved(conflicted=true) < Unique < Frozen < Disabled`. /// `Reserved` that have incompatible `ty_is_freeze` are incomparable to each other. /// This ordering matches the reachability by transitions, as asserted by the exhaustive test /// `permissionpriv_partialord_is_reachability`. @@ -76,8 +76,8 @@ impl PartialOrd for PermissionPriv { (_, Disabled) => Less, (Frozen, _) => Greater, (_, Frozen) => Less, - (Active, _) => Greater, - (_, Active) => Less, + (Unique, _) => Greater, + (_, Unique) => Less, (ReservedIM, ReservedIM) => Equal, (ReservedFrz { conflicted: c1 }, ReservedFrz { conflicted: c2 }) => { // `bool` is ordered such that `false <= true`, so this works as intended. @@ -115,8 +115,8 @@ impl PermissionPriv { // Famously, ReservedIM survives foreign writes. It is never protected. ReservedIM if prot => unreachable!("Protected ReservedIM should not exist!"), ReservedIM => IdempotentForeignAccess::Write, - // Active changes on any foreign access (becomes Frozen/Disabled). - Active => IdempotentForeignAccess::None, + // Unique changes on any foreign access (becomes Frozen/Disabled). + Unique => IdempotentForeignAccess::None, // Frozen survives foreign reads, but not writes. Frozen => IdempotentForeignAccess::Read, // Disabled survives foreign reads and writes. It survives them @@ -139,12 +139,12 @@ mod transition { Disabled => return None, // The inner data `ty_is_freeze` of `Reserved` is always irrelevant for Read // accesses, since the data is not being mutated. Hence the `{ .. }`. - readable @ (Cell | ReservedFrz { .. } | ReservedIM | Active | Frozen) => readable, + readable @ (Cell | ReservedFrz { .. } | ReservedIM | Unique | Frozen) => readable, }) } /// A non-child node was read-accessed: keep `Reserved` but mark it as `conflicted` if it - /// is protected; invalidate `Active`. + /// is protected; invalidate `Unique`. fn foreign_read(state: PermissionPriv, protected: bool) -> Option<PermissionPriv> { Some(match state { // Cell ignores foreign reads. @@ -167,10 +167,10 @@ mod transition { assert!(!protected); res } - Active => + Unique => if protected { // We wrote, someone else reads -- that's bad. - // (Since Active is always initialized, this move-to-protected will mean insta-UB.) + // (Since Unique is always initialized, this move-to-protected will mean insta-UB.) Disabled } else { // We don't want to disable here to allow read-read reordering: it is crucial @@ -180,7 +180,7 @@ mod transition { }) } - /// A child node was write-accessed: `Reserved` must become `Active` to obtain + /// A child node was write-accessed: `Reserved` must become `Unique` to obtain /// write permissions, `Frozen` and `Disabled` cannot obtain such permissions and produce UB. fn child_write(state: PermissionPriv, protected: bool) -> Option<PermissionPriv> { Some(match state { @@ -192,7 +192,7 @@ mod transition { ReservedFrz { conflicted: true } if protected => return None, // A write always activates the 2-phase borrow, even with interior // mutability - ReservedFrz { .. } | ReservedIM | Active => Active, + ReservedFrz { .. } | ReservedIM | Unique => Unique, Frozen | Disabled => return None, }) } @@ -266,8 +266,8 @@ impl Permission { /// Default initial permission of the root of a new tree at inbounds positions. /// Must *only* be used for the root, this is not in general an "initial" permission! - pub fn new_active() -> Self { - Self { inner: Active } + pub fn new_unique() -> Self { + Self { inner: Unique } } /// Default initial permission of a reborrowed mutable reference that is either @@ -309,7 +309,7 @@ impl Permission { // Do not do perform access if it is a `Cell`, as this // can cause data races when using thread-safe data types. Cell => None, - Active => Some(AccessKind::Write), + Unique => Some(AccessKind::Write), _ => Some(AccessKind::Read), } } @@ -344,7 +344,7 @@ impl Permission { (_, Cell) => false, // ReservedIM can be replaced by anything besides Cell. // ReservedIM allows all transitions, but unlike Cell, a local write - // to ReservedIM transitions to Active, while it is a no-op for Cell. + // to ReservedIM transitions to Unique, while it is a no-op for Cell. (ReservedIM, _) => true, (_, ReservedIM) => false, // Reserved (as parent, where conflictedness does not matter) @@ -352,12 +352,12 @@ impl Permission { // since ReservedIM and Cell alone would survive foreign writes (ReservedFrz { .. }, _) => true, (_, ReservedFrz { .. }) => false, - // Active can not be replaced by something surviving + // Unique can not be replaced by something surviving // foreign reads and then remaining writable (i.e., Reserved*). // Replacing a state by itself is always okay, even if the child state is protected. - // Active can be replaced by Frozen, since it is not protected. - (Active, Active | Frozen | Disabled) => true, - (_, Active) => false, + // Unique can be replaced by Frozen, since it is not protected. + (Unique, Unique | Frozen | Disabled) => true, + (_, Unique) => false, // Frozen can only be replaced by Disabled (and itself). (Frozen, Frozen | Disabled) => true, (_, Frozen) => false, @@ -410,7 +410,7 @@ pub mod diagnostics { ReservedFrz { conflicted: false } => "Reserved", ReservedFrz { conflicted: true } => "Reserved (conflicted)", ReservedIM => "Reserved (interior mutable)", - Active => "Active", + Unique => "Unique", Frozen => "Frozen", Disabled => "Disabled", } @@ -441,7 +441,7 @@ pub mod diagnostics { ReservedFrz { conflicted: false } => "Res ", ReservedFrz { conflicted: true } => "ResC", ReservedIM => "ReIM", - Active => "Act ", + Unique => "Act ", Frozen => "Frz ", Disabled => "Dis ", } @@ -455,7 +455,7 @@ pub mod diagnostics { assert!(self.is_possible()); assert!(!self.is_noop()); match (self.from, self.to) { - (_, Active) => "the first write to a 2-phase borrowed mutable reference", + (_, Unique) => "the first write to a 2-phase borrowed mutable reference", (_, Frozen) => "a loss of write permissions", (ReservedFrz { conflicted: false }, ReservedFrz { conflicted: true }) => "a temporary loss of write permissions until function exit", @@ -472,8 +472,8 @@ pub mod diagnostics { /// /// Irrelevant events: /// - modifications of write permissions when the error is related to read permissions - /// (on failed reads and protected `Frozen -> Disabled`, ignore `Reserved -> Active`, - /// `Reserved(conflicted=false) -> Reserved(conflicted=true)`, and `Active -> Frozen`) + /// (on failed reads and protected `Frozen -> Disabled`, ignore `Reserved -> Unique`, + /// `Reserved(conflicted=false) -> Reserved(conflicted=true)`, and `Unique -> Frozen`) /// - all transitions for attempts to deallocate strongly protected tags /// /// # Panics @@ -481,10 +481,10 @@ pub mod diagnostics { /// This function assumes that its arguments apply to the same location /// and that they were obtained during a normal execution. It will panic otherwise. /// - all transitions involved in `self` and `err` should be increasing - /// (Reserved < Active < Frozen < Disabled); + /// (Reserved < Unique < Frozen < Disabled); /// - between `self` and `err` the permission should also be increasing, /// so all permissions inside `err` should be greater than `self.1`; - /// - `Active`, `Reserved(conflicted=false)`, and `Cell` cannot cause an error + /// - `Unique`, `Reserved(conflicted=false)`, and `Cell` cannot cause an error /// due to insufficient permissions, so `err` cannot be a `ChildAccessForbidden(_)` /// of either of them; /// - `err` should not be `ProtectedDisabled(Disabled)`, because the protected @@ -500,11 +500,11 @@ pub mod diagnostics { TransitionError::ChildAccessForbidden(insufficient) => { // Show where the permission was gained then lost, // but ignore unrelated permissions. - // This eliminates transitions like `Active -> Frozen` + // This eliminates transitions like `Unique -> Frozen` // when the error is a failed `Read`. match (self.to, insufficient.inner) { (Frozen, Frozen) => true, - (Active, Frozen) => true, + (Unique, Frozen) => true, (Disabled, Disabled) => true, ( ReservedFrz { conflicted: true, .. }, @@ -512,14 +512,14 @@ pub mod diagnostics { ) => true, // A pointer being `Disabled` is a strictly stronger source of // errors than it being `Frozen`. If we try to access a `Disabled`, - // then where it became `Frozen` (or `Active` or `Reserved`) is the least + // then where it became `Frozen` (or `Unique` or `Reserved`) is the least // of our concerns for now. - (ReservedFrz { conflicted: true } | Active | Frozen, Disabled) => false, + (ReservedFrz { conflicted: true } | Unique | Frozen, Disabled) => false, (ReservedFrz { conflicted: true }, Frozen) => false, - // `Active`, `Reserved`, and `Cell` have all permissions, so a - // `ChildAccessForbidden(Reserved | Active)` can never exist. - (_, Active) | (_, ReservedFrz { conflicted: false }) | (_, Cell) => + // `Unique`, `Reserved`, and `Cell` have all permissions, so a + // `ChildAccessForbidden(Reserved | Unique)` can never exist. + (_, Unique) | (_, ReservedFrz { conflicted: false }) | (_, Cell) => unreachable!("this permission cannot cause an error"), // No transition has `Reserved { conflicted: false }` or `ReservedIM` // as its `.to` unless it's a noop. `Cell` cannot be in its `.to` @@ -527,11 +527,11 @@ pub mod diagnostics { (ReservedFrz { conflicted: false } | ReservedIM | Cell, _) => unreachable!("self is a noop transition"), // All transitions produced in normal executions (using `apply_access`) - // change permissions in the order `Reserved -> Active -> Frozen -> Disabled`. + // change permissions in the order `Reserved -> Unique -> Frozen -> Disabled`. // We assume that the error was triggered on the same location that // the transition `self` applies to, so permissions found must be increasing // in the order `self.from < self.to <= insufficient.inner` - (Active | Frozen | Disabled, ReservedFrz { .. } | ReservedIM) + (Unique | Frozen | Disabled, ReservedFrz { .. } | ReservedIM) | (Disabled, Frozen) | (ReservedFrz { .. }, ReservedIM) => unreachable!("permissions between self and err must be increasing"), @@ -540,29 +540,29 @@ pub mod diagnostics { TransitionError::ProtectedDisabled(before_disabled) => { // Show how we got to the starting point of the forbidden transition, // but ignore what came before. - // This eliminates transitions like `Reserved -> Active` + // This eliminates transitions like `Reserved -> Unique` // when the error is a `Frozen -> Disabled`. match (self.to, before_disabled.inner) { // We absolutely want to know where it was activated/frozen/marked // conflicted. - (Active, Active) => true, + (Unique, Unique) => true, (Frozen, Frozen) => true, ( ReservedFrz { conflicted: true, .. }, ReservedFrz { conflicted: true, .. }, ) => true, // If the error is a transition `Frozen -> Disabled`, then we don't really - // care whether before that was `Reserved -> Active -> Frozen` or + // care whether before that was `Reserved -> Unique -> Frozen` or // `Frozen` directly. // The error will only show either // - created as Reserved { conflicted: false }, // then Reserved { .. } -> Disabled is forbidden // - created as Reserved { conflicted: false }, - // then Active -> Disabled is forbidden + // then Unique -> Disabled is forbidden // A potential `Reserved { conflicted: false } // -> Reserved { conflicted: true }` is inexistant or irrelevant, - // and so is the `Reserved { conflicted: false } -> Active` - (Active, Frozen) => false, + // and so is the `Reserved { conflicted: false } -> Unique` + (Unique, Frozen) => false, (ReservedFrz { conflicted: true }, _) => false, (_, Disabled) => @@ -575,12 +575,12 @@ pub mod diagnostics { (ReservedFrz { conflicted: false } | ReservedIM | Cell, _) => unreachable!("self is a noop transition"), - // Permissions only evolve in the order `Reserved -> Active -> Frozen -> Disabled`, + // Permissions only evolve in the order `Reserved -> Unique -> Frozen -> Disabled`, // so permissions found must be increasing in the order // `self.from < self.to <= forbidden.from < forbidden.to`. - (Disabled, Cell | ReservedFrz { .. } | ReservedIM | Active | Frozen) - | (Frozen, Cell | ReservedFrz { .. } | ReservedIM | Active) - | (Active, Cell | ReservedFrz { .. } | ReservedIM) => + (Disabled, Cell | ReservedFrz { .. } | ReservedIM | Unique | Frozen) + | (Frozen, Cell | ReservedFrz { .. } | ReservedIM | Unique) + | (Unique, Cell | ReservedFrz { .. } | ReservedIM) => unreachable!("permissions between self and err must be increasing"), } } @@ -617,7 +617,7 @@ mod propagation_optimization_checks { impl Exhaustive for PermissionPriv { fn exhaustive() -> Box<dyn Iterator<Item = Self>> { Box::new( - vec![Active, Frozen, Disabled, ReservedIM, Cell] + vec![Unique, Frozen, Disabled, ReservedIM, Cell] .into_iter() .chain(<bool>::exhaustive().map(|conflicted| ReservedFrz { conflicted })), ) @@ -730,7 +730,7 @@ mod propagation_optimization_checks { #[test] // Check that all transitions are consistent with the order on PermissionPriv, - // i.e. Reserved -> Active -> Frozen -> Disabled + // i.e. Reserved -> Unique -> Frozen -> Disabled fn permissionpriv_partialord_is_reachability() { let reach = { let mut reach = rustc_data_structures::fx::FxHashSet::default(); 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 22bd63bd6b6..c4345c63289 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -11,7 +11,7 @@ //! - idempotency properties asserted in `perms.rs` (for optimizations) use std::ops::Range; -use std::{fmt, mem}; +use std::{cmp, fmt, mem}; use rustc_abi::Size; use rustc_data_structures::fx::FxHashSet; @@ -57,7 +57,7 @@ pub(super) struct LocationState { impl LocationState { /// Constructs a new initial state. It has neither been accessed, nor been subjected /// to any foreign access yet. - /// The permission is not allowed to be `Active`. + /// The permission is not allowed to be `Unique`. /// `sifa` is the (strongest) idempotent foreign access, see `foreign_access_skipping.rs` pub fn new_non_accessed(permission: Permission, sifa: IdempotentForeignAccess) -> Self { assert!(permission.is_initial() || permission.is_disabled()); @@ -73,23 +73,10 @@ impl LocationState { /// Check if the location has been accessed, i.e. if it has /// ever been accessed through a child pointer. - pub fn is_accessed(&self) -> bool { + pub fn accessed(&self) -> bool { self.accessed } - /// Check if the state can exist as the initial permission of a pointer. - /// - /// Do not confuse with `is_accessed`, the two are almost orthogonal - /// as apart from `Active` which is not initial and must be accessed, - /// any other permission can have an arbitrary combination of being - /// initial/accessed. - /// FIXME: when the corresponding `assert` in `tree_borrows/mod.rs` finally - /// passes and can be uncommented, remove this `#[allow(dead_code)]`. - #[cfg_attr(not(test), allow(dead_code))] - pub fn is_initial(&self) -> bool { - self.permission.is_initial() - } - pub fn permission(&self) -> Permission { self.permission } @@ -170,7 +157,7 @@ impl LocationState { } if self.permission.is_frozen() && access_kind == AccessKind::Read { // A foreign read to a `Frozen` tag will have almost no observable effect. - // It's a theorem that `Frozen` nodes have no active children, so all children + // It's a theorem that `Frozen` nodes have no `Unique` children, so all children // already survive foreign reads. Foreign reads in general have almost no // effect, the only further thing they could do is make protected `Reserved` // nodes become conflicted, i.e. make them reject child writes for the further @@ -265,7 +252,7 @@ pub(super) struct Node { pub children: SmallVec<[UniIndex; 4]>, /// Either `Reserved`, `Frozen`, or `Disabled`, it is the permission this tag will /// lazily be initialized to on the first access. - /// It is only ever `Disabled` for a tree root, since the root is initialized to `Active` by + /// It is only ever `Disabled` for a tree root, since the root is initialized to `Unique` by /// its own separate mechanism. default_initial_perm: Permission, /// The default initial (strongest) idempotent foreign access. @@ -598,14 +585,14 @@ impl Tree { }; let rperms = { let mut perms = UniValMap::default(); - // We manually set it to `Active` on all in-bounds positions. - // We also ensure that it is accessed, so that no `Active` but + // We manually set it to `Unique` on all in-bounds positions. + // We also ensure that it is accessed, so that no `Unique` but // not yet accessed nodes exist. Essentially, we pretend there - // was a write that initialized these to `Active`. + // was a write that initialized these to `Unique`. perms.insert( root_idx, LocationState::new_accessed( - Permission::new_active(), + Permission::new_unique(), IdempotentForeignAccess::None, ), ); @@ -618,30 +605,26 @@ impl Tree { impl<'tcx> Tree { /// Insert a new tag in the tree. /// - /// `initial_perms` defines the initial permissions for the part of memory - /// that is already considered "initialized" immediately. The ranges in this - /// map are relative to `base_offset`. - /// `default_perm` defines the initial permission for the rest of the allocation. - /// - /// For all non-accessed locations in the RangeMap (those that haven't had an - /// implicit read), their SIFA must be weaker than or as weak as the SIFA of - /// `default_perm`. + /// `inside_perm` defines the initial permissions for a block of memory starting at + /// `base_offset`. These may nor may not be already marked as "accessed". + /// `outside_perm` defines the initial permission for the rest of the allocation. + /// These are definitely not "accessed". pub(super) fn new_child( &mut self, base_offset: Size, parent_tag: BorTag, new_tag: BorTag, - initial_perms: DedupRangeMap<LocationState>, - default_perm: Permission, + inside_perms: DedupRangeMap<LocationState>, + outside_perm: Permission, protected: bool, span: Span, ) -> InterpResult<'tcx> { let idx = self.tag_mapping.insert(new_tag); let parent_idx = self.tag_mapping.get(&parent_tag).unwrap(); - assert!(default_perm.is_initial()); + assert!(outside_perm.is_initial()); let default_strongest_idempotent = - default_perm.strongest_idempotent_foreign_access(protected); + outside_perm.strongest_idempotent_foreign_access(protected); // Create the node self.nodes.insert( idx, @@ -649,47 +632,57 @@ impl<'tcx> Tree { tag: new_tag, parent: Some(parent_idx), children: SmallVec::default(), - default_initial_perm: default_perm, + default_initial_perm: outside_perm, default_initial_idempotent_foreign_access: default_strongest_idempotent, - debug_info: NodeDebugInfo::new(new_tag, default_perm, span), + debug_info: NodeDebugInfo::new(new_tag, outside_perm, span), }, ); // Register new_tag as a child of parent_tag self.nodes.get_mut(parent_idx).unwrap().children.push(idx); + // We need to know the weakest SIFA for `update_idempotent_foreign_access_after_retag`. + let mut min_sifa = default_strongest_idempotent; for (Range { start, end }, &perm) in - initial_perms.iter(Size::from_bytes(0), initial_perms.size()) + inside_perms.iter(Size::from_bytes(0), inside_perms.size()) { - assert!(perm.is_initial()); + assert!(perm.permission.is_initial()); + assert_eq!( + perm.idempotent_foreign_access, + perm.permission.strongest_idempotent_foreign_access(protected) + ); + + min_sifa = cmp::min(min_sifa, perm.idempotent_foreign_access); for (_perms_range, perms) in self .rperms .iter_mut(Size::from_bytes(start) + base_offset, Size::from_bytes(end - start)) { - assert!( - default_strongest_idempotent - >= perm.permission.strongest_idempotent_foreign_access(protected) - ); perms.insert(idx, perm); } } - // Inserting the new perms might have broken the SIFA invariant (see `foreign_access_skipping.rs`). - // We now weaken the recorded SIFA for our parents, until the invariant is restored. - // We could weaken them all to `LocalAccess`, but it is more efficient to compute the SIFA - // for the new permission statically, and use that. - // See the comment in `tb_reborrow` for why it is correct to use the SIFA of `default_uninit_perm`. - self.update_last_accessed_after_retag(parent_idx, default_strongest_idempotent); + // Inserting the new perms might have broken the SIFA invariant (see + // `foreign_access_skipping.rs`) if the SIFA we inserted is weaker than that of some parent. + // We now weaken the recorded SIFA for our parents, until the invariant is restored. We + // could weaken them all to `None`, but it is more efficient to compute the SIFA for the new + // permission statically, and use that. For this we need the *minimum* SIFA (`None` needs + // more fixup than `Write`). + self.update_idempotent_foreign_access_after_retag(parent_idx, min_sifa); interp_ok(()) } - /// Restores the SIFA "children are stronger" invariant after a retag. - /// See `foreign_access_skipping` and `new_child`. - fn update_last_accessed_after_retag( + /// Restores the SIFA "children are stronger"/"parents are weaker" invariant after a retag: + /// reduce the SIFA of `current` and its parents to be no stronger than `strongest_allowed`. + /// See `foreign_access_skipping.rs` and [`Tree::new_child`]. + fn update_idempotent_foreign_access_after_retag( &mut self, mut current: UniIndex, strongest_allowed: IdempotentForeignAccess, ) { + if strongest_allowed == IdempotentForeignAccess::Write { + // Nothing is stronger than `Write`. + return; + } // We walk the tree upwards, until the invariant is restored loop { let current_node = self.nodes.get_mut(current).unwrap(); @@ -755,9 +748,9 @@ impl<'tcx> Tree { == Some(&ProtectorKind::StrongProtector) // Don't check for protector if it is a Cell (see `unsafe_cell_deallocate` in `interior_mutability.rs`). // Related to https://github.com/rust-lang/rust/issues/55005. - && !perm.permission().is_cell() + && !perm.permission.is_cell() // Only trigger UB if the accessed bit is set, i.e. if the protector is actually protecting this offset. See #4579. - && perm.is_accessed() + && perm.accessed { Err(TransitionError::ProtectedDealloc) } else { @@ -790,7 +783,7 @@ impl<'tcx> Tree { /// - the access will be applied only to accessed locations of the allocation, /// - it will not be visible to children, /// - it will be recorded as a `FnExit` diagnostic access - /// - and it will be a read except if the location is `Active`, i.e. has been written to, + /// - and it will be a read except if the location is `Unique`, i.e. has been written to, /// in which case it will be a write. /// /// `LocationState::perform_access` will take care of raising transition diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs index d9b3696e4f8..189e48eca72 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs @@ -106,7 +106,7 @@ fn tree_compacting_is_sound() { as_foreign_or_child(rel), kind, parent.permission(), - as_lazy_or_accessed(child.is_accessed()), + as_lazy_or_accessed(child.accessed()), child.permission(), as_protected(child_protected), np.permission(), @@ -122,7 +122,7 @@ fn tree_compacting_is_sound() { as_foreign_or_child(rel), kind, parent.permission(), - as_lazy_or_accessed(child.is_accessed()), + as_lazy_or_accessed(child.accessed()), child.permission(), as_protected(child_protected), nc.permission() @@ -375,7 +375,7 @@ mod spurious_read { impl LocStateProt { fn is_initial(&self) -> bool { - self.state.is_initial() + self.state.permission().is_initial() } fn perform_access(&self, kind: AccessKind, rel: AccessRelatedness) -> Result<Self, ()> { @@ -420,7 +420,7 @@ mod spurious_read { /// `(LocStateProt, LocStateProt)` where the two states are not guaranteed /// to be updated at the same time. /// Some `LocStateProtPair` may be unreachable through normal means - /// such as `x: Active, y: Active` in the case of mutually foreign pointers. + /// such as `x: Unique, y: Unique` in the case of mutually foreign pointers. struct LocStateProtPair { xy_rel: RelPosXY, x: LocStateProt, @@ -709,7 +709,7 @@ mod spurious_read { let mut err = 0; for pat in Pattern::exhaustive() { let Ok(initial_source) = pat.initial_state() else { - // Failed to retag `x` in the source (e.g. `y` was protected Active) + // Failed to retag `x` in the source (e.g. `y` was protected Unique) continue; }; // `x` must stay protected, but the function protecting `y` might return here diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 15d486c27e3..e4e7fb1d725 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -381,8 +381,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // We need to drop our mutex borrow before unblock_thread // because it will be borrowed again in the unblock callback. drop(mutex); - if thread_id.is_some() { - this.unblock_thread(thread_id.unwrap(), BlockReason::Mutex)?; + if let Some(thread_id) = thread_id { + this.unblock_thread(thread_id, BlockReason::Mutex)?; } } Some(old_lock_count) diff --git a/src/tools/miri/tests/fail/async-shared-mutable.tree.stderr b/src/tools/miri/tests/fail/async-shared-mutable.tree.stderr index 449a29088a0..dc8b4f6665a 100644 --- a/src/tools/miri/tests/fail/async-shared-mutable.tree.stderr +++ b/src/tools/miri/tests/fail/async-shared-mutable.tree.stderr @@ -16,7 +16,7 @@ LL | | Poll::<()>::Pending LL | | }) LL | | .await | |______________^ -help: the accessed tag <TAG> later transitioned to Active due to a child write access at offsets [OFFSET] +help: the accessed tag <TAG> later transitioned to Unique due to a child write access at offsets [OFFSET] --> tests/fail/async-shared-mutable.rs:LL:CC | LL | *x = 1; diff --git a/src/tools/miri/tests/fail/both_borrows/box_noalias_violation.tree.stderr b/src/tools/miri/tests/fail/both_borrows/box_noalias_violation.tree.stderr index 3c8ec7f7d3e..6a1f7761a41 100644 --- a/src/tools/miri/tests/fail/both_borrows/box_noalias_violation.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/box_noalias_violation.tree.stderr @@ -7,7 +7,7 @@ LL | *y = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information = help: the accessed tag <TAG> is foreign to the protected tag <TAG> (i.e., it is not a child) - = help: this foreign read access would cause the protected tag <TAG> (currently Active) to become Disabled + = help: this foreign read access would cause the protected tag <TAG> (currently Unique) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <TAG> was created here --> tests/fail/both_borrows/box_noalias_violation.rs:LL:CC @@ -19,7 +19,7 @@ help: the protected tag <TAG> was created here, in the initial state Reserved | LL | unsafe fn test(mut x: Box<i32>, y: *const i32) -> i32 { | ^^^^^ -help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the protected tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/both_borrows/box_noalias_violation.rs:LL:CC | LL | *x = 5; diff --git a/src/tools/miri/tests/fail/both_borrows/illegal_write6.tree.stderr b/src/tools/miri/tests/fail/both_borrows/illegal_write6.tree.stderr index 6780e52c3ba..1547a6ca73a 100644 --- a/src/tools/miri/tests/fail/both_borrows/illegal_write6.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/illegal_write6.tree.stderr @@ -7,7 +7,7 @@ LL | unsafe { *y = 2 }; = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information = help: the accessed tag <TAG> 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: this foreign write access would cause the protected tag <TAG> (currently Unique) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <TAG> was created here --> tests/fail/both_borrows/illegal_write6.rs:LL:CC @@ -19,7 +19,7 @@ help: the protected tag <TAG> was created here, in the initial state Reserved | LL | fn foo(a: &mut u32, y: *mut u32) -> u32 { | ^ -help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the protected tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/both_borrows/illegal_write6.rs:LL:CC | LL | *a = 1; diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias.tree.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias.tree.stderr index 2266a9c39f9..2d9ce2aa1fb 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias.tree.stderr @@ -7,7 +7,7 @@ LL | Call(_unit = callee(Move(non_copy), Move(non_copy)), ReturnTo(a = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information = 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 read access would cause the protected tag <TAG> (currently Active) to become Disabled + = help: this foreign read access would cause the protected tag <TAG> (currently Unique) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <TAG> was created here --> tests/fail/function_calls/arg_inplace_locals_alias.rs:LL:CC @@ -19,7 +19,7 @@ help: the protected tag <TAG> was created here, in the initial state Reserved | LL | y.0 = 0; | ^^^^^^^ -help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the protected tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/function_calls/arg_inplace_locals_alias.rs:LL:CC | LL | y.0 = 0; diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.tree.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.tree.stderr index b7f514de0af..42e391b5daf 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_locals_alias_ret.tree.stderr @@ -7,7 +7,7 @@ LL | Call(_non_copy = callee(Move(_non_copy)), ReturnTo(after_call), = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information = 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: this reborrow (acting as a foreign read access) would cause the protected tag <TAG> (currently Unique) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <TAG> was created here --> tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC @@ -19,7 +19,7 @@ help: the protected tag <TAG> was created here, in the initial state Reserved | LL | x | ^ -help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the protected tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/function_calls/arg_inplace_locals_alias_ret.rs:LL:CC | LL | x diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr index 1995528e9f9..74706d6b9f6 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr @@ -7,7 +7,7 @@ LL | unsafe { ptr.write(S(0)) }; = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information = 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: this foreign write access would cause the protected tag <TAG> (currently Unique) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <TAG> was created here --> tests/fail/function_calls/arg_inplace_mutate.rs:LL:CC @@ -24,7 +24,7 @@ help: the protected tag <TAG> was created here, in the initial state Reserved | LL | unsafe { ptr.write(S(0)) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the protected tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/function_calls/arg_inplace_mutate.rs:LL:CC | LL | unsafe { ptr.write(S(0)) }; diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.tree.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.tree.stderr index e506a61c6bb..c8c0e5c37ef 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.tree.stderr @@ -7,7 +7,7 @@ LL | unsafe { ptr.read() }; = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information = 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 read access would cause the protected tag <TAG> (currently Active) to become Disabled + = help: this foreign read access would cause the protected tag <TAG> (currently Unique) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <TAG> was created here --> tests/fail/function_calls/arg_inplace_observe_during.rs:LL:CC @@ -24,7 +24,7 @@ help: the protected tag <TAG> was created here, in the initial state Reserved | LL | x.0 = 0; | ^^^^^^^ -help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the protected tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/function_calls/arg_inplace_observe_during.rs:LL:CC | LL | x.0 = 0; diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr index b1aa2ba2886..b43e19c3905 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr @@ -7,7 +7,7 @@ LL | unsafe { ptr.read() }; = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information = 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 read access would cause the protected tag <TAG> (currently Active) to become Disabled + = help: this foreign read access would cause the protected tag <TAG> (currently Unique) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <TAG> was created here --> tests/fail/function_calls/return_pointer_aliasing_read.rs:LL:CC @@ -24,7 +24,7 @@ help: the protected tag <TAG> was created here, in the initial state Reserved | LL | unsafe { ptr.read() }; | ^^^^^^^^^^^^^^^^^^^^^ -help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the protected tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/function_calls/return_pointer_aliasing_read.rs:LL:CC | LL | unsafe { ptr.read() }; diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr index 0cf449ea3ec..deefb24b785 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr @@ -7,7 +7,7 @@ LL | unsafe { ptr.write(0) }; = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information = 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: this foreign write access would cause the protected tag <TAG> (currently Unique) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <TAG> was created here --> tests/fail/function_calls/return_pointer_aliasing_write.rs:LL:CC @@ -24,7 +24,7 @@ help: the protected tag <TAG> was created here, in the initial state Reserved | LL | unsafe { ptr.write(0) }; | ^^^^^^^^^^^^^^^^^^^^^^^ -help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the protected tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/function_calls/return_pointer_aliasing_write.rs:LL:CC | LL | unsafe { ptr.write(0) }; diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr index a006c6feae4..76ccf39744d 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr @@ -7,7 +7,7 @@ LL | unsafe { ptr.write(0) }; = 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: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information = 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: this foreign write access would cause the protected tag <TAG> (currently Unique) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <TAG> was created here --> tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs:LL:CC @@ -24,7 +24,7 @@ help: the protected tag <TAG> was created here, in the initial state Reserved | LL | unsafe { ptr.write(0) }; | ^^^^^^^^^^^^^^^^^^^^^^^ -help: the protected tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the protected tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs:LL:CC | LL | unsafe { ptr.write(0) }; diff --git a/src/tools/miri/tests/fail/tree_borrows/alternate-read-write.stderr b/src/tools/miri/tests/fail/tree_borrows/alternate-read-write.stderr index 9e955a6d5b1..aff482abfa0 100644 --- a/src/tools/miri/tests/fail/tree_borrows/alternate-read-write.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/alternate-read-write.stderr @@ -12,7 +12,7 @@ help: the accessed tag <TAG> was created here, in the initial state Reserved | LL | let y = unsafe { &mut *(x as *mut u8) }; | ^^^^^^^^^^^^^^^^^^^^ -help: the accessed tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x1] +help: the accessed tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x1] --> tests/fail/tree_borrows/alternate-read-write.rs:LL:CC | LL | *y += 1; // Success diff --git a/src/tools/miri/tests/fail/tree_borrows/fnentry_invalidation.stderr b/src/tools/miri/tests/fail/tree_borrows/fnentry_invalidation.stderr index 7886029dccf..bfd6854514e 100644 --- a/src/tools/miri/tests/fail/tree_borrows/fnentry_invalidation.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/fnentry_invalidation.stderr @@ -12,7 +12,7 @@ help: the accessed tag <TAG> was created here, in the initial state Reserved | LL | let z = &mut x as *mut i32; | ^^^^^^ -help: the accessed tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the accessed tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/tree_borrows/fnentry_invalidation.rs:LL:CC | LL | *z = 1; diff --git a/src/tools/miri/tests/fail/tree_borrows/parent_read_freezes_raw_mut.stderr b/src/tools/miri/tests/fail/tree_borrows/parent_read_freezes_raw_mut.stderr index 2edbbd80569..7a713abcbc4 100644 --- a/src/tools/miri/tests/fail/tree_borrows/parent_read_freezes_raw_mut.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/parent_read_freezes_raw_mut.stderr @@ -12,7 +12,7 @@ help: the accessed tag <TAG> was created here, in the initial state Reserved | LL | let mref = &mut root; | ^^^^^^^^^ -help: the accessed tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x1] +help: the accessed tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x1] --> tests/fail/tree_borrows/parent_read_freezes_raw_mut.rs:LL:CC | LL | *ptr = 0; // Write diff --git a/src/tools/miri/tests/fail/tree_borrows/pass_invalid_mut.stderr b/src/tools/miri/tests/fail/tree_borrows/pass_invalid_mut.stderr index c00c67173b7..9a70d248aa0 100644 --- a/src/tools/miri/tests/fail/tree_borrows/pass_invalid_mut.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/pass_invalid_mut.stderr @@ -18,7 +18,7 @@ help: the conflicting tag <TAG> was created here, in the initial state Reserved | LL | let xref = unsafe { &mut *xraw }; | ^^^^^^^^^^ -help: the conflicting tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x4] +help: the conflicting tag <TAG> later transitioned to Unique due to a child write access at offsets [0x0..0x4] --> tests/fail/tree_borrows/pass_invalid_mut.rs:LL:CC | LL | *xref = 18; // activate xref diff --git a/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.rs b/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.rs index 024a14600b1..3e5d83911ee 100644 --- a/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.rs +++ b/src/tools/miri/tests/fail/tree_borrows/reservedim_spurious_write.rs @@ -60,8 +60,7 @@ fn main() { fn inner(x: &mut u8, b: IdxBarrier) { *x = 42; // activate immediately synchronized!(b, "[lazy] retag y (&mut, protect, IM)"); - // A spurious write should be valid here because `x` is - // `Active` and protected. + // A spurious write should be valid here because `x` is `Unique` and protected. if cfg!(with) { synchronized!(b, "spurious write x (executed)"); *x = 64; diff --git a/src/tools/miri/tests/fail/tree_borrows/return_invalid_mut.stderr b/src/tools/miri/tests/fail/tree_borrows/return_invalid_mut.stderr index ba8ab472872..4b6308847bb 100644 --- a/src/tools/miri/tests/fail/tree_borrows/return_invalid_mut.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/return_invalid_mut.stderr @@ -18,7 +18,7 @@ help: the conflicting tag <TAG> was created here, in the initial state Reserved | LL | let ret = unsafe { &mut (*xraw).1 }; | ^^^^^^^^^^^^^^ -help: the conflicting tag <TAG> later transitioned to Active due to a child write access at offsets [0x4..0x8] +help: the conflicting tag <TAG> later transitioned to Unique due to a child write access at offsets [0x4..0x8] --> tests/fail/tree_borrows/return_invalid_mut.rs:LL:CC | LL | *ret = *ret; // activate diff --git a/src/tools/miri/tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.rs b/src/tools/miri/tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.rs index 6514334b09d..94a3bb80544 100644 --- a/src/tools/miri/tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.rs +++ b/src/tools/miri/tests/fail/tree_borrows/subtree_traversal_skipping_diagnostics.rs @@ -5,7 +5,7 @@ // When this method is called, the tree will be a single line and look like this, // with other_ptr being the root at the top -// other_ptr = root : Active +// other_ptr = root : Unique // intermediary : Frozen // an intermediary node // m : Reserved fn write_to_mut(m: &mut u8, other_ptr: *const u8) { diff --git a/src/tools/miri/tests/fail/tree_borrows/unique.default.stderr b/src/tools/miri/tests/fail/tree_borrows/unique.default.stderr deleted file mode 100644 index c7d72f70f40..00000000000 --- a/src/tools/miri/tests/fail/tree_borrows/unique.default.stderr +++ /dev/null @@ -1,32 +0,0 @@ -error: Undefined Behavior: write access through <TAG> at ALLOC[0x0] is forbidden - --> tests/fail/tree_borrows/unique.rs:LL:CC - | -LL | *uniq.as_ptr() = 3; - | ^^^^^^^^^^^^^^^^^^ write access through <TAG> at ALLOC[0x0] 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> has state Frozen which forbids this child write access -help: the accessed tag <TAG> was created here, in the initial state Reserved - --> tests/fail/tree_borrows/unique.rs:LL:CC - | -LL | let refmut = &mut data; - | ^^^^^^^^^ -help: the accessed tag <TAG> later transitioned to Active due to a child write access at offsets [0x0..0x1] - --> tests/fail/tree_borrows/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 <TAG> later transitioned to Frozen due to a foreign read access at offsets [0x0..0x1] - --> tests/fail/tree_borrows/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 tests/fail/tree_borrows/unique.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to 1 previous error - diff --git a/src/tools/miri/tests/pass/0weak_memory/weak.rs b/src/tools/miri/tests/pass/0weak_memory/weak.rs index c752fc114ba..611733d0dac 100644 --- a/src/tools/miri/tests/pass/0weak_memory/weak.rs +++ b/src/tools/miri/tests/pass/0weak_memory/weak.rs @@ -13,6 +13,10 @@ use std::sync::atomic::Ordering::*; use std::sync::atomic::{AtomicUsize, fence}; use std::thread::spawn; +#[path = "../../utils/mod.rs"] +mod utils; +use utils::check_all_outcomes; + #[allow(dead_code)] #[derive(Copy, Clone)] struct EvilSend<T>(pub T); @@ -33,35 +37,6 @@ fn spin_until(loc: &AtomicUsize, val: usize) -> usize { val } -/// Check that the function produces the intended set of outcomes. -#[track_caller] -fn check_all_outcomes<T: Eq + std::hash::Hash + std::fmt::Debug>( - expected: impl IntoIterator<Item = T>, - generate: impl Fn() -> T, -) { - use std::collections::HashSet; - - let expected: HashSet<T> = HashSet::from_iter(expected); - let mut seen = HashSet::new(); - // Let's give it N times as many tries as we are expecting values. - let tries = expected.len() * 16; - for i in 0..tries { - let val = generate(); - assert!(expected.contains(&val), "got an unexpected value: {val:?}"); - seen.insert(val); - if i > tries / 2 && expected.len() == seen.len() { - // We saw everything and we did quite a few tries, let's avoid wasting time. - return; - } - } - // Let's see if we saw them all. - for val in expected { - if !seen.contains(&val) { - panic!("did not get value that should be possible: {val:?}"); - } - } -} - fn relaxed() { check_all_outcomes([0, 1, 2], || { let x = static_atomic(0); diff --git a/src/tools/miri/tests/pass/float.rs b/src/tools/miri/tests/pass/float.rs index 3a764329f9b..67a14c2b389 100644 --- a/src/tools/miri/tests/pass/float.rs +++ b/src/tools/miri/tests/pass/float.rs @@ -8,12 +8,16 @@ #![allow(internal_features)] #![allow(unnecessary_transmutes)] +#[path = "../utils/mod.rs"] +mod utils; use std::any::type_name; use std::cmp::min; use std::fmt::{Debug, Display, LowerHex}; use std::hint::black_box; use std::{f32, f64}; +use utils::check_nondet; + /// Compare the two floats, allowing for $ulp many ULPs of error. /// /// ULP means "Units in the Last Place" or "Units of Least Precision". @@ -1429,29 +1433,14 @@ fn test_fmuladd() { /// `min` and `max` on equal arguments are non-deterministic. fn test_min_max_nondet() { - /// Ensure that if we call the closure often enough, we see both `true` and `false.` - #[track_caller] - fn ensure_both(f: impl Fn() -> bool) { - let rounds = 32; - let first = f(); - for _ in 1..rounds { - if f() != first { - // We saw two different values! - return; - } - } - // We saw the same thing N times. - panic!("expected non-determinism, got {rounds} times the same result: {first:?}"); - } - - ensure_both(|| f16::min(0.0, -0.0).is_sign_positive()); - ensure_both(|| f16::max(0.0, -0.0).is_sign_positive()); - ensure_both(|| f32::min(0.0, -0.0).is_sign_positive()); - ensure_both(|| f32::max(0.0, -0.0).is_sign_positive()); - ensure_both(|| f64::min(0.0, -0.0).is_sign_positive()); - ensure_both(|| f64::max(0.0, -0.0).is_sign_positive()); - ensure_both(|| f128::min(0.0, -0.0).is_sign_positive()); - ensure_both(|| f128::max(0.0, -0.0).is_sign_positive()); + check_nondet(|| f16::min(0.0, -0.0).is_sign_positive()); + check_nondet(|| f16::max(0.0, -0.0).is_sign_positive()); + check_nondet(|| f32::min(0.0, -0.0).is_sign_positive()); + check_nondet(|| f32::max(0.0, -0.0).is_sign_positive()); + check_nondet(|| f64::min(0.0, -0.0).is_sign_positive()); + check_nondet(|| f64::max(0.0, -0.0).is_sign_positive()); + check_nondet(|| f128::min(0.0, -0.0).is_sign_positive()); + check_nondet(|| f128::max(0.0, -0.0).is_sign_positive()); } fn test_non_determinism() { @@ -1461,35 +1450,20 @@ fn test_non_determinism() { }; use std::{f32, f64}; - /// Ensure that the operation is non-deterministic - #[track_caller] - fn ensure_nondet<T: PartialEq + std::fmt::Debug>(f: impl Fn() -> T) { - let rounds = 16; - let first = f(); - for _ in 1..rounds { - if f() != first { - // We saw two different values! - return; - } - } - // We saw the same thing N times. - panic!("expected non-determinism, got {rounds} times the same result: {first:?}"); - } - macro_rules! test_operations_f { ($a:expr, $b:expr) => { - ensure_nondet(|| fadd_algebraic($a, $b)); - ensure_nondet(|| fsub_algebraic($a, $b)); - ensure_nondet(|| fmul_algebraic($a, $b)); - ensure_nondet(|| fdiv_algebraic($a, $b)); - ensure_nondet(|| frem_algebraic($a, $b)); + check_nondet(|| fadd_algebraic($a, $b)); + check_nondet(|| fsub_algebraic($a, $b)); + check_nondet(|| fmul_algebraic($a, $b)); + check_nondet(|| fdiv_algebraic($a, $b)); + check_nondet(|| frem_algebraic($a, $b)); unsafe { - ensure_nondet(|| fadd_fast($a, $b)); - ensure_nondet(|| fsub_fast($a, $b)); - ensure_nondet(|| fmul_fast($a, $b)); - ensure_nondet(|| fdiv_fast($a, $b)); - ensure_nondet(|| frem_fast($a, $b)); + check_nondet(|| fadd_fast($a, $b)); + check_nondet(|| fsub_fast($a, $b)); + check_nondet(|| fmul_fast($a, $b)); + check_nondet(|| fdiv_fast($a, $b)); + check_nondet(|| frem_fast($a, $b)); } }; } @@ -1499,70 +1473,70 @@ fn test_non_determinism() { } pub fn test_operations_f32(a: f32, b: f32) { test_operations_f!(a, b); - ensure_nondet(|| a.powf(b)); - ensure_nondet(|| a.powi(2)); - ensure_nondet(|| a.log(b)); - ensure_nondet(|| a.exp()); - ensure_nondet(|| 10f32.exp2()); - ensure_nondet(|| f32::consts::E.ln()); - ensure_nondet(|| 10f32.log10()); - ensure_nondet(|| 8f32.log2()); - ensure_nondet(|| 1f32.ln_1p()); - ensure_nondet(|| 27.0f32.cbrt()); - ensure_nondet(|| 3.0f32.hypot(4.0f32)); - ensure_nondet(|| 1f32.sin()); - ensure_nondet(|| 1f32.cos()); + check_nondet(|| a.powf(b)); + check_nondet(|| a.powi(2)); + check_nondet(|| a.log(b)); + check_nondet(|| a.exp()); + check_nondet(|| 10f32.exp2()); + check_nondet(|| f32::consts::E.ln()); + check_nondet(|| 10f32.log10()); + check_nondet(|| 8f32.log2()); + check_nondet(|| 1f32.ln_1p()); + check_nondet(|| 27.0f32.cbrt()); + check_nondet(|| 3.0f32.hypot(4.0f32)); + check_nondet(|| 1f32.sin()); + check_nondet(|| 1f32.cos()); // On i686-pc-windows-msvc , these functions are implemented by calling the `f64` version, // which means the little rounding errors Miri introduces are discarded by the cast down to // `f32`. Just skip the test for them. if !cfg!(all(target_os = "windows", target_env = "msvc", target_arch = "x86")) { - ensure_nondet(|| 1.0f32.tan()); - ensure_nondet(|| 1.0f32.asin()); - ensure_nondet(|| 5.0f32.acos()); - ensure_nondet(|| 1.0f32.atan()); - ensure_nondet(|| 1.0f32.atan2(2.0f32)); - ensure_nondet(|| 1.0f32.sinh()); - ensure_nondet(|| 1.0f32.cosh()); - ensure_nondet(|| 1.0f32.tanh()); + check_nondet(|| 1.0f32.tan()); + check_nondet(|| 1.0f32.asin()); + check_nondet(|| 5.0f32.acos()); + check_nondet(|| 1.0f32.atan()); + check_nondet(|| 1.0f32.atan2(2.0f32)); + check_nondet(|| 1.0f32.sinh()); + check_nondet(|| 1.0f32.cosh()); + check_nondet(|| 1.0f32.tanh()); } - ensure_nondet(|| 1.0f32.asinh()); - ensure_nondet(|| 2.0f32.acosh()); - ensure_nondet(|| 0.5f32.atanh()); - ensure_nondet(|| 5.0f32.gamma()); - ensure_nondet(|| 5.0f32.ln_gamma()); - ensure_nondet(|| 5.0f32.erf()); - ensure_nondet(|| 5.0f32.erfc()); + check_nondet(|| 1.0f32.asinh()); + check_nondet(|| 2.0f32.acosh()); + check_nondet(|| 0.5f32.atanh()); + check_nondet(|| 5.0f32.gamma()); + check_nondet(|| 5.0f32.ln_gamma()); + check_nondet(|| 5.0f32.erf()); + check_nondet(|| 5.0f32.erfc()); } pub fn test_operations_f64(a: f64, b: f64) { test_operations_f!(a, b); - ensure_nondet(|| a.powf(b)); - ensure_nondet(|| a.powi(2)); - ensure_nondet(|| a.log(b)); - ensure_nondet(|| a.exp()); - ensure_nondet(|| 50f64.exp2()); - ensure_nondet(|| 3f64.ln()); - ensure_nondet(|| f64::consts::E.log10()); - ensure_nondet(|| f64::consts::E.log2()); - ensure_nondet(|| 1f64.ln_1p()); - ensure_nondet(|| 27.0f64.cbrt()); - ensure_nondet(|| 3.0f64.hypot(4.0f64)); - ensure_nondet(|| 1f64.sin()); - ensure_nondet(|| 1f64.cos()); - ensure_nondet(|| 1.0f64.tan()); - ensure_nondet(|| 1.0f64.asin()); - ensure_nondet(|| 5.0f64.acos()); - ensure_nondet(|| 1.0f64.atan()); - ensure_nondet(|| 1.0f64.atan2(2.0f64)); - ensure_nondet(|| 1.0f64.sinh()); - ensure_nondet(|| 1.0f64.cosh()); - ensure_nondet(|| 1.0f64.tanh()); - ensure_nondet(|| 1.0f64.asinh()); - ensure_nondet(|| 3.0f64.acosh()); - ensure_nondet(|| 0.5f64.atanh()); - ensure_nondet(|| 5.0f64.gamma()); - ensure_nondet(|| 5.0f64.ln_gamma()); - ensure_nondet(|| 5.0f64.erf()); - ensure_nondet(|| 5.0f64.erfc()); + check_nondet(|| a.powf(b)); + check_nondet(|| a.powi(2)); + check_nondet(|| a.log(b)); + check_nondet(|| a.exp()); + check_nondet(|| 50f64.exp2()); + check_nondet(|| 3f64.ln()); + check_nondet(|| f64::consts::E.log10()); + check_nondet(|| f64::consts::E.log2()); + check_nondet(|| 1f64.ln_1p()); + check_nondet(|| 27.0f64.cbrt()); + check_nondet(|| 3.0f64.hypot(4.0f64)); + check_nondet(|| 1f64.sin()); + check_nondet(|| 1f64.cos()); + check_nondet(|| 1.0f64.tan()); + check_nondet(|| 1.0f64.asin()); + check_nondet(|| 5.0f64.acos()); + check_nondet(|| 1.0f64.atan()); + check_nondet(|| 1.0f64.atan2(2.0f64)); + check_nondet(|| 1.0f64.sinh()); + check_nondet(|| 1.0f64.cosh()); + check_nondet(|| 1.0f64.tanh()); + check_nondet(|| 1.0f64.asinh()); + check_nondet(|| 3.0f64.acosh()); + check_nondet(|| 0.5f64.atanh()); + check_nondet(|| 5.0f64.gamma()); + check_nondet(|| 5.0f64.ln_gamma()); + check_nondet(|| 5.0f64.erf()); + check_nondet(|| 5.0f64.erfc()); } pub fn test_operations_f128(a: f128, b: f128) { test_operations_f!(a, b); @@ -1574,15 +1548,15 @@ fn test_non_determinism() { test_operations_f128(25., 18.); // SNaN^0 = (1 | NaN) - ensure_nondet(|| f32::powf(SNAN_F32, 0.0).is_nan()); - ensure_nondet(|| f64::powf(SNAN_F64, 0.0).is_nan()); + check_nondet(|| f32::powf(SNAN_F32, 0.0).is_nan()); + check_nondet(|| f64::powf(SNAN_F64, 0.0).is_nan()); // 1^SNaN = (1 | NaN) - ensure_nondet(|| f32::powf(1.0, SNAN_F32).is_nan()); - ensure_nondet(|| f64::powf(1.0, SNAN_F64).is_nan()); + check_nondet(|| f32::powf(1.0, SNAN_F32).is_nan()); + check_nondet(|| f64::powf(1.0, SNAN_F64).is_nan()); // same as powf (keep it consistent): // x^SNaN = (1 | NaN) - ensure_nondet(|| f32::powi(SNAN_F32, 0).is_nan()); - ensure_nondet(|| f64::powi(SNAN_F64, 0).is_nan()); + check_nondet(|| f32::powi(SNAN_F32, 0).is_nan()); + check_nondet(|| f64::powi(SNAN_F64, 0).is_nan()); } diff --git a/src/tools/miri/tests/pass/float_nan.rs b/src/tools/miri/tests/pass/float_nan.rs index 90281630740..c07ffdf9740 100644 --- a/src/tools/miri/tests/pass/float_nan.rs +++ b/src/tools/miri/tests/pass/float_nan.rs @@ -5,6 +5,10 @@ use std::fmt; use std::hint::black_box; +#[path = "../utils/mod.rs"] +mod utils; +use utils::check_all_outcomes; + fn ldexp(a: f64, b: i32) -> f64 { extern "C" { fn ldexp(x: f64, n: i32) -> f64; @@ -26,35 +30,6 @@ enum NaNKind { } use NaNKind::*; -/// Check that the function produces the intended set of outcomes. -#[track_caller] -fn check_all_outcomes<T: Eq + std::hash::Hash + fmt::Display>( - expected: impl IntoIterator<Item = T>, - generate: impl Fn() -> T, -) { - use std::collections::HashSet; - - let expected: HashSet<T> = HashSet::from_iter(expected); - let mut seen = HashSet::new(); - // Let's give it N times as many tries as we are expecting values. - let tries = expected.len() * 12; - for i in 0..tries { - let val = generate(); - assert!(expected.contains(&val), "got an unexpected value: {val}"); - seen.insert(val); - if i > tries / 2 && expected.len() == seen.len() { - // We saw everything and we did quite a few tries, let's avoid wasting time. - return; - } - } - // Let's see if we saw them all. - for val in expected { - if !seen.contains(&val) { - panic!("did not get value that should be possible: {val}"); - } - } -} - // -- f32 support #[repr(C)] #[derive(Copy, Clone, Eq, PartialEq, Hash)] @@ -81,7 +56,7 @@ const F32_EXP: u32 = 8; // 8 bits of exponent const F32_MANTISSA: u32 = F32_SIGN_BIT - F32_EXP; const F32_NAN_PAYLOAD: u32 = F32_MANTISSA - 1; -impl fmt::Display for F32 { +impl fmt::Debug for F32 { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // Alaways show raw bits. write!(f, "0x{:08x} ", self.0)?; @@ -154,7 +129,7 @@ const F64_EXP: u32 = 11; // 11 bits of exponent const F64_MANTISSA: u32 = F64_SIGN_BIT - F64_EXP; const F64_NAN_PAYLOAD: u32 = F64_MANTISSA - 1; -impl fmt::Display for F64 { +impl fmt::Debug for F64 { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // Alaways show raw bits. write!(f, "0x{:08x} ", self.0)?; diff --git a/src/tools/miri/tests/pass/intrinsics/fmuladd_nondeterministic.rs b/src/tools/miri/tests/pass/intrinsics/fmuladd_nondeterministic.rs index 4d3e91c4cba..abc156d49cb 100644 --- a/src/tools/miri/tests/pass/intrinsics/fmuladd_nondeterministic.rs +++ b/src/tools/miri/tests/pass/intrinsics/fmuladd_nondeterministic.rs @@ -3,73 +3,48 @@ use std::intrinsics::simd::simd_relaxed_fma; use std::intrinsics::{fmuladdf32, fmuladdf64}; use std::simd::prelude::*; -fn ensure_both_happen(f: impl Fn() -> bool) -> bool { - let mut saw_true = false; - let mut saw_false = false; - for _ in 0..50 { - let b = f(); - if b { - saw_true = true; - } else { - saw_false = true; - } - if saw_true && saw_false { - return true; - } - } - false -} +#[path = "../../utils/mod.rs"] +mod utils; +use utils::check_nondet; fn main() { - assert!( - ensure_both_happen(|| { - let a = std::hint::black_box(0.1_f64); - let b = std::hint::black_box(0.2); - let c = std::hint::black_box(-a * b); - // It is unspecified whether the following operation is fused or not. The - // following evaluates to 0.0 if unfused, and nonzero (-1.66e-18) if fused. - let x = fmuladdf64(a, b, c); - x == 0.0 - }), - "`fmuladdf64` failed to be evaluated as both fused and unfused" - ); + check_nondet(|| { + let a = std::hint::black_box(0.1_f64); + let b = std::hint::black_box(0.2); + let c = std::hint::black_box(-a * b); + // It is unspecified whether the following operation is fused or not. The + // following evaluates to 0.0 if unfused, and nonzero (-1.66e-18) if fused. + let x = fmuladdf64(a, b, c); + x == 0.0 + }); - assert!( - ensure_both_happen(|| { - let a = std::hint::black_box(0.1_f32); - let b = std::hint::black_box(0.2); - let c = std::hint::black_box(-a * b); - // It is unspecified whether the following operation is fused or not. The - // following evaluates to 0.0 if unfused, and nonzero (-8.1956386e-10) if fused. - let x = fmuladdf32(a, b, c); - x == 0.0 - }), - "`fmuladdf32` failed to be evaluated as both fused and unfused" - ); + check_nondet(|| { + let a = std::hint::black_box(0.1_f32); + let b = std::hint::black_box(0.2); + let c = std::hint::black_box(-a * b); + // It is unspecified whether the following operation is fused or not. The + // following evaluates to 0.0 if unfused, and nonzero (-8.1956386e-10) if fused. + let x = fmuladdf32(a, b, c); + x == 0.0 + }); - assert!( - ensure_both_happen(|| { - let a = f32x4::splat(std::hint::black_box(0.1)); - let b = f32x4::splat(std::hint::black_box(0.2)); - let c = std::hint::black_box(-a * b); - let x = unsafe { simd_relaxed_fma(a, b, c) }; - // Whether we fuse or not is a per-element decision, so sometimes these should be - // the same and sometimes not. - x[0] == x[1] - }), - "`simd_relaxed_fma` failed to be evaluated as both fused and unfused" - ); + check_nondet(|| { + let a = f32x4::splat(std::hint::black_box(0.1)); + let b = f32x4::splat(std::hint::black_box(0.2)); + let c = std::hint::black_box(-a * b); + let x = unsafe { simd_relaxed_fma(a, b, c) }; + // Whether we fuse or not is a per-element decision, so sometimes these should be + // the same and sometimes not. + x[0] == x[1] + }); - assert!( - ensure_both_happen(|| { - let a = f64x4::splat(std::hint::black_box(0.1)); - let b = f64x4::splat(std::hint::black_box(0.2)); - let c = std::hint::black_box(-a * b); - let x = unsafe { simd_relaxed_fma(a, b, c) }; - // Whether we fuse or not is a per-element decision, so sometimes these should be - // the same and sometimes not. - x[0] == x[1] - }), - "`simd_relaxed_fma` failed to be evaluated as both fused and unfused" - ); + check_nondet(|| { + let a = f64x4::splat(std::hint::black_box(0.1)); + let b = f64x4::splat(std::hint::black_box(0.2)); + let c = std::hint::black_box(-a * b); + let x = unsafe { simd_relaxed_fma(a, b, c) }; + // Whether we fuse or not is a per-element decision, so sometimes these should be + // the same and sometimes not. + x[0] == x[1] + }); } diff --git a/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs index adf2f4e845b..4a868455c84 100644 --- a/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs +++ b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs @@ -20,7 +20,7 @@ pub fn main() { name!(ptr2); // We perform a write through `x`. - // Because `ptr1` is ReservedIM, a child write will make it transition to Active. + // Because `ptr1` is ReservedIM, a child write will make it transition to Unique. // Because `ptr2` is ReservedIM, a foreign write doesn't have any effect on it. let x = (*ptr1).get(); *x = 1; diff --git a/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.rs b/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.rs index 4fbccef2367..edd649b91ed 100644 --- a/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.rs +++ b/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.rs @@ -6,7 +6,7 @@ mod utils; // To check that a reborrow is counted as a Read access, we use a reborrow -// with no additional Read to Freeze an Active pointer. +// with no additional Read to Freeze an Unique pointer. fn main() { unsafe { @@ -15,7 +15,7 @@ fn main() { let alloc_id = alloc_id!(parent); let x = &mut *parent; name!(x); - *x = 0; // x is now Active + *x = 0; // x is now Unique print_state!(alloc_id); let y = &mut *parent; name!(y); diff --git a/src/tools/miri/tests/pass/tree_borrows/reserved.rs b/src/tools/miri/tests/pass/tree_borrows/reserved.rs index c57cd7fcf0a..83e1d9c70ed 100644 --- a/src/tools/miri/tests/pass/tree_borrows/reserved.rs +++ b/src/tools/miri/tests/pass/tree_borrows/reserved.rs @@ -68,7 +68,7 @@ unsafe fn cell_unprotected_read() { } // Foreign Write on an interior mutable pointer is a noop. -// Also y must become Active. +// Also y must become Unique. unsafe fn cell_unprotected_write() { print("[interior mut] Foreign Write: Re* -> Re*"); let base = &mut UnsafeCell::new(0u64); @@ -97,7 +97,7 @@ unsafe fn int_protected_read() { } // Foreign Read on a Reserved is a noop. -// Also y must become Active. +// Also y must become Unique. unsafe fn int_unprotected_read() { print("[] Foreign Read: Res -> Res"); let base = &mut 0u8; diff --git a/src/tools/miri/tests/utils/mod.rs b/src/tools/miri/tests/utils/mod.rs index 138ada4e20d..37f99962163 100644 --- a/src/tools/miri/tests/utils/mod.rs +++ b/src/tools/miri/tests/utils/mod.rs @@ -16,3 +16,53 @@ pub fn run_provenance_gc() { // SAFETY: No preconditions. The GC is fine to run at any time. unsafe { miri_run_provenance_gc() } } + +/// Check that the function produces the intended set of outcomes. +#[track_caller] +pub fn check_all_outcomes<T: Eq + std::hash::Hash + std::fmt::Debug>( + expected: impl IntoIterator<Item = T>, + generate: impl Fn() -> T, +) { + use std::collections::HashSet; + + let expected: HashSet<T> = HashSet::from_iter(expected); + let mut seen = HashSet::new(); + // Let's give it N times as many tries as we are expecting values. + let min_tries = std::cmp::max(20, expected.len() * 4); + let max_tries = expected.len() * 50; + for i in 0..max_tries { + let val = generate(); + assert!(expected.contains(&val), "got an unexpected value: {val:?}"); + seen.insert(val); + if i >= min_tries && expected.len() == seen.len() { + // We saw everything and we did enough tries, let's avoid wasting time. + return; + } + } + // Let's see if we saw them all. + if expected.len() == seen.len() { + return; + } + // Find the missing one. + for val in expected { + if !seen.contains(&val) { + panic!("did not get value that should be possible: {val:?}"); + } + } + unreachable!() +} + +/// Check that the operation is non-deterministic +#[track_caller] +pub fn check_nondet<T: PartialEq + std::fmt::Debug>(f: impl Fn() -> T) { + let rounds = 50; + let first = f(); + for _ in 1..rounds { + if f() != first { + // We saw two different values! + return; + } + } + // We saw the same thing N times. + panic!("expected non-determinism, got {rounds} times the same result: {first:?}"); +} |