From 67e780e29e8ebc6057c8fa24e364f5641c423808 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Jul 2023 16:45:18 +0200 Subject: [PATCH] make full field retagging the default --- README.md | 14 +++---- src/eval.rs | 2 +- tests/fail/generator-pinned-moved.rs | 5 ++- tests/pass/generator.rs | 38 +++++++++++++------ .../stacked-borrows/interior_mutability.rs | 1 - tests/pass/stacked-borrows/stacked-borrows.rs | 1 - .../zst-field-retagging-terminates.rs | 1 - 7 files changed, 35 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 601101905f..d2f81f911f 100644 --- a/README.md +++ b/README.md @@ -400,15 +400,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/eval.rs b/src/eval.rs index ed3d741db1..79a7115f24 100644 --- a/src/eval.rs +++ b/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/tests/fail/generator-pinned-moved.rs b/tests/fail/generator-pinned-moved.rs index 240ae18cc4..29dc6e56f7 100644 --- a/tests/fail/generator-pinned-moved.rs +++ b/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/tests/pass/generator.rs b/tests/pass/generator.rs index e059c7114e..2009960345 100644 --- a/tests/pass/generator.rs +++ b/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, true, static || { + let mut x = 5; + let y = &mut x; + *y = 5; + yield *y; + *y = 10; + x + }), + 10 + ); assert_eq!( - finish(5, || { + 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/tests/pass/stacked-borrows/interior_mutability.rs b/tests/pass/stacked-borrows/interior_mutability.rs index c6373a7eaf..830e9c3384 100644 --- a/tests/pass/stacked-borrows/interior_mutability.rs +++ b/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/tests/pass/stacked-borrows/stacked-borrows.rs b/tests/pass/stacked-borrows/stacked-borrows.rs index d7d7d1f97d..43ae7d6f52 100644 --- a/tests/pass/stacked-borrows/stacked-borrows.rs +++ b/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/tests/pass/stacked-borrows/zst-field-retagging-terminates.rs b/tests/pass/stacked-borrows/zst-field-retagging-terminates.rs index 6e13a9ea83..4faf6004ad 100644 --- a/tests/pass/stacked-borrows/zst-field-retagging-terminates.rs +++ b/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)]