Auto merge of #1080 - rust-lang:stacked_borrow_tracing, r=RalfJung

Add a scheme to find the place where an id was destroyed

cc https://github.com/rust-lang/miri/issues/974

I'm not too happy with it, but since stacked borrows don't have access to the current call stack, I can't just report a warning as per https://github.com/rust-lang/miri/issues/797

We could add some global mutex that we can throw strings at and `step` will clear out that mutex and report warnings before moving the `statement_id` or the `block_id`, not sure how well that would work. For now I think this is sufficient

README.md

@@ -160,7 +160,7 @@ Several `-Z` flags are relevant for Miri:
   the program has access to host resources such as environment variables and
   randomness (and, eventually, file systems and more).
 * `-Zmiri-ignore-leaks` disables the memory leak checker.
-* `-Zmiri-env-exclude=<var>` keeps the `var` environment variable isolated from 
+* `-Zmiri-env-exclude=<var>` keeps the `var` environment variable isolated from
   the host. Can be used multiple times to exclude several variables. The `TERM`
   environment variable is excluded by default.
 * `-Zmir-opt-level` controls how many MIR optimizations are performed.  Miri
@@ -171,6 +171,11 @@ Several `-Z` flags are relevant for Miri:
   sets this flag per default.
 * `-Zmir-emit-retag` controls whether `Retag` statements are emitted. Miri
   enables this per default because it is needed for validation.
+* `-Zmiri-track-pointer-tag=<tag>` aborts interpretation with a backtrace when the
+  given pointer tag is popped from a borrow stack (which is where the tag
+  becomes invalid and any future use of it will error anyway).  This helps you
+  in finding out why UB is happening and where in your code would be a good
+  place to look for it.
 
 Moreover, Miri recognizes some environment variables:
 

benches/helpers/miri_helper.rs

@@ -32,6 +32,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls<'_> {
                     excluded_env_vars: vec![],
                     args: vec![],
                     seed: None,
+                    tracked_pointer_tag: None,
                 };
                 eval_main(tcx, entry_def_id, config);
             });

src/bin/miri-rustc-tests.rs

@@ -45,6 +45,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls {
                                     excluded_env_vars: vec![],
                                     args: vec![],
                                     seed: None,
+                                    tracked_pointer_tag: None,
                                 };
                                 let did = self.0.hir().body_owner_def_id(body_id);
                                 println!("running test: {}", self.0.def_path_debug_str(did));
@@ -64,7 +65,8 @@ impl rustc_driver::Callbacks for MiriCompilerCalls {
                     ignore_leaks: false,
                     excluded_env_vars: vec![],
                     args: vec![],
-                    seed: None
+                    seed: None,
+                    tracked_pointer_tag: None,
                 };
                 miri::eval_main(tcx, entry_def_id, config);
 

src/bin/miri.rs

