Auto merge of #1603 - RalfJung:track-raw, r=oli-obk

add an option to track raw pointer tags in Stacked Borrows

Also make error messages more informative by printing the affected memory location

Author: bors

README.md

@@ -230,13 +230,20 @@ environment variable:
 * `-Zmiri-track-alloc-id=<id>` shows a backtrace when the given allocation is
   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
+  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
   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.
-* `-Zmiri-track-call-id=<id>` shows a backtrace when the given call id is
-  assigned to a stack frame.  This helps in debugging UB related to Stacked
-  Borrows "protectors".
+* `-Zmiri-track-raw-pointers` makes Stacked Borrows track a pointer tag even for
+  raw pointers. This can make valid code 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 "<untagged>" occurring in the message -- this
+  indicates a pointer that was cast from an integer, so Miri was unable to track
+  this pointer. Make sure to use a non-Windows target with this flag, as the
+  Windows runtime makes use of integer-pointer casts.
 
 Some native rustc `-Z` flags are also very relevant for Miri:
 

src/bin/miri.rs

@@ -207,6 +207,9 @@ fn main() {
                 "-Zmiri-ignore-leaks" => {
                     miri_config.ignore_leaks = true;
                 }
+                "-Zmiri-track-raw-pointers" => {
+                    miri_config.track_raw = true;
+                }
                 "--" => {
                     after_dashdash = true;
                 }

src/eval.rs

@@ -3,8 +3,6 @@
 use std::convert::TryFrom;
 use std::ffi::OsStr;
 
-use rand::rngs::StdRng;
-use rand::SeedableRng;
 use log::info;
 
 use rustc_hir::def_id::DefId;
@@ -48,6 +46,8 @@ pub struct MiriConfig {
     pub tracked_call_id: Option<CallId>,
     /// The allocation id to report about.
     pub tracked_alloc_id: Option<AllocId>,
+    /// Whether to track raw pointers in stacked borrows.
+    pub track_raw: bool,
 }
 
 impl Default for MiriConfig {
@@ -64,6 +64,7 @@ impl Default for MiriConfig {
             tracked_pointer_tag: None,
             tracked_call_id: None,
             tracked_alloc_id: None,
+            track_raw: false,
         }
     }
 }
@@ -84,14 +85,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
         rustc_span::source_map::DUMMY_SP,
         param_env,
         Evaluator::new(config.communicate, config.validate, layout_cx),
-        MemoryExtra::new(
-            StdRng::seed_from_u64(config.seed.unwrap_or(0)),
-            config.stacked_borrows,
-            config.tracked_pointer_tag,
-            config.tracked_call_id,
-            config.tracked_alloc_id,
-            config.check_alignment,
-        ),
+        MemoryExtra::new(&config),
     );
     // Complete initialization.
     EnvVars::init(&mut ecx, config.excluded_env_vars)?;

src/machine.rs

@@ -10,6 +10,7 @@ use std::fmt;
 
 use log::trace;
 use rand::rngs::StdRng;
