diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index 041254e6f43..9d24d3c6f47 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -59,6 +59,7 @@ harness = false [features] default = ["stack-cache"] stack-cache = [] +stack-cache-consistency-check = ["stack-cache"] # Be aware that this file is inside a workspace when used via the # submodule in the rustc repo. That means there are many cargo features diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 8c0f605fd6e..b61bff2716c 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -179,18 +179,27 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { ); } cmd.env("RUSTC_WRAPPER", &cargo_miri_path); + // There's also RUSTC_WORKSPACE_WRAPPER, which gets in the way of our own wrapping. + if env::var_os("RUSTC_WORKSPACE_WRAPPER").is_some() { + println!( + "WARNING: Ignoring `RUSTC_WORKSPACE_WRAPPER` environment variable, Miri does not support wrapping." + ); + } + cmd.env_remove("RUSTC_WORKSPACE_WRAPPER"); // We are going to invoke `MIRI` for everything, not `RUSTC`. if env::var_os("RUSTC").is_some() && env::var_os("MIRI").is_none() { println!( "WARNING: Ignoring `RUSTC` environment variable; set `MIRI` if you want to control the binary used as the driver." ); } - // Build scripts (and also cargo: https://github.com/rust-lang/cargo/issues/10885) will invoke - // `rustc` even when `RUSTC_WRAPPER` is set. To make sure everything is coherent, we want that - // to be the Miri driver, but acting as rustc, on the target level. (Target, rather than host, - // is needed for cross-interpretation situations.) This is not a perfect emulation of real rustc - // (it might be unable to produce binaries since the sysroot is check-only), but it's as close - // as we can get, and it's good enough for autocfg. + // Ideally we would set RUSTC to some non-existent path, so we can be sure our wrapping is + // always applied. However, buggy build scripts (https://github.com/eyre-rs/eyre/issues/84) and + // also cargo (https://github.com/rust-lang/cargo/issues/10885) will invoke `rustc` even when + // `RUSTC_WRAPPER` is set, bypassing the wrapper. To make sure everything is coherent, we want + // that to be the Miri driver, but acting as rustc, on the target level. (Target, rather than + // host, is needed for cross-interpretation situations.) This is not a perfect emulation of real + // rustc (it might be unable to produce binaries since the sysroot is check-only), but it's as + // close as we can get, and it's good enough for autocfg. // // In `main`, we need the value of `RUSTC` to distinguish RUSTC_WRAPPER invocations from rustdoc // or TARGET_RUNNER invocations, so we canonicalize it here to make it exceedingly unlikely that @@ -247,6 +256,16 @@ fn is_target_crate() -> bool { /// Cargo does not give us this information directly, so we need to check /// various command-line flags. fn is_runnable_crate() -> bool { + // Determine whether this is cargo invoking rustc to get some infos. Ideally we'd check "is + // there a filename passed to rustc", but that's very hard as we would have to know whether + // e.g. `--print foo` is a booolean flag `--print` followed by filename `foo` or equivalent + // to `--print=foo`. So instead we use this more fragile approach of detecting the presence + // of a "query" flag rather than the absence of a filename. + let info_query = get_arg_flag_value("--print").is_some() || has_arg_flag("-vV"); + if info_query { + // Nothing to run. + return false; + } let is_bin = get_arg_flag_value("--crate-type").as_deref().unwrap_or("bin") == "bin"; let is_test = has_arg_flag("--test"); is_bin || is_test @@ -285,16 +304,9 @@ fn out_filenames() -> Vec { } } - // phase_cargo_miri set `MIRI_BE_RUSTC` for when build scripts directly invoke the driver; - // however, if we get called back by cargo here, we'll carefully compute the right flags - // ourselves, so we first un-do what the earlier phase did. - env::remove_var("MIRI_BE_RUSTC"); - let verbose = std::env::var("MIRI_VERBOSE") .map_or(0, |verbose| verbose.parse().expect("verbosity flag must be an integer")); let target_crate = is_target_crate(); - // Determine whether this is cargo invoking rustc to get some infos. - let info_query = get_arg_flag_value("--print").is_some() || has_arg_flag("-vV"); let store_json = |info: CrateRunInfo| { if get_arg_flag_value("--emit").unwrap_or_default().split(',').any(|e| e == "dep-info") { @@ -321,7 +333,7 @@ fn out_filenames() -> Vec { } }; - let runnable_crate = !info_query && is_runnable_crate(); + let runnable_crate = is_runnable_crate(); if runnable_crate && target_crate { assert!( @@ -395,7 +407,7 @@ fn out_filenames() -> Vec { let mut emit_link_hack = false; // Arguments are treated very differently depending on whether this crate is // for interpretation by Miri, or for use by a build script / proc macro. - if !info_query && target_crate { + if target_crate { // Forward arguments, but remove "link" from "--emit" to make this a check-only build. let emit_flag = "--emit"; while let Some(arg) = args.next() { @@ -429,17 +441,14 @@ fn out_filenames() -> Vec { cmd.arg("-C").arg("panic=abort"); } } else { - // For host crates (but not when we are just printing some info), - // we might still have to set the sysroot. - if !info_query { - // When we're running `cargo-miri` from `x.py` we need to pass the sysroot explicitly - // due to bootstrap complications. - if let Some(sysroot) = std::env::var_os("MIRI_HOST_SYSROOT") { - cmd.arg("--sysroot").arg(sysroot); - } + // This is a host crate. + // When we're running `cargo-miri` from `x.py` we need to pass the sysroot explicitly + // due to bootstrap complications. + if let Some(sysroot) = std::env::var_os("MIRI_HOST_SYSROOT") { + cmd.arg("--sysroot").arg(sysroot); } - // For host crates or when we are printing, just forward everything. + // Forward everything. cmd.args(args); } @@ -451,9 +460,7 @@ fn out_filenames() -> Vec { // Run it. if verbose > 0 { - eprintln!( - "[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate} info_query={info_query}" - ); + eprintln!("[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate}"); } // Create a stub .rlib file if "link" was requested by cargo. @@ -480,11 +487,6 @@ pub enum RunnerPhase { } pub fn phase_runner(mut binary_args: impl Iterator, phase: RunnerPhase) { - // phase_cargo_miri set `MIRI_BE_RUSTC` for when build scripts directly invoke the driver; - // however, if we get called back by cargo here, we'll carefully compute the right flags - // ourselves, so we first un-do what the earlier phase did. - env::remove_var("MIRI_BE_RUSTC"); - let verbose = std::env::var("MIRI_VERBOSE") .map_or(0, |verbose| verbose.parse().expect("verbosity flag must be an integer")); @@ -542,15 +544,13 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner // but when we run here, cargo does not interpret the JSON any more. `--json` // then also needs to be dropped. let mut args = info.args.into_iter(); - let error_format_flag = "--error-format"; - let json_flag = "--json"; while let Some(arg) = args.next() { if arg == "--extern" { forward_patched_extern_arg(&mut args, &mut cmd); - } else if let Some(suffix) = arg.strip_prefix(error_format_flag) { + } else if let Some(suffix) = arg.strip_prefix("--error-format") { assert!(suffix.starts_with('=')); // Drop this argument. - } else if let Some(suffix) = arg.strip_prefix(json_flag) { + } else if let Some(suffix) = arg.strip_prefix("--json") { assert!(suffix.starts_with('=')); // Drop this argument. } else { @@ -589,13 +589,11 @@ pub fn phase_rustdoc(mut args: impl Iterator) { let rustdoc = env::var("MIRI_ORIG_RUSTDOC").unwrap_or("rustdoc".to_string()); let mut cmd = Command::new(rustdoc); - let extern_flag = "--extern"; - let runtool_flag = "--runtool"; while let Some(arg) = args.next() { - if arg == extern_flag { + if arg == "--extern" { // Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files. forward_patched_extern_arg(&mut args, &mut cmd); - } else if arg == runtool_flag { + } else if arg == "--runtool" { // An existing --runtool flag indicates cargo is running in cross-target mode, which we don't support. // Note that this is only passed when cargo is run with the unstable -Zdoctest-xcompile flag; // otherwise, we won't be called as rustdoc at all. diff --git a/src/tools/miri/cargo-miri/src/util.rs b/src/tools/miri/cargo-miri/src/util.rs index 6c1a074cd8c..d99957d9c22 100644 --- a/src/tools/miri/cargo-miri/src/util.rs +++ b/src/tools/miri/cargo-miri/src/util.rs @@ -101,7 +101,12 @@ pub fn find_miri() -> PathBuf { } pub fn miri() -> Command { - Command::new(find_miri()) + let mut cmd = Command::new(find_miri()); + // We never want to inherit this from the environment. + // However, this is sometimes set in the environment to work around build scripts that don't + // honor RUSTC_WRAPPER. So remove it again in case it is set. + cmd.env_remove("MIRI_BE_RUSTC"); + cmd } pub fn miri_for_host() -> Command { diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 54c5d3087fd..cccf10a7d70 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -22,17 +22,24 @@ if [ "$HOST_TARGET" = i686-pc-windows-msvc ]; then BASH="C:/Program Files/Git/usr/bin/bash" fi -# Determine configuration for installed build -echo "Installing release version of Miri" +# Global configuration export RUSTFLAGS="-D warnings" export CARGO_INCREMENTAL=0 export CARGO_EXTRA_FLAGS="--locked" + +# Determine configuration for installed build +echo "Installing release version of Miri" ./miri install -# Prepare debug build for direct `./miri` invocations -echo "Building debug version of Miri" +echo "Checking various feature flag configurations" ./miri check --no-default-features # make sure this can be built -./miri check --all-features # and this, too +./miri check # and this, too +# `--all-features` is used for the build below, so no extra check needed. + +# Prepare debug build for direct `./miri` invocations. +# We enable all features to make sure the Stacked Borrows consistency check runs. +echo "Building debug version of Miri" +export CARGO_EXTRA_FLAGS="$CARGO_EXTRA_FLAGS --all-features" ./miri build --all-targets # the build that all the `./miri test` below will use endgroup @@ -46,8 +53,8 @@ function run_tests { fi ## ui test suite - # On the host and on Linux, also stress-test the GC. - if [ -z "${MIRI_TEST_TARGET:-}" ] || [ "$HOST_TARGET" = x86_64-unknown-linux-gnu ]; then + # On the host, also stress-test the GC. + if [ -z "${MIRI_TEST_TARGET:-}" ]; then MIRIFLAGS="${MIRIFLAGS:-} -Zmiri-provenance-gc=1" ./miri test else ./miri test @@ -130,6 +137,8 @@ case $HOST_TARGET in MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests MIRI_TEST_TARGET=aarch64-apple-darwin run_tests MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests + MIRI_TEST_TARGET=x86_64-pc-windows-gnu run_tests + MIRI_TEST_TARGET=arm-unknown-linux-gnueabi run_tests # Some targets are only partially supported. MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthread-threadname libc-getentropy libc-getrandom libc-misc libc-fs atomic env align num_cpus MIRI_TEST_TARGET=i686-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthread-threadname libc-getentropy libc-getrandom libc-misc libc-fs atomic env align num_cpus @@ -145,9 +154,7 @@ case $HOST_TARGET in MIRI_TEST_TARGET=x86_64-pc-windows-msvc run_tests ;; i686-pc-windows-msvc) - MIRI_TEST_TARGET=arm-unknown-linux-gnueabi run_tests MIRI_TEST_TARGET=x86_64-unknown-linux-gnu run_tests - MIRI_TEST_TARGET=x86_64-pc-windows-gnu run_tests ;; *) echo "FATAL: unknown OS" diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 66f323290b2..58deac66560 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -479,10 +479,11 @@ fn test(bless: bool, flags: Vec) -> Result<()> { Ok(()) } - fn run(dep: bool, flags: Vec) -> Result<()> { + fn run(dep: bool, mut flags: Vec) -> Result<()> { let mut e = MiriEnv::new()?; // Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so - // that we set the MIRI_SYSROOT up the right way. + // that we set the MIRI_SYSROOT up the right way. We must make sure that + // MIRI_TEST_TARGET and `--target` are in sync. use itertools::Itertools; let target = flags .iter() @@ -493,33 +494,35 @@ fn run(dep: bool, flags: Vec) -> Result<()> { // Found it! e.sh.set_var("MIRI_TEST_TARGET", target); } else if let Ok(target) = std::env::var("MIRI_TEST_TARGET") { - // Make sure miri actually uses this target. - let miriflags = e.sh.var("MIRIFLAGS").unwrap_or_default(); - e.sh.set_var("MIRIFLAGS", format!("{miriflags} --target {target}")); + // Convert `MIRI_TEST_TARGET` into `--target`. + flags.push("--target".into()); + flags.push(target.into()); } - // Scan for "--edition" (we'll set one ourselves if that flag is not present). + // Scan for "--edition", set one ourselves if that flag is not present. let have_edition = flags.iter().take_while(|arg| *arg != "--").any(|arg| *arg == "--edition"); + if !have_edition { + flags.push("--edition=2021".into()); // keep in sync with `tests/ui.rs`.` + } // Prepare a sysroot. e.build_miri_sysroot(/* quiet */ true)?; - // Then run the actual command. + // Then run the actual command. Also add MIRIFLAGS. let miri_manifest = path!(e.miri_dir / "Cargo.toml"); let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default(); let miri_flags = flagsplit(&miri_flags); let toolchain = &e.toolchain; let extra_flags = &e.cargo_extra_flags; - let edition_flags = (!have_edition).then_some("--edition=2021"); // keep in sync with `tests/ui.rs`.` if dep { cmd!( e.sh, - "cargo +{toolchain} --quiet test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode {miri_flags...} {edition_flags...} {flags...}" + "cargo +{toolchain} --quiet test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode {miri_flags...} {flags...}" ).quiet().run()?; } else { cmd!( e.sh, - "cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {edition_flags...} {flags...}" + "cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {flags...}" ).quiet().run()?; } Ok(()) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index fa06a069d54..187756851c7 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -cb7c63606e53715f94f3ba04d38e50772e4cd23d +5baf1e13f568b61e121953bf6a3d09faee7dd446 diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs index 712c26a9afd..76430498e2b 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs @@ -146,7 +146,7 @@ impl<'tcx> Stack { /// Panics if any of the caching mechanisms have broken, /// - The StackCache indices don't refer to the parallel items, /// - There are no Unique items outside of first_unique..last_unique - #[cfg(all(feature = "stack-cache", debug_assertions))] + #[cfg(feature = "stack-cache-consistency-check")] fn verify_cache_consistency(&self) { // Only a full cache needs to be valid. Also see the comments in find_granting_cache // and set_unknown_bottom. @@ -190,7 +190,7 @@ pub(super) fn find_granting( tag: ProvenanceExtra, exposed_tags: &FxHashSet, ) -> Result, ()> { - #[cfg(all(feature = "stack-cache", debug_assertions))] + #[cfg(feature = "stack-cache-consistency-check")] self.verify_cache_consistency(); let ProvenanceExtra::Concrete(tag) = tag else { @@ -327,7 +327,7 @@ fn insert_cache(&mut self, new_idx: usize, new: Item) { // This primes the cache for the next access, which is almost always the just-added tag. self.cache.add(new_idx, new); - #[cfg(debug_assertions)] + #[cfg(feature = "stack-cache-consistency-check")] self.verify_cache_consistency(); } @@ -410,7 +410,7 @@ pub fn disable_uniques_starting_at( self.unique_range.end = self.unique_range.end.min(disable_start); } - #[cfg(all(feature = "stack-cache", debug_assertions))] + #[cfg(feature = "stack-cache-consistency-check")] self.verify_cache_consistency(); Ok(()) @@ -465,7 +465,7 @@ pub fn pop_items_after crate::InterpResult<'tcx>>( self.unique_range = 0..0; } - #[cfg(all(feature = "stack-cache", debug_assertions))] + #[cfg(feature = "stack-cache-consistency-check")] self.verify_cache_consistency(); Ok(()) } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs index 4394e3c2c86..2690bc026a1 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -365,7 +365,7 @@ pub fn build<'tcx>(self) -> InterpError<'tcx> { /// Pretty-printing details /// /// Example: -/// ``` +/// ```rust,ignore (private type) /// DisplayFmtWrapper { /// top: '>', /// bot: '<', @@ -393,7 +393,7 @@ struct DisplayFmtWrapper { /// Formating of the permissions on each range. /// /// Example: -/// ``` +/// ```rust,ignore (private type) /// DisplayFmtPermission { /// open: "[", /// sep: "|", @@ -425,7 +425,7 @@ struct DisplayFmtPermission { /// Formating of the tree structure. /// /// Example: -/// ``` +/// ```rust,ignore (private type) /// DisplayFmtPadding { /// join_middle: "|-", /// join_last: "'-", @@ -463,7 +463,7 @@ struct DisplayFmtPadding { /// How to show whether a location has been accessed /// /// Example: -/// ``` +/// ```rust,ignore (private type) /// DisplayFmtAccess { /// yes: " ", /// no: "?", diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index 5c7f5ea46ba..bec51c7cdf2 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -181,8 +181,12 @@ impl Permission { pub fn is_initial(&self) -> bool { self.inner.is_initial() } + /// Check if `self` is the terminal state of a pointer (is `Disabled`). + pub fn is_disabled(&self) -> bool { + self.inner == Disabled + } - /// Default initial permission of the root of a new tree. + /// Default initial permission of the root of a new tree at inbounds positions. /// Must *only* be used for the root, this is not in general an "initial" permission! pub fn new_active() -> Self { Self { inner: Active } @@ -193,11 +197,17 @@ pub fn new_reserved(ty_is_freeze: bool) -> Self { Self { inner: Reserved { ty_is_freeze, conflicted: false } } } - /// Default initial permission of a reborrowed shared reference + /// Default initial permission of a reborrowed shared reference. pub fn new_frozen() -> Self { Self { inner: Frozen } } + /// Default initial permission of the root of a new tre at out-of-bounds positions. + /// Must *only* be used for the root, this is not in general an "initial" permission! + pub fn new_disabled() -> Self { + Self { inner: Disabled } + } + /// Apply the transition to the inner PermissionPriv. pub fn perform_access( kind: AccessKind, diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index dda1c7cca19..0fea78daa88 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -42,6 +42,7 @@ pub(super) struct LocationState { /// protected, that is *not* the case for uninitialized locations. Instead we just have a latent /// "future initial permission" of `Disabled`, causing UB only if an access is ever actually /// performed. + /// Note that the tree root is also always initialized, as if the allocation was a write access. initialized: bool, /// This pointer's current permission / future initial permission. permission: Permission, @@ -55,17 +56,18 @@ pub(super) struct LocationState { } impl LocationState { - /// Default initial state has never been accessed and has been subjected to no - /// foreign access. - fn new(permission: Permission) -> Self { + /// Constructs a new initial state. It has neither been accessed, nor been subjected + /// to any foreign access yet. + /// The permission is not allowed to be `Active`. + fn new_uninit(permission: Permission) -> Self { + assert!(permission.is_initial() || permission.is_disabled()); Self { permission, initialized: false, latest_foreign_access: None } } - /// Record that this location was accessed through a child pointer by - /// marking it as initialized - fn with_access(mut self) -> Self { - self.initialized = true; - self + /// Constructs a new initial state. It has not yet been subjected + /// to any foreign access. However, it is already marked as having been accessed. + fn new_init(permission: Permission) -> Self { + Self { permission, initialized: true, latest_foreign_access: None } } /// Check if the location has been initialized, i.e. if it has @@ -238,8 +240,10 @@ pub(super) struct Node { /// If the pointer was reborrowed, it has children. // FIXME: bench to compare this to FxHashSet and to other SmallVec sizes pub children: SmallVec<[UniIndex; 4]>, - /// Either `Reserved` or `Frozen`, the permission this tag will be lazily initialized - /// to on the first access. + /// Either `Reserved`, `Frozen`, or `Disabled`, it is the permission this tag will + /// lazily be initialized to on the first access. + /// It is only ever `Disabled` for a tree root, since the root is initialized to `Active` by + /// its own separate mechanism. default_initial_perm: Permission, /// Some extra information useful only for debugging purposes pub debug_info: NodeDebugInfo, @@ -444,12 +448,14 @@ fn traverse_nonchildren( impl Tree { /// Create a new tree, with only a root pointer. pub fn new(root_tag: BorTag, size: Size, span: Span) -> Self { - let root_perm = Permission::new_active(); + // The root has `Disabled` as the default permission, + // so that any access out of bounds is invalid. + let root_default_perm = Permission::new_disabled(); let mut tag_mapping = UniKeyMap::default(); let root_idx = tag_mapping.insert(root_tag); let nodes = { let mut nodes = UniValMap::::default(); - let mut debug_info = NodeDebugInfo::new(root_tag, root_perm, span); + let mut debug_info = NodeDebugInfo::new(root_tag, root_default_perm, span); // name the root so that all allocations contain one named pointer debug_info.add_name("root of the allocation"); nodes.insert( @@ -458,7 +464,7 @@ pub fn new(root_tag: BorTag, size: Size, span: Span) -> Self { tag: root_tag, parent: None, children: SmallVec::default(), - default_initial_perm: root_perm, + default_initial_perm: root_default_perm, debug_info, }, ); @@ -466,7 +472,11 @@ pub fn new(root_tag: BorTag, size: Size, span: Span) -> Self { }; let rperms = { let mut perms = UniValMap::default(); - perms.insert(root_idx, LocationState::new(root_perm).with_access()); + // We manually set it to `Active` on all in-bounds positions. + // We also ensure that it is initalized, so that no `Active` but + // not yet initialized nodes exist. Essentially, we pretend there + // was a write that initialized these to `Active`. + perms.insert(root_idx, LocationState::new_init(Permission::new_active())); RangeMap::new(size, perms) }; Self { root: root_idx, nodes, rperms, tag_mapping } @@ -500,7 +510,7 @@ pub fn new_child( // Register new_tag as a child of parent_tag self.nodes.get_mut(parent_idx).unwrap().children.push(idx); // Initialize perms - let perm = LocationState::new(default_initial_perm).with_access(); + let perm = LocationState::new_init(default_initial_perm); for (_perms_range, perms) in self.rperms.iter_mut(reborrow_range.start, reborrow_range.size) { perms.insert(idx, perm); @@ -599,7 +609,7 @@ pub fn perform_access( -> Result { let NodeAppArgs { node, mut perm, rel_pos } = args; - let old_state = perm.or_insert(LocationState::new(node.default_initial_perm)); + let old_state = perm.or_insert(LocationState::new_uninit(node.default_initial_perm)); match old_state.skip_if_known_noop(access_kind, rel_pos) { ContinueTraversal::SkipChildren => return Ok(ContinueTraversal::SkipChildren), diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs index 35f3b53afdb..f568850d8db 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs @@ -516,11 +516,11 @@ fn reserved_aliased_protected_indistinguishable() { let source = LocStateProtPair { xy_rel: RelPosXY::MutuallyForeign, x: LocStateProt { - state: LocationState::new(Permission::new_frozen()).with_access(), + state: LocationState::new_init(Permission::new_frozen()), prot: true, }, y: LocStateProt { - state: LocationState::new(Permission::new_reserved(false)), + state: LocationState::new_uninit(Permission::new_reserved(false)), prot: true, }, }; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/unimap.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/unimap.rs index d9cad9c8e0b..f45b2d9e00a 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/unimap.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/unimap.rs @@ -132,7 +132,7 @@ pub fn get_or_insert(&mut self, key: K) -> UniIndex { /// the associated `UniIndex` from ALL `UniValMap`s. /// /// Example of such behavior: - /// ``` + /// ```rust,ignore (private type can't be doctested) /// let mut keymap = UniKeyMap::::default(); /// let mut valmap = UniValMap::::default(); /// // Insert 'a' -> _ -> 'A' diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index e2e18d3a734..d0d73bb1b34 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -1046,7 +1046,7 @@ fn unregister_timeout_callback_if_exists(&mut self, thread: ThreadId) { /// Run the core interpreter loop. Returns only when an interrupt occurs (an error or program /// termination). fn run_threads(&mut self) -> InterpResult<'tcx, !> { - let this = self.eval_context_mut(); + let this = self.eval_context_mut(); loop { if CTRL_C_RECEIVED.load(Relaxed) { this.machine.handle_abnormal_termination(); diff --git a/src/tools/miri/test-cargo-miri/Cargo.lock b/src/tools/miri/test-cargo-miri/Cargo.lock index f75d68f4e29..4783f79ea8f 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.lock +++ b/src/tools/miri/test-cargo-miri/Cargo.lock @@ -2,17 +2,11 @@ # It is not intended for manual editing. version = 3 -[[package]] -name = "anyhow" -version = "1.0.81" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0952808a6c2afd1aa8947271f3a60f1a6763c7b912d210184c5149b5cf147247" - [[package]] name = "autocfg" -version = "1.1.0" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +checksum = "f1fdabc7756949593fe60f30ec81974b613357de856987752631dea1e3394c80" [[package]] name = "byteorder" @@ -22,33 +16,33 @@ checksum = "0fc10e8cc6b2580fda3f36eb6dc5316657f812a3df879a44a66fc9f0fdbc4855" [[package]] name = "byteorder" -version = "1.4.3" +version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "cargo-miri-test" version = "0.1.0" dependencies = [ - "anyhow", "autocfg", "byteorder 0.5.3", - "byteorder 1.4.3", + "byteorder 1.5.0", "cdylib", "exported_symbol", + "eyre", "issue_1567", "issue_1691", "issue_1705", - "issue_1760", "issue_rust_86261", - "serde_derive", + "proc-macro2", + "proc_macro_crate", ] [[package]] name = "cdylib" version = "0.1.0" dependencies = [ - "byteorder 1.4.3", + "byteorder 1.5.0", ] [[package]] @@ -62,11 +56,27 @@ dependencies = [ name = "exported_symbol_dep" version = "0.1.0" +[[package]] +name = "eyre" +version = "0.6.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cd915d99f24784cdc19fd37ef22b97e3ff0ae756c7e492e9fbfe897d61e2aec" +dependencies = [ + "indenter", + "once_cell", +] + +[[package]] +name = "indenter" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce23b50ad8242c51a442f3ff322d56b02f08852c77e4c0b4d3fd684abc89c683" + [[package]] name = "issue_1567" version = "0.1.0" dependencies = [ - "byteorder 1.4.3", + "byteorder 1.5.0", ] [[package]] @@ -77,66 +87,44 @@ version = "0.1.0" name = "issue_1705" version = "0.1.0" dependencies = [ - "byteorder 1.4.3", + "byteorder 1.5.0", ] -[[package]] -name = "issue_1760" -version = "0.1.0" - [[package]] name = "issue_rust_86261" version = "0.1.0" [[package]] -name = "proc-macro2" -version = "1.0.66" +name = "once_cell" +version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18fb31db3f9bddb2ea821cde30a9f70117e3f119938b5ee630b7403aa6e2ead9" +checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" + +[[package]] +name = "proc-macro2" +version = "1.0.79" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e835ff2298f5721608eb1a980ecaee1aef2c132bf95ecc026a11b7bf3c01c02e" dependencies = [ "unicode-ident", ] [[package]] -name = "quote" -version = "1.0.33" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" +name = "proc_macro_crate" +version = "0.1.0" dependencies = [ "proc-macro2", ] -[[package]] -name = "serde_derive" -version = "1.0.185" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc59dfdcbad1437773485e0367fea4b090a2e0a16d9ffc46af47764536a298ec" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "subcrate" version = "0.1.0" dependencies = [ - "byteorder 1.4.3", -] - -[[package]] -name = "syn" -version = "2.0.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a" -dependencies = [ - "proc-macro2", - "quote", - "unicode-ident", + "byteorder 1.5.0", ] [[package]] name = "unicode-ident" -version = "1.0.6" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "84a22b9f218b40614adcb3f4ff08b703773ad44fa9423e4e0d346d5db86e4ebc" +checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" diff --git a/src/tools/miri/test-cargo-miri/Cargo.toml b/src/tools/miri/test-cargo-miri/Cargo.toml index 58c451741bb..bfef388669d 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.toml +++ b/src/tools/miri/test-cargo-miri/Cargo.toml @@ -12,19 +12,21 @@ edition = "2018" byteorder = "1.0" cdylib = { path = "cdylib" } exported_symbol = { path = "exported-symbol" } +proc_macro_crate = { path = "proc-macro-crate" } issue_1567 = { path = "issue-1567" } issue_1691 = { path = "issue-1691" } issue_1705 = { path = "issue-1705" } -issue_1760 = { path = "issue-1760" } issue_rust_86261 = { path = "issue-rust-86261" } [dev-dependencies] byteorder_2 = { package = "byteorder", version = "0.5" } # to test dev-dependencies behave as expected, with renaming -# Not actually used, but exercises some unique code path (`--extern` .so file). -serde_derive = "1.0.185" -# Not actually used, but uses a custom build probe so let's make sure that works. +## More dependencies that we don't actually use, but add just for extra test coverage. +# These use custom build probes, let's make sure they don't explode. # (Ideally we'd check if the probe was successful, but that's not easily possible.) -anyhow = "1.0" +# proc-macro2 is extra exciting because it is both a host-dependency (of proc_macro_crate above) +# and a target-dependency. +proc-macro2 = "1.0" +eyre = "0.6" [build-dependencies] autocfg = "1" diff --git a/src/tools/miri/test-cargo-miri/issue-1760/Cargo.toml b/src/tools/miri/test-cargo-miri/issue-1760/Cargo.toml deleted file mode 100644 index 80925c74746..00000000000 --- a/src/tools/miri/test-cargo-miri/issue-1760/Cargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -name = "issue_1760" -version = "0.1.0" -authors = ["Miri Team"] -edition = "2018" - -[lib] -proc-macro = true diff --git a/src/tools/miri/test-cargo-miri/proc-macro-crate/Cargo.toml b/src/tools/miri/test-cargo-miri/proc-macro-crate/Cargo.toml new file mode 100644 index 00000000000..89652f9b042 --- /dev/null +++ b/src/tools/miri/test-cargo-miri/proc-macro-crate/Cargo.toml @@ -0,0 +1,13 @@ +[package] +# regression test for issue 1760 +name = "proc_macro_crate" +version = "0.1.0" +authors = ["Miri Team"] +edition = "2018" + +[lib] +proc-macro = true + +[dependencies] +# A common dependency of proc macros, let's make sure that works. +proc-macro2 = "1.0" diff --git a/src/tools/miri/test-cargo-miri/issue-1760/build.rs b/src/tools/miri/test-cargo-miri/proc-macro-crate/build.rs similarity index 100% rename from src/tools/miri/test-cargo-miri/issue-1760/build.rs rename to src/tools/miri/test-cargo-miri/proc-macro-crate/build.rs diff --git a/src/tools/miri/test-cargo-miri/issue-1760/src/lib.rs b/src/tools/miri/test-cargo-miri/proc-macro-crate/src/lib.rs similarity index 100% rename from src/tools/miri/test-cargo-miri/issue-1760/src/lib.rs rename to src/tools/miri/test-cargo-miri/proc-macro-crate/src/lib.rs diff --git a/src/tools/miri/test-cargo-miri/src/lib.rs b/src/tools/miri/test-cargo-miri/src/lib.rs index e6b8c4ef65b..003341d0974 100644 --- a/src/tools/miri/test-cargo-miri/src/lib.rs +++ b/src/tools/miri/test-cargo-miri/src/lib.rs @@ -28,9 +28,9 @@ /// ``` #[no_mangle] pub fn make_true() -> bool { + proc_macro_crate::use_the_dependency!(); issue_1567::use_the_dependency(); issue_1705::use_the_dependency(); - issue_1760::use_the_dependency!(); issue_1691::use_me() } diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 129d1dfd732..a75fa4cf986 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -54,34 +54,13 @@ fn build_so_for_c_ffi_tests() -> PathBuf { so_file_path } -fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> Config { +/// Does *not* set any args or env vars, since it is shared between the test runner and +/// run_dep_mode. +fn miri_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> Config { // Miri is rustc-like, so we create a default builder for rustc and modify it let mut program = CommandBuilder::rustc(); program.program = miri_path(); - // Add some flags we always want. - program.args.push("-Dwarnings".into()); - program.args.push("-Dunused".into()); - program.args.push("-Ainternal_features".into()); - if let Ok(extra_flags) = env::var("MIRIFLAGS") { - for flag in extra_flags.split_whitespace() { - program.args.push(flag.into()); - } - } - program.args.push("-Zui-testing".into()); - program.args.push("--target".into()); - program.args.push(target.into()); - - // If we're on linux, and we're testing the extern-so functionality, - // then build the shared object file for testing external C function calls - // and push the relevant compiler flag. - if cfg!(target_os = "linux") && path.starts_with("tests/extern-so/") { - let so_file_path = build_so_for_c_ffi_tests(); - let mut flag = std::ffi::OsString::from("-Zmiri-extern-so-file="); - flag.push(so_file_path.into_os_string()); - program.args.push(flag); - } - let mut config = Config { target: Some(target.to_owned()), stderr_filters: STDERR.clone(), @@ -119,17 +98,38 @@ fn run_tests( with_dependencies: bool, tmpdir: &Path, ) -> Result<()> { - let mut config = test_config(target, path, mode, with_dependencies); + let mut config = miri_config(target, path, mode, with_dependencies); // Add a test env var to do environment communication tests. config.program.envs.push(("MIRI_ENV_VAR_TEST".into(), Some("0".into()))); - // Let the tests know where to store temp files (they might run for a different target, which can make this hard to find). config.program.envs.push(("MIRI_TEMP".into(), Some(tmpdir.to_owned().into()))); - // If a test ICEs, we want to see a backtrace. config.program.envs.push(("RUST_BACKTRACE".into(), Some("1".into()))); + // Add some flags we always want. + config.program.args.push("-Dwarnings".into()); + config.program.args.push("-Dunused".into()); + config.program.args.push("-Ainternal_features".into()); + if let Ok(extra_flags) = env::var("MIRIFLAGS") { + for flag in extra_flags.split_whitespace() { + config.program.args.push(flag.into()); + } + } + config.program.args.push("-Zui-testing".into()); + config.program.args.push("--target".into()); + config.program.args.push(target.into()); + + // If we're on linux, and we're testing the extern-so functionality, + // then build the shared object file for testing external C function calls + // and push the relevant compiler flag. + if cfg!(target_os = "linux") && path.starts_with("tests/extern-so/") { + let so_file_path = build_so_for_c_ffi_tests(); + let mut flag = std::ffi::OsString::from("-Zmiri-extern-so-file="); + flag.push(so_file_path.into_os_string()); + config.program.args.push(flag); + } + // Handle command-line arguments. let args = ui_test::Args::test()?; let default_bless = env::var_os("RUSTC_BLESS").is_some_and(|v| v != "0"); @@ -292,13 +292,12 @@ fn main() -> Result<()> { fn run_dep_mode(target: String, mut args: impl Iterator) -> Result<()> { let path = args.next().expect("./miri run-dep must be followed by a file name"); - let mut config = test_config( + let config = miri_config( &target, "", Mode::Yolo { rustfix: RustfixMode::Disabled }, /* with dependencies */ true, ); - config.program.args.clear(); // We want to give the user full control over flags let dep_args = config.build_dependencies()?; let mut cmd = config.program.build(&config.out_dir);