From d86bbd5094badaf55ade2073fbb41835ffc641b6 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 23 Oct 2022 12:32:45 -0400 Subject: [PATCH 1/2] Rename, improve docs, fail better --- src/tools/miri/README.md | 15 ++++++--- src/tools/miri/src/shims/foreign_items.rs | 8 +++-- src/tools/miri/src/stacked_borrows/mod.rs | 3 ++ .../pass/stacked-borrows/stack-printing.rs | 31 ++++++++++++++----- .../stacked-borrows/stack-printing.stdout | 1 + 5 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 81c4f5ffef4..bd175b46b7a 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -538,15 +538,20 @@ extern "Rust" { fn miri_start_panic(payload: *mut u8) -> !; /// Miri-provided extern function to get the internal unique identifier for the allocation that a pointer - /// points to. This is only useful as an input to `miri_print_stacks`, and it is a separate call because + /// points to. This is only useful as an input to `miri_print_borrow_stacks`, and it is a separate call because /// getting a pointer to an allocation at runtime can change the borrow stacks in the allocation. + /// This function should be considered unstable. It exists only to support `miri_print_borrow_stacks` and so + /// inherits all of its instability. fn miri_get_alloc_id(ptr: *const ()) -> u64; /// Miri-provided extern function to print (from the interpreter, not the program) the contents of all - /// borrow stacks in an allocation. The format of what this emits is unstable and may change at any time. - /// In particular, users should be aware that Miri will periodically attempt to garbage collect the - /// contents of all stacks. Callers of this function may wish to pass `-Zmiri-tag-gc=0` to disable the GC. - fn miri_print_stacks(alloc_id: u64); + /// borrow stacks in an allocation. The leftmost tag is the bottom of the stack. + /// The format of what this emits is unstable and may change at any time. In particular, users should be + /// aware that Miri will periodically attempt to garbage collect the contents of all stacks. Callers of + /// this function may wish to pass `-Zmiri-tag-gc=0` to disable the GC. + /// This function is extremely unstable. At any time the format of its output may change, its signature may + /// change, or it may be removed entirely. + fn miri_print_borrow_stacks(alloc_id: u64); /// Miri-provided extern function to print (from the interpreter, not the /// program) the contents of a section of program memory, as bytes. Bytes diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 9d0e47cd06f..1b3205aabc9 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -420,10 +420,14 @@ fn emulate_foreign_item_by_name( "miri_get_alloc_id" => { let [ptr] = this.check_shim(abi, Abi::Rust, link_name, args)?; let ptr = this.read_pointer(ptr)?; - let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr)?; + let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr).map_err(|_e| { + err_machine_stop!(TerminationInfo::Abort( + format!("pointer passed to miri_get_alloc_id must not be dangling, got {ptr:?}") + )) + })?; this.write_scalar(Scalar::from_u64(alloc_id.0.get()), dest)?; } - "miri_print_stacks" => { + "miri_print_borrow_stacks" => { let [id] = this.check_shim(abi, Abi::Rust, link_name, args)?; let id = this.read_scalar(id)?.to_u64()?; if let Some(id) = std::num::NonZeroU64::new(id) { diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index a2f003e6cc8..cc27b71eb56 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -1154,6 +1154,9 @@ fn print_stacks(&mut self, alloc_id: AllocId) -> InterpResult<'tcx> { let stacks = alloc_extra.stacked_borrows.as_ref().unwrap().borrow(); for (range, stack) in stacks.stacks.iter_all() { print!("{range:?}: ["); + if let Some(bottom) = stack.unknown_bottom() { + print!(" unknown-bottom(..{bottom:?})"); + } for i in 0..stack.len() { let item = stack.get(i).unwrap(); print!(" {:?}{:?}", item.perm(), item.tag()); diff --git a/src/tools/miri/tests/pass/stacked-borrows/stack-printing.rs b/src/tools/miri/tests/pass/stacked-borrows/stack-printing.rs index 8d96a2e1ca9..3ca937ae13d 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/stack-printing.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/stack-printing.rs @@ -1,3 +1,5 @@ +//@compile-flags: -Zmiri-permissive-provenance +#![feature(strict_provenance)] use std::{ alloc::{self, Layout}, mem::ManuallyDrop, @@ -5,25 +7,40 @@ extern "Rust" { fn miri_get_alloc_id(ptr: *const u8) -> u64; - fn miri_print_stacks(alloc_id: u64); + fn miri_print_borrow_stacks(alloc_id: u64); +} + +fn get_alloc_id(ptr: *const u8) -> u64 { + unsafe { miri_get_alloc_id(ptr) } +} + +fn print_borrow_stacks(alloc_id: u64) { + unsafe { miri_print_borrow_stacks(alloc_id) } } fn main() { let ptr = unsafe { alloc::alloc(Layout::new::()) }; - let alloc_id = unsafe { miri_get_alloc_id(ptr) }; - unsafe { miri_print_stacks(alloc_id) }; + let alloc_id = get_alloc_id(ptr); + print_borrow_stacks(alloc_id); assert!(!ptr.is_null()); - unsafe { miri_print_stacks(alloc_id) }; + print_borrow_stacks(alloc_id); unsafe { *ptr = 42 }; - unsafe { miri_print_stacks(alloc_id) }; + print_borrow_stacks(alloc_id); let _b = unsafe { ManuallyDrop::new(Box::from_raw(ptr)) }; - unsafe { miri_print_stacks(alloc_id) }; + print_borrow_stacks(alloc_id); let _ptr = unsafe { &*ptr }; - unsafe { miri_print_stacks(alloc_id) }; + print_borrow_stacks(alloc_id); + + // Create an unknown bottom, and print it + let ptr = ptr as usize as *mut u8; + unsafe { + *ptr = 5; + } + print_borrow_stacks(alloc_id); unsafe { alloc::dealloc(ptr, Layout::new::()) }; } diff --git a/src/tools/miri/tests/pass/stacked-borrows/stack-printing.stdout b/src/tools/miri/tests/pass/stacked-borrows/stack-printing.stdout index 660ee71e6f5..83873307820 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/stack-printing.stdout +++ b/src/tools/miri/tests/pass/stacked-borrows/stack-printing.stdout @@ -3,3 +3,4 @@ 0..1: [ SharedReadWrite ] 0..1: [ SharedReadWrite Unique Unique Unique Unique Unique ] 0..1: [ SharedReadWrite Disabled Disabled Disabled Disabled Disabled SharedReadOnly ] +0..1: [ unknown-bottom(..) ] From 71e6815885f54631a2035e0c4d180a6de25ebccf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 26 Oct 2022 09:58:21 +0200 Subject: [PATCH 2/2] tweak docs --- src/tools/miri/README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index bd175b46b7a..5803a88c0e7 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -538,7 +538,9 @@ extern "Rust" { fn miri_start_panic(payload: *mut u8) -> !; /// Miri-provided extern function to get the internal unique identifier for the allocation that a pointer - /// points to. This is only useful as an input to `miri_print_borrow_stacks`, and it is a separate call because + /// points to. If this pointer is invalid (not pointing to an allocation), interpretation will abort. + /// + /// This is only useful as an input to `miri_print_borrow_stacks`, and it is a separate call because /// getting a pointer to an allocation at runtime can change the borrow stacks in the allocation. /// This function should be considered unstable. It exists only to support `miri_print_borrow_stacks` and so /// inherits all of its instability. @@ -549,6 +551,7 @@ extern "Rust" { /// The format of what this emits is unstable and may change at any time. In particular, users should be /// aware that Miri will periodically attempt to garbage collect the contents of all stacks. Callers of /// this function may wish to pass `-Zmiri-tag-gc=0` to disable the GC. + /// /// This function is extremely unstable. At any time the format of its output may change, its signature may /// change, or it may be removed entirely. fn miri_print_borrow_stacks(alloc_id: u64);