don't UB on dangling ptr deref, instead check inbounds on projections

Author: RalfJung

src/helpers.rs

@@ -700,23 +700,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         let mplace = MPlaceTy::from_aligned_ptr(ptr, layout);
 
-        this.check_mplace(&mplace)?;
-
-        Ok(mplace)
-    }
-
-    /// Deref' a pointer *without* checking that the place is dereferenceable.
-    fn deref_pointer_unchecked(
-        &self,
-        val: &ImmTy<'tcx, Provenance>,
-        layout: TyAndLayout<'tcx>,
-    ) -> InterpResult<'tcx, MPlaceTy<'tcx, Provenance>> {
-        let this = self.eval_context_ref();
-        let mut mplace = this.ref_to_mplace(val)?;
-
-        mplace.layout = layout;
-        mplace.align = layout.align.abi;
-
         Ok(mplace)
     }
 

src/machine.rs

@@ -1285,6 +1285,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
         // We do need to write `uninit` so that even after the call ends, the former contents of
         // this place cannot be observed any more. We do the write after retagging so that for
         // Tree Borrows, this is considered to activate the new tag.
+        // Conveniently this also ensures that the place actually points to suitable memory.
         ecx.write_uninit(&protected_place)?;
         // Now we throw away the protected place, ensuring its tag is never used again.
         Ok(())

src/shims/unix/linux/sync.rs

@@ -85,9 +85,8 @@ pub fn futex<'tcx>(
                 return Ok(());
             }
 
