From cb985670c1997534a0cd1cfee606e81e4fa136dc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 16 Aug 2020 17:08:34 +0200 Subject: [PATCH 1/8] make alignment check integer-based by default, and add an option to make it symbolic --- src/bin/miri.rs | 54 ++++++------------- src/eval.rs | 16 ++++-- src/lib.rs | 2 +- src/machine.rs | 11 ++-- .../unaligned_pointers/atomic_unaligned.rs | 1 + .../unaligned_pointers/dyn_alignment.rs | 4 +- .../intptrcast_alignment_check.rs | 10 ++-- .../unaligned_pointers/reference_to_packed.rs | 2 +- .../unaligned_pointers/unaligned_ptr_zst.rs | 4 +- tests/run-pass/align.rs | 11 ++++ 10 files changed, 62 insertions(+), 53 deletions(-) create mode 100644 tests/run-pass/align.rs diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 73e52af4ec..1347020475 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -172,48 +172,41 @@ fn main() { init_early_loggers(); // Parse our arguments and split them across `rustc` and `miri`. - let mut validate = true; - let mut stacked_borrows = true; - let mut check_alignment = true; - let mut communicate = false; - let mut ignore_leaks = false; - let mut seed: Option = None; - let mut tracked_pointer_tag: Option = None; - let mut tracked_call_id: Option = None; - let mut tracked_alloc_id: Option = None; + let mut miri_config = miri::MiriConfig::default(); let mut rustc_args = vec![]; - let mut crate_args = vec![]; let mut after_dashdash = false; - let mut excluded_env_vars = vec![]; for arg in env::args() { if rustc_args.is_empty() { // Very first arg: binary name. rustc_args.push(arg); } else if after_dashdash { // Everything that comes after `--` is forwarded to the interpreted crate. - crate_args.push(arg); + miri_config.args.push(arg); } else { match arg.as_str() { "-Zmiri-disable-validation" => { - validate = false; + miri_config.validate = false; } "-Zmiri-disable-stacked-borrows" => { - stacked_borrows = false; + miri_config.stacked_borrows = false; } "-Zmiri-disable-alignment-check" => { - check_alignment = false; + miri_config.check_alignment = miri::AlignmentCheck::None; + } + "-Zmiri-symbolic-alignment-check" => { + miri_config.check_alignment = miri::AlignmentCheck::Symbolic; } "-Zmiri-disable-isolation" => { - communicate = true; + miri_config.communicate = true; } "-Zmiri-ignore-leaks" => { - ignore_leaks = true; + miri_config.ignore_leaks = true; } "--" => { after_dashdash = true; } arg if arg.starts_with("-Zmiri-seed=") => { - if seed.is_some() { + if miri_config.seed.is_some() { panic!("Cannot specify -Zmiri-seed multiple times!"); } let seed_raw = hex::decode(arg.strip_prefix("-Zmiri-seed=").unwrap()) @@ -234,10 +227,10 @@ fn main() { let mut bytes = [0; 8]; bytes[..seed_raw.len()].copy_from_slice(&seed_raw); - seed = Some(u64::from_be_bytes(bytes)); + miri_config.seed = Some(u64::from_be_bytes(bytes)); } arg if arg.starts_with("-Zmiri-env-exclude=") => { - excluded_env_vars + miri_config.excluded_env_vars .push(arg.strip_prefix("-Zmiri-env-exclude=").unwrap().to_owned()); } arg if arg.starts_with("-Zmiri-track-pointer-tag=") => { @@ -249,7 +242,7 @@ fn main() { ), }; if let Some(id) = miri::PtrId::new(id) { - tracked_pointer_tag = Some(id); + miri_config.tracked_pointer_tag = Some(id); } else { panic!("-Zmiri-track-pointer-tag requires a nonzero argument"); } @@ -263,7 +256,7 @@ fn main() { ), }; if let Some(id) = miri::CallId::new(id) { - tracked_call_id = Some(id); + miri_config.tracked_call_id = Some(id); } else { panic!("-Zmiri-track-call-id requires a nonzero argument"); } @@ -276,7 +269,7 @@ fn main() { err ), }; - tracked_alloc_id = Some(miri::AllocId(id)); + miri_config.tracked_alloc_id = Some(miri::AllocId(id)); } _ => { // Forward to rustc. @@ -287,19 +280,6 @@ fn main() { } debug!("rustc arguments: {:?}", rustc_args); - debug!("crate arguments: {:?}", crate_args); - let miri_config = miri::MiriConfig { - validate, - stacked_borrows, - check_alignment, - communicate, - ignore_leaks, - excluded_env_vars, - seed, - args: crate_args, - tracked_pointer_tag, - tracked_call_id, - tracked_alloc_id, - }; + debug!("crate arguments: {:?}", miri_config.args); run_compiler(rustc_args, &mut MiriCompilerCalls { miri_config }) } diff --git a/src/eval.rs b/src/eval.rs index cc5a6eb21f..8e4604c336 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -13,6 +13,16 @@ use rustc_target::abi::LayoutOf; use crate::*; +#[derive(Copy, Clone, Debug, PartialEq)] +pub enum AlignmentCheck { + /// Do not check alignment. + None, + /// Check alignment "symbolically", i.e., using only the requested alignment for an allocation and not its real base address. + Symbolic, + /// Check alignment on the actual physical integer address. + Int, +} + /// Configuration needed to spawn a Miri instance. #[derive(Clone)] pub struct MiriConfig { @@ -20,8 +30,8 @@ pub struct MiriConfig { pub validate: bool, /// Determines if Stacked Borrows is enabled. pub stacked_borrows: bool, - /// Determines if alignment checking is enabled. - pub check_alignment: bool, + /// Controls alignment checking. + pub check_alignment: AlignmentCheck, /// Determines if communication with the host environment is enabled. pub communicate: bool, /// Determines if memory leaks should be ignored. @@ -45,7 +55,7 @@ impl Default for MiriConfig { MiriConfig { validate: true, stacked_borrows: true, - check_alignment: true, + check_alignment: AlignmentCheck::Int, communicate: false, ignore_leaks: false, excluded_env_vars: vec![], diff --git a/src/lib.rs b/src/lib.rs index 816917081a..1b66d5ff6f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -55,7 +55,7 @@ pub use crate::diagnostics::{ register_diagnostic, report_error, EvalContextExt as DiagnosticsEvalContextExt, TerminationInfo, NonHaltingDiagnostic, }; -pub use crate::eval::{create_ecx, eval_main, MiriConfig}; +pub use crate::eval::{create_ecx, eval_main, AlignmentCheck, MiriConfig}; pub use crate::helpers::EvalContextExt as HelpersEvalContextExt; pub use crate::machine::{ AllocExtra, Evaluator, FrameData, MemoryExtra, MiriEvalContext, MiriEvalContextExt, diff --git a/src/machine.rs b/src/machine.rs index 04bba6c33c..f2abb2ae0e 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -128,7 +128,7 @@ pub struct MemoryExtra { tracked_alloc_id: Option, /// Controls whether alignment of memory accesses is being checked. - check_alignment: bool, + check_alignment: AlignmentCheck, } impl MemoryExtra { @@ -138,7 +138,7 @@ impl MemoryExtra { tracked_pointer_tag: Option, tracked_call_id: Option, tracked_alloc_id: Option, - check_alignment: bool, + check_alignment: AlignmentCheck, ) -> Self { let stacked_borrows = if stacked_borrows { Some(Rc::new(RefCell::new(stacked_borrows::GlobalState::new(tracked_pointer_tag, tracked_call_id)))) @@ -336,7 +336,12 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> { #[inline(always)] fn enforce_alignment(memory_extra: &MemoryExtra) -> bool { - memory_extra.check_alignment + memory_extra.check_alignment != AlignmentCheck::None + } + + #[inline(always)] + fn force_int_for_alignment_check(memory_extra: &Self::MemoryExtra) -> bool { + memory_extra.check_alignment == AlignmentCheck::Int } #[inline(always)] diff --git a/tests/compile-fail/unaligned_pointers/atomic_unaligned.rs b/tests/compile-fail/unaligned_pointers/atomic_unaligned.rs index bdec0ff504..77eff5087d 100644 --- a/tests/compile-fail/unaligned_pointers/atomic_unaligned.rs +++ b/tests/compile-fail/unaligned_pointers/atomic_unaligned.rs @@ -1,3 +1,4 @@ +// compile-flags: -Zmiri-symbolic-alignment-check #![feature(core_intrinsics)] fn main() { diff --git a/tests/compile-fail/unaligned_pointers/dyn_alignment.rs b/tests/compile-fail/unaligned_pointers/dyn_alignment.rs index aa293a5d21..a40db99a72 100644 --- a/tests/compile-fail/unaligned_pointers/dyn_alignment.rs +++ b/tests/compile-fail/unaligned_pointers/dyn_alignment.rs @@ -1,4 +1,4 @@ -// should find the bug even without these, but gets masked by optimizations +// should find the bug even without validation and stacked borrows, but gets masked by optimizations // compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Zmir-opt-level=0 #[repr(align(256))] @@ -15,5 +15,5 @@ fn main() { // Overwrite the data part of `ptr` so it points to `buf`. unsafe { (&mut ptr as *mut _ as *mut *const u8).write(&buf as *const _ as *const u8); } // Re-borrow that. This should be UB. - let _ptr = &*ptr; //~ ERROR accessing memory with alignment 4, but alignment 256 is required + let _ptr = &*ptr; //~ ERROR alignment 256 is required } diff --git a/tests/compile-fail/unaligned_pointers/intptrcast_alignment_check.rs b/tests/compile-fail/unaligned_pointers/intptrcast_alignment_check.rs index 0a3b48dab5..3865d45786 100644 --- a/tests/compile-fail/unaligned_pointers/intptrcast_alignment_check.rs +++ b/tests/compile-fail/unaligned_pointers/intptrcast_alignment_check.rs @@ -1,7 +1,9 @@ -// Even with intptrcast and without validation, we want to be *sure* to catch bugs -// that arise from pointers being insufficiently aligned. The only way to achieve -// that is not not let programs exploit integer information for alignment, so here -// we test that this is indeed the case. +// compile-flags: -Zmiri-symbolic-alignment-check +// With the symbolic alignment check, even with intptrcast and without +// validation, we want to be *sure* to catch bugs that arise from pointers being +// insufficiently aligned. The only way to achieve that is not not let programs +// exploit integer information for alignment, so here we test that this is +// indeed the case. // // See https://github.com/rust-lang/miri/issues/1074. fn main() { diff --git a/tests/compile-fail/unaligned_pointers/reference_to_packed.rs b/tests/compile-fail/unaligned_pointers/reference_to_packed.rs index 08104e917d..a1240c9018 100644 --- a/tests/compile-fail/unaligned_pointers/reference_to_packed.rs +++ b/tests/compile-fail/unaligned_pointers/reference_to_packed.rs @@ -15,5 +15,5 @@ fn main() { y: 99, }; let p = unsafe { &foo.x }; - let i = *p; //~ ERROR memory with alignment 1, but alignment 4 is required + let i = *p; //~ ERROR alignment 4 is required } diff --git a/tests/compile-fail/unaligned_pointers/unaligned_ptr_zst.rs b/tests/compile-fail/unaligned_pointers/unaligned_ptr_zst.rs index 31f88c8381..beba47359b 100644 --- a/tests/compile-fail/unaligned_pointers/unaligned_ptr_zst.rs +++ b/tests/compile-fail/unaligned_pointers/unaligned_ptr_zst.rs @@ -2,9 +2,9 @@ // compile-flags: -Zmiri-disable-validation fn main() { - let x = &2u16; + let x = &2u8; let x = x as *const _ as *const [u32; 0]; // This must fail because alignment is violated. Test specifically for loading ZST. let _x = unsafe { *x }; - //~^ ERROR memory with alignment 2, but alignment 4 is required + //~^ ERROR alignment 4 is required } diff --git a/tests/run-pass/align.rs b/tests/run-pass/align.rs new file mode 100644 index 0000000000..b3d268f45e --- /dev/null +++ b/tests/run-pass/align.rs @@ -0,0 +1,11 @@ +// This manually makes sure that we have a pointer with the proper alignment. +// Do this a couple times in a loop because it may work "by chance". +fn main() { + for _ in 0..10 { + let x = &mut [0u8; 3]; + let base_addr = x as *mut _ as usize; + let base_addr_aligned = if base_addr % 2 == 0 { base_addr } else { base_addr+1 }; + let u16_ptr = base_addr_aligned as *mut u16; + unsafe { *u16_ptr = 2; } + } +} From 5a579f281d913d7d838413564272fafc23c6c336 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 16 Aug 2020 17:12:32 +0200 Subject: [PATCH 2/8] document -Zmiri-symbolic-alignment-check --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index f97639ea5b..a189fd233f 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,17 @@ Miri adds its own set of `-Z` flags: 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-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 + program passes the alignment check by pure chance, because things "happened to + be" sufficiently aligned. To avoid such cases, the symbolic alignment check + only takes into account the requested alignment of the relevant allocation, + and the offset into that allocation. This avoids such false negatives, but it + also incurs some false positives when the code does manual integer arithmetic + to ensure alignment. (The standard library `align_to` method works fine in + both modes; under symbolic alignment it only fills the middle slice when the + allocation guarantees sufficient alignment.) * `-Zmiri-track-alloc-id=` shows a backtrace when the given allocation is being allocated or freed. This helps in debugging memory leaks and use after free bugs. From 664706662ff1221e88e70dd5c6ceef391e2528a6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 16 Aug 2020 17:16:53 +0200 Subject: [PATCH 3/8] adjust diagnostics to alignment check mode --- src/diagnostics.rs | 7 ++++--- src/machine.rs | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/diagnostics.rs b/src/diagnostics.rs index eed60c2696..81cd049217 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -100,11 +100,12 @@ pub fn report_error<'tcx, 'mir>( panic!("Error should never be raised by Miri: {:?}", e.kind), Unsupported(_) => vec![format!("this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support")], - UndefinedBehavior(UndefinedBehaviorInfo::AlignmentCheckFailed { .. }) => + UndefinedBehavior(UndefinedBehaviorInfo::AlignmentCheckFailed { .. }) + if ecx.memory.extra.check_alignment == AlignmentCheck::Symbolic + => vec![ format!("this usually indicates that your program performed an invalid operation and caused Undefined Behavior"), - format!("but alignment errors can also be false positives, see https://github.com/rust-lang/miri/issues/1074"), - format!("you can disable the alignment check with `-Zmiri-disable-alignment-check`, but that could hide true bugs") + format!("but due to `-Zmiri-symbolic-alignment-check`, alignment errors can also be false positives"), ], UndefinedBehavior(_) => vec![ diff --git a/src/machine.rs b/src/machine.rs index f2abb2ae0e..b76159694d 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -128,7 +128,7 @@ pub struct MemoryExtra { tracked_alloc_id: Option, /// Controls whether alignment of memory accesses is being checked. - check_alignment: AlignmentCheck, + pub(crate) check_alignment: AlignmentCheck, } impl MemoryExtra { From d4e5943259de7573556711ed6496c409dd754ea3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 16 Aug 2020 18:16:34 +0200 Subject: [PATCH 4/8] use real align_offset unless we symbolic alignment check is enabled --- src/shims/mod.rs | 35 ++++++++++--------- tests/run-pass/align.rs | 28 +++++++++++---- ...ign_offset.rs => align_offset_symbolic.rs} | 2 ++ ...et.stdout => align_offset_symbolic.stdout} | 0 4 files changed, 41 insertions(+), 24 deletions(-) rename tests/run-pass/{align_offset.rs => align_offset_symbolic.rs} (98%) rename tests/run-pass/{align_offset.stdout => align_offset_symbolic.stdout} (100%) diff --git a/src/shims/mod.rs b/src/shims/mod.rs index 05dd4059eb..eab27496cb 100644 --- a/src/shims/mod.rs +++ b/src/shims/mod.rs @@ -13,8 +13,6 @@ pub mod tls; // End module management, begin local code -use std::convert::TryFrom; - use log::trace; use rustc_middle::{mir, ty}; @@ -37,8 +35,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // There are some more lang items we want to hook that CTFE does not hook (yet). if this.tcx.lang_items().align_offset_fn() == Some(instance.def.def_id()) { let &[ptr, align] = check_arg_count(args)?; - this.align_offset(ptr, align, ret, unwind)?; - return Ok(None); + if this.align_offset(ptr, align, ret, unwind)? { + return Ok(None); + } } // Try to see if we can do something about foreign items. @@ -56,46 +55,48 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(Some(&*this.load_mir(instance.def, None)?)) } + /// Returns `true` if the computation was performed, and `false` if we should just evaluate + /// the actual MIR of `align_offset`. fn align_offset( &mut self, ptr_op: OpTy<'tcx, Tag>, align_op: OpTy<'tcx, Tag>, ret: Option<(PlaceTy<'tcx, Tag>, mir::BasicBlock)>, unwind: Option, - ) -> InterpResult<'tcx> { + ) -> InterpResult<'tcx, bool> { let this = self.eval_context_mut(); let (dest, ret) = ret.unwrap(); + if this.memory.extra.check_alignment != AlignmentCheck::Symbolic { + // Just use actual implementation. + return Ok(false); + } + let req_align = this .force_bits(this.read_scalar(align_op)?.check_init()?, this.pointer_size())?; // Stop if the alignment is not a power of two. if !req_align.is_power_of_two() { - return this.start_panic("align_offset: align is not a power-of-two", unwind); + this.start_panic("align_offset: align is not a power-of-two", unwind)?; + return Ok(true); // nothing left to do } let ptr_scalar = this.read_scalar(ptr_op)?.check_init()?; - // Default: no result. - let mut result = this.machine_usize_max(); if let Ok(ptr) = this.force_ptr(ptr_scalar) { // Only do anything if we can identify the allocation this goes to. let cur_align = this.memory.get_size_and_align(ptr.alloc_id, AllocCheck::MaybeDead)?.1.bytes(); if u128::from(cur_align) >= req_align { // If the allocation alignment is at least the required alignment we use the - // libcore implementation. - // FIXME: is this correct in case of truncation? - result = u64::try_from( - (this.force_bits(ptr_scalar, this.pointer_size())? as *const i8) - .align_offset(usize::try_from(req_align).unwrap()) - ).unwrap(); + // real implementation. + return Ok(false); } } - // Return result, and jump to caller. - this.write_scalar(Scalar::from_machine_usize(result, this), dest)?; + // Return error result (usize::MAX), and jump to caller. + this.write_scalar(Scalar::from_machine_usize(this.machine_usize_max(), this), dest)?; this.go_to_block(ret); - Ok(()) + Ok(true) } } diff --git a/tests/run-pass/align.rs b/tests/run-pass/align.rs index b3d268f45e..81e7e8c7cc 100644 --- a/tests/run-pass/align.rs +++ b/tests/run-pass/align.rs @@ -1,11 +1,25 @@ -// This manually makes sure that we have a pointer with the proper alignment. -// Do this a couple times in a loop because it may work "by chance". +/// This manually makes sure that we have a pointer with the proper alignment. +fn manual_alignment() { + let x = &mut [0u8; 3]; + let base_addr = x as *mut _ as usize; + let base_addr_aligned = if base_addr % 2 == 0 { base_addr } else { base_addr+1 }; + let u16_ptr = base_addr_aligned as *mut u16; + unsafe { *u16_ptr = 2; } +} + +/// Test standard library `align_to`. +fn align_to() { + const LEN: usize = 128; + let buf = &[0u8; LEN]; + let (l, m, r) = unsafe { buf.align_to::() }; + assert!(m.len()*4 >= LEN-4); + assert!(l.len() + r.len() <= 4); +} + fn main() { + // Do this a couple times in a loop because it may work "by chance". for _ in 0..10 { - let x = &mut [0u8; 3]; - let base_addr = x as *mut _ as usize; - let base_addr_aligned = if base_addr % 2 == 0 { base_addr } else { base_addr+1 }; - let u16_ptr = base_addr_aligned as *mut u16; - unsafe { *u16_ptr = 2; } + manual_alignment(); + align_to(); } } diff --git a/tests/run-pass/align_offset.rs b/tests/run-pass/align_offset_symbolic.rs similarity index 98% rename from tests/run-pass/align_offset.rs rename to tests/run-pass/align_offset_symbolic.rs index f921634647..70b2e00896 100644 --- a/tests/run-pass/align_offset.rs +++ b/tests/run-pass/align_offset_symbolic.rs @@ -1,3 +1,5 @@ +// compile-flags: -Zmiri-symbolic-alignment-check + fn test_align_offset() { let d = Box::new([0u32; 4]); // Get u8 pointer to base diff --git a/tests/run-pass/align_offset.stdout b/tests/run-pass/align_offset_symbolic.stdout similarity index 100% rename from tests/run-pass/align_offset.stdout rename to tests/run-pass/align_offset_symbolic.stdout From 0913653e065896cbbacf164c36673fec48f947d1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 16 Aug 2020 18:31:48 +0200 Subject: [PATCH 5/8] make sure we test panic of interpreter-impelemted align_offset --- tests/run-pass/panic/catch_panic.rs | 5 ++++- tests/run-pass/panic/catch_panic.stderr | 24 ++++++++++++------------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/run-pass/panic/catch_panic.rs b/tests/run-pass/panic/catch_panic.rs index 6dc47f2f59..3afff1d36d 100644 --- a/tests/run-pass/panic/catch_panic.rs +++ b/tests/run-pass/panic/catch_panic.rs @@ -1,4 +1,7 @@ // normalize-stderr-test "[^ ]*core/[a-z_/]+.rs[0-9:]*" -> "$$LOC" +// normalize-stderr-test "catch_panic\.rs:[0-9]{2}" -> "catch_panic.rs:LL" +// We test the `align_offset` panic below, make sure we test the interpreter impl and not the "real" one. +// compile-flags: -Zmiri-symbolic-alignment-check #![feature(never_type)] #![allow(unconditional_panic)] @@ -99,7 +102,7 @@ fn test(expect_msg: Option<&str>, do_panic: impl FnOnce(usize) -> !) { eprintln!("Caught panic message (&str): {}", s); Some(*s) } else { - eprintln!("Failed get caught panic message."); + eprintln!("Failed to get caught panic message."); None }; if let Some(expect_msg) = expect_msg { diff --git a/tests/run-pass/panic/catch_panic.stderr b/tests/run-pass/panic/catch_panic.stderr index f64ff11866..c31f54aac7 100644 --- a/tests/run-pass/panic/catch_panic.stderr +++ b/tests/run-pass/panic/catch_panic.stderr @@ -1,26 +1,26 @@ -thread 'main' panicked at 'Hello from panic: std', $DIR/catch_panic.rs:50:27 +thread 'main' panicked at 'Hello from panic: std', $DIR/catch_panic.rs:LL:27 Caught panic message (&str): Hello from panic: std -thread 'main' panicked at 'Hello from panic: 1', $DIR/catch_panic.rs:51:26 +thread 'main' panicked at 'Hello from panic: 1', $DIR/catch_panic.rs:LL:26 Caught panic message (String): Hello from panic: 1 -thread 'main' panicked at 'Hello from panic: 2', $DIR/catch_panic.rs:52:26 +thread 'main' panicked at 'Hello from panic: 2', $DIR/catch_panic.rs:LL:26 Caught panic message (String): Hello from panic: 2 -thread 'main' panicked at 'Box', $DIR/catch_panic.rs:53:27 -Failed get caught panic message. -thread 'main' panicked at 'Hello from panic: core', $DIR/catch_panic.rs:56:27 +thread 'main' panicked at 'Box', $DIR/catch_panic.rs:LL:27 +Failed to get caught panic message. +thread 'main' panicked at 'Hello from panic: core', $DIR/catch_panic.rs:LL:27 Caught panic message (String): Hello from panic: core -thread 'main' panicked at 'Hello from panic: 5', $DIR/catch_panic.rs:57:26 +thread 'main' panicked at 'Hello from panic: 5', $DIR/catch_panic.rs:LL:26 Caught panic message (String): Hello from panic: 5 -thread 'main' panicked at 'Hello from panic: 6', $DIR/catch_panic.rs:58:26 +thread 'main' panicked at 'Hello from panic: 6', $DIR/catch_panic.rs:LL:26 Caught panic message (String): Hello from panic: 6 -thread 'main' panicked at 'index out of bounds: the len is 3 but the index is 4', $DIR/catch_panic.rs:63:33 +thread 'main' panicked at 'index out of bounds: the len is 3 but the index is 4', $DIR/catch_panic.rs:LL:33 Caught panic message (String): index out of bounds: the len is 3 but the index is 4 -thread 'main' panicked at 'attempt to divide by zero', $DIR/catch_panic.rs:67:33 +thread 'main' panicked at 'attempt to divide by zero', $DIR/catch_panic.rs:LL:33 Caught panic message (String): attempt to divide by zero thread 'main' panicked at 'align_offset: align is not a power-of-two', $LOC Caught panic message (String): align_offset: align is not a power-of-two -thread 'main' panicked at 'assertion failed: false', $DIR/catch_panic.rs:76:29 +thread 'main' panicked at 'assertion failed: false', $DIR/catch_panic.rs:LL:29 Caught panic message (&str): assertion failed: false -thread 'main' panicked at 'assertion failed: false', $DIR/catch_panic.rs:77:29 +thread 'main' panicked at 'assertion failed: false', $DIR/catch_panic.rs:LL:29 Caught panic message (&str): assertion failed: false thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', $LOC Caught panic message (String): called `Option::unwrap()` on a `None` value From f691e573b242a046b9c411eb8e362b8a0715f07e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Aug 2020 11:51:18 +0200 Subject: [PATCH 6/8] tweak alignment check docs --- README.md | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index a189fd233f..37ed718a1a 100644 --- a/README.md +++ b/README.md @@ -165,9 +165,8 @@ up the sysroot. If you are using `miri` (the Miri driver) directly, see the Miri adds its own set of `-Z` flags: -* `-Zmiri-disable-alignment-check` disables checking pointer alignment. This is - useful to avoid [false positives][alignment-false-positives]. However, setting - this flag means Miri could miss bugs in your program. +* `-Zmiri-disable-alignment-check` disables checking pointer alignment, so you + can focus on other failures. * `-Zmiri-disable-stacked-borrows` disables checking the experimental [Stacked Borrows] aliasing rules. This can make Miri run faster, but it also means no aliasing violations will be detected. @@ -193,12 +192,13 @@ Miri adds its own set of `-Z` flags: 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 program passes the alignment check by pure chance, because things "happened to - be" sufficiently aligned. To avoid such cases, the symbolic alignment check - only takes into account the requested alignment of the relevant allocation, - and the offset into that allocation. This avoids such false negatives, but it - also incurs some false positives when the code does manual integer arithmetic - to ensure alignment. (The standard library `align_to` method works fine in - both modes; under symbolic alignment it only fills the middle slice when the + be" sufficiently aligned -- there is no UB in this execution but there would + be UB in others. To avoid such cases, the symbolic alignment check only takes + into account the requested alignment of the relevant allocation, and the + offset into that allocation. This avoids missing such bugs, but it also + incurs some false positives when the code does manual integer arithmetic to + ensure alignment. (The standard library `align_to` method works fine in both + modes; under symbolic alignment it only fills the middle slice when the allocation guarantees sufficient alignment.) * `-Zmiri-track-alloc-id=` shows a backtrace when the given allocation is being allocated or freed. This helps in debugging memory leaks and @@ -211,8 +211,6 @@ Miri adds its own set of `-Z` flags: assigned to a stack frame. This helps in debugging UB related to Stacked Borrows "protectors". -[alignment-false-positives]: https://github.com/rust-lang/miri/issues/1074 - Some native rustc `-Z` flags are also very relevant for Miri: * `-Zmir-opt-level` controls how many MIR optimizations are performed. Miri From db159b8709dac1cb8adeac91c3257906e55319ec Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Aug 2020 16:51:48 +0200 Subject: [PATCH 7/8] rustup --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 198b3970e0..e322ff61d1 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -814bc4fe9364865bfaa94d7825b8eabc11245c7c +8cdc94e84040ce797fd33d0a7cfda4ec4f2f2421 From 5b1bc4ba94cdbb403388cd7c5b45f5ecf4ccd922 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Aug 2020 17:09:09 +0200 Subject: [PATCH 8/8] make another test more robust against random alignment --- tests/compile-fail/unaligned_pointers/alignment.rs | 8 ++++---- .../unaligned_pointers/reference_to_packed.rs | 14 ++++++++------ .../unaligned_pointers/unaligned_ptr1.rs | 10 ++++++---- .../unaligned_pointers/unaligned_ptr2.rs | 12 +++++++----- .../unaligned_pointers/unaligned_ptr3.rs | 14 ++++++++------ .../unaligned_pointers/unaligned_ptr_addr_of.rs | 12 +++++++----- 6 files changed, 40 insertions(+), 30 deletions(-) diff --git a/tests/compile-fail/unaligned_pointers/alignment.rs b/tests/compile-fail/unaligned_pointers/alignment.rs index 8532f91a5c..e4d7621b8b 100644 --- a/tests/compile-fail/unaligned_pointers/alignment.rs +++ b/tests/compile-fail/unaligned_pointers/alignment.rs @@ -1,11 +1,11 @@ +// error-pattern: but alignment 4 is required + fn main() { let mut x = [0u8; 20]; let x_ptr: *mut u8 = x.as_mut_ptr(); // At least one of these is definitely unaligned. - // Currently, we guarantee to complain about the first one already (https://github.com/rust-lang/miri/issues/1074). unsafe { - *(x_ptr as *mut u64) = 42; //~ ERROR accessing memory with alignment 1, but alignment - *(x_ptr.add(1) as *mut u64) = 42; + *(x_ptr as *mut u32) = 42; + *(x_ptr.add(1) as *mut u32) = 42; } - panic!("unreachable in miri"); } diff --git a/tests/compile-fail/unaligned_pointers/reference_to_packed.rs b/tests/compile-fail/unaligned_pointers/reference_to_packed.rs index a1240c9018..998394c6c7 100644 --- a/tests/compile-fail/unaligned_pointers/reference_to_packed.rs +++ b/tests/compile-fail/unaligned_pointers/reference_to_packed.rs @@ -10,10 +10,12 @@ struct Foo { } fn main() { - let foo = Foo { - x: 42, - y: 99, - }; - let p = unsafe { &foo.x }; - let i = *p; //~ ERROR alignment 4 is required + for _ in 0..10 { // Try many times as this might work by chance. + let foo = Foo { + x: 42, + y: 99, + }; + let p = unsafe { &foo.x }; + let i = *p; //~ ERROR alignment 4 is required + } } diff --git a/tests/compile-fail/unaligned_pointers/unaligned_ptr1.rs b/tests/compile-fail/unaligned_pointers/unaligned_ptr1.rs index 0a67cfc5a1..43e6fd67d2 100644 --- a/tests/compile-fail/unaligned_pointers/unaligned_ptr1.rs +++ b/tests/compile-fail/unaligned_pointers/unaligned_ptr1.rs @@ -2,8 +2,10 @@ // compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows fn main() { - let x = [2u16, 3, 4]; // Make it big enough so we don't get an out-of-bounds error. - let x = &x[0] as *const _ as *const u32; - // This must fail because alignment is violated: the allocation's base is not sufficiently aligned. - let _x = unsafe { *x }; //~ ERROR memory with alignment 2, but alignment 4 is required + for _ in 0..10 { // Try many times as this might work by chance. + let x = [2u16, 3, 4]; // Make it big enough so we don't get an out-of-bounds error. + let x = &x[0] as *const _ as *const u32; + // This must fail because alignment is violated: the allocation's base is not sufficiently aligned. + let _x = unsafe { *x }; //~ ERROR memory with alignment 2, but alignment 4 is required + } } diff --git a/tests/compile-fail/unaligned_pointers/unaligned_ptr2.rs b/tests/compile-fail/unaligned_pointers/unaligned_ptr2.rs index b1fb2f4aa9..f4ed8d47b5 100644 --- a/tests/compile-fail/unaligned_pointers/unaligned_ptr2.rs +++ b/tests/compile-fail/unaligned_pointers/unaligned_ptr2.rs @@ -2,9 +2,11 @@ // compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows fn main() { - let x = [2u32, 3]; // Make it big enough so we don't get an out-of-bounds error. - let x = (x.as_ptr() as *const u8).wrapping_offset(3) as *const u32; - // This must fail because alignment is violated: the offset is not sufficiently aligned. - // Also make the offset not a power of 2, that used to ICE. - let _x = unsafe { *x }; //~ ERROR memory with alignment 1, but alignment 4 is required + for _ in 0..10 { // Try many times as this might work by chance. + let x = [2u32, 3]; // Make it big enough so we don't get an out-of-bounds error. + let x = (x.as_ptr() as *const u8).wrapping_offset(3) as *const u32; + // This must fail because alignment is violated: the offset is not sufficiently aligned. + // Also make the offset not a power of 2, that used to ICE. + let _x = unsafe { *x }; //~ ERROR memory with alignment 1, but alignment 4 is required + } } diff --git a/tests/compile-fail/unaligned_pointers/unaligned_ptr3.rs b/tests/compile-fail/unaligned_pointers/unaligned_ptr3.rs index c5a3398384..61c2a3cde8 100644 --- a/tests/compile-fail/unaligned_pointers/unaligned_ptr3.rs +++ b/tests/compile-fail/unaligned_pointers/unaligned_ptr3.rs @@ -2,10 +2,12 @@ // compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows fn main() { - let x = [2u16, 3, 4, 5]; // Make it big enough so we don't get an out-of-bounds error. - let x = &x[0] as *const _ as *const *const u8; // cast to ptr-to-ptr, so that we load a ptr - // This must fail because alignment is violated. Test specifically for loading pointers, - // which have special code in miri's memory. - let _x = unsafe { *x }; - //~^ ERROR memory with alignment 2, but alignment + for _ in 0..10 { // Try many times as this might work by chance. + let x = [2u16, 3, 4, 5]; // Make it big enough so we don't get an out-of-bounds error. + let x = &x[0] as *const _ as *const *const u8; // cast to ptr-to-ptr, so that we load a ptr + // This must fail because alignment is violated. Test specifically for loading pointers, + // which have special code in miri's memory. + let _x = unsafe { *x }; + //~^ ERROR but alignment + } } diff --git a/tests/compile-fail/unaligned_pointers/unaligned_ptr_addr_of.rs b/tests/compile-fail/unaligned_pointers/unaligned_ptr_addr_of.rs index cd52cd44c2..88e2634efa 100644 --- a/tests/compile-fail/unaligned_pointers/unaligned_ptr_addr_of.rs +++ b/tests/compile-fail/unaligned_pointers/unaligned_ptr_addr_of.rs @@ -4,9 +4,11 @@ use std::ptr; fn main() { - let x = [2u16, 3, 4]; // Make it big enough so we don't get an out-of-bounds error. - let x = &x[0] as *const _ as *const u32; - // This must fail because alignment is violated: the allocation's base is not sufficiently aligned. - // The deref is UB even if we just put the result into a raw pointer. - let _x = unsafe { ptr::raw_const!(*x) }; //~ ERROR memory with alignment 2, but alignment 4 is required + for _ in 0..10 { // Try many times as this might work by chance. + let x = [2u16, 3, 4]; // Make it big enough so we don't get an out-of-bounds error. + let x = &x[0] as *const _ as *const u32; + // This must fail because alignment is violated: the allocation's base is not sufficiently aligned. + // The deref is UB even if we just put the result into a raw pointer. + let _x = unsafe { ptr::raw_const!(*x) }; //~ ERROR memory with alignment 2, but alignment 4 is required + } }