Auto merge of #2030 - saethlin:track-alloc-history, r=oli-obk

Print spans where tags are created and invalidated

5225225 called this "automatic tag tracking" and I think that may be a reasonable description, but I would like to kill tag tracking as a primary use of Miri if possible. Tag tracking isn't always possible; for example if the UB is only detected with isolation off and the failing tag is made unstable by removing isolation. (also it's bad UX to run the tool twice)

This is just one of the things we can do with #2024

The memory usage of this is _shockingly_ low, I think because the memory usage of Miri is driven by allocations where each byte ends up with its own very large stack. The memory usage in this change is linear with the number of tags, not tags * bytes. If memory usage gets out of control we can cap the number of events we save per allocation, from experience we tend to only use the most recent few in diagnostics but of course there's no guarantee of that so if we can manage to keep everything that would be best.

In many cases now I can tell exactly what these codebases are doing wrong just from the new outputs here, which I think is extremely cool.

New helps generated with plain old `cargo miri test` on `rust-argon2` v1.0.0:
```
test argon2::tests::single_thread_verification_multi_lane_hash ... error: Undefined Behavior: trying to reborrow <1485898> for Unique permission at alloc110523[0x0], but that tag does not exist in the borrow stack for this location
   --> /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/mem/manually_drop.rs:89:9
    |
89  |         slot.value
    |         ^^^^^^^^^^
    |         |
    |         trying to reborrow <1485898> for Unique permission at alloc110523[0x0], but that tag does not exist in the borrow stack for this location
    |         this error occurs as part of a reborrow at alloc110523[0x0..0x20]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <1485898> was created by a retag at offsets [0x0..0x20]
   --> src/memory.rs:42:13
    |
42  |             vec.push(unsafe { &mut (*ptr) });
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <1485898> was later invalidated at offsets [0x0..0x20]
   --> src/memory.rs:42:31
    |
42  |             vec.push(unsafe { &mut (*ptr) });
    |                               ^^^^^^^^^^^
```

And with `-Zmiri-tag-raw-pointers` on `slab` v0.4.5
```
error: Undefined Behavior: trying to reborrow <2915> for Unique permission at alloc1418[0x0], but that tag does not exist in the borrow stack for this location
   --> /tmp/slab-0.4.5/src/lib.rs:835:16
    |
835 |         match (&mut *ptr1, &mut *ptr2) {
    |                ^^^^^^^^^^
    |                |
    |                trying to reborrow <2915> for Unique permission at alloc1418[0x0], but that tag does not exist in the borrow stack for this location
    |                this error occurs as part of a reborrow at alloc1418[0x0..0x10]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <2915> was created by a retag at offsets [0x0..0x10]
   --> /tmp/slab-0.4.5/src/lib.rs:833:20
    |
833 |         let ptr1 = self.entries.get_unchecked_mut(key1) as *mut Entry<T>;
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <2915> was later invalidated at offsets [0x0..0x20]
   --> /tmp/slab-0.4.5/src/lib.rs:834:20
    |
834 |         let ptr2 = self.entries.get_unchecked_mut(key2) as *mut Entry<T>;
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

And without raw pointer tagging, `cargo miri test` on `half` v1.8.2
```
error: Undefined Behavior: trying to reborrow <untagged> for Unique permission at alloc1340[0x0], but that tag only grants SharedReadOnly permission for this location
   --> /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/slice/raw.rs:141:9
    |
141 |         &mut *ptr::slice_from_raw_parts_mut(data, len)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |         |
    |         trying to reborrow <untagged> for Unique permission at alloc1340[0x0], but that tag only grants SharedReadOnly permission for this location
    |         this error occurs as part of a reborrow at alloc1340[0x0..0x6]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: tag was most recently created at offsets [0x0..0x6]
   --> /tmp/half-1.8.2/src/slice.rs:309:22
    |
309 |         let length = self.len();
    |                      ^^^^^^^^^^
help: this tag was also created here at offsets [0x0..0x6]
   --> /tmp/half-1.8.2/src/slice.rs:308:23
    |
308 |         let pointer = self.as_ptr() as *mut u16;
    |                       ^^^^^^^^^^^^^
