miri subtree update

src/tools/miri/CONTRIBUTING.md

@@ -66,6 +66,23 @@ process for such contributions:
 This process is largely informal, and its primary goal is to more clearly communicate expectations.
 Please get in touch with us if you have any questions!
 
+## Scope of Miri shims
+
+Miri has "shims" to implement functionality that is usually implemented in C libraries which are
+invoked from Rust code, such as opening files or spawning threads, as well as for
+CPU-vendor-provided SIMD intrinsics. However, the set of C functions that Rust code invokes this way
+is enormous, and for obvious reasons we have no intention of implementing every C API ever written
+in Miri.
+
+At the moment, the general guideline for "could this function have a shim in Miri" is: we will
+generally only add shims for functions that can be implemented in a portable way using just what is
+provided by the Rust standard library. The function should also be reasonably widely-used in Rust
+code to justify the review and maintenance effort (i.e. the easier the function is to implement, the
+lower the barrier). Other than that, we might make exceptions for certain cases if (a) there is a
+good case for why Miri should support those APIs, and (b) robust and widely-used portable libraries
+exist in the Rust ecosystem. We will generally not add shims to Miri that would require Miri to
+directly interact with platform-specific APIs (such as `libc` or `windows-sys`).
+
 ## Preparing the build environment
 
 Miri heavily relies on internal and unstable rustc interfaces to execute MIR,

src/tools/miri/Cargo.lock

@@ -562,9 +562,9 @@ checksum = "e629b9b98ef3dd8afe6ca2bd0f89306cec16d43d907889945bc5d6687f2f13c7"
 
 [[package]]
 name = "git2"