@@ -126,6 +126,7 @@ fn main() {
     let mut communicate = false;
     let mut ignore_leaks = false;
     let mut seed: Option<u64> = None;
+    let mut tracked_pointer_tag: Option<miri::PtrId> = None;
     let mut rustc_args = vec![];
     let mut miri_args = vec![];
     let mut after_dashdash = false;
@@ -176,6 +177,17 @@ fn main() {
                 arg if arg.starts_with("-Zmiri-env-exclude=") => {
                     excluded_env_vars.push(arg.trim_start_matches("-Zmiri-env-exclude=").to_owned());
                 },
+                arg if arg.starts_with("-Zmiri-track-pointer-tag=") => {
+                    let id: u64 = match arg.trim_start_matches("-Zmiri-track-pointer-tag=").parse() {
+                        Ok(id) => id,
+                        Err(err) => panic!("-Zmiri-track-pointer-tag requires a valid `u64` as the argument: {}", err),
+                    };
+                    if let Some(id) = miri::PtrId::new(id) {
+                        tracked_pointer_tag = Some(id);
+                    } else {
+                        panic!("-Zmiri-track-pointer-tag must be a nonzero id");
+                    }
+                },
                 _ => {
                     rustc_args.push(arg);
                 }
@@ -208,6 +220,7 @@ fn main() {
         excluded_env_vars,
         seed,
         args: miri_args,
+        tracked_pointer_tag,
     };
     rustc_driver::install_ice_hook();
     let result = rustc_driver::catch_fatal_errors(move || {

src/eval.rs

@@ -26,11 +26,14 @@ pub struct MiriConfig {
     pub args: Vec<String>,
     /// The seed to use when non-determinism or randomness are required (e.g. ptr-to-int cast, `getrandom()`).
     pub seed: Option<u64>,
+    /// The stacked borrow id to report about
+    pub tracked_pointer_tag: Option<PtrId>,
 }
 
 /// Details of premature program termination.
 pub enum TerminationInfo {
     Exit(i64),
+    PoppedTrackedPointerTag(Item),
     Abort,
 }
 
@@ -47,7 +50,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
         tcx.at(syntax::source_map::DUMMY_SP),
         ty::ParamEnv::reveal_all(),
         Evaluator::new(config.communicate),
-        MemoryExtra::new(StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate),
+        MemoryExtra::new(StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate, config.tracked_pointer_tag),
     );
     // Complete initialization.
     EnvVars::init(&mut ecx, config.excluded_env_vars);
@@ -216,6 +219,8 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
                         .expect("invalid MachineStop payload");
                     match info {
                         TerminationInfo::Exit(code) => return Some(*code),
+                        TerminationInfo::PoppedTrackedPointerTag(item) =>
+                            format!("popped tracked tag for item {:?}", item),
                         TerminationInfo::Abort =>
                             format!("the evaluated program aborted execution")
                     }

src/lib.rs

@@ -43,7 +43,10 @@ pub use crate::operator::EvalContextExt as OperatorEvalContextExt;
 pub use crate::range_map::RangeMap;
 pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt};
 pub use crate::mono_hash_map::MonoHashMap;
-pub use crate::stacked_borrows::{EvalContextExt as StackedBorEvalContextExt, Tag, Permission, Stack, Stacks, Item};
+pub use crate::stacked_borrows::{
+    EvalContextExt as StackedBorEvalContextExt, Tag, Permission, Stack, Stacks, Item, PtrId,
+    GlobalState,
+};
 pub use crate::machine::{
     PAGE_SIZE, STACK_ADDR, STACK_SIZE, NUM_CPUS,
     MemoryExtra, AllocExtra, FrameData, MiriMemoryKind, Evaluator, MiriEvalContext, MiriEvalContextExt,

src/machine.rs

@@ -77,9 +77,9 @@ pub struct MemoryExtra {
 }
 
 impl MemoryExtra {
-    pub fn new(rng: StdRng, validate: bool) -> Self {
+    pub fn new(rng: StdRng, validate: bool, tracked_pointer_tag: Option<PtrId>) -> Self {
         MemoryExtra {
-            stacked_borrows: Default::default(),
+            stacked_borrows: Rc::new(RefCell::new(GlobalState::new(tracked_pointer_tag))),
             intptrcast: Default::default(),
             rng: RefCell::new(rng),
             validate,

src/stacked_borrows.rs

@@ -12,7 +12,7 @@ use rustc::hir::Mutability::{Mutable, Immutable};
 use rustc::mir::RetagKind;
 
 use crate::{
-    InterpResult, HelpersEvalContextExt,
+    InterpResult, HelpersEvalContextExt, TerminationInfo,
     MemoryKind, MiriMemoryKind, RangeMap, AllocId, Pointer, Immediate, ImmTy, PlaceTy, MPlaceTy,
 };
 
@@ -105,6 +105,8 @@ pub struct GlobalState {
     next_call_id: CallId,
     /// Those call IDs corresponding to functions that are still running.
     active_calls: HashSet<CallId>,
+    /// The id to trace in this execution run
+    tracked_pointer_tag: Option<PtrId>,
 }
 /// Memory extra state gives us interior mutable access to the global state.
 pub type MemoryExtra = Rc<RefCell<GlobalState>>;
@@ -151,18 +153,17 @@ impl fmt::Display for RefKind {
 }
 
 /// Utilities for initialization and ID generation
-impl Default for GlobalState {
-    fn default() -> Self {
+impl GlobalState {
+    pub fn new(tracked_pointer_tag: Option<PtrId>) -> Self {
         GlobalState {
             next_ptr_id: NonZeroU64::new(1).unwrap(),
             base_ptr_ids: HashMap::default(),
             next_call_id: NonZeroU64::new(1).unwrap(),
             active_calls: HashSet::default(),
+            tracked_pointer_tag,
         }
     }
-}
 
-impl GlobalState {
     fn new_ptr(&mut self) -> PtrId {
         let id = self.next_ptr_id;
         self.next_ptr_id = NonZeroU64::new(id.get() + 1).unwrap();
@@ -270,6 +271,11 @@ impl<'tcx> Stack {
 
     /// Check if the given item is protected.
     fn check_protector(item: &Item, tag: Option<Tag>, global: &GlobalState) -> InterpResult<'tcx> {
+        if let Tag::Tagged(id) = item.tag {
+            if Some(id) == global.tracked_pointer_tag {
+                throw_machine_stop!(TerminationInfo::PoppedTrackedPointerTag(item.clone()));
+            }
+        }
         if let Some(call) = item.protector {
             if global.is_active(call) {
                 if let Some(tag) = tag {