From 648d74a068be79fa6c1349a4740962527af8d448 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Sun, 16 Jul 2023 06:24:54 +0000 Subject: [PATCH 01/15] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 3bef3be2a53..18b17451bbe 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -33a2c2487ac5d9927830ea4c1844335c6b9f77db +ffb9b61294b96c389d343a4c55b15400249d74e6 From 8480847ed8d835239c55e345a0c1b0935b23a19c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 15 Jul 2023 10:26:35 +0200 Subject: [PATCH 02/15] style checks: use latest rustc for cron job --- src/tools/miri/.github/workflows/ci.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index c87b3e42323..452677d9146 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -115,7 +115,12 @@ jobs: run: cargo install -f rustup-toolchain-install-master - name: Install "master" toolchain - run: ./miri toolchain + run: | + if [[ ${{ github.event_name }} == 'schedule' ]]; then + echo "Building against latest rustc git version" + git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1 > rust-version + fi + ./miri toolchain - name: Show Rust version run: | @@ -203,7 +208,7 @@ jobs: ./miri fmt --check || (./miri fmt && git commit -am "fmt") - name: Push changes to a branch run: | - BRANCH="rustup$(date -u +%Y-%m-%d)" + BRANCH="rustup-$(date -u +%Y-%m-%d)" git switch -c $BRANCH git push -u origin $BRANCH - name: Create Pull Request From d537a7aaa26b48e723dcea0e1d065690a5632b45 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 16 Jul 2023 13:47:16 +0200 Subject: [PATCH 03/15] clarify that we do not prove soundness --- src/tools/miri/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 601101905f1..d51384d0a14 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -74,6 +74,12 @@ behavior** in your program, and cannot run all programs: unobservable by compiled programs running on real hardware when `SeqCst` fences are used, and it cannot produce all behaviors possibly observable on real hardware. +Moreover, Miri fundamentally cannot tell you whether your code is *sound*. Soundness is the property +of never causing undefined behavior when invoked from arbitrary safe code, even in combination with +other sound code. In contrast, Miri can just tell you if *a particular way of interacting with your +code* (e.g., a test suite) causes any undefined behavior. It is up to you to ensure sufficient +coverage. + [rust]: https://www.rust-lang.org/ [mir]: https://github.com/rust-lang/rfcs/blob/master/text/1211-mir.md [`unreachable_unchecked`]: https://doc.rust-lang.org/stable/std/hint/fn.unreachable_unchecked.html From 75da07bf28e64aadbc6f452da4d0cb44e4223b5a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 16 Jul 2023 15:51:46 +0200 Subject: [PATCH 04/15] link to a definition of soundness --- src/tools/miri/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index d51384d0a14..a965f44a4e6 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -74,7 +74,7 @@ behavior** in your program, and cannot run all programs: unobservable by compiled programs running on real hardware when `SeqCst` fences are used, and it cannot produce all behaviors possibly observable on real hardware. -Moreover, Miri fundamentally cannot tell you whether your code is *sound*. Soundness is the property +Moreover, Miri fundamentally cannot tell you whether your code is *sound*. [Soundness] is the property of never causing undefined behavior when invoked from arbitrary safe code, even in combination with other sound code. In contrast, Miri can just tell you if *a particular way of interacting with your code* (e.g., a test suite) causes any undefined behavior. It is up to you to ensure sufficient @@ -86,6 +86,7 @@ coverage. [`copy_nonoverlapping`]: https://doc.rust-lang.org/stable/std/ptr/fn.copy_nonoverlapping.html [Stacked Borrows]: https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md [Tree Borrows]: https://perso.crans.org/vanille/treebor/ +[Soundness]: https://rust-lang.github.io/unsafe-code-guidelines/glossary.html#soundness-of-code--of-a-library ## Using Miri From 7948f1ac472fc7b11a80cc33ee8f99b2f0a93f8c Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Tue, 18 Jul 2023 06:58:26 +0000 Subject: [PATCH 05/15] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 18b17451bbe..2cc690989d1 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -ffb9b61294b96c389d343a4c55b15400249d74e6 +ec362f0ae8a05618b75727cfdca853540cb2950e From f5aa3131b4d8d2ed54cb3eda084c82c18cb38d36 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 18 Jul 2023 11:25:06 +0200 Subject: [PATCH 06/15] make './miri toolchain' work even if we cannot write to rustup dir --- src/tools/miri/miri | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/miri b/src/tools/miri/miri index ace3d17ae2a..1bc4e254ad4 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -97,7 +97,9 @@ toolchain) CUR_COMMIT=$(rustc +miri --version -v 2>/dev/null | grep "^commit-hash: " | cut -d " " -f 2) if [[ "$CUR_COMMIT" == "$NEW_COMMIT" ]]; then echo "miri toolchain is already at commit $CUR_COMMIT." - rustup override set miri + if [[ "$TOOLCHAIN" != "miri" ]]; then + rustup override set miri + fi exit 0 fi # Install and setup new toolchain. From 6ba56ac7b900d04afec2f9c81323e8508ce74109 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 18 Jul 2023 11:25:17 +0200 Subject: [PATCH 07/15] fix clippy warnings --- src/tools/miri/src/borrow_tracker/mod.rs | 4 ++-- src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs | 6 +++--- src/tools/miri/src/shims/unix/sync.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index fcfa8f64570..b6dfd9944ee 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -413,7 +413,7 @@ impl AllocState { alloc_id: AllocId, prov_extra: ProvenanceExtra, range: AllocRange, - machine: &mut MiriMachine<'_, 'tcx>, + machine: &MiriMachine<'_, 'tcx>, ) -> InterpResult<'tcx> { match self { AllocState::StackedBorrows(sb) => @@ -434,7 +434,7 @@ impl AllocState { alloc_id: AllocId, prov_extra: ProvenanceExtra, range: AllocRange, - machine: &mut MiriMachine<'_, 'tcx>, + machine: &MiriMachine<'_, 'tcx>, ) -> InterpResult<'tcx> { match self { AllocState::StackedBorrows(sb) => diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 15a7d72edf1..7e89d3a0e8c 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -244,7 +244,7 @@ impl<'tcx> Stack { fn item_invalidated( item: &Item, global: &GlobalStateInner, - dcx: &mut DiagnosticCx<'_, '_, '_, 'tcx>, + dcx: &DiagnosticCx<'_, '_, '_, 'tcx>, cause: ItemInvalidationCause, ) -> InterpResult<'tcx> { if !global.tracked_pointer_tags.is_empty() { @@ -575,7 +575,7 @@ impl Stacks { alloc_id: AllocId, tag: ProvenanceExtra, range: AllocRange, - machine: &mut MiriMachine<'_, 'tcx>, + machine: &MiriMachine<'_, 'tcx>, ) -> InterpResult<'tcx> { trace!( "write access with tag {:?}: {:?}, size {}", @@ -596,7 +596,7 @@ impl Stacks { alloc_id: AllocId, tag: ProvenanceExtra, range: AllocRange, - machine: &mut MiriMachine<'_, 'tcx>, + machine: &MiriMachine<'_, 'tcx>, ) -> InterpResult<'tcx> { trace!("deallocation with tag {:?}: {:?}, size {}", tag, alloc_id, range.size.bytes()); let dcx = DiagnosticCxBuilder::dealloc(machine, tag); diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index 94b831da5c8..1fa0ffd8ee7 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -18,14 +18,14 @@ use crate::*; const PTHREAD_MUTEX_NORMAL_FLAG: i32 = 0x8000000; fn is_mutex_kind_default<'mir, 'tcx: 'mir>( - ecx: &mut MiriInterpCx<'mir, 'tcx>, + ecx: &MiriInterpCx<'mir, 'tcx>, kind: i32, ) -> InterpResult<'tcx, bool> { Ok(kind == ecx.eval_libc_i32("PTHREAD_MUTEX_DEFAULT")) } fn is_mutex_kind_normal<'mir, 'tcx: 'mir>( - ecx: &mut MiriInterpCx<'mir, 'tcx>, + ecx: &MiriInterpCx<'mir, 'tcx>, kind: i32, ) -> InterpResult<'tcx, bool> { let mutex_normal_kind = ecx.eval_libc_i32("PTHREAD_MUTEX_NORMAL"); From 7bee1c86161a7189508597902895b5bb5d460321 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 18 Jul 2023 11:26:25 +0200 Subject: [PATCH 08/15] ignore test on apple --- src/tools/miri/tests/fail/function_calls/target_feature.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/miri/tests/fail/function_calls/target_feature.rs b/src/tools/miri/tests/fail/function_calls/target_feature.rs index be95241ea67..84e01eb4803 100644 --- a/src/tools/miri/tests/fail/function_calls/target_feature.rs +++ b/src/tools/miri/tests/fail/function_calls/target_feature.rs @@ -1,4 +1,5 @@ //@only-target-x86_64: uses x86 target features +//@ignore-target-x86_64-apple-darwin: that target actually has ssse3 fn main() { assert!(!is_x86_feature_detected!("ssse3")); From dfb552e37ea7a8fca3bf19ed3a88be5d366bf27b Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Thu, 20 Jul 2023 06:32:05 +0000 Subject: [PATCH 09/15] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 2cc690989d1..da89b71ccd5 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -ec362f0ae8a05618b75727cfdca853540cb2950e +0646a5d1aa3745cb448db247f6fa432890a1812b From 245c52e7801ae00f07f3876c7a03820ce6bd1c0c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Jul 2023 16:45:18 +0200 Subject: [PATCH 10/15] make full field retagging the default --- src/tools/miri/README.md | 14 +++---- src/tools/miri/src/eval.rs | 2 +- .../fail/both_borrows/buggy_split_at_mut.rs | 2 +- .../buggy_split_at_mut.stack.stderr | 20 +++++++--- .../miri/tests/fail/generator-pinned-moved.rs | 5 ++- src/tools/miri/tests/pass/generator.rs | 38 +++++++++++++------ .../stacked-borrows/interior_mutability.rs | 1 - .../pass/stacked-borrows/stacked-borrows.rs | 1 - .../zst-field-retagging-terminates.rs | 1 - 9 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index a965f44a4e6..eaf58340d7b 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -407,15 +407,11 @@ to Miri failing to detect cases of undefined behavior in a program. application instead of raising an error within the context of Miri (and halting execution). Note that code might not expect these operations to ever panic, so this flag can lead to strange (mis)behavior. -* `-Zmiri-retag-fields` changes Stacked Borrows retagging to recurse into *all* fields. - This means that references in fields of structs/enums/tuples/arrays/... are retagged, - and in particular, they are protected when passed as function arguments. - (The default is to recurse only in cases where rustc would actually emit a `noalias` attribute.) -* `-Zmiri-retag-fields=` controls when Stacked Borrows retagging recurses into - fields. `all` means it always recurses (like `-Zmiri-retag-fields`), `none` means it never - recurses, `scalar` (the default) means it only recurses for types where we would also emit - `noalias` annotations in the generated LLVM IR (types passed as individual scalars or pairs of - scalars). Setting this to `none` is **unsound**. +* `-Zmiri-retag-fields[=]` controls when Stacked Borrows retagging recurses into + fields. `all` means it always recurses (the default, and equivalent to `-Zmiri-retag-fields` + without an explicit value), `none` means it never recurses, `scalar` means it only recurses for + types where we would also emit `noalias` annotations in the generated LLVM IR (types passed as + individual scalars or pairs of scalars). Setting this to `none` is **unsound**. * `-Zmiri-tag-gc=` configures how often the pointer tag garbage collector runs. The default is to search for and remove unreachable tags once every `10000` basic blocks. Setting this to `0` disables the garbage collector, which causes some programs to have explosive memory usage diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index ed3d741db1c..79a7115f24e 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -183,7 +183,7 @@ impl Default for MiriConfig { mute_stdout_stderr: false, preemption_rate: 0.01, // 1% report_progress: None, - retag_fields: RetagFields::OnlyScalar, + retag_fields: RetagFields::Yes, external_so_file: None, gc_interval: 10_000, num_cpus: 1, diff --git a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.rs b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.rs index 4dc6eaf7cd3..80ededd2100 100644 --- a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.rs +++ b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.rs @@ -14,6 +14,7 @@ mod safe { from_raw_parts_mut(ptr, len - mid), // BUG: should be "mid" instead of "len - mid" from_raw_parts_mut(ptr.offset(mid as isize), len - mid), ) + //~[stack]^^^^ ERROR: /retag .* tag does not exist in the borrow stack/ } } } @@ -21,7 +22,6 @@ mod safe { fn main() { let mut array = [1, 2, 3, 4]; let (a, b) = safe::split_at_mut(&mut array, 0); - //~[stack]^ ERROR: /retag .* tag does not exist in the borrow stack/ a[1] = 5; b[1] = 6; //~[tree]^ ERROR: /write access through .* is forbidden/ diff --git a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr index c75d8cab3fc..0cb6111ecb9 100644 --- a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr @@ -1,11 +1,14 @@ error: Undefined Behavior: trying to retag from for Unique permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location --> $DIR/buggy_split_at_mut.rs:LL:CC | -LL | let (a, b) = safe::split_at_mut(&mut array, 0); - | ^ - | | - | trying to retag from for Unique permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x0..0x10] +LL | / ( +LL | | from_raw_parts_mut(ptr, len - mid), // BUG: should be "mid" instead of "len - mid" +LL | | from_raw_parts_mut(ptr.offset(mid as isize), len - mid), +LL | | ) + | | ^ + | | | + | |_____________trying to retag from for Unique permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + | this error occurs as part of retag at ALLOC[0x0..0x10] | = 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 @@ -20,7 +23,12 @@ help: was later invalidated at offsets [0x0..0x10] by a Unique retag LL | from_raw_parts_mut(ptr.offset(mid as isize), len - mid), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: BACKTRACE (of the first span): - = note: inside `main` at $DIR/buggy_split_at_mut.rs:LL:CC + = note: inside `safe::split_at_mut::` at $DIR/buggy_split_at_mut.rs:LL:CC +note: inside `main` + --> $DIR/buggy_split_at_mut.rs:LL:CC + | +LL | let (a, b) = safe::split_at_mut(&mut array, 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/generator-pinned-moved.rs b/src/tools/miri/tests/fail/generator-pinned-moved.rs index 240ae18cc45..29dc6e56f7c 100644 --- a/src/tools/miri/tests/fail/generator-pinned-moved.rs +++ b/src/tools/miri/tests/fail/generator-pinned-moved.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-validation +//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows #![feature(generators, generator_trait)] use std::{ @@ -10,9 +10,10 @@ fn firstn() -> impl Generator { static move || { let mut num = 0; let num = &mut num; + *num += 0; yield *num; - *num += 1; //~ ERROR: dereferenced after this allocation got freed + *num += 1; //~ERROR: dereferenced after this allocation got freed } } diff --git a/src/tools/miri/tests/pass/generator.rs b/src/tools/miri/tests/pass/generator.rs index e059c7114e3..20099603455 100644 --- a/src/tools/miri/tests/pass/generator.rs +++ b/src/tools/miri/tests/pass/generator.rs @@ -13,7 +13,7 @@ use std::ptr; use std::sync::atomic::{AtomicUsize, Ordering}; fn basic() { - fn finish(mut amt: usize, mut t: T) -> T::Return + fn finish(mut amt: usize, self_referential: bool, mut t: T) -> T::Return where T: Generator, { @@ -22,7 +22,10 @@ fn basic() { loop { let state = t.as_mut().resume(()); // Test if the generator is valid (according to type invariants). - let _ = unsafe { ManuallyDrop::new(ptr::read(t.as_mut().get_unchecked_mut())) }; + // For self-referential generators however this is UB! + if !self_referential { + let _ = unsafe { ManuallyDrop::new(ptr::read(t.as_mut().get_unchecked_mut())) }; + } match state { GeneratorState::Yielded(y) => { amt -= y; @@ -40,9 +43,9 @@ fn basic() { panic!() } - finish(1, || yield 1); + finish(1, false, || yield 1); - finish(3, || { + finish(3, false, || { let mut x = 0; yield 1; x += 1; @@ -52,27 +55,27 @@ fn basic() { assert_eq!(x, 2); }); - finish(7 * 8 / 2, || { + finish(7 * 8 / 2, false, || { for i in 0..8 { yield i; } }); - finish(1, || { + finish(1, false, || { if true { yield 1; } else { } }); - finish(1, || { + finish(1, false, || { if false { } else { yield 1; } }); - finish(2, || { + finish(2, false, || { if { yield 1; false @@ -83,9 +86,20 @@ fn basic() { yield 1; }); - // also test a self-referential generator + // also test self-referential generators assert_eq!( - finish(5, || { + finish(5, true, static || { + let mut x = 5; + let y = &mut x; + *y = 5; + yield *y; + *y = 10; + x + }), + 10 + ); + assert_eq!( + finish(5, true, || { let mut x = Box::new(5); let y = &mut *x; *y = 5; @@ -97,7 +111,7 @@ fn basic() { ); let b = true; - finish(1, || { + finish(1, false, || { yield 1; if b { return; @@ -109,7 +123,7 @@ fn basic() { drop(x); }); - finish(3, || { + finish(3, false, || { yield 1; #[allow(unreachable_code)] let _x: (String, !) = (String::new(), { diff --git a/src/tools/miri/tests/pass/stacked-borrows/interior_mutability.rs b/src/tools/miri/tests/pass/stacked-borrows/interior_mutability.rs index c6373a7eaf1..830e9c33847 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/interior_mutability.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/interior_mutability.rs @@ -1,4 +1,3 @@ -//@compile-flags: -Zmiri-retag-fields use std::cell::{Cell, Ref, RefCell, RefMut, UnsafeCell}; use std::mem::{self, MaybeUninit}; diff --git a/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs b/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs index d7d7d1f97d6..43ae7d6f522 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs @@ -1,4 +1,3 @@ -//@compile-flags: -Zmiri-retag-fields #![feature(allocator_api)] use std::ptr; diff --git a/src/tools/miri/tests/pass/stacked-borrows/zst-field-retagging-terminates.rs b/src/tools/miri/tests/pass/stacked-borrows/zst-field-retagging-terminates.rs index 6e13a9ea836..4faf6004ad6 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/zst-field-retagging-terminates.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/zst-field-retagging-terminates.rs @@ -1,4 +1,3 @@ -//@compile-flags: -Zmiri-retag-fields // Checks that the test does not run forever (which relies on a fast path). #![allow(dropping_copy_types)] From b8b92db1ee5de5e9add6370627aceb592755e998 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jul 2023 16:08:30 +0200 Subject: [PATCH 11/15] SB: track whether a retag occurred nested inside a field --- .../stacked_borrows/diagnostics.rs | 41 +++++++++++------ .../src/borrow_tracker/stacked_borrows/mod.rs | 45 ++++++++++++------- .../buggy_split_at_mut.stack.stderr | 2 +- .../pass_invalid_shr_option.stack.stderr | 2 +- .../pass_invalid_shr_tuple.stack.stderr | 2 +- .../return_invalid_shr_option.stack.stderr | 2 +- .../return_invalid_shr_tuple.stack.stderr | 2 +- .../return_invalid_mut_option.stderr | 2 +- .../return_invalid_mut_tuple.stderr | 2 +- 9 files changed, 63 insertions(+), 37 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs index 5ec8d80fb32..33b777abd9f 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -61,12 +61,15 @@ struct Invalidation { #[derive(Clone, Debug)] enum InvalidationCause { Access(AccessKind), - Retag(Permission, RetagCause), + Retag(Permission, RetagInfo), } impl Invalidation { fn generate_diagnostic(&self) -> (String, SpanData) { - let message = if let InvalidationCause::Retag(_, RetagCause::FnEntry) = self.cause { + let message = if matches!( + self.cause, + InvalidationCause::Retag(_, RetagInfo { cause: RetagCause::FnEntry, .. }) + ) { // For a FnEntry retag, our Span points at the caller. // See `DiagnosticCx::log_invalidation`. format!( @@ -87,8 +90,8 @@ impl fmt::Display for InvalidationCause { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { InvalidationCause::Access(kind) => write!(f, "{kind}"), - InvalidationCause::Retag(perm, kind) => - write!(f, "{perm:?} {retag}", retag = kind.summary()), + InvalidationCause::Retag(perm, info) => + write!(f, "{perm:?} {retag}", retag = info.summary()), } } } @@ -129,13 +132,13 @@ impl<'ecx, 'mir, 'tcx> DiagnosticCxBuilder<'ecx, 'mir, 'tcx> { pub fn retag( machine: &'ecx MiriMachine<'mir, 'tcx>, - cause: RetagCause, + info: RetagInfo, new_tag: BorTag, orig_tag: ProvenanceExtra, range: AllocRange, ) -> Self { let operation = - Operation::Retag(RetagOp { cause, new_tag, orig_tag, range, permission: None }); + Operation::Retag(RetagOp { info, new_tag, orig_tag, range, permission: None }); DiagnosticCxBuilder { machine, operation } } @@ -179,13 +182,19 @@ enum Operation { #[derive(Debug, Clone)] struct RetagOp { - cause: RetagCause, + info: RetagInfo, new_tag: BorTag, orig_tag: ProvenanceExtra, range: AllocRange, permission: Option, } +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct RetagInfo { + pub cause: RetagCause, + pub in_field: bool, +} + #[derive(Debug, Clone, Copy, PartialEq)] pub enum RetagCause { Normal, @@ -258,11 +267,11 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { pub fn log_invalidation(&mut self, tag: BorTag) { let mut span = self.machine.current_span(); let (range, cause) = match &self.operation { - Operation::Retag(RetagOp { cause, range, permission, .. }) => { - if *cause == RetagCause::FnEntry { + Operation::Retag(RetagOp { info, range, permission, .. }) => { + if info.cause == RetagCause::FnEntry { span = self.machine.caller_span(); } - (*range, InvalidationCause::Retag(permission.unwrap(), *cause)) + (*range, InvalidationCause::Retag(permission.unwrap(), *info)) } Operation::Access(AccessOp { kind, range, .. }) => (*range, InvalidationCause::Access(*kind)), @@ -374,7 +383,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { ); err_sb_ub( format!("{action}{}", error_cause(stack, op.orig_tag)), - Some(operation_summary(&op.cause.summary(), self.history.id, op.range)), + Some(operation_summary(&op.info.summary(), self.history.id, op.range)), op.orig_tag.and_then(|orig_tag| self.get_logs_relevant_to(orig_tag, None)), ) } @@ -496,14 +505,18 @@ fn error_cause(stack: &Stack, prov_extra: ProvenanceExtra) -> &'static str { } } -impl RetagCause { +impl RetagInfo { fn summary(&self) -> String { - match self { + let mut s = match self.cause { RetagCause::Normal => "retag", RetagCause::FnEntry => "function-entry retag", RetagCause::InPlaceFnPassing => "in-place function argument/return passing protection", RetagCause::TwoPhase => "two-phase retag", } - .to_string() + .to_string(); + if self.in_field { + s.push_str(" (of a reference/box inside this compound value)"); + } + s } } diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 7e89d3a0e8c..5e1e0d75436 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -5,9 +5,11 @@ pub mod diagnostics; mod item; mod stack; -use log::trace; use std::cmp; use std::fmt::Write; +use std::mem; + +use log::trace; use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir::{Mutability, RetagKind}; @@ -24,7 +26,7 @@ use crate::borrow_tracker::{ }; use crate::*; -use diagnostics::RetagCause; +use diagnostics::{RetagCause, RetagInfo}; pub use item::{Item, Permission}; pub use stack::Stack; @@ -623,7 +625,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' size: Size, new_perm: NewPermission, new_tag: BorTag, - retag_cause: RetagCause, // What caused this retag, for diagnostics only + retag_info: RetagInfo, // diagnostics info about this retag ) -> InterpResult<'tcx, Option> { let this = self.eval_context_mut(); @@ -670,7 +672,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // FIXME: can this be done cleaner? let dcx = DiagnosticCxBuilder::retag( &this.machine, - retag_cause, + retag_info, new_tag, orig_tag, alloc_range(base_offset, size), @@ -761,7 +763,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let global = machine.borrow_tracker.as_ref().unwrap().borrow(); let dcx = DiagnosticCxBuilder::retag( machine, - retag_cause, + retag_info, new_tag, orig_tag, alloc_range(base_offset, size), @@ -804,7 +806,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let global = this.machine.borrow_tracker.as_ref().unwrap().borrow(); let dcx = DiagnosticCxBuilder::retag( &this.machine, - retag_cause, + retag_info, new_tag, orig_tag, alloc_range(base_offset, size), @@ -834,7 +836,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' &mut self, val: &ImmTy<'tcx, Provenance>, new_perm: NewPermission, - cause: RetagCause, // What caused this retag, for diagnostics only + info: RetagInfo, // diagnostics info about this retag ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); // We want a place for where the ptr *points to*, so we get one. @@ -852,7 +854,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let new_tag = this.machine.borrow_tracker.as_mut().unwrap().get_mut().new_ptr(); // Reborrow. - let alloc_id = this.sb_reborrow(&place, size, new_perm, new_tag, cause)?; + let alloc_id = this.sb_reborrow(&place, size, new_perm, new_tag, info)?; // Adjust pointer. let new_place = place.map_provenance(|p| { @@ -886,12 +888,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); let new_perm = NewPermission::from_ref_ty(val.layout.ty, kind, this); - let retag_cause = match kind { + let cause = match kind { RetagKind::TwoPhase { .. } => RetagCause::TwoPhase, RetagKind::FnEntry => unreachable!(), RetagKind::Raw | RetagKind::Default => RetagCause::Normal, }; - this.sb_retag_reference(val, new_perm, retag_cause) + this.sb_retag_reference(val, new_perm, RetagInfo { cause, in_field: false }) } fn sb_retag_place_contents( @@ -906,7 +908,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { RetagKind::FnEntry => RetagCause::FnEntry, RetagKind::Default => RetagCause::Normal, }; - let mut visitor = RetagVisitor { ecx: this, kind, retag_cause, retag_fields }; + let mut visitor = + RetagVisitor { ecx: this, kind, retag_cause, retag_fields, in_field: false }; return visitor.visit_value(place); // The actual visitor. @@ -915,6 +918,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { kind: RetagKind, retag_cause: RetagCause, retag_fields: RetagFields, + in_field: bool, } impl<'ecx, 'mir, 'tcx> RetagVisitor<'ecx, 'mir, 'tcx> { #[inline(always)] // yes this helps in our benchmarks @@ -922,10 +926,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { &mut self, place: &PlaceTy<'tcx, Provenance>, new_perm: NewPermission, - retag_cause: RetagCause, ) -> InterpResult<'tcx> { let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?; - let val = self.ecx.sb_retag_reference(&val, new_perm, retag_cause)?; + let val = self.ecx.sb_retag_reference( + &val, + new_perm, + RetagInfo { cause: self.retag_cause, in_field: self.in_field }, + )?; self.ecx.write_immediate(*val, place)?; Ok(()) } @@ -943,7 +950,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { // Boxes get a weak protectors, since they may be deallocated. let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx); - self.retag_ptr_inplace(place, new_perm, self.retag_cause) + self.retag_ptr_inplace(place, new_perm) } fn visit_value(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { @@ -960,7 +967,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ty::Ref(..) => { let new_perm = NewPermission::from_ref_ty(place.layout.ty, self.kind, self.ecx); - self.retag_ptr_inplace(place, new_perm, self.retag_cause)?; + self.retag_ptr_inplace(place, new_perm)?; } ty::RawPtr(..) => { // We do *not* want to recurse into raw pointers -- wide raw pointers have @@ -984,7 +991,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } }; if recurse { + let in_field = mem::replace(&mut self.in_field, true); // remember and restore old value self.walk_value(place)?; + self.in_field = in_field; } } } @@ -1011,7 +1020,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { access: Some(AccessKind::Write), protector: Some(ProtectorKind::StrongProtector), }; - let _new_ptr = this.sb_retag_reference(&ptr, new_perm, RetagCause::InPlaceFnPassing)?; + let _new_ptr = this.sb_retag_reference( + &ptr, + new_perm, + RetagInfo { cause: RetagCause::InPlaceFnPassing, in_field: false }, + )?; // We just throw away `new_ptr`, so nobody can access this memory while it is protected. Ok(()) diff --git a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr index 0cb6111ecb9..b957464f95f 100644 --- a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr @@ -8,7 +8,7 @@ LL | | ) | | ^ | | | | |_____________trying to retag from for Unique permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x0..0x10] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x0..0x10] | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr index e35d7918c03..96121f0659f 100644 --- a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr @@ -5,7 +5,7 @@ LL | foo(some_xref); | ^^^^^^^^^ | | | trying to retag from for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x0..0x4] + | this error occurs as part of retag (of a reference/box inside this compound value) 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 diff --git a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr index 10c566d0612..0cfaf123554 100644 --- a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr @@ -5,7 +5,7 @@ LL | foo(pair_xref); | ^^^^^^^^^ | | | trying to retag from for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x0..0x4] + | this error occurs as part of retag (of a reference/box inside this compound value) 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 diff --git a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr index f14e8b8532f..d5b8433568f 100644 --- a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr @@ -5,7 +5,7 @@ LL | ret | ^^^ | | | trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr index 9ddaad4d1be..9ced0da96c4 100644 --- a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr @@ -5,7 +5,7 @@ LL | ret | ^^^ | | | trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] | = 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 diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr index ff00c54570c..89b6cee7d97 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr @@ -5,7 +5,7 @@ LL | ret | ^^^ | | | trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] | = 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 diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr index 61d041a8816..388b00c7146 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr @@ -5,7 +5,7 @@ LL | ret | ^^^ | | | trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] | = 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 From 3cdd2922cb828dee30bacdbed9f0a37602200e18 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jul 2023 16:28:29 +0200 Subject: [PATCH 12/15] ask people to reach out if we declare too much UB --- .../stacked_borrows/diagnostics.rs | 26 ++++++++++++++----- .../src/borrow_tracker/stacked_borrows/mod.rs | 11 +------- src/tools/miri/src/diagnostics.rs | 5 ++-- .../buggy_split_at_mut.stack.stderr | 5 ++-- .../pass_invalid_shr_option.stack.stderr | 1 + .../pass_invalid_shr_tuple.stack.stderr | 1 + .../return_invalid_shr_option.stack.stderr | 1 + .../return_invalid_shr_tuple.stack.stderr | 1 + .../return_invalid_mut_option.stderr | 1 + .../return_invalid_mut_tuple.stderr | 1 + 10 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs index 33b777abd9f..9b0f13dd62c 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -6,11 +6,19 @@ use rustc_span::{Span, SpanData}; use rustc_target::abi::Size; use crate::borrow_tracker::{ - stacked_borrows::{err_sb_ub, Permission}, - AccessKind, GlobalStateInner, ProtectorKind, + stacked_borrows::Permission, AccessKind, GlobalStateInner, ProtectorKind, }; use crate::*; +/// Error reporting +fn err_sb_ub<'tcx>( + msg: String, + help: Vec, + history: Option, +) -> InterpError<'tcx> { + err_machine_stop!(TerminationInfo::StackedBorrowsUb { msg, help, history }) +} + #[derive(Clone, Debug)] pub struct AllocHistory { id: AllocId, @@ -381,9 +389,13 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { self.history.id, self.offset.bytes(), ); + let mut helps = vec![operation_summary(&op.info.summary(), self.history.id, op.range)]; + if op.info.in_field { + helps.push(format!("errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling")); + } err_sb_ub( format!("{action}{}", error_cause(stack, op.orig_tag)), - Some(operation_summary(&op.info.summary(), self.history.id, op.range)), + helps, op.orig_tag.and_then(|orig_tag| self.get_logs_relevant_to(orig_tag, None)), ) } @@ -406,7 +418,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { ); err_sb_ub( format!("{action}{}", error_cause(stack, op.tag)), - Some(operation_summary("an access", self.history.id, op.range)), + vec![operation_summary("an access", self.history.id, op.range)], op.tag.and_then(|tag| self.get_logs_relevant_to(tag, None)), ) } @@ -432,7 +444,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { Operation::Dealloc(_) => err_sb_ub( format!("deallocating while item {item:?} is {protected} by call {call_id:?}",), - None, + vec![], None, ), Operation::Retag(RetagOp { orig_tag: tag, .. }) @@ -441,7 +453,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { format!( "not granting access to tag {tag:?} because that would remove {item:?} which is {protected} because it is an argument of call {call_id:?}", ), - None, + vec![], tag.and_then(|tag| self.get_logs_relevant_to(tag, Some(item.tag()))), ), } @@ -459,7 +471,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { alloc_id = self.history.id, cause = error_cause(stack, op.tag), ), - None, + vec![], op.tag.and_then(|tag| self.get_logs_relevant_to(tag, None)), ) } diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 5e1e0d75436..1aed436e88d 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -21,7 +21,7 @@ use rustc_middle::ty::{ use rustc_target::abi::{Abi, Size}; use crate::borrow_tracker::{ - stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder, TagHistory}, + stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder}, AccessKind, GlobalStateInner, ProtectorKind, RetagFields, }; use crate::*; @@ -170,15 +170,6 @@ impl NewPermission { } } -/// Error reporting -pub fn err_sb_ub<'tcx>( - msg: String, - help: Option, - history: Option, -) -> InterpError<'tcx> { - err_machine_stop!(TerminationInfo::StackedBorrowsUb { msg, help, history }) -} - // # Stacked Borrows Core Begin /// We need to make at least the following things true: diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 2a06bd871ef..8d9901807ec 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -22,7 +22,7 @@ pub enum TerminationInfo { UnsupportedInIsolation(String), StackedBorrowsUb { msg: String, - help: Option, + help: Vec, history: Option, }, TreeBorrowsUb { @@ -224,11 +224,10 @@ pub fn report_error<'tcx, 'mir>( (None, format!("or pass `-Zmiri-isolation-error=warn` to configure Miri to return an error code from isolated operations (if supported for that operation) and continue with a warning")), ], StackedBorrowsUb { help, history, .. } => { - let url = "https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md"; msg.extend(help.clone()); let mut helps = vec![ (None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental")), - (None, format!("see {url} for further information")), + (None, format!("see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information")), ]; if let Some(TagHistory {created, invalidated, protected}) = history.clone() { helps.push((Some(created.1), created.0)); diff --git a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr index b957464f95f..daa4339225d 100644 --- a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr @@ -7,8 +7,9 @@ LL | | from_raw_parts_mut(ptr.offset(mid as isize), len - mid), LL | | ) | | ^ | | | - | |_____________trying to retag from for Unique permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x0..0x10] + | | trying to retag from for Unique permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + | |_____________this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x0..0x10] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr index 96121f0659f..26d9f38f239 100644 --- a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr @@ -6,6 +6,7 @@ LL | foo(some_xref); | | | trying to retag from for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x0..0x4] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr index 0cfaf123554..5f0fbf12759 100644 --- a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr @@ -6,6 +6,7 @@ LL | foo(pair_xref); | | | trying to retag from for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x0..0x4] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr index d5b8433568f..7a9f061228a 100644 --- a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr @@ -6,6 +6,7 @@ LL | ret | | | trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr index 9ced0da96c4..6a98c9121ef 100644 --- a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr @@ -6,6 +6,7 @@ LL | ret | | | trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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 diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr index 89b6cee7d97..d357ab9639b 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr @@ -6,6 +6,7 @@ LL | ret | | | trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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 diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr index 388b00c7146..d346e6fa895 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr @@ -6,6 +6,7 @@ LL | ret | | | trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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 From 6e3932e10d02c68d2013e3f6c889a95ec588f28f Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Sat, 22 Jul 2023 06:25:15 +0000 Subject: [PATCH 13/15] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index da89b71ccd5..6f8ad244fa2 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -0646a5d1aa3745cb448db247f6fa432890a1812b +a5e2eca40ec17f17b6641bcc7c069380ac395acf From 32198e1b5c334ca3f70a1f14921e6df17628c9dd Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Sat, 22 Jul 2023 06:36:56 +0000 Subject: [PATCH 14/15] fmt --- src/tools/miri/src/shims/mod.rs | 2 +- src/tools/miri/src/shims/unix/fs.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/src/shims/mod.rs b/src/tools/miri/src/shims/mod.rs index 0caa9b522f9..63e55e7369a 100644 --- a/src/tools/miri/src/shims/mod.rs +++ b/src/tools/miri/src/shims/mod.rs @@ -20,8 +20,8 @@ pub mod tls; use log::trace; use rustc_middle::{mir, ty}; -use rustc_target::spec::abi::Abi; use rustc_target::abi::HasDataLayout as _; +use rustc_target::spec::abi::Abi; use crate::*; use helpers::check_arg_count; diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 5da66801694..b973a9e4766 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -12,7 +12,7 @@ use log::trace; use rustc_data_structures::fx::FxHashMap; use rustc_middle::ty::TyCtxt; -use rustc_target::abi::{Align, Size, HasDataLayout as _}; +use rustc_target::abi::{Align, HasDataLayout as _, Size}; use crate::shims::os_str::bytes_to_os_str; use crate::*; From 46ddf460afb6b277700f1bac9a3e26b34dc2ed62 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 23 Jul 2023 09:25:15 +0200 Subject: [PATCH 15/15] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 6f8ad244fa2..4958c377215 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -a5e2eca40ec17f17b6641bcc7c069380ac395acf +cec34a43b1b14f4e39363f3b283d7ac4f593ee81