-            // `read_timespec` will check the place when it is not null.
-            let timeout = this.deref_pointer_unchecked(
-                &this.read_immediate(&args[3])?,
+            let timeout = this.deref_pointer_as(
+                &args[3],
                 this.libc_ty_layout("timespec"),
             )?;
             let timeout_time = if this.ptr_is_null(timeout.ptr())? {

tests/compiletest.rs

@@ -181,6 +181,7 @@ regexes! {
     r"0x[0-9a-fA-F]+[0-9a-fA-F]{2,2}" => "$$HEX",
     // erase specific alignments
     "alignment [0-9]+"               => "alignment ALIGN",
+    "[0-9]+ byte alignment but found [0-9]+" => "ALIGN byte alignment but found ALIGN",
     // erase thread caller ids
     r"call [0-9]+"                  => "call ID",
     // erase platform module paths

tests/fail-dep/shims/mmap_use_after_munmap.stderr

@@ -13,11 +13,11 @@ LL |         libc::munmap(ptr, 4096);
    = note: BACKTRACE:
    = note: inside `main` at $DIR/mmap_use_after_munmap.rs:LL:CC
 
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
   --> $DIR/mmap_use_after_munmap.rs:LL:CC
    |
 LL |         let _x = *(ptr as *mut u8);
-   |                  ^^^^^^^^^^^^^^^^^ dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+   |                  ^^^^^^^^^^^^^^^^^ memory access failed: ALLOC has been freed, so this pointer is dangling
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/alloc/reallocate-change-alloc.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
   --> $DIR/reallocate-change-alloc.rs:LL:CC
    |
 LL |         let _z = *x;
-   |                  ^^ dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+   |                  ^^ memory access failed: ALLOC has been freed, so this pointer is dangling
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/concurrency/thread_local_static_dealloc.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
   --> $DIR/thread_local_static_dealloc.rs:LL:CC
    |
 LL |         let _val = *dangling_ptr.0;
-   |                    ^^^^^^^^^^^^^^^ dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+   |                    ^^^^^^^^^^^^^^^ memory access failed: ALLOC has been freed, so this pointer is dangling
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/dangling_pointers/dangling_pointer_addr_of.rs

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
   --> $DIR/dangling_pointer_deref.rs:LL:CC
    |
 LL |     let x = unsafe { *p };
-   |                      ^^ dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+   |                      ^^ memory access failed: ALLOC has been freed, so this pointer is dangling
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/dangling_pointers/dangling_pointer_addr_of.stderr

@@ -4,10 +4,9 @@
 fn main() {
     let p = {
         let b = Box::new(42);
-        &*b as *const i32
+        &*b as *const i32 as *const (u8, u8, u8, u8)
     };
     unsafe {
-        let _ = *p; //~ ERROR: has been freed
+        let _ = (*p).1; //~ ERROR: out-of-bounds pointer arithmetic
     }
-    panic!("this should never print");
 }

tests/fail/dangling_pointers/dangling_pointer_deref.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+error: Undefined Behavior: out-of-bounds pointer arithmetic: ALLOC has been freed, so this pointer is dangling
   --> $DIR/dangling_pointer_project_underscore.rs:LL:CC
    |
-LL |         let _ = *p;
-   |                 ^^ dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+LL |         let _ = (*p).1;
+   |                 ^^^^^^ out-of-bounds pointer arithmetic: ALLOC has been freed, so this pointer is dangling
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/dangling_pointers/dangling_pointer_project_underscore.rs

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
   --> $DIR/dangling_primitive.rs:LL:CC
    |
 LL |         dbg!(*ptr);
-   |         ^^^^^^^^^^ dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+   |         ^^^^^^^^^^ memory access failed: ALLOC has been freed, so this pointer is dangling
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/dangling_pointers/dangling_pointer_project_underscore.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
   --> $DIR/dangling_zst_deref.rs:LL:CC
    |
 LL |     let _x = unsafe { *p };
-   |                       ^^ dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
+   |                       ^^ memory access failed: ALLOC has been freed, so this pointer is dangling
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/dangling_pointers/dangling_primitive.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: 0x10[noalloc] is a dangling pointer (it has no provenance)
+error: Undefined Behavior: out-of-bounds pointer use: 0x10[noalloc] is a dangling pointer (it has no provenance)
   --> $DIR/deref-invalid-ptr.rs:LL:CC
    |
 LL |     let _y = unsafe { &*x as *const u32 };
-   |                       ^^^ dereferencing pointer failed: 0x10[noalloc] is a dangling pointer (it has no provenance)
+   |                       ^^^ out-of-bounds pointer use: 0x10[noalloc] is a dangling pointer (it has no provenance)
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/dangling_pointers/dangling_zst_deref.stderr

@@ -0,0 +1,16 @@
+// Should be caught even without retagging
+//@compile-flags: -Zmiri-disable-stacked-borrows
+#![feature(strict_provenance)]
+use std::ptr::{addr_of_mut, self};
+
+// Deref'ing a dangling raw pointer is fine, but for a dangling box it is not.
+// We do this behind a pointer indirection to potentially fool validity checking.
+// (This test relies on the `deref_copy` pass that lowers `**ptr` to materialize the intermediate pointer.)
+
+fn main() {
+    let mut inner = ptr::invalid::<i32>(24);
+    let outer = addr_of_mut!(inner).cast::<Box<i32>>();
+    // Now `outer` is a pointer to a dangling reference.
+    // Deref'ing that should be UB.
+    let _val = unsafe { addr_of_mut!(**outer) }; //~ERROR: dangling box
+}

tests/fail/dangling_pointers/deref-invalid-ptr.stderr

@@ -0,0 +1,16 @@
+error: Undefined Behavior: constructing invalid value: encountered a dangling box (0x18[noalloc] has no provenance)
+  --> $DIR/deref_dangling_box.rs:LL:CC
+   |
+LL |     let _val = unsafe { addr_of_mut!(**outer) };
+   |                         ^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a dangling box (0x18[noalloc] has no provenance)
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at RUSTLIB/core/src/ptr/mod.rs:LL:CC
+   = note: this error originates in the macro `addr_of_mut` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+

tests/fail/dangling_pointers/deref-partially-dangling.rs

@@ -0,0 +1,16 @@
+// Should be caught even without retagging
+//@compile-flags: -Zmiri-disable-stacked-borrows
+#![feature(strict_provenance)]
+use std::ptr::{addr_of_mut, self};
+
+// Deref'ing a dangling raw pointer is fine, but for a dangling reference it is not.
+// We do this behind a pointer indirection to potentially fool validity checking.
+// (This test relies on the `deref_copy` pass that lowers `**ptr` to materialize the intermediate pointer.)
+
+fn main() {
+    let mut inner = ptr::invalid::<i32>(24);
+    let outer = addr_of_mut!(inner).cast::<&'static mut i32>();
+    // Now `outer` is a pointer to a dangling reference.
+    // Deref'ing that should be UB.
+    let _val = unsafe { addr_of_mut!(**outer) }; //~ERROR: dangling reference
+}

tests/fail/dangling_pointers/deref-partially-dangling.stderr

@@ -0,0 +1,16 @@
+error: Undefined Behavior: constructing invalid value: encountered a dangling reference (0x18[noalloc] has no provenance)
+  --> $DIR/deref_dangling_ref.rs:LL:CC
+   |
+LL |     let _val = unsafe { addr_of_mut!(**outer) };
+   |                         ^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a dangling reference (0x18[noalloc] has no provenance)
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at RUSTLIB/core/src/ptr/mod.rs:LL:CC
+   = note: this error originates in the macro `addr_of_mut` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+

tests/fail/dangling_pointers/deref_dangling_box.rs

@@ -1,13 +1,13 @@
-// should find the bug even without these
-//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
+// should find the bug even without retagging
+//@compile-flags: -Zmiri-disable-stacked-borrows
 
 struct SliceWithHead(u8, [u8]);
 
 fn main() {
     let buf = [0u32; 1];
     // We craft a wide pointer `*const SliceWithHead` such that the unsized tail is only partially allocated.
-    // That should be UB, as the reference is not fully dereferencable.
+    // That should lead to UB, as the reference is not fully dereferenceable.
     let ptr: *const SliceWithHead = unsafe { std::mem::transmute((&buf, 4usize)) };
     // Re-borrow that. This should be UB.
-    let _ptr = unsafe { &*ptr }; //~ ERROR: pointer to 5 bytes starting at offset 0 is out-of-bounds
+    let _ptr = unsafe { &*ptr }; //~ ERROR: encountered a dangling reference (going beyond the bounds of its allocation)
 }

tests/fail/dangling_pointers/deref_dangling_box.stderr

@@ -1,17 +1,12 @@
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has size 4, so pointer to 5 bytes starting at offset 0 is out-of-bounds
+error: Undefined Behavior: constructing invalid value: encountered a dangling reference (going beyond the bounds of its allocation)
   --> $DIR/dyn_size.rs:LL:CC
    |
 LL |     let _ptr = unsafe { &*ptr };
-   |                         ^^^^^ dereferencing pointer failed: ALLOC has size 4, so pointer to 5 bytes starting at offset 0 is out-of-bounds
+   |                         ^^^^^ constructing invalid value: encountered a dangling reference (going beyond the bounds of its allocation)
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-help: ALLOC was allocated here:
-  --> $DIR/dyn_size.rs:LL:CC
-   |
-LL |     let buf = [0u32; 1];
-   |         ^^^
-   = note: BACKTRACE (of the first span):
+   = note: BACKTRACE:
    = note: inside `main` at $DIR/dyn_size.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/dangling_pointers/deref_dangling_ref.rs

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has size 1, so pointer at offset -2048 is out-of-bounds
+error: Undefined Behavior: memory access failed: ALLOC has size 1, so pointer at offset -2048 is out-of-bounds
   --> $DIR/maybe_null_pointer_deref_zst.rs:LL:CC
    |
 LL |     let _x: () = unsafe { *ptr };
-   |                           ^^^^ dereferencing pointer failed: ALLOC has size 1, so pointer at offset -2048 is out-of-bounds
+   |                           ^^^^ memory access failed: ALLOC has size 1, so pointer at offset -2048 is out-of-bounds
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/dangling_pointers/deref_dangling_ref.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: ALLOC has size 1, so pointer at offset -2048 is out-of-bounds
+error: Undefined Behavior: memory access failed: ALLOC has size 1, so pointer at offset -2048 is out-of-bounds
   --> $DIR/maybe_null_pointer_write_zst.rs:LL:CC
    |
 LL |     unsafe { *ptr = zst_val };
-   |              ^^^^^^^^^^^^^^ dereferencing pointer failed: ALLOC has size 1, so pointer at offset -2048 is out-of-bounds
+   |              ^^^^^^^^^^^^^^ memory access failed: ALLOC has size 1, so pointer at offset -2048 is out-of-bounds
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/dangling_pointers/dyn_size.rs

@@ -1,8 +1,8 @@
-error: Undefined Behavior: dereferencing pointer failed: null pointer is a dangling pointer (it has no provenance)
+error: Undefined Behavior: memory access failed: null pointer is a dangling pointer (it has no provenance)
   --> $DIR/null_pointer_deref.rs:LL:CC
    |
 LL |     let x: i32 = unsafe { *std::ptr::null() };
-   |                           ^^^^^^^^^^^^^^^^^ dereferencing pointer failed: null pointer is a dangling pointer (it has no provenance)
+   |                           ^^^^^^^^^^^^^^^^^ memory access failed: null pointer is a dangling pointer (it has no provenance)
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/fail/dangling_pointers/dyn_size.stderr

@@ -1,5 +1,5 @@
 #[allow(deref_nullptr)]
 fn main() {
-    let x: () = unsafe { *std::ptr::null() }; //~ ERROR: dereferencing pointer failed: null pointer is a dangling pointer
+    let x: () = unsafe { *std::ptr::null() }; //~ ERROR: memory access failed: null pointer is a dangling pointer
     panic!("this should never print: {:?}", x);
 }