Auto merge of #2075 - y86-dev:master, r=RalfJung

Allow to track multiple alloc-ids, call-ids and pointer tags

Closes #2073.

Author: bors

README.md

@@ -314,16 +314,20 @@ environment variable:
   ensure alignment.  (The standard library `align_to` method works fine in both
   modes; under symbolic alignment it only fills the middle slice when the
   allocation guarantees sufficient alignment.)
-* `-Zmiri-track-alloc-id=<id>` shows a backtrace when the given allocation is
+* `-Zmiri-track-alloc-id=<id1>,<id2>,...` shows a backtrace when the given allocations are
   being allocated or freed.  This helps in debugging memory leaks and
-  use after free bugs.
-* `-Zmiri-track-call-id=<id>` shows a backtrace when the given call id is
+  use after free bugs. Specifying this argument multiple times does not overwrite the previous
+  values, instead it appends its values to the list. Listing an id multiple times has no effect.
+* `-Zmiri-track-call-id=<id1>,<id2>,...` shows a backtrace when the given call ids are
   assigned to a stack frame.  This helps in debugging UB related to Stacked
-  Borrows "protectors".
-* `-Zmiri-track-pointer-tag=<tag>` shows a backtrace when the given pointer tag
+  Borrows "protectors". Specifying this argument multiple times does not overwrite the previous
+  values, instead it appends its values to the list. Listing an id multiple times has no effect.
+* `-Zmiri-track-pointer-tag=<tag1>,<tag2>,...` shows a backtrace when a given pointer tag
   is popped from a borrow stack (which is where the tag becomes invalid and any
   future use of it will error).  This helps you in finding out why UB is
   happening and where in your code would be a good place to look for it.
+  Specifying this argument multiple times does not overwrite the previous
+  values, instead it appends its values to the list. Listing a tag multiple times has no effect.
 * `-Zmiri-tag-raw-pointers` makes Stacked Borrows assign proper tags even for raw pointers. This can
   make valid code using int-to-ptr casts fail to pass the checks, but also can help identify latent
   aliasing issues in code that Miri accepts by default. You can recognize false positives by

src/bin/miri.rs

@@ -251,6 +251,13 @@ fn run_compiler(
     std::process::exit(exit_code)
 }
 
