Auto merge of #1332 - RalfJung:disable-alignment-check, r=RalfJung

Add option to disable alignment check

Requires rust-lang/rust#71101
Fixes #1326

Author: bors

README.md

@@ -165,43 +165,51 @@ up the sysroot.  If you are using `miri` (the Miri driver) directly, see
 ## Miri `-Z` flags and environment variables
 [miri-flags]: #miri--z-flags-and-environment-variables
 
-Several `-Z` flags are relevant for Miri:
-
-* `-Zmiri-seed=<hex>` is a custom `-Z` flag added by Miri.  It configures the
-  seed of the RNG that Miri uses to resolve non-determinism.  This RNG is used
-  to pick base addresses for allocations.  When isolation is enabled (the default),
-  this is also used to emulate system 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-disable-validation` disables enforcing validity invariants, which are
-  enforced by default.  This is mostly useful for debugging.  It means Miri will
-  miss bugs in your program.  However, this can also help to make Miri run
-  faster.
+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-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.
+* `-Zmiri-disable-validation` disables enforcing validity invariants, which are
+  enforced by default.  This is mostly useful to focus on other failures (such
+  as out-of-bounds accesses) first.  Setting this flag means Miri will miss bugs
+  in your program.  However, this can also help to make Miri run faster.
 * `-Zmiri-disable-isolation` disables host isolation.  As a consequence,
   the program has access to host resources such as environment variables, file
   systems, and randomness.
-* `-Zmiri-ignore-leaks` disables the memory leak checker.
 * `-Zmiri-env-exclude=<var>` keeps the `var` environment variable isolated from
-  the host. Can be used multiple times to exclude several variables. The `TERM`
-  environment variable is excluded by default.
+  the host so that it cannot be accessed by the program.  Can be used multiple
+  times to exclude several variables.  On Windows, the `TERM` environment
+  variable is excluded by default.
+* `-Zmiri-ignore-leaks` disables the memory leak checker.
+* `-Zmiri-seed=<hex>` configures the seed of the RNG that Miri uses to resolve
+  non-determinism.  This RNG is used to pick base addresses for allocations.
+  When isolation is enabled (the default), this is also used to emulate system
+  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-track-alloc-id=<id>` shows a backtrace when the given allocation is
+  being allocated.  This helps in debugging memory leaks.
+* `-Zmiri-track-pointer-tag=<tag>` shows a backtrace when the given pointer tag
+  is popped from a borrow stack (which is where the tag becomes invalid and any
+  future use of it will error).  This helps you in finding out why UB is
+  happening and where in your code would be a good place to look for it.
+
+[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
   overrides the default to be `0`; be advised that using any higher level can
   make Miri miss bugs in your program because they got optimized away.
 * `-Zalways-encode-mir` makes rustc dump MIR even for completely monomorphic
   functions.  This is needed so that Miri can execute such functions, so Miri
   sets this flag per default.
 * `-Zmir-emit-retag` controls whether `Retag` statements are emitted. Miri
-  enables this per default because it is needed for validation.
-* `-Zmiri-track-pointer-tag=<tag>` shows a backtrace when the given pointer tag
-  is popped from a borrow stack (which is where the tag becomes invalid and any
-  future use of it will error).  This helps you in finding out why UB is
-  happening and where in your code would be a good place to look for it.
-* `-Zmiri-track-alloc-id=<id>` shows a backtrace when the given allocation is
-  being allocated.  This helps in debugging memory leaks.
+  enables this per default because it is needed for [Stacked Borrows].
 
 Moreover, Miri recognizes some environment variables:
 

rust-version

@@ -1 +1 @@
-4d1fbaccb822b6d52dc786589de7918d3c5effb1
+47f49695dfb4fe9e584239fdc59c771887148a57

src/bin/miri.rs

@@ -128,6 +128,7 @@ fn main() {
     // 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<u64> = None;
@@ -152,6 +153,9 @@ fn main() {
                 "-Zmiri-disable-stacked-borrows" => {
                     stacked_borrows = false;
                 }
+                "-Zmiri-disable-alignment-check" => {
+                    check_alignment = false;
+                }
                 "-Zmiri-disable-isolation" => {
                     communicate = true;
                 }
@@ -243,6 +247,7 @@ fn main() {
     let miri_config = miri::MiriConfig {
         validate,
         stacked_borrows,
+        check_alignment,
         communicate,
         ignore_leaks,
         excluded_env_vars,

src/diagnostics.rs

@@ -97,6 +97,7 @@ pub fn report_error<'tcx, 'mir>(
                     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")
                     ],
                 UndefinedBehavior(_) =>
                     vec![

src/eval.rs

@@ -19,6 +19,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,
     /// Determines if communication with the host environment is enabled.
     pub communicate: bool,
     /// Determines if memory leaks should be ignored.
@@ -40,6 +42,7 @@ impl Default for MiriConfig {
         MiriConfig {
             validate: true,
             stacked_borrows: true,
+            check_alignment: true,
             communicate: false,
             ignore_leaks: false,
             excluded_env_vars: vec![],
@@ -72,6 +75,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
             config.stacked_borrows,
             config.tracked_pointer_tag,
             config.tracked_alloc_id,
+            config.check_alignment,
         ),
     );
     // Complete initialization.

src/machine.rs

@@ -118,10 +118,19 @@ pub struct MemoryExtra {
     /// An allocation ID to report when it is being allocated
     /// (helps for debugging memory leaks).
     tracked_alloc_id: Option<AllocId>,
+
+    /// Controls whether alignment of memory accesses is being checked.
+    check_alignment: bool,
 }
 
 impl MemoryExtra {
-    pub fn new(rng: StdRng, stacked_borrows: bool, tracked_pointer_tag: Option<PtrId>, tracked_alloc_id: Option<AllocId>) -> Self {
+    pub fn new(
+        rng: StdRng,
+        stacked_borrows: bool,
+        tracked_pointer_tag: Option<PtrId>,
+        tracked_alloc_id: Option<AllocId>,
+        check_alignment: bool,
+    ) -> Self {
         let stacked_borrows = if stacked_borrows {
             Some(Rc::new(RefCell::new(stacked_borrows::GlobalState::new(tracked_pointer_tag))))
         } else {
@@ -133,6 +142,7 @@ impl MemoryExtra {
             extern_statics: FxHashMap::default(),
             rng: RefCell::new(rng),
             tracked_alloc_id,
+            check_alignment,
         }
     }
 
@@ -299,7 +309,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
 
     const GLOBAL_KIND: Option<MiriMemoryKind> = Some(MiriMemoryKind::Global);
 
-    const CHECK_ALIGN: bool = true;
+    #[inline(always)]
+    fn enforce_alignment(memory_extra: &MemoryExtra) -> bool {
+        memory_extra.check_alignment
+    }
 
     #[inline(always)]
     fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {

tests/compile-fail/unaligned_pointers/alignment.rs

@@ -1,11 +1,11 @@
 fn main() {
-    // miri always gives allocations the worst possible alignment, so a `u8` array is guaranteed
-    // to be at the virtual location 1 (so one byte offset from the ultimate alignemnt location 0)
     let mut x = [0u8; 20];
-    let x_ptr: *mut u8 = &mut x[0];
-    let y_ptr = x_ptr as *mut u64;
+    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 {
-        *y_ptr = 42; //~ ERROR accessing memory with alignment 1, but alignment
+        *(x_ptr as *mut u64) = 42; //~ ERROR accessing memory with alignment 1, but alignment
+        *(x_ptr.add(1) as *mut u64) = 42;
     }
     panic!("unreachable in miri");
 }

tests/compile-fail/unaligned_pointers/intptrcast_alignment_check.rs

@@ -2,6 +2,8 @@
 // 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() {
     let x = &mut [0u8; 3];
     let base_addr = x as *mut _ as usize;

tests/run-pass/disable-alignment-check.rs

@@ -0,0 +1,11 @@
+// compile-flags: -Zmiri-disable-alignment-check
+
+fn main() {
+    let mut x = [0u8; 20];
+    let x_ptr: *mut u8 = x.as_mut_ptr();
+    // At least one of these is definitely unaligned.
+    unsafe {
+        *(x_ptr as *mut u64) = 42;
+        *(x_ptr.add(1) as *mut u64) = 42;
+    }
+}