From ad001dccca8c1505ae1e2db5c106800404a5d0fb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 6 Nov 2022 13:00:09 +0100 Subject: [PATCH 1/3] interpret: support for per-byte provenance --- src/machine.rs | 23 ++- src/tag_gc.rs | 2 +- tests/fail/copy_half_a_pointer.rs | 21 --- tests/fail/copy_half_a_pointer.stderr | 14 -- .../pointer_partial_overwrite.rs | 7 +- .../pointer_partial_overwrite.stderr | 4 +- tests/pass/provenance.rs | 139 ++++++++++++++++++ 7 files changed, 153 insertions(+), 57 deletions(-) delete mode 100644 tests/fail/copy_half_a_pointer.rs delete mode 100644 tests/fail/copy_half_a_pointer.stderr rename tests/fail/{ => provenance}/pointer_partial_overwrite.rs (50%) rename tests/fail/{ => provenance}/pointer_partial_overwrite.stderr (70%) create mode 100644 tests/pass/provenance.rs diff --git a/src/machine.rs b/src/machine.rs index 231a99c1d034e..5887d26462ba2 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -133,7 +133,7 @@ impl fmt::Display for MiriMemoryKind { } /// Pointer provenance. -#[derive(Debug, Clone, Copy)] +#[derive(Clone, Copy)] pub enum Provenance { Concrete { alloc_id: AllocId, @@ -176,18 +176,9 @@ static_assert_size!(Pointer, 24); #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] static_assert_size!(Scalar, 32); -impl interpret::Provenance for Provenance { - /// We use absolute addresses in the `offset` of a `Pointer`. - const OFFSET_IS_ADDR: bool = true; - - /// We cannot err on partial overwrites, it happens too often in practice (due to unions). - const ERR_ON_PARTIAL_PTR_OVERWRITE: bool = false; - - fn fmt(ptr: &Pointer, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let (prov, addr) = ptr.into_parts(); // address is absolute - write!(f, "{:#x}", addr.bytes())?; - - match prov { +impl fmt::Debug for Provenance { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { Provenance::Concrete { alloc_id, sb } => { // Forward `alternate` flag to `alloc_id` printing. if f.alternate() { @@ -202,9 +193,13 @@ impl interpret::Provenance for Provenance { write!(f, "[wildcard]")?; } } - Ok(()) } +} + +impl interpret::Provenance for Provenance { + /// We use absolute addresses in the `offset` of a `Pointer`. + const OFFSET_IS_ADDR: bool = true; fn get_alloc_id(self) -> Option { match self { diff --git a/src/tag_gc.rs b/src/tag_gc.rs index 5aa653632f395..73712348f0d5f 100644 --- a/src/tag_gc.rs +++ b/src/tag_gc.rs @@ -127,7 +127,7 @@ impl VisitTags for Operand { impl VisitTags for Allocation { fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { - for (_size, prov) in self.provenance().iter() { + for prov in self.provenance().provenances() { prov.visit_tags(visit); } diff --git a/tests/fail/copy_half_a_pointer.rs b/tests/fail/copy_half_a_pointer.rs deleted file mode 100644 index e1dcdda7fdfe3..0000000000000 --- a/tests/fail/copy_half_a_pointer.rs +++ /dev/null @@ -1,21 +0,0 @@ -//@normalize-stderr-test: "\+0x[48]" -> "+HALF_PTR" -#![allow(dead_code)] - -// We use packed structs to get around alignment restrictions -#[repr(packed)] -struct Data { - pad: u8, - ptr: &'static i32, -} - -static G: i32 = 0; - -fn main() { - let mut d = Data { pad: 0, ptr: &G }; - - // Get a pointer to the beginning of the Data struct (one u8 byte, then the pointer bytes). - let d_alias = &mut d as *mut _ as *mut *const u8; - unsafe { - let _x = d_alias.read_unaligned(); //~ERROR: unable to copy parts of a pointer - } -} diff --git a/tests/fail/copy_half_a_pointer.stderr b/tests/fail/copy_half_a_pointer.stderr deleted file mode 100644 index 21797757084ee..0000000000000 --- a/tests/fail/copy_half_a_pointer.stderr +++ /dev/null @@ -1,14 +0,0 @@ -error: unsupported operation: unable to copy parts of a pointer from memory at ALLOC+HALF_PTR - --> $DIR/copy_half_a_pointer.rs:LL:CC - | -LL | let _x = d_alias.read_unaligned(); - | ^^^^^^^^^^^^^^^^^^^^^^^^ unable to copy parts of a pointer from memory at ALLOC+HALF_PTR - | - = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support - = note: BACKTRACE: - = note: inside `main` at $DIR/copy_half_a_pointer.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to previous error - diff --git a/tests/fail/pointer_partial_overwrite.rs b/tests/fail/provenance/pointer_partial_overwrite.rs similarity index 50% rename from tests/fail/pointer_partial_overwrite.rs rename to tests/fail/provenance/pointer_partial_overwrite.rs index 63f0649b8ed3e..d3a68fbdd018f 100644 --- a/tests/fail/pointer_partial_overwrite.rs +++ b/tests/fail/provenance/pointer_partial_overwrite.rs @@ -2,16 +2,13 @@ //@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation // Test what happens when we overwrite parts of a pointer. -// Also see . fn main() { let mut p = &42; unsafe { let ptr: *mut _ = &mut p; - *(ptr as *mut u8) = 123; // if we ever support 8 bit pointers, this is gonna cause - // "attempted to interpret some raw bytes as a pointer address" instead of - // "attempted to read undefined bytes" + *(ptr as *mut u8) = 123; // this removes provenance from one of the bytes, meaning the entire ptr is considered to have no provenance. } - let x = *p; //~ ERROR: this operation requires initialized memory + let x = *p; //~ ERROR: no provenance panic!("this should never print: {}", x); } diff --git a/tests/fail/pointer_partial_overwrite.stderr b/tests/fail/provenance/pointer_partial_overwrite.stderr similarity index 70% rename from tests/fail/pointer_partial_overwrite.stderr rename to tests/fail/provenance/pointer_partial_overwrite.stderr index 7d10b75e8805f..06e5ede8c7788 100644 --- a/tests/fail/pointer_partial_overwrite.stderr +++ b/tests/fail/provenance/pointer_partial_overwrite.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory +error: Undefined Behavior: dereferencing pointer failed: $HEX[noalloc] is a dangling pointer (it has no provenance) --> $DIR/pointer_partial_overwrite.rs:LL:CC | LL | let x = *p; - | ^^ using uninitialized data, but this operation requires initialized memory + | ^^ dereferencing pointer failed: $HEX[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 diff --git a/tests/pass/provenance.rs b/tests/pass/provenance.rs new file mode 100644 index 0000000000000..b18d903e36ceb --- /dev/null +++ b/tests/pass/provenance.rs @@ -0,0 +1,139 @@ +#![feature(strict_provenance)] +#![feature(pointer_byte_offsets)] +use std::{mem, ptr}; + +const PTR_SIZE: usize = mem::size_of::<&i32>(); + +fn main() { + basic(); + partial_overwrite_then_restore(); + bytewise_ptr_methods(); + bytewise_custom_memcpy(); + bytewise_custom_memcpy_chunked(); +} + +/// Some basic smoke tests for provenance. +fn basic() { + let x = &42; + let ptr = x as *const i32; + let addr: usize = unsafe { mem::transmute(ptr) }; // an integer without provenance + // But we can give provenance back via `with_addr`. + let ptr_back = ptr.with_addr(addr); + assert_eq!(unsafe { *ptr_back }, 42); + + // It is preserved by MaybeUninit. + let addr_mu: mem::MaybeUninit = unsafe { mem::transmute(ptr) }; + let ptr_back: *const i32 = unsafe { mem::transmute(addr_mu) }; + assert_eq!(unsafe { *ptr_back }, 42); +} + +/// Overwrite one byte of a pointer, then restore it. +fn partial_overwrite_then_restore() { + unsafe fn ptr_bytes<'x>(ptr: &'x mut *const i32) -> &'x mut [mem::MaybeUninit; PTR_SIZE] { + mem::transmute(ptr) + } + + // Returns a value with the same provenance as `x` but 0 for the integer value. + // `x` must be initialized. + unsafe fn zero_with_provenance(x: mem::MaybeUninit) -> mem::MaybeUninit { + let ptr = [x; PTR_SIZE]; + let ptr: *const i32 = mem::transmute(ptr); + let mut ptr = ptr.with_addr(0); + ptr_bytes(&mut ptr)[0] + } + + unsafe { + let ptr = &42; + let mut ptr = ptr as *const i32; + // Get a bytewise view of the pointer. + let ptr_bytes = ptr_bytes(&mut ptr); + + // The highest bytes must be 0 for this to work. + let hi = if cfg!(target_endian = "little") { ptr_bytes.len() - 1 } else { 0 }; + assert_eq!(*ptr_bytes[hi].as_ptr().cast::(), 0); + // Overwrite provenance on the last byte. + ptr_bytes[hi] = mem::MaybeUninit::new(0); + // Restore it from the another byte. + ptr_bytes[hi] = zero_with_provenance(ptr_bytes[1]); + + // Now ptr should be good again. + assert_eq!(*ptr, 42); + } +} + +fn bytewise_ptr_methods() { + let mut ptr1 = &1; + let mut ptr2 = &2; + + // Swap them, bytewise. + unsafe { + ptr::swap_nonoverlapping( + &mut ptr1 as *mut _ as *mut mem::MaybeUninit, + &mut ptr2 as *mut _ as *mut mem::MaybeUninit, + mem::size_of::<&i32>(), + ); + } + + // Make sure they still work. + assert_eq!(*ptr1, 2); + assert_eq!(*ptr2, 1); + + // TODO: also test ptr::swap, ptr::copy, ptr::copy_nonoverlapping. +} + +fn bytewise_custom_memcpy() { + unsafe fn memcpy(to: *mut T, from: *const T) { + let to = to.cast::>(); + let from = from.cast::>(); + for i in 0..mem::size_of::() { + let b = from.add(i).read(); + to.add(i).write(b); + } + } + + let ptr1 = &1; + let mut ptr2 = &2; + + // Copy, bytewise. + unsafe { memcpy(&mut ptr2, &ptr1) }; + + // Make sure they still work. + assert_eq!(*ptr1, 1); + assert_eq!(*ptr2, 1); +} + +fn bytewise_custom_memcpy_chunked() { + unsafe fn memcpy(to: *mut T, from: *const T) { + assert!(mem::size_of::() % mem::size_of::() == 0); + let count = mem::size_of::() / mem::size_of::(); + let to = to.cast::>(); + let from = from.cast::>(); + for i in 0..count { + let b = from.add(i).read(); + to.add(i).write(b); + } + } + + // Prepare an array where pointers are stored at... interesting... offsets. + let mut data = [0usize; 2 * PTR_SIZE]; + let mut offsets = vec![]; + for i in 0..mem::size_of::() { + // We have 2*PTR_SIZE room for each of these pointers. + let base = i * 2 * PTR_SIZE; + // This one is mis-aligned by `i`. + let offset = base + i; + offsets.push(offset); + // Store it there. + unsafe { data.as_mut_ptr().byte_add(offset).cast::<&i32>().write_unaligned(&42) }; + } + + // Now memcpy that. + let mut data2 = [0usize; 2 * PTR_SIZE]; + unsafe { memcpy(&mut data2, &data) }; + + // And check the result. + for &offset in &offsets { + let ptr = unsafe { data2.as_ptr().byte_add(offset).cast::<&i32>().read_unaligned() }; + assert_eq!(*ptr, 42); + } +} From 238da08f1fea1258e598850edb27932876f33a17 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 6 Nov 2022 14:11:46 +0100 Subject: [PATCH 2/3] add test for printing per-byte provenance --- tests/fail/uninit_buffer_with_provenance.rs | 32 +++++++++++++++++++ .../fail/uninit_buffer_with_provenance.stderr | 32 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 tests/fail/uninit_buffer_with_provenance.rs create mode 100644 tests/fail/uninit_buffer_with_provenance.stderr diff --git a/tests/fail/uninit_buffer_with_provenance.rs b/tests/fail/uninit_buffer_with_provenance.rs new file mode 100644 index 0000000000000..170bc6e1ed12b --- /dev/null +++ b/tests/fail/uninit_buffer_with_provenance.rs @@ -0,0 +1,32 @@ +//@error-pattern: memory is uninitialized at [0x4..0x8] +//@normalize-stderr-test: "a[0-9]+" -> "ALLOC" +#![feature(strict_provenance)] + +// Test printing allocations that contain single-byte provenance. + +use std::alloc::{alloc, dealloc, Layout}; +use std::mem::{self, MaybeUninit}; +use std::slice::from_raw_parts; + +fn byte_with_provenance(val: u8, prov: *const T) -> MaybeUninit { + let ptr = prov.with_addr(val as usize); + let bytes: [MaybeUninit; mem::size_of::<*const ()>()] = unsafe { mem::transmute(ptr) }; + let lsb = if cfg!(target_endian = "little") { 0 } else { bytes.len() - 1 }; + bytes[lsb] +} + +fn main() { + let layout = Layout::from_size_align(16, 8).unwrap(); + unsafe { + let ptr = alloc(layout); + let ptr_raw = ptr.cast::>(); + *ptr_raw.add(0) = byte_with_provenance(0x42, &42u8); + *ptr.add(1) = 0x12; + *ptr.add(2) = 0x13; + *ptr_raw.add(3) = byte_with_provenance(0x43, &0u8); + let slice1 = from_raw_parts(ptr, 8); + let slice2 = from_raw_parts(ptr.add(8), 8); + drop(slice1.cmp(slice2)); + dealloc(ptr, layout); + } +} diff --git a/tests/fail/uninit_buffer_with_provenance.stderr b/tests/fail/uninit_buffer_with_provenance.stderr new file mode 100644 index 0000000000000..715d76aa1c2e7 --- /dev/null +++ b/tests/fail/uninit_buffer_with_provenance.stderr @@ -0,0 +1,32 @@ +error: Undefined Behavior: reading memory at ALLOC[0x0..0x8], but memory is uninitialized at [0x4..0x8], and this operation requires initialized memory + --> RUSTLIB/core/src/slice/cmp.rs:LL:CC + | +LL | let mut order = unsafe { memcmp(left.as_ptr(), right.as_ptr(), len) as isize }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading memory at ALLOC[0x0..0x8], but memory is uninitialized at [0x4..0x8], and this operation requires initialized memory + | + = 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 `::compare` at RUSTLIB/core/src/slice/cmp.rs:LL:CC + = note: inside `core::slice::cmp::::cmp` at RUSTLIB/core/src/slice/cmp.rs:LL:CC +note: inside `main` at $DIR/uninit_buffer_with_provenance.rs:LL:CC + --> $DIR/uninit_buffer_with_provenance.rs:LL:CC + | +LL | drop(slice1.cmp(slice2)); + | ^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +Uninitialized memory occurred at ALLOC[0x4..0x8], in this allocation: +ALLOC (Rust heap, size: 16, align: 8) { + ╾42[ALLOC] (1 ptr byte)╼ 12 13 ╾43[ALLOC] (1 ptr byte)╼ __ __ __ __ __ __ __ __ __ __ __ __ │ ━..━░░░░░░░░░░░░ +} +ALLOC (global (static or const), size: 1, align: 1) { + 2a │ * +} +ALLOC (global (static or const), size: 1, align: 1) { + 00 │ . +} + +error: aborting due to previous error + From be593491deca0162a646f9e189b44826f6dd9892 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 7 Nov 2022 19:12:24 +0100 Subject: [PATCH 3/3] less unsupported errors in Miri, and clarifying comments --- src/diagnostics.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/diagnostics.rs b/src/diagnostics.rs index ec81ffd3cd5c9..0cfa3812e400d 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -229,13 +229,13 @@ pub fn report_error<'tcx, 'mir>( Unsupported( UnsupportedOpInfo::ThreadLocalStatic(_) | UnsupportedOpInfo::ReadExternStatic(_) | - UnsupportedOpInfo::PartialPointerOverwrite(_) | // we make memory uninit instead + UnsupportedOpInfo::PartialPointerOverwrite(_) | + UnsupportedOpInfo::PartialPointerCopy(_) | UnsupportedOpInfo::ReadPointerAsBytes ) => panic!("Error should never be raised by Miri: {kind:?}", kind = e.kind()), Unsupported( - UnsupportedOpInfo::Unsupported(_) | - UnsupportedOpInfo::PartialPointerCopy(_) + UnsupportedOpInfo::Unsupported(_) ) => vec![(None, 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 { .. })