Rollup merge of rust-lang#150483 - RalfJung:miri, r=RalfJung

miri subtree update

Subtree update of `miri` to rust-lang/miri@70e2451.

Created using https://github.com/rust-lang/josh-sync.

r? `@ghost`

Author: JonathanBrouwer

src/tools/miri/rust-version

@@ -1 +1 @@
-cb79c42008b970269f6a06b257e5f04b93f24d03
+7fefa09b90ca57b8a0e0e4717d672d38a0ae58b5

src/tools/miri/src/bin/miri.rs

@@ -710,20 +710,18 @@ fn main() {
     if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict {
         fatal_error!("strict provenance is not compatible with calling native functions");
     }
+    // Native calls and many-seeds are an "intersting" combination.
+    if !miri_config.native_lib.is_empty() && many_seeds.is_some() {
+        eprintln!(
+            "warning: `-Zmiri-many-seeds` runs multiple instances of the program in the same address space, \
+            so if the native library has global state, it will leak across execution bundaries"
+        );
+    }
     // You can set either one seed or many.
     if many_seeds.is_some() && miri_config.seed.is_some() {
         fatal_error!("Only one of `-Zmiri-seed` and `-Zmiri-many-seeds can be set");
     }
-
-    // Ensure we have parallelism for many-seeds mode.
-    if many_seeds.is_some() && !rustc_args.iter().any(|arg| arg.starts_with("-Zthreads=")) {
-        // Clamp to 20 threads; things get a less efficient beyond that due to lock contention.
-        let threads = std::thread::available_parallelism().map_or(1, |n| n.get()).min(20);
-        rustc_args.push(format!("-Zthreads={threads}"));
-    }
-    let many_seeds =
-        many_seeds.map(|seeds| ManySeedsConfig { seeds, keep_going: many_seeds_keep_going });
-
+    // We cannot emulate weak memory without the data race detector.
     if miri_config.weak_memory_emulation && !miri_config.data_race_detector {
         fatal_error!(
             "Weak memory emulation cannot be enabled when the data race detector is disabled"
@@ -737,6 +735,15 @@ fn main() {
         fatal_error!("Invalid settings: {err}");
     }
 
+    // Ensure we have parallelism for many-seeds mode.
+    if many_seeds.is_some() && !rustc_args.iter().any(|arg| arg.starts_with("-Zthreads=")) {
+        // Clamp to 20 threads; things get a less efficient beyond that due to lock contention.
+        let threads = std::thread::available_parallelism().map_or(1, |n| n.get()).min(20);
+        rustc_args.push(format!("-Zthreads={threads}"));
+    }
+    let many_seeds =
+        many_seeds.map(|seeds| ManySeedsConfig { seeds, keep_going: many_seeds_keep_going });
+
     debug!("rustc arguments: {:?}", rustc_args);
     debug!("crate arguments: {:?}", miri_config.args);
     if !miri_config.native_lib.is_empty() && miri_config.native_lib_enable_tracing {

src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs

@@ -661,6 +661,7 @@ impl<'tcx> Tree {
                 global,
                 ChildrenVisitMode::VisitChildrenOfAccessed,
                 &diagnostics,
+                /* min_exposed_child */ None, // only matters for protector end access,
             )?;
         }
         interp_ok(())
@@ -686,6 +687,13 @@ impl<'tcx> Tree {
 
         let source_idx = self.tag_mapping.get(&tag).unwrap();
 
+        let min_exposed_child = if self.roots.len() > 1 {
+            LocationTree::get_min_exposed_child(source_idx, &self.nodes)
+        } else {
+            // There's no point in computing this when there is just one tree.
+            None
+        };
+
         // This is a special access through the entire allocation.
         // It actually only affects `accessed` locations, so we need
         // to filter on those before initiating the traversal.
@@ -716,6 +724,7 @@ impl<'tcx> Tree {
                     global,
                     ChildrenVisitMode::SkipChildrenOfAccessed,
                     &diagnostics,
+                    min_exposed_child,
                 )?;
             }
         }
@@ -876,9 +885,36 @@ impl Tree {
 }
 
 impl<'tcx> LocationTree {
