From 9af03bf342708f591f5596916b08d5d5d784245e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 1 Apr 2022 14:10:24 -0400 Subject: [PATCH] add -Zmiri-strict-provenance --- README.md | 4 ++++ src/bin/miri.rs | 4 ++++ src/eval.rs | 7 ++++++- src/intptrcast.rs | 20 +++++++++++++------ src/machine.rs | 2 +- tests/compile-fail/box-cell-alias.rs | 2 +- .../compile-fail/stacked_borrows/zst_slice.rs | 2 +- .../compile-fail/strict-provenance-offset.rs | 9 +++++++++ tests/run-pass/btreemap.rs | 2 +- tests/run-pass/concurrency/channels.rs | 1 - tests/run-pass/concurrency/sync.rs | 2 +- tests/run-pass/concurrency/thread_locals.rs | 2 +- .../concurrency/tls_lib_drop_single_thread.rs | 1 - tests/run-pass/rc.rs | 2 +- tests/run-pass/slices.rs | 2 +- tests/run-pass/strings.rs | 2 +- tests/run-pass/vec.rs | 2 +- tests/run-pass/vecdeque.rs | 2 +- 18 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 tests/compile-fail/strict-provenance-offset.rs diff --git a/README.md b/README.md index 5d8b9034f3..2203a0643a 100644 --- a/README.md +++ b/README.md @@ -294,6 +294,10 @@ environment variable: entropy. The default seed is 0. **NOTE**: This entropy is not good enough for cryptographic use! Do not generate secret keys in Miri or perform other kinds of cryptographic operations that rely on proper random numbers. +* `-Zmiri-strict-provenance` enables [strict + provenance](https://github.com/rust-lang/rust/issues/95228) checking in Miri. This means that + casting an integer to a pointer yields a result with 'invalid' provenance, i.e., with provenance + that cannot be used for any memory access. Also implies `-Zmiri-tag-raw-pointers`. * `-Zmiri-symbolic-alignment-check` makes the alignment check more strict. By default, alignment is checked by casting the pointer to an integer, and making sure that is a multiple of the alignment. This can lead to cases where a diff --git a/src/bin/miri.rs b/src/bin/miri.rs index a1f7c617f0..5a9c96ef99 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -363,6 +363,10 @@ fn main() { "-Zmiri-tag-raw-pointers" => { miri_config.tag_raw = true; } + "-Zmiri-strict-provenance" => { + miri_config.strict_provenance = true; + miri_config.tag_raw = true; + } "-Zmiri-track-raw-pointers" => { eprintln!( "WARNING: -Zmiri-track-raw-pointers has been renamed to -Zmiri-tag-raw-pointers, the old name is deprecated." diff --git a/src/eval.rs b/src/eval.rs index 97856d9202..788e30c9e7 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -108,9 +108,13 @@ pub struct MiriConfig { /// If `Some`, enable the `measureme` profiler, writing results to a file /// with the specified prefix. pub measureme_out: Option, - /// Panic when unsupported functionality is encountered + /// Panic when unsupported functionality is encountered. pub panic_on_unsupported: bool, + /// Which style to use for printing backtraces. pub backtrace_style: BacktraceStyle, + /// Whether to enforce "strict provenance" rules. Enabling this means int2ptr casts return + /// pointers with an invalid provenance, i.e., not valid for any memory access. + pub strict_provenance: bool, } impl Default for MiriConfig { @@ -136,6 +140,7 @@ impl Default for MiriConfig { measureme_out: None, panic_on_unsupported: false, backtrace_style: BacktraceStyle::Short, + strict_provenance: false, } } } diff --git a/src/intptrcast.rs b/src/intptrcast.rs index 6f4169e950..9333536c9c 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -15,23 +15,27 @@ pub type MemoryExtra = RefCell; pub struct GlobalState { /// This is used as a map between the address of each allocation and its `AllocId`. /// It is always sorted - pub int_to_ptr_map: Vec<(u64, AllocId)>, + int_to_ptr_map: Vec<(u64, AllocId)>, /// The base address for each allocation. We cannot put that into /// `AllocExtra` because function pointers also have a base address, and /// they do not have an `AllocExtra`. /// This is the inverse of `int_to_ptr_map`. - pub base_addr: FxHashMap, + base_addr: FxHashMap, /// This is used as a memory address when a new pointer is casted to an integer. It /// is always larger than any address that was previously made part of a block. - pub next_base_addr: u64, + next_base_addr: u64, + /// Whether to enforce "strict provenance" rules. Enabling this means int2ptr casts return + /// pointers with an invalid provenance, i.e., not valid for any memory access. + strict_provenance: bool, } -impl Default for GlobalState { - fn default() -> Self { +impl GlobalState { + pub fn new(config: &MiriConfig) -> Self { GlobalState { int_to_ptr_map: Vec::default(), base_addr: FxHashMap::default(), next_base_addr: STACK_ADDR, + strict_provenance: config.strict_provenance, } } } @@ -43,8 +47,12 @@ impl<'mir, 'tcx> GlobalState { ) -> Pointer> { trace!("Casting 0x{:x} to a pointer", addr); let global_state = memory.extra.intptrcast.borrow(); - let pos = global_state.int_to_ptr_map.binary_search_by_key(&addr, |(addr, _)| *addr); + if global_state.strict_provenance { + return Pointer::new(None, Size::from_bytes(addr)); + } + + let pos = global_state.int_to_ptr_map.binary_search_by_key(&addr, |(addr, _)| *addr); let alloc_id = match pos { Ok(pos) => Some(global_state.int_to_ptr_map[pos].1), Err(0) => None, diff --git a/src/machine.rs b/src/machine.rs index 9c763149ff..e9ed507244 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -205,7 +205,7 @@ impl MemoryExtra { MemoryExtra { stacked_borrows, data_race, - intptrcast: Default::default(), + intptrcast: RefCell::new(intptrcast::GlobalState::new(config)), extern_statics: FxHashMap::default(), rng: RefCell::new(rng), tracked_alloc_id: config.tracked_alloc_id, diff --git a/tests/compile-fail/box-cell-alias.rs b/tests/compile-fail/box-cell-alias.rs index 5b9614f79f..49cce27507 100644 --- a/tests/compile-fail/box-cell-alias.rs +++ b/tests/compile-fail/box-cell-alias.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-tag-raw-pointers +// compile-flags: -Zmiri-strict-provenance // Taken from . diff --git a/tests/compile-fail/stacked_borrows/zst_slice.rs b/tests/compile-fail/stacked_borrows/zst_slice.rs index 065bf77d04..d45b3dcac0 100644 --- a/tests/compile-fail/stacked_borrows/zst_slice.rs +++ b/tests/compile-fail/stacked_borrows/zst_slice.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-tag-raw-pointers +// compile-flags: -Zmiri-strict-provenance // error-pattern: does not exist in the borrow stack fn main() { diff --git a/tests/compile-fail/strict-provenance-offset.rs b/tests/compile-fail/strict-provenance-offset.rs new file mode 100644 index 0000000000..6955d0243a --- /dev/null +++ b/tests/compile-fail/strict-provenance-offset.rs @@ -0,0 +1,9 @@ +// compile-flags: -Zmiri-strict-provenance +// error-pattern: not a valid pointer + +fn main() { + let x = 22; + let ptr = &x as *const _ as *const u8; + let roundtrip = ptr as usize as *const u8; + let _ = unsafe { roundtrip.offset(1) }; +} diff --git a/tests/run-pass/btreemap.rs b/tests/run-pass/btreemap.rs index 842ba0f4a8..4e11aa5917 100644 --- a/tests/run-pass/btreemap.rs +++ b/tests/run-pass/btreemap.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-tag-raw-pointers +// compile-flags: -Zmiri-strict-provenance -Zmiri-check-number-validity #![feature(btree_drain_filter)] use std::collections::{BTreeMap, BTreeSet}; use std::mem; diff --git a/tests/run-pass/concurrency/channels.rs b/tests/run-pass/concurrency/channels.rs index 58d073c85a..7d28cd726d 100644 --- a/tests/run-pass/concurrency/channels.rs +++ b/tests/run-pass/concurrency/channels.rs @@ -1,5 +1,4 @@ // ignore-windows: Concurrency on Windows is not supported yet. -// compile-flags: -Zmiri-disable-isolation use std::sync::mpsc::{channel, sync_channel}; use std::thread; diff --git a/tests/run-pass/concurrency/sync.rs b/tests/run-pass/concurrency/sync.rs index d47aa2a8d2..95ede8e6c0 100644 --- a/tests/run-pass/concurrency/sync.rs +++ b/tests/run-pass/concurrency/sync.rs @@ -1,5 +1,5 @@ // ignore-windows: Concurrency on Windows is not supported yet. -// compile-flags: -Zmiri-disable-isolation -Zmiri-check-number-validity +// compile-flags: -Zmiri-disable-isolation -Zmiri-strict-provenance -Zmiri-check-number-validity use std::sync::{Arc, Barrier, Condvar, Mutex, Once, RwLock}; use std::thread; diff --git a/tests/run-pass/concurrency/thread_locals.rs b/tests/run-pass/concurrency/thread_locals.rs index 0fd4a9f137..8b4f2a6f79 100644 --- a/tests/run-pass/concurrency/thread_locals.rs +++ b/tests/run-pass/concurrency/thread_locals.rs @@ -1,5 +1,5 @@ // ignore-windows: Concurrency on Windows is not supported yet. -// compile-flags: -Zmiri-check-number-validity +// compile-flags: -Zmiri-strict-provenance -Zmiri-check-number-validity //! The main purpose of this test is to check that if we take a pointer to //! thread's `t1` thread-local `A` and send it to another thread `t2`, diff --git a/tests/run-pass/concurrency/tls_lib_drop_single_thread.rs b/tests/run-pass/concurrency/tls_lib_drop_single_thread.rs index c8be1273bd..ef8b2c02ed 100644 --- a/tests/run-pass/concurrency/tls_lib_drop_single_thread.rs +++ b/tests/run-pass/concurrency/tls_lib_drop_single_thread.rs @@ -1,4 +1,3 @@ -// compile-flags: -Zmiri-tag-raw-pointers //! Check that destructors of the thread locals are executed on all OSes. use std::cell::RefCell; diff --git a/tests/run-pass/rc.rs b/tests/run-pass/rc.rs index e00d9df32e..fcc5156de8 100644 --- a/tests/run-pass/rc.rs +++ b/tests/run-pass/rc.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-tag-raw-pointers +// compile-flags: -Zmiri-strict-provenance -Zmiri-check-number-validity #![feature(new_uninit)] #![feature(get_mut_unchecked)] diff --git a/tests/run-pass/slices.rs b/tests/run-pass/slices.rs index 9d98c44741..1b3bef5e07 100644 --- a/tests/run-pass/slices.rs +++ b/tests/run-pass/slices.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-tag-raw-pointers +// compile-flags: -Zmiri-strict-provenance -Zmiri-check-number-validity #![feature(new_uninit)] #![feature(slice_as_chunks)] #![feature(slice_partition_dedup)] diff --git a/tests/run-pass/strings.rs b/tests/run-pass/strings.rs index 6998ec6e59..5c36168a6e 100644 --- a/tests/run-pass/strings.rs +++ b/tests/run-pass/strings.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-tag-raw-pointers +// compile-flags: -Zmiri-strict-provenance -Zmiri-check-number-validity fn empty() -> &'static str { "" diff --git a/tests/run-pass/vec.rs b/tests/run-pass/vec.rs index 102396f4b9..0d4c8016cd 100644 --- a/tests/run-pass/vec.rs +++ b/tests/run-pass/vec.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-tag-raw-pointers -Zmiri-check-number-validity +// compile-flags: -Zmiri-strict-provenance -Zmiri-check-number-validity // Gather all references from a mutable iterator and make sure Miri notices if // using them is dangerous. fn test_all_refs<'a, T: 'a>(dummy: &mut T, iter: impl Iterator) { diff --git a/tests/run-pass/vecdeque.rs b/tests/run-pass/vecdeque.rs index f45c21d207..8e8b395cbd 100644 --- a/tests/run-pass/vecdeque.rs +++ b/tests/run-pass/vecdeque.rs @@ -1,4 +1,4 @@ -// compile-flags: -Zmiri-tag-raw-pointers +// compile-flags: -Zmiri-strict-provenance -Zmiri-check-number-validity use std::collections::VecDeque; fn test_all_refs<'a, T: 'a>(dummy: &mut T, iter: impl Iterator) {