+/// Parses a comma separated list of `T` from the given string:
+///
+/// `<value1>,<value2>,<value3>,...`
+fn parse_comma_list<T: FromStr>(input: &str) -> Result<Vec<T>, T::Err> {
+    input.split(',').map(str::parse::<T>).collect()
+}
+
 fn main() {
     rustc_driver::install_ice_hook();
 
@@ -397,46 +404,55 @@ fn main() {
                         .push(arg.strip_prefix("-Zmiri-env-forward=").unwrap().to_owned());
                 }
                 arg if arg.starts_with("-Zmiri-track-pointer-tag=") => {
-                    let id: u64 =
-                        match arg.strip_prefix("-Zmiri-track-pointer-tag=").unwrap().parse() {
-                            Ok(id) => id,
-                            Err(err) =>
-                                panic!(
-                                    "-Zmiri-track-pointer-tag requires a valid `u64` argument: {}",
-                                    err
-                                ),
-                        };
-                    if let Some(id) = miri::PtrId::new(id) {
-                        miri_config.tracked_pointer_tag = Some(id);
-                    } else {
-                        panic!("-Zmiri-track-pointer-tag requires a nonzero argument");
+                    let ids: Vec<u64> = match parse_comma_list(
+                        arg.strip_prefix("-Zmiri-track-pointer-tag=").unwrap(),
+                    ) {
+                        Ok(ids) => ids,
+                        Err(err) =>
+                            panic!(
+                                "-Zmiri-track-pointer-tag requires a comma separated list of valid `u64` arguments: {}",
+                                err
+                            ),
+                    };
+                    for id in ids.into_iter().map(miri::PtrId::new) {
+                        if let Some(id) = id {
+                            miri_config.tracked_pointer_tags.insert(id);
+                        } else {
+                            panic!("-Zmiri-track-pointer-tag requires nonzero arguments");
+                        }
                     }
                 }
                 arg if arg.starts_with("-Zmiri-track-call-id=") => {
-                    let id: u64 = match arg.strip_prefix("-Zmiri-track-call-id=").unwrap().parse() {
-                        Ok(id) => id,
+                    let ids: Vec<u64> = match parse_comma_list(
+                        arg.strip_prefix("-Zmiri-track-call-id=").unwrap(),
+                    ) {
+                        Ok(ids) => ids,
                         Err(err) =>
-                            panic!("-Zmiri-track-call-id requires a valid `u64` argument: {}", err),
+                            panic!(
+                                "-Zmiri-track-call-id requires a comma separated list of valid `u64` arguments: {}",
+                                err
+                            ),
                     };
-                    if let Some(id) = miri::CallId::new(id) {
-                        miri_config.tracked_call_id = Some(id);
-                    } else {
-                        panic!("-Zmiri-track-call-id requires a nonzero argument");
+                    for id in ids.into_iter().map(miri::CallId::new) {
+                        if let Some(id) = id {
+                            miri_config.tracked_call_ids.insert(id);
+                        } else {
+                            panic!("-Zmiri-track-call-id requires a nonzero argument");
+                        }
                     }
                 }
                 arg if arg.starts_with("-Zmiri-track-alloc-id=") => {
-                    let id = match arg
-                        .strip_prefix("-Zmiri-track-alloc-id=")
-                        .unwrap()
-                        .parse()
-                        .ok()
-                        .and_then(NonZeroU64::new)
-                    {
-                        Some(id) => id,
-                        None =>
-                            panic!("-Zmiri-track-alloc-id requires a valid non-zero `u64` argument"),
+                    let ids: Vec<miri::AllocId> = match parse_comma_list::<NonZeroU64>(
+                        arg.strip_prefix("-Zmiri-track-alloc-id=").unwrap(),
+                    ) {
+                        Ok(ids) => ids.into_iter().map(miri::AllocId).collect(),
+                        Err(err) =>
+                            panic!(
+                                "-Zmiri-track-alloc-id requires a comma separated list of valid non-zero `u64` arguments: {}",
+                                err
+                            ),
                     };
-                    miri_config.tracked_alloc_id = Some(miri::AllocId(id));
+                    miri_config.tracked_alloc_ids.extend(ids);
                 }
                 arg if arg.starts_with("-Zmiri-compare-exchange-weak-failure-rate=") => {
                     let rate = match arg

src/eval.rs

@@ -15,6 +15,8 @@ use rustc_target::spec::abi::Abi;
 
 use rustc_session::config::EntryFnType;
 
+use std::collections::HashSet;
+
 use crate::*;
 
 #[derive(Copy, Clone, Debug, PartialEq)]
@@ -91,12 +93,12 @@ 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 borrows pointer id to report about
-    pub tracked_pointer_tag: Option<PtrId>,
-    /// The stacked borrows call ID to report about
-    pub tracked_call_id: Option<CallId>,
-    /// The allocation id to report about.
-    pub tracked_alloc_id: Option<AllocId>,
+    /// The stacked borrows pointer ids to report about
+    pub tracked_pointer_tags: HashSet<PtrId>,
+    /// The stacked borrows call IDs to report about
+    pub tracked_call_ids: HashSet<CallId>,
+    /// The allocation ids to report about.
+    pub tracked_alloc_ids: HashSet<AllocId>,
     /// Whether to track raw pointers in stacked borrows.
     pub tag_raw: bool,
     /// Determine if data race detection should be enabled
@@ -130,9 +132,9 @@ impl Default for MiriConfig {
             forwarded_env_vars: vec![],
             args: vec![],
             seed: None,
-            tracked_pointer_tag: None,
-            tracked_call_id: None,
-            tracked_alloc_id: None,
+            tracked_pointer_tags: HashSet::default(),
+            tracked_call_ids: HashSet::default(),
+            tracked_alloc_ids: HashSet::default(),
             tag_raw: false,
             data_race_detector: true,
             cmpxchg_weak_failure_rate: 0.8,

src/machine.rs

@@ -3,6 +3,7 @@
 
 use std::borrow::Cow;
 use std::cell::RefCell;
+use std::collections::HashSet;
 use std::fmt;
 use std::num::NonZeroU64;
 use std::time::Instant;
@@ -281,9 +282,9 @@ pub struct Evaluator<'mir, 'tcx> {
     /// Needs to be queried by ptr_to_int, hence needs interior mutability.
     pub(crate) rng: RefCell<StdRng>,
 
-    /// An allocation ID to report when it is being allocated
+    /// The allocation IDs to report when they are being allocated
     /// (helps for debugging memory leaks and use after free bugs).
-    tracked_alloc_id: Option<AllocId>,
+    tracked_alloc_ids: HashSet<AllocId>,
 
     /// Controls whether alignment of memory accesses is being checked.
     pub(crate) check_alignment: AlignmentCheck,
@@ -303,8 +304,8 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
         let rng = StdRng::seed_from_u64(config.seed.unwrap_or(0));
         let stacked_borrows = if config.stacked_borrows {
             Some(RefCell::new(stacked_borrows::GlobalStateInner::new(
-                config.tracked_pointer_tag,
-                config.tracked_call_id,
+                config.tracked_pointer_tags.clone(),
+                config.tracked_call_ids.clone(),
                 config.tag_raw,
             )))
         } else {
@@ -340,7 +341,7 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
             local_crates,
             extern_statics: FxHashMap::default(),
             rng: RefCell::new(rng),
-            tracked_alloc_id: config.tracked_alloc_id,
+            tracked_alloc_ids: config.tracked_alloc_ids.clone(),
             check_alignment: config.check_alignment,
             cmpxchg_weak_failure_rate: config.cmpxchg_weak_failure_rate,
         }
@@ -560,7 +561,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         alloc: Cow<'b, Allocation>,
         kind: Option<MemoryKind<Self::MemoryKind>>,
     ) -> Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>> {
-        if Some(id) == ecx.machine.tracked_alloc_id {
+        if ecx.machine.tracked_alloc_ids.contains(&id) {
             register_diagnostic(NonHaltingDiagnostic::CreatedAlloc(id));
         }
 
@@ -669,7 +670,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         (alloc_id, tag): (AllocId, Self::TagExtra),
         range: AllocRange,
     ) -> InterpResult<'tcx> {
-        if Some(alloc_id) == machine.tracked_alloc_id {
+        if machine.tracked_alloc_ids.contains(&alloc_id) {
             register_diagnostic(NonHaltingDiagnostic::FreedAlloc(alloc_id));
         }
         if let Some(data_race) = &mut alloc_extra.data_race {

src/stacked_borrows.rs

@@ -15,6 +15,7 @@ use rustc_middle::ty::{
 };
 use rustc_span::DUMMY_SP;
 use rustc_target::abi::Size;
+use std::collections::HashSet;
 
 use crate::*;
 
@@ -104,10 +105,10 @@ pub struct GlobalStateInner {
     next_call_id: CallId,
     /// Those call IDs corresponding to functions that are still running.
     active_calls: FxHashSet<CallId>,
-    /// The pointer id to trace
-    tracked_pointer_tag: Option<PtrId>,
-    /// The call id to trace
-    tracked_call_id: Option<CallId>,
+    /// The pointer ids to trace
+    tracked_pointer_tags: HashSet<PtrId>,
+    /// The call ids to trace
+    tracked_call_ids: HashSet<CallId>,
     /// Whether to track raw pointers.
     tag_raw: bool,
 }
@@ -158,24 +159,24 @@ impl fmt::Display for RefKind {
 /// Utilities for initialization and ID generation
 impl GlobalStateInner {
     pub fn new(
-        tracked_pointer_tag: Option<PtrId>,
-        tracked_call_id: Option<CallId>,
+        tracked_pointer_tags: HashSet<PtrId>,
+        tracked_call_ids: HashSet<CallId>,
         tag_raw: bool,
     ) -> Self {
         GlobalStateInner {
             next_ptr_id: NonZeroU64::new(1).unwrap(),
             base_ptr_ids: FxHashMap::default(),
             next_call_id: NonZeroU64::new(1).unwrap(),
             active_calls: FxHashSet::default(),
-            tracked_pointer_tag,
-            tracked_call_id,
+            tracked_pointer_tags,
+            tracked_call_ids,
             tag_raw,
         }
     }
 
     fn new_ptr(&mut self) -> PtrId {
         let id = self.next_ptr_id;
-        if Some(id) == self.tracked_pointer_tag {
+        if self.tracked_pointer_tags.contains(&id) {
             register_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(id));
         }
         self.next_ptr_id = NonZeroU64::new(id.get() + 1).unwrap();
@@ -185,7 +186,7 @@ impl GlobalStateInner {
     pub fn new_call(&mut self) -> CallId {
         let id = self.next_call_id;
         trace!("new_call: Assigning ID {}", id);
-        if Some(id) == self.tracked_call_id {
+        if self.tracked_call_ids.contains(&id) {
             register_diagnostic(NonHaltingDiagnostic::CreatedCallId(id));
         }
         assert!(self.active_calls.insert(id));
@@ -311,7 +312,7 @@ impl<'tcx> Stack {
         global: &GlobalStateInner,
     ) -> InterpResult<'tcx> {
         if let SbTag::Tagged(id) = item.tag {
-            if Some(id) == global.tracked_pointer_tag {
+            if global.tracked_pointer_tags.contains(&id) {
                 register_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(
                     item.clone(),
                     provoking_access,