```
The second suggestion is close to guesswork, but from experience it tends to be correct (as in, it tends to locate the pointer the user wanted) more often that it doesn't.

Author: bors

src/diagnostics.rs

@@ -7,7 +7,8 @@ use log::trace;
 use rustc_middle::ty;
 use rustc_span::{source_map::DUMMY_SP, Span, SpanData, Symbol};
 
-use crate::stacked_borrows::{AccessKind, SbTag};
+use crate::helpers::HexRange;
+use crate::stacked_borrows::{diagnostics::TagHistory, AccessKind, SbTag};
 use crate::*;
 
 /// Details of premature program termination.
@@ -19,6 +20,7 @@ pub enum TerminationInfo {
         msg: String,
         help: Option<String>,
         url: String,
+        history: Option<TagHistory>,
     },
     Deadlock,
     MultipleSymbolDefinitions {
@@ -155,12 +157,46 @@ pub fn report_error<'tcx, 'mir>(
                         (None, format!("pass the flag `-Zmiri-disable-isolation` to disable isolation;")),
                         (None, format!("or pass `-Zmiri-isolation-error=warn` to configure Miri to return an error code from isolated operations (if supported for that operation) and continue with a warning")),
                     ],
-                ExperimentalUb { url, help, .. } => {
+                ExperimentalUb { url, help, history, .. } => {
                     msg.extend(help.clone());
-                    vec![
+                    let mut helps = vec![
                         (None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental")),
-                        (None, format!("see {} for further information", url))
-                    ]
+                        (None, format!("see {} for further information", url)),
+                    ];
+                    match history {
+                        Some(TagHistory::Tagged {tag, created: (created_range, created_span), invalidated, protected }) => {
+                            let msg = format!("{:?} was created by a retag at offsets {}", tag, HexRange(*created_range));
+                            helps.push((Some(created_span.clone()), msg));
+                            if let Some((invalidated_range, invalidated_span)) = invalidated {
+                                let msg = format!("{:?} was later invalidated at offsets {}", tag, HexRange(*invalidated_range));
+                                helps.push((Some(invalidated_span.clone()), msg));
+                            }
+                            if let Some((protecting_tag, protecting_tag_span, protection_span)) = protected {
+                                helps.push((Some(protecting_tag_span.clone()), format!("{:?} was protected due to {:?} which was created here", tag, protecting_tag)));
+                                helps.push((Some(protection_span.clone()), "this protector is live for this call".to_string()));
+                            }
+                        }
+                        Some(TagHistory::Untagged{ recently_created, recently_invalidated, matching_created, protected }) => {
+                            if let Some((range, span)) = recently_created {
+                                let msg = format!("tag was most recently created at offsets {}", HexRange(*range));
+                                helps.push((Some(span.clone()), msg));
+                            }
+                            if let Some((range, span)) = recently_invalidated {
+                                let msg = format!("tag was later invalidated at offsets {}", HexRange(*range));
+                                helps.push((Some(span.clone()), msg));
+                            }
+                            if let Some((range, span)) = matching_created {
+                                let msg = format!("this tag was also created here at offsets {}", HexRange(*range));
+                                helps.push((Some(span.clone()), msg));
+                            }
+                            if let Some((protecting_tag, protecting_tag_span, protection_span)) = protected {
+                                helps.push((Some(protecting_tag_span.clone()), format!("{:?} was protected due to a tag which was created here", protecting_tag)));
+                                helps.push((Some(protection_span.clone()), "this protector is live for this call".to_string()));
+                            }
+                        }
+                        None => {}
+                    }
+                    helps
                 }
                 MultipleSymbolDefinitions { first, first_crate, second, second_crate, .. } =>
                     vec![

src/helpers.rs

@@ -2,6 +2,7 @@ pub mod convert;
 
 use std::mem;
 use std::num::NonZeroUsize;
+use std::rc::Rc;
 use std::time::Duration;
 
 use log::trace;
@@ -816,7 +817,7 @@ pub fn isolation_abort_error(name: &str) -> InterpResult<'static> {
 
 /// Retrieve the list of local crates that should have been passed by cargo-miri in
 /// MIRI_LOCAL_CRATES and turn them into `CrateNum`s.
-pub fn get_local_crates(tcx: &TyCtxt<'_>) -> Vec<CrateNum> {
+pub fn get_local_crates(tcx: &TyCtxt<'_>) -> Rc<[CrateNum]> {
     // Convert the local crate names from the passed-in config into CrateNums so that they can
     // be looked up quickly during execution
     let local_crate_names = std::env::var("MIRI_LOCAL_CRATES")
@@ -830,5 +831,14 @@ pub fn get_local_crates(tcx: &TyCtxt<'_>) -> Vec<CrateNum> {
             local_crates.push(crate_num);
         }
     }
-    local_crates
+    Rc::from(local_crates.as_slice())
+}
+
+/// Formats an AllocRange like [0x1..0x3], for use in diagnostics.
+pub struct HexRange(pub AllocRange);
+
+impl std::fmt::Display for HexRange {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "[{:#x}..{:#x}]", self.0.start.bytes(), self.0.end().bytes())
+    }
 }

src/machine.rs

@@ -6,6 +6,7 @@ use std::cell::RefCell;
 use std::collections::HashSet;
 use std::fmt;
 use std::num::NonZeroU64;
+use std::rc::Rc;
 use std::time::Instant;
 
 use rand::rngs::StdRng;
@@ -273,7 +274,7 @@ pub struct Evaluator<'mir, 'tcx> {
     pub(crate) backtrace_style: BacktraceStyle,
 
     /// Crates which are considered local for the purposes of error reporting.
-    pub(crate) local_crates: Vec<CrateNum>,
+    pub(crate) local_crates: Rc<[CrateNum]>,
 
     /// Mapping extern static names to their base pointer.
     extern_statics: FxHashMap<Symbol, Pointer<Tag>>,
@@ -569,7 +570,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         let kind = kind.expect("we set our STATIC_KIND so this cannot be None");
         let alloc = alloc.into_owned();
         let stacks = if let Some(stacked_borrows) = &ecx.machine.stacked_borrows {
-            Some(Stacks::new_allocation(id, alloc.size(), stacked_borrows, kind))
+            Some(Stacks::new_allocation(
+                id,
+                alloc.size(),
+                stacked_borrows,
+                kind,
+                &ecx.machine.threads,
+                ecx.machine.local_crates.clone(),
+            ))
         } else {
             None
         };
@@ -634,6 +642,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
                 tag,
                 range,
                 machine.stacked_borrows.as_ref().unwrap(),
+                &machine.threads,
             )
         } else {
             Ok(())
@@ -656,7 +665,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
                 alloc_id,
                 tag,
                 range,
-                machine.stacked_borrows.as_mut().unwrap(),
+                machine.stacked_borrows.as_ref().unwrap(),
+                &machine.threads,
             )
         } else {
             Ok(())
@@ -682,7 +692,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
                 alloc_id,
                 tag,
                 range,
-                machine.stacked_borrows.as_mut().unwrap(),
+                machine.stacked_borrows.as_ref().unwrap(),
             )
         } else {
             Ok(())