From ede470e1fcb8b6e488f39c48a84cde0f639adcc9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Mar 2022 13:10:20 -0400 Subject: [PATCH 1/4] ensure that -Zmiri-check-number-validity detects integers with provenance --- tests/compile-fail/ptr_integer_array_transmute.rs | 6 ++++++ tests/compile-fail/ptr_integer_transmute.rs | 6 ++++++ 2 files changed, 12 insertions(+) create mode 100644 tests/compile-fail/ptr_integer_array_transmute.rs create mode 100644 tests/compile-fail/ptr_integer_transmute.rs diff --git a/tests/compile-fail/ptr_integer_array_transmute.rs b/tests/compile-fail/ptr_integer_array_transmute.rs new file mode 100644 index 00000000000..7a1ae2f3c9a --- /dev/null +++ b/tests/compile-fail/ptr_integer_array_transmute.rs @@ -0,0 +1,6 @@ +// compile-flags: -Zmiri-check-number-validity + +fn main() { + let r = &mut 42; + let _i: [usize; 1] = unsafe { std::mem::transmute(r) }; //~ ERROR encountered a pointer, but expected plain (non-pointer) bytes +} diff --git a/tests/compile-fail/ptr_integer_transmute.rs b/tests/compile-fail/ptr_integer_transmute.rs new file mode 100644 index 00000000000..e15a1576375 --- /dev/null +++ b/tests/compile-fail/ptr_integer_transmute.rs @@ -0,0 +1,6 @@ +// compile-flags: -Zmiri-check-number-validity + +fn main() { + let r = &mut 42; + let _i: usize = unsafe { std::mem::transmute(r) }; //~ ERROR expected initialized plain (non-pointer) bytes +} From 552b77e3b9841f697d954f2ab341711dbff8d029 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Mar 2022 14:29:58 -0400 Subject: [PATCH 2/4] fix types in env shim to avoid ptr-int transmutes --- src/machine.rs | 23 ++++++++++++++--------- src/shims/backtrace.rs | 5 ++--- src/shims/env.rs | 7 ++++--- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/machine.rs b/src/machine.rs index 2cf7cd0fae0..9c763149ffa 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -10,13 +10,14 @@ use rand::rngs::StdRng; use rand::SeedableRng; +use rustc_ast::ast::Mutability; use rustc_data_structures::fx::FxHashMap; use rustc_middle::{ mir, ty::{ self, layout::{LayoutCx, LayoutError, LayoutOf, TyAndLayout}, - Instance, TyCtxt, + Instance, TyCtxt, TypeAndMut, }, }; use rustc_span::def_id::{CrateNum, DefId}; @@ -269,19 +270,23 @@ pub struct PrimitiveLayouts<'tcx> { pub u32: TyAndLayout<'tcx>, pub usize: TyAndLayout<'tcx>, pub bool: TyAndLayout<'tcx>, + pub mut_raw_ptr: TyAndLayout<'tcx>, } impl<'mir, 'tcx: 'mir> PrimitiveLayouts<'tcx> { fn new(layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) -> Result> { + let tcx = layout_cx.tcx; + let mut_raw_ptr = tcx.mk_ptr(TypeAndMut { ty: tcx.types.unit, mutbl: Mutability::Mut }); Ok(Self { - unit: layout_cx.layout_of(layout_cx.tcx.mk_unit())?, - i8: layout_cx.layout_of(layout_cx.tcx.types.i8)?, - i32: layout_cx.layout_of(layout_cx.tcx.types.i32)?, - isize: layout_cx.layout_of(layout_cx.tcx.types.isize)?, - u8: layout_cx.layout_of(layout_cx.tcx.types.u8)?, - u32: layout_cx.layout_of(layout_cx.tcx.types.u32)?, - usize: layout_cx.layout_of(layout_cx.tcx.types.usize)?, - bool: layout_cx.layout_of(layout_cx.tcx.types.bool)?, + unit: layout_cx.layout_of(tcx.mk_unit())?, + i8: layout_cx.layout_of(tcx.types.i8)?, + i32: layout_cx.layout_of(tcx.types.i32)?, + isize: layout_cx.layout_of(tcx.types.isize)?, + u8: layout_cx.layout_of(tcx.types.u8)?, + u32: layout_cx.layout_of(tcx.types.u32)?, + usize: layout_cx.layout_of(tcx.types.usize)?, + bool: layout_cx.layout_of(tcx.types.bool)?, + mut_raw_ptr: layout_cx.layout_of(mut_raw_ptr)?, }) } } diff --git a/src/shims/backtrace.rs b/src/shims/backtrace.rs index 541cec8e847..6a8a9553e93 100644 --- a/src/shims/backtrace.rs +++ b/src/shims/backtrace.rs @@ -1,7 +1,7 @@ use crate::*; use rustc_ast::ast::Mutability; use rustc_middle::ty::layout::LayoutOf as _; -use rustc_middle::ty::{self, Instance, TypeAndMut}; +use rustc_middle::ty::{self, Instance}; use rustc_span::{BytePos, Loc, Symbol}; use rustc_target::{abi::Size, spec::abi::Abi}; use std::convert::TryInto as _; @@ -71,8 +71,7 @@ fn handle_miri_get_backtrace( let len: u64 = ptrs.len().try_into().unwrap(); - let ptr_ty = tcx.mk_ptr(TypeAndMut { ty: tcx.types.unit, mutbl: Mutability::Mut }); - + let ptr_ty = this.machine.layouts.mut_raw_ptr.ty; let array_layout = this.layout_of(tcx.mk_array(ptr_ty, len)).unwrap(); match flags { diff --git a/src/shims/env.rs b/src/shims/env.rs index 1916d7d70a4..c2050647abc 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -440,7 +440,7 @@ fn update_environ(&mut self) -> InterpResult<'tcx> { } else { // No `environ` allocated yet, let's do that. // This is memory backing an extern static, hence `ExternStatic`, not `Env`. - let layout = this.machine.layouts.usize; + let layout = this.machine.layouts.mut_raw_ptr; let place = this.allocate(layout, MiriMemoryKind::ExternStatic.into())?; this.machine.env_vars.environ = Some(place); } @@ -452,8 +452,9 @@ fn update_environ(&mut self) -> InterpResult<'tcx> { vars.push(Pointer::null()); // Make an array with all these pointers inside Miri. let tcx = this.tcx; - let vars_layout = - this.layout_of(tcx.mk_array(tcx.types.usize, u64::try_from(vars.len()).unwrap()))?; + let vars_layout = this.layout_of( + tcx.mk_array(this.machine.layouts.mut_raw_ptr.ty, u64::try_from(vars.len()).unwrap()), + )?; let vars_place = this.allocate(vars_layout, MiriMemoryKind::Runtime.into())?; for (idx, var) in vars.into_iter().enumerate() { let place = this.mplace_field(&vars_place, idx)?; From 5d7c495de5d4ed870fe9c4fc2337c5ce0c96dec9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Mar 2022 14:33:17 -0400 Subject: [PATCH 3/4] channels do ptr-int transmutes so move them to non-check-number-validity test --- tests/run-pass/concurrency/channels.rs | 57 +++++++++++++++++++++ tests/run-pass/concurrency/channels.stderr | 2 + tests/run-pass/concurrency/simple.rs | 1 + tests/run-pass/concurrency/simple.stderr | 4 +- tests/run-pass/concurrency/sync.rs | 50 ------------------ tests/run-pass/concurrency/thread_locals.rs | 1 + 6 files changed, 63 insertions(+), 52 deletions(-) create mode 100644 tests/run-pass/concurrency/channels.rs create mode 100644 tests/run-pass/concurrency/channels.stderr diff --git a/tests/run-pass/concurrency/channels.rs b/tests/run-pass/concurrency/channels.rs new file mode 100644 index 00000000000..58d073c85ac --- /dev/null +++ b/tests/run-pass/concurrency/channels.rs @@ -0,0 +1,57 @@ +// ignore-windows: Concurrency on Windows is not supported yet. +// compile-flags: -Zmiri-disable-isolation + +use std::sync::mpsc::{channel, sync_channel}; +use std::thread; + +// Check if channels are working. + +/// The test taken from the Rust documentation. +fn simple_send() { + let (tx, rx) = channel(); + thread::spawn(move || { + tx.send(10).unwrap(); + }); + assert_eq!(rx.recv().unwrap(), 10); +} + +/// The test taken from the Rust documentation. +fn multiple_send() { + let (tx, rx) = channel(); + for i in 0..10 { + let tx = tx.clone(); + thread::spawn(move || { + tx.send(i).unwrap(); + }); + } + + let mut sum = 0; + for _ in 0..10 { + let j = rx.recv().unwrap(); + assert!(0 <= j && j < 10); + sum += j; + } + assert_eq!(sum, 45); +} + +/// The test taken from the Rust documentation. +fn send_on_sync() { + let (sender, receiver) = sync_channel(1); + + // this returns immediately + sender.send(1).unwrap(); + + thread::spawn(move || { + // this will block until the previous message has been received + sender.send(2).unwrap(); + }); + + assert_eq!(receiver.recv().unwrap(), 1); + assert_eq!(receiver.recv().unwrap(), 2); +} + +fn main() { + simple_send(); + multiple_send(); + send_on_sync(); +} diff --git a/tests/run-pass/concurrency/channels.stderr b/tests/run-pass/concurrency/channels.stderr new file mode 100644 index 00000000000..03676519d4f --- /dev/null +++ b/tests/run-pass/concurrency/channels.stderr @@ -0,0 +1,2 @@ +warning: thread support is experimental and incomplete: weak memory effects are not emulated. + diff --git a/tests/run-pass/concurrency/simple.rs b/tests/run-pass/concurrency/simple.rs index c22506821f5..c659cfbc3fd 100644 --- a/tests/run-pass/concurrency/simple.rs +++ b/tests/run-pass/concurrency/simple.rs @@ -1,4 +1,5 @@ // ignore-windows: Concurrency on Windows is not supported yet. +// compile-flags: -Zmiri-check-number-validity use std::thread; diff --git a/tests/run-pass/concurrency/simple.stderr b/tests/run-pass/concurrency/simple.stderr index f46b1442d74..35f5f10274c 100644 --- a/tests/run-pass/concurrency/simple.stderr +++ b/tests/run-pass/concurrency/simple.stderr @@ -1,5 +1,5 @@ warning: thread support is experimental and incomplete: weak memory effects are not emulated. -thread '' panicked at 'Hello!', $DIR/simple.rs:54:9 +thread '' panicked at 'Hello!', $DIR/simple.rs:55:9 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace -thread 'childthread' panicked at 'Hello, world!', $DIR/simple.rs:64:9 +thread 'childthread' panicked at 'Hello, world!', $DIR/simple.rs:65:9 diff --git a/tests/run-pass/concurrency/sync.rs b/tests/run-pass/concurrency/sync.rs index da6f6f25ec1..d47aa2a8d23 100644 --- a/tests/run-pass/concurrency/sync.rs +++ b/tests/run-pass/concurrency/sync.rs @@ -1,7 +1,6 @@ // ignore-windows: Concurrency on Windows is not supported yet. // compile-flags: -Zmiri-disable-isolation -Zmiri-check-number-validity -use std::sync::mpsc::{channel, sync_channel}; use std::sync::{Arc, Barrier, Condvar, Mutex, Once, RwLock}; use std::thread; use std::time::{Duration, Instant}; @@ -181,52 +180,6 @@ fn check_rwlock_read_no_deadlock() { handle.join().unwrap(); } -// Check if channels are working. - -/// The test taken from the Rust documentation. -fn simple_send() { - let (tx, rx) = channel(); - thread::spawn(move || { - tx.send(10).unwrap(); - }); - assert_eq!(rx.recv().unwrap(), 10); -} - -/// The test taken from the Rust documentation. -fn multiple_send() { - let (tx, rx) = channel(); - for i in 0..10 { - let tx = tx.clone(); - thread::spawn(move || { - tx.send(i).unwrap(); - }); - } - - let mut sum = 0; - for _ in 0..10 { - let j = rx.recv().unwrap(); - assert!(0 <= j && j < 10); - sum += j; - } - assert_eq!(sum, 45); -} - -/// The test taken from the Rust documentation. -fn send_on_sync() { - let (sender, receiver) = sync_channel(1); - - // this returns immediately - sender.send(1).unwrap(); - - thread::spawn(move || { - // this will block until the previous message has been received - sender.send(2).unwrap(); - }); - - assert_eq!(receiver.recv().unwrap(), 1); - assert_eq!(receiver.recv().unwrap(), 2); -} - // Check if Rust once statics are working. static mut VAL: usize = 0; @@ -353,9 +306,6 @@ fn main() { check_mutex(); check_rwlock_write(); check_rwlock_read_no_deadlock(); - simple_send(); - multiple_send(); - send_on_sync(); check_once(); check_rwlock_unlock_bug1(); check_rwlock_unlock_bug2(); diff --git a/tests/run-pass/concurrency/thread_locals.rs b/tests/run-pass/concurrency/thread_locals.rs index 384c2ac9155..0fd4a9f1372 100644 --- a/tests/run-pass/concurrency/thread_locals.rs +++ b/tests/run-pass/concurrency/thread_locals.rs @@ -1,4 +1,5 @@ // ignore-windows: Concurrency on Windows is not supported yet. +// compile-flags: -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`, From f3c35d51050a1d89da252734ff5fc8b9ba0b9937 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 27 Mar 2022 09:40:46 -0400 Subject: [PATCH 4/4] rustup --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 7c9e50da5ce..2497974aa7e 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -d2df372bca13bb60979c909660e69f2451630e81 +100f12d17026fccfc5d80527b5976dd66b228b13