+    /// Returns the smallest exposed tag, if any, that is a transitive child of `root`.
+    fn get_min_exposed_child(root: UniIndex, nodes: &UniValMap<Node>) -> Option<BorTag> {
+        // We cannot use the wildcard datastructure to improve this lookup. This is because
+        // the datastructure only tracks enabled nodes and we need to also consider disabled ones.
+        let mut stack = vec![root];
+        let mut min_tag = None;
+        while let Some(idx) = stack.pop() {
+            let node = nodes.get(idx).unwrap();
+            if min_tag.is_some_and(|min| min < node.tag) {
+                // The minimum we found before is bigger than this tag, and therefore
+                // also bigger than all its children, so we can skip this subtree.
+                continue;
+            }
+            stack.extend_from_slice(node.children.as_slice());
+            if node.is_exposed {
+                min_tag = match min_tag {
+                    Some(prev) if prev < node.tag => Some(prev),
+                    _ => Some(node.tag),
+                };
+            }
+        }
+        min_tag
+    }
+
     /// Performs an access on this location.
     /// * `access_source`: The index, if any, where the access came from.
     /// * `visit_children`: Whether to skip updating the children of `access_source`.
+    /// * `min_exposed_child`: The tag of the smallest exposed (transitive) child of the accessed node.
+    ///   This is only used with `visit_children == SkipChildrenOfAccessed`, where we need to skip children
+    ///   of the accessed node.
     fn perform_access(
         &mut self,
         roots: impl Iterator<Item = UniIndex>,
@@ -888,6 +924,7 @@ impl<'tcx> LocationTree {
         global: &GlobalState,
         visit_children: ChildrenVisitMode,
         diagnostics: &DiagnosticInfo,
+        min_exposed_child: Option<BorTag>,
     ) -> InterpResult<'tcx> {
         let accessed_root = if let Some(idx) = access_source {
             Some(self.perform_normal_access(
@@ -906,11 +943,22 @@ impl<'tcx> LocationTree {
         };
 
         let accessed_root_tag = accessed_root.map(|idx| nodes.get(idx).unwrap().tag);
-        if matches!(visit_children, ChildrenVisitMode::SkipChildrenOfAccessed) {
-            // FIXME: approximate which roots could be children of the accessed node and only skip them instead of all other trees.
-            return interp_ok(());
-        }
         for root in roots {
+            let tag = nodes.get(root).unwrap().tag;
+            // On a protector release access we have to skip the children of the accessed tag.
+            // However, if the tag has exposed children then some of the wildcard subtrees could
+            // also be children of the accessed node and would also need to be skipped. We can
+            // narrow down which wildcard trees might be children by comparing their root tag to the
+            // minimum exposed child of the accessed node. As the parent tag is always smaller
+            // than the child tag this means we only need to skip subtrees with a root tag larger
+            // than `min_exposed_child`. Once we find such a root, we can leave the loop because roots
+            // are sorted by tag.
+            if matches!(visit_children, ChildrenVisitMode::SkipChildrenOfAccessed)
+                && let Some(min_exposed_child) = min_exposed_child
+                && tag > min_exposed_child
+            {
+                break;
+            }
             // We don't perform a wildcard access on the tree we already performed a
             // normal access on.
             if Some(root) == accessed_root {

src/tools/miri/src/concurrency/data_race.rs

@@ -1116,6 +1116,7 @@ impl VClockAlloc {
             {
                 (AccessType::AtomicStore, idx, &atomic.write_vector)
             } else if !access.is_atomic() &&
+                !access.is_read() &&
                 let Some(atomic) = mem_clocks.atomic() &&
                 let Some(idx) = Self::find_gt_index(&atomic.read_vector, &active_clocks.clock)
             {
@@ -1124,7 +1125,7 @@ impl VClockAlloc {
             } else if mem_clocks.write.1 > active_clocks.clock[mem_clocks.write.0] {
                 write_clock = mem_clocks.write();
                 (AccessType::NaWrite(mem_clocks.write_type), mem_clocks.write.0, &write_clock)
-            } else if let Some(idx) = Self::find_gt_index(&mem_clocks.read, &active_clocks.clock) {
+            } else if !access.is_read() && let Some(idx) = Self::find_gt_index(&mem_clocks.read, &active_clocks.clock) {
                 (AccessType::NaRead(mem_clocks.read[idx].read_type()), idx, &mem_clocks.read)
             // Finally, mixed-size races.
             } else if access.is_atomic() && let Some(atomic) = mem_clocks.atomic() && atomic.size != Some(access_size) {
@@ -1157,7 +1158,9 @@ impl VClockAlloc {
             assert!(!involves_non_atomic);
             Some("overlapping unsynchronized atomic accesses must use the same access size")
         } else if access.is_read() && other_access.is_read() {
-            panic!("there should be no same-size read-read races")
+            panic!(
+                "there should be no same-size read-read races\naccess: {access:?}\nother_access: {other_access:?}"
+            )
         } else {
             None
         };

src/tools/miri/src/concurrency/vector_clock.rs

@@ -48,14 +48,24 @@ const SMALL_VECTOR: usize = 4;
 /// The time-stamps recorded in the data-race detector consist of both
 /// a 32-bit unsigned integer which is the actual timestamp, and a `Span`
 /// so that diagnostics can report what code was responsible for an operation.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy)]
 pub(super) struct VTimestamp {
     /// The lowest bit indicates read type, the rest is the time.
     /// `1` indicates a retag read, `0` a regular read.
     time_and_read_type: u32,
     pub span: Span,
 }
 
+impl Debug for VTimestamp {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("VTimestamp")
+            .field("time", &self.time())
+            .field("read_type", &self.read_type())
+            .field("span", &self.span)
+            .finish()
+    }
+}
+
 impl VTimestamp {
     pub const ZERO: VTimestamp = VTimestamp::new(0, NaReadType::Read, DUMMY_SP);
 

src/tools/miri/src/diagnostics.rs

@@ -548,7 +548,7 @@ pub fn report_leaks<'tcx>(
 /// We want to present a multi-line span message for some errors. Diagnostics do not support this
 /// directly, so we pass the lines as a `Vec<String>` and display each line after the first with an
 /// additional `span_label` or `note` call.
-pub fn report_msg<'tcx>(
+fn report_msg<'tcx>(
     diag_level: DiagLevel,
     title: String,
     span_msg: Vec<String>,

src/tools/miri/tests/fail/data_race/mixed_size_read_write_read.rs

@@ -0,0 +1,27 @@
+//@compile-flags: -Zmiri-deterministic-concurrency
+// A case that is not covered by `mixed_size_read_write`.
+#![feature(ptr_as_ref_unchecked)]
+
+use std::sync::atomic::*;
+use std::thread;
+
+fn main() {
+    let data = AtomicI32::new(0);
+
+    thread::scope(|s| {
+        s.spawn(|| unsafe {
+            let _val = (&raw const data).read();
+            let _val = (&raw const data).cast::<AtomicI8>().as_ref_unchecked().compare_exchange(
+                0,
+                1,
+                Ordering::Relaxed,
+                Ordering::Relaxed,
+            );
+            thread::yield_now();
+            unreachable!();
+        });
+        s.spawn(|| {
+            let _val = data.load(Ordering::Relaxed); //~ERROR: /Race condition detected between \(1\) 1-byte atomic store .* and \(2\) 4-byte atomic load/
+        });
+    });
+}

src/tools/miri/tests/fail/data_race/mixed_size_read_write_read.stderr

@@ -0,0 +1,26 @@
+error: Undefined Behavior: Race condition detected between (1) 1-byte atomic store on thread `unnamed-ID` and (2) 4-byte atomic load on thread `unnamed-ID` at ALLOC
+  --> tests/fail/data_race/mixed_size_read_write_read.rs:LL:CC
+   |
+LL | ...   let _val = data.load(Ordering::Relaxed);
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (2) just happened here
+   |
+help: and (1) occurred earlier here
+  --> tests/fail/data_race/mixed_size_read_write_read.rs:LL:CC
+   |
+LL |               let _val = (&raw const data).cast::<AtomicI8>().as_ref_unchecked().compare_exchange(
+   |  ________________________^
+LL | |                 0,
+LL | |                 1,
+LL | |                 Ordering::Relaxed,
+LL | |                 Ordering::Relaxed,
+LL | |             );
+   | |_____________^
+   = help: overlapping unsynchronized atomic accesses must use the same access size
+   = help: see https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#memory-model-for-atomic-accesses for more information about the Rust memory model
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/tree_borrows/wildcard/protector_release.rs

@@ -0,0 +1,79 @@
+//@compile-flags: -Zmiri-tree-borrows -Zmiri-permissive-provenance
+use std::cell::UnsafeCell;
+
+/// This is a variant of the test in `../protector-write-lazy.rs`, but with
+/// wildcard references.
+/// Checks that a protector release access correctly determines that certain tags
+/// cannot be children of the protected tag and that it updates them accordingly.
+///
+/// For this version we know the tag is not a child because its wildcard root has
+/// a smaller tag then the released reference.
+pub fn main() {
+    // We need two locations so that we can create a new reference
+    // that is foreign to an already active tag.
+    let mut x: UnsafeCell<[u32; 2]> = UnsafeCell::new([32, 33]);
+    let ref1 = &mut x;
+    let cell_ptr = ref1.get() as *mut u32;
+
+    let int = ref1 as *mut UnsafeCell<[u32; 2]> as usize;
+    let wild = int as *mut UnsafeCell<u32>;
+
+    let ref2 = unsafe { &mut *cell_ptr };
+
+    // `ref3` gets created before the protected ref `arg4`.
+    let ref3 = unsafe { &mut *wild.wrapping_add(1) };
+
+    let protect = |arg4: &mut u32| {
+        // Activates arg4. This would disable ref3  at [0] if it wasn't a cell.
+        *arg4 = 41;
+
+        // Creates an exposed child of arg4.
+        let ref5 = &mut *arg4;
+        let _int = ref5 as *mut u32 as usize;
+
+        // This creates ref6 from ref3 at [1], so that it doesn't disable arg4 at [0].
+        let ref6 = unsafe { &mut *ref3.get() };
+
+        //    ┌───────────┐
+        //    │    ref1*  │
+        //    │ Cel │ Cel │           *
+        //    └─────┬─────┘           │
+        //          │                 │
+        //          │                 │
+        //          ▼                 ▼
+        //    ┌───────────┐     ┌───────────┐
+        //    │ ref2│     │     │     │ ref3│
+        //    │ Act │ Res │     │ Cel │ Cel │
+        //    └─────┬─────┘     └─────┬─────┘
+        //          │                 │
+        //          │                 │
+        //          ▼                 ▼
+        //    ┌───────────┐     ┌───────────┐
+        //    │ arg4│     │     │     │ ref6│
+        //    │ Act │ Res │     │ Res │ Res │
+        //    └─────┬─────┘     └───────────┘
+        //          │
+        //          │
+        //          ▼
+        //    ┌───────────┐
+        //    │ref5*|     │
+        //    │ Res │ Res │
+        //    └───────────┘
+
+        // Creates a pointer to [0] with the provenance of ref6.
+        return (ref6 as *mut u32).wrapping_sub(1);
+
+        // Protector release on arg4 happens here.
+        // This should cause a foreign write on all foreign nodes,
+        // unless they could be a child of arg4.
+        // arg4 has an exposed child, so some tags with a wildcard
+        // ancestor could be its children.
+        // However, the root of ref6 was created before arg4, so it
+        // cannot be a child of it. So it should get disabled
+        // (at location [0]).
+    };
+    // ptr has provenance of ref6
+    let ptr = protect(ref2);
+    // ref6 is disabled at [0].
+    let _fail = unsafe { *ptr }; //~ ERROR: /read access through .* is forbidden/
+}

src/tools/miri/tests/fail/tree_borrows/wildcard/protector_release.stderr

@@ -0,0 +1,25 @@
+error: Undefined Behavior: read access through <TAG> at ALLOC[0x0] is forbidden
+  --> tests/fail/tree_borrows/wildcard/protector_release.rs:LL:CC
+   |
+LL |     let _fail = unsafe { *ptr };
+   |                          ^^^^ Undefined Behavior occurred here
+   |
+   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental
+   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information
+   = help: the accessed tag <TAG> has state Disabled which forbids this child read access
+help: the accessed tag <TAG> was created here, in the initial state Reserved
+  --> tests/fail/tree_borrows/wildcard/protector_release.rs:LL:CC
+   |
+LL |         let ref6 = unsafe { &mut *ref3.get() };
+   |                             ^^^^^^^^^^^^^^^^
+help: the accessed tag <TAG> later transitioned to Disabled due to a protector release (acting as a foreign write access) on every location previously accessed by this tag
+  --> tests/fail/tree_borrows/wildcard/protector_release.rs:LL:CC
+   |
+LL |     };
+   |      ^
+   = help: this transition corresponds to a loss of read and write permissions
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+