Auto merge of #2614 - saethlin:stack-inspection-tools, r=RalfJung

Improve miri_print_borrow_stacks

Per post-merge review on rust-lang/miri#2322

* `miri_print_stacks` renamed to `miri_print_borrow_stacks`
* A bit more details in docs, clarified how unstable these functions are meant to be
* Print an `unknown_bottom` if one exists

Open question: Currently `miri_get_alloc_id` gets the expected `AllocId` for `Wildcard` pointers, but for pointers with no provenance, the function reports UB and halts the interpreter. That's definitely wrong. But what _should_ we do? Is it reasonable to check if the pointer has `None` provenance and try to get an `AllocId` for its address? That still leaves us with a failure path, which in this case might be best-handled as an ICE? I'm just not sure that changing the return type of `miri_get_alloc_id` to `Option` is a win because it complicates all normal uses of this.

Author: bors

src/tools/miri/README.md

@@ -538,15 +538,23 @@ 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. 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.
     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

src/tools/miri/src/shims/foreign_items.rs

@@ -420,10 +420,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "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) {

src/tools/miri/src/stacked_borrows/mod.rs

@@ -1154,6 +1154,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, '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());

src/tools/miri/tests/pass/stacked-borrows/stack-printing.rs

@@ -1,29 +1,46 @@
+//@compile-flags: -Zmiri-permissive-provenance
+#![feature(strict_provenance)]
 use std::{
     alloc::{self, Layout},
     mem::ManuallyDrop,
 };
 
 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::<u8>()) };
-    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::<u8>()) };
 }

src/tools/miri/tests/pass/stacked-borrows/stack-printing.stdout

@@ -3,3 +3,4 @@
 0..1: [ SharedReadWrite<TAG> ]
 0..1: [ SharedReadWrite<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> ]
 0..1: [ SharedReadWrite<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> SharedReadOnly<TAG> ]
+0..1: [ unknown-bottom(..<TAG>) ]