From 297ddffff3248c0d60a07a8a96cb2d06d6191a2b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 15 Aug 2022 18:31:40 -0400 Subject: [PATCH 1/4] add test that we do not merge neighboring SRW --- .../disable_mut_does_not_merge_srw.rs | 17 +++++++++++ .../disable_mut_does_not_merge_srw.stderr | 28 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.rs create mode 100644 tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.stderr diff --git a/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.rs b/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.rs new file mode 100644 index 00000000000..1dcd7621acd --- /dev/null +++ b/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.rs @@ -0,0 +1,17 @@ +// This tests demonstrates the effect of 'Disabling' mutable references on reads, rather than +// removing them from the stack -- the latter would 'merge' neighboring SRW groups which we would +// like to avoid. +fn main() { + unsafe { + let mut mem = 0; + let base = &mut mem as *mut i32; // the base pointer we build the rest of the stack on + let mutref = &mut *base; + let raw = mutref as *mut i32; + // in the stack, we now have [base, mutref, raw] + let _val = *base; + // now mutref is disabled + *base = 1; + // this should pop raw from the stack, since it is in a different SRW group + let _val = *raw; //~ERROR: that tag does not exist in the borrow stack + } +} diff --git a/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.stderr b/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.stderr new file mode 100644 index 00000000000..b3f0204c09f --- /dev/null +++ b/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.stderr @@ -0,0 +1,28 @@ +error: Undefined Behavior: attempting a read access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + --> $DIR/disable_mut_does_not_merge_srw.rs:LL:CC + | +LL | let _val = *raw; + | ^^^^ + | | + | 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 +help: was created by a retag at offsets [0x0..0x4] + --> $DIR/disable_mut_does_not_merge_srw.rs:LL:CC + | +LL | let raw = mutref as *mut i32; + | ^^^^^^ +help: was later invalidated at offsets [0x0..0x4] + --> $DIR/disable_mut_does_not_merge_srw.rs:LL:CC + | +LL | *base = 1; + | ^^^^^^^^^ + = note: backtrace: + = note: inside `main` at $DIR/disable_mut_does_not_merge_srw.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + From 79ebfa25dcde23f3eb95238a23707b69cc01a528 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 15 Aug 2022 18:35:11 -0400 Subject: [PATCH 2/4] make Miri build without the stack-cache feature --- src/stacked_borrows/stack.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/stacked_borrows/stack.rs b/src/stacked_borrows/stack.rs index 6fd8d85f2b5..4a9a13d35b5 100644 --- a/src/stacked_borrows/stack.rs +++ b/src/stacked_borrows/stack.rs @@ -81,7 +81,7 @@ impl<'tcx> Stack { /// Panics if any of the caching mechanisms have broken, /// - The StackCache indices don't refer to the parallel items, /// - There are no Unique items outside of first_unique..last_unique - #[cfg(debug_assertions)] + #[cfg(all(feature = "stack-cache", debug_assertions))] fn verify_cache_consistency(&self) { // Only a full cache needs to be valid. Also see the comments in find_granting_cache // and set_unknown_bottom. @@ -128,7 +128,7 @@ pub(super) fn find_granting( tag: ProvenanceExtra, exposed_tags: &FxHashSet, ) -> Result, ()> { - #[cfg(debug_assertions)] + #[cfg(all(feature = "stack-cache", debug_assertions))] self.verify_cache_consistency(); let ProvenanceExtra::Concrete(tag) = tag else { @@ -320,13 +320,14 @@ pub fn disable_uniques_starting_at crate::InterpResult<'tcx>>( if disable_start <= unique_range.end { let lower = unique_range.start.max(disable_start); - let upper = self.unique_range.end; + let upper = unique_range.end; for item in &mut self.borrows[lower..upper] { if item.perm() == Permission::Unique { log::trace!("access: disabling item {:?}", item); visitor(*item)?; item.set_permission(Permission::Disabled); // Also update all copies of this item in the cache. + #[cfg(feature = "stack-cache")] for it in &mut self.cache.items { if it.tag() == item.tag() { it.set_permission(Permission::Disabled); @@ -347,7 +348,7 @@ pub fn disable_uniques_starting_at crate::InterpResult<'tcx>>( self.unique_range.end = self.unique_range.end.min(disable_start); } - #[cfg(debug_assertions)] + #[cfg(all(feature = "stack-cache", debug_assertions))] self.verify_cache_consistency(); Ok(()) @@ -402,7 +403,7 @@ pub fn pop_items_after crate::InterpResult<'tcx>>( self.unique_range = 0..0; } - #[cfg(debug_assertions)] + #[cfg(all(feature = "stack-cache", debug_assertions))] self.verify_cache_consistency(); Ok(()) } From 2e3da5d8c30238cddbc63131776af485d2d10cda Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 15 Aug 2022 19:33:07 -0400 Subject: [PATCH 3/4] check no-default-features and all-features build on CI --- ci.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci.sh b/ci.sh index a29fc3deca5..8189596dba7 100755 --- a/ci.sh +++ b/ci.sh @@ -5,11 +5,12 @@ set -x # Determine configuration export RUSTFLAGS="-D warnings" export CARGO_INCREMENTAL=0 -export CARGO_EXTRA_FLAGS="--all-features" # Prepare echo "Build and install miri" ./miri install # implicitly locked +./miri check --no-default-features # make sure this can be built +./miri check --all-features # and this, too ./miri build --all-targets --locked # the build that all the `./miri test` below will use echo From db43ee5714900900a52ad5050b77993288472ce2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 15 Aug 2022 20:23:19 -0400 Subject: [PATCH 4/4] defend test against overly smart Miri --- .../stacked_borrows/disable_mut_does_not_merge_srw.rs | 11 ++++++++--- .../disable_mut_does_not_merge_srw.stderr | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.rs b/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.rs index 1dcd7621acd..fed5cd26f4d 100644 --- a/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.rs +++ b/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.rs @@ -5,9 +5,14 @@ fn main() { unsafe { let mut mem = 0; let base = &mut mem as *mut i32; // the base pointer we build the rest of the stack on - let mutref = &mut *base; - let raw = mutref as *mut i32; - // in the stack, we now have [base, mutref, raw] + let raw = { + let mutref = &mut *base; + mutref as *mut i32 + }; + // In the stack, we now have [base, mutref, raw]. + // We do this in a weird way where `mutref` is out of scope here, just in case + // Miri decides to get smart and argue that items for tags that are no longer + // used by any pointer stored anywhere in the machine can be removed. let _val = *base; // now mutref is disabled *base = 1; diff --git a/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.stderr b/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.stderr index b3f0204c09f..9f5182ae98f 100644 --- a/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.stderr +++ b/tests/fail/stacked_borrows/disable_mut_does_not_merge_srw.stderr @@ -12,8 +12,8 @@ LL | let _val = *raw; help: was created by a retag at offsets [0x0..0x4] --> $DIR/disable_mut_does_not_merge_srw.rs:LL:CC | -LL | let raw = mutref as *mut i32; - | ^^^^^^ +LL | mutref as *mut i32 + | ^^^^^^ help: was later invalidated at offsets [0x0..0x4] --> $DIR/disable_mut_does_not_merge_srw.rs:LL:CC |