+use rand::SeedableRng;
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_middle::{
@@ -132,16 +133,14 @@ pub struct MemoryExtra {
 }
 
 impl MemoryExtra {
-    pub fn new(
-        rng: StdRng,
-        stacked_borrows: bool,
-        tracked_pointer_tag: Option<PtrId>,
-        tracked_call_id: Option<CallId>,
-        tracked_alloc_id: Option<AllocId>,
-        check_alignment: AlignmentCheck,
-    ) -> Self {
-        let stacked_borrows = if stacked_borrows {
-            Some(Rc::new(RefCell::new(stacked_borrows::GlobalState::new(tracked_pointer_tag, tracked_call_id))))
+    pub fn new(config: &MiriConfig) -> Self {
+        let rng = StdRng::seed_from_u64(config.seed.unwrap_or(0));
+        let stacked_borrows = if config.stacked_borrows {
+            Some(Rc::new(RefCell::new(stacked_borrows::GlobalState::new(
+                config.tracked_pointer_tag,
+                config.tracked_call_id,
+                config.track_raw,
+            ))))
         } else {
             None
         };
@@ -150,8 +149,8 @@ impl MemoryExtra {
             intptrcast: Default::default(),
             extern_statics: FxHashMap::default(),
             rng: RefCell::new(rng),
-            tracked_alloc_id,
-            check_alignment,
+            tracked_alloc_id: config.tracked_alloc_id,
+            check_alignment: config.check_alignment,
         }
     }
 

src/range_map.rs

@@ -61,7 +61,9 @@ impl<T> RangeMap<T> {
     /// Provides read-only iteration over everything in the given range. This does
     /// *not* split items if they overlap with the edges. Do not use this to mutate
     /// through interior mutability.
-    pub fn iter<'a>(&'a self, offset: Size, len: Size) -> impl Iterator<Item = &'a T> + 'a {
+    ///
+    /// The iterator also provides the offset of the given element.
+    pub fn iter<'a>(&'a self, offset: Size, len: Size) -> impl Iterator<Item = (Size, &'a T)> + 'a {
         let offset = offset.bytes();
         let len = len.bytes();
         // Compute a slice starting with the elements we care about.
@@ -75,7 +77,7 @@ impl<T> RangeMap<T> {
         };
         // The first offset that is not included any more.
         let end = offset + len;
-        slice.iter().take_while(move |elem| elem.range.start < end).map(|elem| &elem.data)
+        slice.iter().take_while(move |elem| elem.range.start < end).map(|elem| (Size::from_bytes(elem.range.start), &elem.data))
     }
 
     pub fn iter_mut_all<'a>(&'a mut self) -> impl Iterator<Item = &'a mut T> + 'a {
@@ -112,11 +114,13 @@ impl<T> RangeMap<T> {
     /// this will split entries in the map that are only partially hit by the given range,
     /// to make sure that when they are mutated, the effect is constrained to the given range.
     /// Moreover, this will opportunistically merge neighbouring equal blocks.
+    ///
+    /// The iterator also provides the offset of the given element.
     pub fn iter_mut<'a>(
         &'a mut self,
         offset: Size,
         len: Size,
-    ) -> impl Iterator<Item = &'a mut T> + 'a
+    ) -> impl Iterator<Item = (Size, &'a mut T)> + 'a
     where
         T: Clone + PartialEq,
     {
@@ -197,7 +201,7 @@ impl<T> RangeMap<T> {
             // Now we yield the slice. `end` is inclusive.
             &mut self.v[first_idx..=end_idx]
         };
-        slice.iter_mut().map(|elem| &mut elem.data)
+        slice.iter_mut().map(|elem| (Size::from_bytes(elem.range.start), &mut elem.data))
     }
 }
 
@@ -209,26 +213,26 @@ mod tests {
     fn to_vec<T: Copy>(map: &RangeMap<T>, offset: u64, len: u64) -> Vec<T> {
         (offset..offset + len)
             .into_iter()
-            .map(|i| map.iter(Size::from_bytes(i), Size::from_bytes(1)).next().map(|&t| t).unwrap())
+            .map(|i| map.iter(Size::from_bytes(i), Size::from_bytes(1)).next().map(|(_, &t)| t).unwrap())
             .collect()
     }
 
     #[test]
     fn basic_insert() {
         let mut map = RangeMap::<i32>::new(Size::from_bytes(20), -1);
         // Insert.
-        for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(1)) {
+        for (_, x) in map.iter_mut(Size::from_bytes(10), Size::from_bytes(1)) {
             *x = 42;
         }
         // Check.
         assert_eq!(to_vec(&map, 10, 1), vec![42]);
         assert_eq!(map.v.len(), 3);
 
         // Insert with size 0.
-        for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(0)) {
+        for (_, x) in map.iter_mut(Size::from_bytes(10), Size::from_bytes(0)) {
             *x = 19;
         }
-        for x in map.iter_mut(Size::from_bytes(11), Size::from_bytes(0)) {
+        for (_, x) in map.iter_mut(Size::from_bytes(11), Size::from_bytes(0)) {
             *x = 19;
         }
         assert_eq!(to_vec(&map, 10, 2), vec![42, -1]);
@@ -238,16 +242,16 @@ mod tests {
     #[test]
     fn gaps() {
         let mut map = RangeMap::<i32>::new(Size::from_bytes(20), -1);
-        for x in map.iter_mut(Size::from_bytes(11), Size::from_bytes(1)) {
+        for (_, x) in map.iter_mut(Size::from_bytes(11), Size::from_bytes(1)) {
             *x = 42;
         }
-        for x in map.iter_mut(Size::from_bytes(15), Size::from_bytes(1)) {
+        for (_, x) in map.iter_mut(Size::from_bytes(15), Size::from_bytes(1)) {
             *x = 43;
         }
         assert_eq!(map.v.len(), 5);
         assert_eq!(to_vec(&map, 10, 10), vec![-1, 42, -1, -1, -1, 43, -1, -1, -1, -1]);
 
-        for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(10)) {
+        for (_, x) in map.iter_mut(Size::from_bytes(10), Size::from_bytes(10)) {
             if *x < 42 {
                 *x = 23;
             }
@@ -256,14 +260,14 @@ mod tests {
         assert_eq!(to_vec(&map, 10, 10), vec![23, 42, 23, 23, 23, 43, 23, 23, 23, 23]);
         assert_eq!(to_vec(&map, 13, 5), vec![23, 23, 43, 23, 23]);
 
-        for x in map.iter_mut(Size::from_bytes(15), Size::from_bytes(5)) {
+        for (_, x) in map.iter_mut(Size::from_bytes(15), Size::from_bytes(5)) {
             *x = 19;
         }
         assert_eq!(map.v.len(), 6);
         assert_eq!(to_vec(&map, 10, 10), vec![23, 42, 23, 23, 23, 19, 19, 19, 19, 19]);
         // Should be seeing two blocks with 19.
         assert_eq!(
-            map.iter(Size::from_bytes(15), Size::from_bytes(2)).map(|&t| t).collect::<Vec<_>>(),
+            map.iter(Size::from_bytes(15), Size::from_bytes(2)).map(|(_, &t)| t).collect::<Vec<_>>(),
             vec![19, 19]
         );