-version = "0.20.2"
+version = "0.20.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2deb07a133b1520dc1a5690e9bd08950108873d7ed5de38dcc74d3b5ebffa110"
+checksum = "7b88256088d75a56f8ecfa070513a775dd9107f6530ef14919dac831af9cfe2b"
 dependencies = [
  "bitflags",
  "libc",
@@ -804,9 +804,9 @@ dependencies = [
 
 [[package]]
 name = "libgit2-sys"
-version = "0.18.2+1.9.1"
+version = "0.18.3+1.9.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1c42fe03df2bd3c53a3a9c7317ad91d80c81cd1fb0caec8d7cc4cd2bfa10c222"
+checksum = "c9b3acc4b91781bb0b3386669d325163746af5f6e4f73e6d2d630e09a35f3487"
 dependencies = [
  "cc",
  "libc",

src/tools/miri/Cargo.toml

@@ -39,7 +39,7 @@ serde = { version = "1.0.219", features = ["derive"], optional = true }
 [target.'cfg(target_os = "linux")'.dependencies]
 nix = { version = "0.30.1", features = ["mman", "ptrace", "signal"], optional = true }
 ipc-channel = { version = "0.20.0", optional = true }
-capstone = { version = "0.14", optional = true }
+capstone = { version = "0.14", features = ["arch_x86", "full"], default-features = false, optional = true}
 
 [target.'cfg(all(target_os = "linux", target_pointer_width = "64", target_endian = "little"))'.dependencies]
 genmc-sys = { path = "./genmc-sys/", version = "0.1.0", optional = true }

src/tools/miri/README.md

@@ -626,6 +626,8 @@ Definite bugs found:
 * [`ReentrantLock` not correctly dealing with reuse of addresses for TLS storage of different threads](https://github.com/rust-lang/rust/pull/141248)
 * [Rare Deadlock in the thread (un)parking example code](https://github.com/rust-lang/rust/issues/145816)
 * [`winit` registering a global constructor with the wrong ABI on Windows](https://github.com/rust-windowing/winit/issues/4435)
+* [`VecDeque::splice` confusing physical and logical indices](https://github.com/rust-lang/rust/issues/151758)
+* [Data race in `oneshot` channel](https://github.com/faern/oneshot/issues/69)
 
 Violations of [Stacked Borrows] found that are likely bugs (but Stacked Borrows is currently just an experiment):
 

src/tools/miri/cargo-miri/src/phases.rs

@@ -63,6 +63,9 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
         "setup" => MiriCommand::Setup,
         "test" | "t" | "run" | "r" | "nextest" => MiriCommand::Forward(subcommand),
         "clean" => MiriCommand::Clean,
+        // For use by the `./miri test` dependency builder.
+        "build" if env::var_os("MIRI_BUILD_TEST_DEPS").is_some() =>
+            MiriCommand::Forward("build".into()),
         _ => {
             // Check for version and help flags.
             if has_arg_flag("--help") || has_arg_flag("-h") {
@@ -309,6 +312,7 @@ pub fn phase_rustc(args: impl Iterator<Item = String>, phase: RustcPhase) {
             // Ask rustc for the filename (since that is target-dependent).
             let mut rustc = miri_for_host(); // sysroot doesn't matter for this so we just use the host
             rustc.arg("--print").arg("file-names");
+            rustc.arg("-Zunstable-options"); // needed for JSON targets
             for flag in ["--crate-name", "--crate-type", "--target"] {
                 for val in get_arg_flag_values(flag) {
                     rustc.arg(flag).arg(val);

src/tools/miri/cargo-miri/src/setup.rs

@@ -88,6 +88,11 @@ pub fn setup(
     };
     let cargo_cmd = {
         let mut command = cargo();
+        // Allow JSON targets since users do not have a good way to set this flag otherwise.
+        if env::var("RUSTC_STAGE").is_err() {
+            // ^ is a HACK for bootstrap cargo. FIXME(cfg(bootstrap)) remove the hack.
+            command.arg("-Zjson-target-spec");
+        }
         // Use Miri as rustc to build a libstd compatible with us (and use the right flags).
         // We set ourselves (`cargo-miri`) instead of Miri directly to be able to patch the flags
         // for `libpanic_abort` (usually this is done by bootstrap but we have to do it ourselves).

src/tools/miri/ci/ci.sh

@@ -129,7 +129,7 @@ function run_tests_minimal {
   time ./miri test $TARGET_FLAG "$@"
 
   # Ensure that a small smoke test of cargo-miri works.
-  time cargo miri run --manifest-path test-cargo-miri/no-std-smoke/Cargo.toml $TARGET_FLAG
+  time cargo miri run --manifest-path test-cargo-miri/no-std-smoke/Cargo.toml -Zjson-target-spec $TARGET_FLAG
 
   endgroup
 }
@@ -173,7 +173,9 @@ case $HOST_TARGET in
     # Host
     MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests
     # Custom target JSON file
-    TEST_TARGET=tests/x86_64-unknown-kernel.json MIRI_NO_STD=1 run_tests_minimal no_std
+    TEST_TARGET=tests/x86_64-unknown-kernel.json MIRI_NO_STD=1 MIRIFLAGS="-Zunstable-options" run_tests_minimal no_std
+    # Not officially supported tier 2
+    MANY_SEEDS=16 TEST_TARGET=x86_64-pc-solaris run_tests
     ;;
   aarch64-apple-darwin)
     # Host
@@ -184,7 +186,6 @@ case $HOST_TARGET in
     # Not officially supported tier 2
     MANY_SEEDS=16 TEST_TARGET=mips-unknown-linux-gnu run_tests # a 32bit big-endian target, and also a target without 64bit atomics
     MANY_SEEDS=16 TEST_TARGET=x86_64-unknown-illumos run_tests
-    MANY_SEEDS=16 TEST_TARGET=x86_64-pc-solaris run_tests
     ;;
   i686-pc-windows-msvc)
     # Host

src/tools/miri/rust-version

@@ -1 +1 @@
-d10ac47c20152feb5e99b1c35a2e6830f77c66dc
+7bee525095c0872e87c038c412c781b9bbb3f5dc

src/tools/miri/src/alloc/isolated_alloc.rs

@@ -66,10 +66,10 @@ impl IsolatedAlloc {
         // And make sure the align is at least one page
         let align = std::cmp::max(layout.align(), self.page_size);
         // pg_count gives us the # of pages needed to satisfy the size. For
-        // align > page_size where align = n * page_size, a sufficently-aligned
+        // align > page_size where align = n * page_size, a sufficiently-aligned
         // address must exist somewhere in the range of
         // some_page_aligned_address..some_page_aligned_address + (n-1) * page_size
-        // (since if some_page_aligned_address + n * page_size is sufficently aligned,
+        // (since if some_page_aligned_address + n * page_size is sufficiently aligned,
         // then so is some_page_aligned_address itself per the definition of n, so we
         // can avoid using that 1 extra page).
         // Thus we allocate n-1 extra pages

src/tools/miri/src/bin/log/tracing_chrome_instant.rs

@@ -2,7 +2,7 @@
 //! <https://github.com/tikv/minstant/blob/27c9ec5ec90b5b67113a748a4defee0d2519518c/src/tsc_now.rs>.
 //! A useful resource is also
 //! <https://www.pingcap.com/blog/how-we-trace-a-kv-database-with-less-than-5-percent-performance-impact/>,
-//! although this file does not implement TSC synchronization but insteads pins threads to CPUs,
+//! although this file does not implement TSC synchronization but instead pins threads to CPUs,
 //! since the former is not reliable (i.e. it might lead to non-monotonic time measurements).
 //! Another useful resource for future improvements might be measureme's time measurement utils:
 //! <https://github.com/rust-lang/measureme/blob/master/measureme/src/counters.rs>.
@@ -11,7 +11,7 @@
 #![cfg(feature = "tracing")]
 
 /// This alternative `TracingChromeInstant` implementation was made entirely to suit the needs of
-/// [crate::log::tracing_chrome], and shouldn't be used for anything else. It featues two functions:
+/// [crate::log::tracing_chrome], and shouldn't be used for anything else. It features two functions:
 /// - [TracingChromeInstant::setup_for_thread_and_start], which sets up the current thread to do
 ///   proper time tracking and returns a point in time to use as "t=0", and
 /// - [TracingChromeInstant::with_elapsed_micros_subtracting_tracing], which allows

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

@@ -578,7 +578,7 @@ pub mod diagnostics {
                         // - created as Reserved { conflicted: false },
                         //   then Unique -> Disabled is forbidden
                         // A potential `Reserved { conflicted: false }
-                        //   -> Reserved { conflicted: true }` is inexistant or irrelevant,
+                        //   -> Reserved { conflicted: true }` is inexistent or irrelevant,
                         // and so is the `Reserved { conflicted: false } -> Unique`
                         (Unique, Frozen) => false,
                         (ReservedFrz { conflicted: true }, _) => false,

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

@@ -26,7 +26,7 @@ use super::foreign_access_skipping::IdempotentForeignAccess;
 use super::perms::{PermTransition, Permission};
 use super::tree_visitor::{ChildrenVisitMode, ContinueTraversal, NodeAppArgs, TreeVisitor};
 use super::unimap::{UniIndex, UniKeyMap, UniValMap};
-use super::wildcard::WildcardState;
+use super::wildcard::ExposedCache;
 use crate::borrow_tracker::{AccessKind, GlobalState, ProtectorKind};
 use crate::*;
 
@@ -89,7 +89,7 @@ impl LocationState {
         &mut self,
         idx: UniIndex,
         nodes: &mut UniValMap<Node>,
-        wildcard_accesses: &mut UniValMap<WildcardState>,
+        exposed_cache: &mut ExposedCache,
         access_kind: AccessKind,
         relatedness: AccessRelatedness,
         protected: bool,
@@ -99,7 +99,7 @@ impl LocationState {
         // ensures it is only called when `skip_if_known_noop` returns
         // `Recurse`, due to the contract of `traverse_this_parents_children_other`.
         self.record_new_access(access_kind, relatedness);
-
+        let old_access_level = self.permission.strongest_allowed_local_access(protected);
         let transition = self.perform_access(access_kind, relatedness, protected)?;
         if !transition.is_noop() {
             let node = nodes.get_mut(idx).unwrap();
@@ -111,8 +111,8 @@ impl LocationState {
             // We need to update the wildcard state, if the permission
             // of an exposed pointer changes.
             if node.is_exposed {
-                let access_type = self.permission.strongest_allowed_local_access(protected);
-                WildcardState::update_exposure(idx, access_type, nodes, wildcard_accesses);
+                let access_level = self.permission.strongest_allowed_local_access(protected);
+                exposed_cache.update_exposure(nodes, idx, old_access_level, access_level);
             }
         }
         Ok(())
@@ -226,7 +226,7 @@ impl LocationState {
 
     /// Records a new access, so that future access can potentially be skipped
     /// by `skip_if_known_noop`. This must be called on child accesses, and otherwise
-    /// shoud be called on foreign accesses for increased performance. It should not be called
+    /// should be called on foreign accesses for increased performance. It should not be called
     /// when `skip_if_known_noop` indicated skipping, since it then is a no-op.
     /// See `foreign_access_skipping.rs`
     fn record_new_access(&mut self, access_kind: AccessKind, rel_pos: AccessRelatedness) {
@@ -261,22 +261,16 @@ pub struct LocationTree {
     ///
     /// We do uphold the fact that `keys(perms)` is a subset of `keys(nodes)`
     pub perms: UniValMap<LocationState>,
-    /// Maps a tag and a location to its wildcard access tracking information,
-    /// with possible lazy initialization.
-    ///
-    /// If this allocation doesn't have any exposed nodes, then this map doesn't get
-    /// initialized. This way we only need to allocate the map if we need it.
-    ///
-    /// NOTE: same guarantees on entry initialization as for `perms`.
-    pub wildcard_accesses: UniValMap<WildcardState>,
+    /// Caches information about the relatedness of nodes for a wildcard access.
+    pub exposed_cache: ExposedCache,
 }
 /// Tree structure with both parents and children since we want to be
 /// able to traverse the tree efficiently in both directions.
 #[derive(Clone, Debug)]
 pub struct Tree {
     /// Mapping from tags to keys. The key obtained can then be used in
     /// any of the `UniValMap` relative to this allocation, i.e.
-    /// `nodes`, `LocationTree::perms` and `LocationTree::wildcard_accesses`
+    /// `nodes`, `LocationTree::perms` and `LocationTree::exposed_cache`
     /// of the same `Tree`.
     /// The parent-child relationship in `Node` is encoded in terms of these same
     /// keys, so traversing the entire tree needs exactly one access to
@@ -372,8 +366,8 @@ impl Tree {
                     IdempotentForeignAccess::None,
                 ),
             );
-            let wildcard_accesses = UniValMap::default();
-            DedupRangeMap::new(size, LocationTree { perms, wildcard_accesses })
+            let exposed_cache = ExposedCache::default();
+            DedupRangeMap::new(size, LocationTree { perms, exposed_cache })
         };
         Self { roots: SmallVec::from_slice(&[root_idx]), nodes, locations, tag_mapping }
     }
@@ -451,19 +445,9 @@ impl<'tcx> Tree {
             }
         }
 
-        // We need to ensure the consistency of the wildcard access tracking data structure.
-        // For this, we insert the correct entry for this tag based on its parent, if it exists.
-        // If we are inserting a new wildcard root (with Wildcard as parent_prov) then we insert
-        // the special wildcard root initial state instead.
-        for (_range, loc) in self.locations.iter_mut_all() {
-            if let Some(parent_idx) = parent_idx {
-                if let Some(parent_access) = loc.wildcard_accesses.get(parent_idx) {
-                    loc.wildcard_accesses.insert(idx, parent_access.for_new_child());
-                }
-            } else {
-                loc.wildcard_accesses.insert(idx, WildcardState::for_wildcard_root());
-            }
-        }
+        // We don't have to update `exposed_cache` as the new node is not exposed and
+        // has no children so the default counts of 0 are correct.
+
         // If the parent is a wildcard pointer, then it doesn't track SIFA and doesn't need to be updated.
         if let Some(parent_idx) = parent_idx {
             // Inserting the new perms might have broken the SIFA invariant (see
@@ -807,7 +791,7 @@ impl Tree {
         let node = self.nodes.remove(this).unwrap();
         for (_range, loc) in self.locations.iter_mut_all() {
             loc.perms.remove(this);
-            loc.wildcard_accesses.remove(this);
+            loc.exposed_cache.remove(this);
         }
         self.tag_mapping.remove(&node.tag);
     }
@@ -943,7 +927,7 @@ impl<'tcx> LocationTree {
         };
 
         let accessed_root_tag = accessed_root.map(|idx| nodes.get(idx).unwrap().tag);
-        for root in roots {
+        for (i, root) in roots.enumerate() {
             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
@@ -981,6 +965,7 @@ impl<'tcx> LocationTree {
                 access_kind,
                 global,
                 diagnostics,
+                /*is_wildcard_tree*/ i != 0,
             )?;
         }
         interp_ok(())
@@ -1029,7 +1014,7 @@ impl<'tcx> LocationTree {
                 .perform_transition(
                     args.idx,
                     args.nodes,
-                    &mut args.data.wildcard_accesses,
+                    &mut args.data.exposed_cache,
                     access_kind,
                     args.rel_pos,
                     protected,
@@ -1074,12 +1059,18 @@ impl<'tcx> LocationTree {
         access_kind: AccessKind,
         global: &GlobalState,
         diagnostics: &DiagnosticInfo,
+        is_wildcard_tree: bool,
     ) -> InterpResult<'tcx> {
         let get_relatedness = |idx: UniIndex, node: &Node, loc: &LocationTree| {
-            let wildcard_state = loc.wildcard_accesses.get(idx).cloned().unwrap_or_default();
             // If the tag is larger than `max_local_tag` then the access can only be foreign.
             let only_foreign = max_local_tag.is_some_and(|max_local_tag| max_local_tag < node.tag);
-            wildcard_state.access_relatedness(access_kind, only_foreign)
+            loc.exposed_cache.access_relatedness(
+                root,
+                idx,
+                access_kind,
+                is_wildcard_tree,
+                only_foreign,
+            )
         };
 
         // Whether there is an exposed node in this tree that allows this access.
@@ -1156,7 +1147,7 @@ impl<'tcx> LocationTree {
                 perm.perform_transition(
                     args.idx,
                     args.nodes,
-                    &mut args.data.wildcard_accesses,
+                    &mut args.data.exposed_cache,
                     access_kind,
                     relatedness,
                     protected,
@@ -1175,19 +1166,11 @@ impl<'tcx> LocationTree {
                 })
             },
         )?;
-        // If there is no exposed node in this tree that allows this access, then the
-        // access *must* be foreign. So we check if the root of this tree would allow this
-        // as a foreign access, and if not, then we can error.
-        // In practice, all wildcard trees accept foreign accesses, but the main tree does
-        // not, so this catches UB when none of the nodes in the main tree allows this access.
-        if !has_valid_exposed
-            && self
-                .wildcard_accesses
-                .get(root)
-                .unwrap()
-                .access_relatedness(access_kind, /* only_foreign */ true)
-                .is_none()
-        {
+        // If there is no exposed node in this tree that allows this access, then the access *must*
+        // be foreign to the entire subtree. Foreign accesses are only possible on wildcard subtrees
+        // as there are no ancestors to the main root. So if we do not find a valid exposed node in
+        // the main tree then this access is UB.
+        if !has_valid_exposed && !is_wildcard_tree {
             return Err(no_valid_exposed_references_error(diagnostics)).into();
         }
         interp_ok(())

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

@@ -741,7 +741,7 @@ mod spurious_read {
                     );
                     eprintln!("  (arbitrary code instanciated with '{opaque}')");
                     err += 1;
-                    // We found an instanciation of the opaque code that makes this Pattern
+                    // We found an instantiation of the opaque code that makes this Pattern
                     // fail, we don't really need to check the rest.
                     break;
                 }

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

@@ -99,7 +99,7 @@ where
         assert!(self.stack.is_empty());
         // First, handle accessed node. A bunch of things need to
         // be handled differently here compared to the further parents
-        // of `accesssed_node`.
+        // of `accessesed_node`.
         {
             self.propagate_at(this, accessed_node, AccessRelatedness::LocalAccess)?;
             if matches!(visit_children, ChildrenVisitMode::VisitChildrenOfAccessed) {