From f836cfa69347ba27097a510d689eb7041ea63d2b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 17 Aug 2023 13:47:08 +0200 Subject: [PATCH 1/7] tree borrows: more comments in foreign_read transition --- .../src/borrow_tracker/tree_borrows/perms.rs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) 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 0ce29ac5437f8..dab216bc5b1c5 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -68,17 +68,31 @@ mod transition { fn foreign_read(state: PermissionPriv, protected: bool) -> Option { use Option::*; Some(match state { + // Non-writeable states just ignore foreign reads. + non_writeable @ (Frozen | Disabled) => non_writeable, + // Writeable states are more tricky, and depend on whether things are protected. // The inner data `ty_is_freeze` of `Reserved` is always irrelevant for Read // accesses, since the data is not being mutated. Hence the `{ .. }` - res @ Reserved { .. } if !protected => res, - Reserved { .. } => Frozen, // protected reserved + res @ Reserved { .. } => + if protected { + // Someone else read, make sure we won't write. + // We could make this `Disabled` but it doesn't look like we get anything out of that extra UB. + Frozen + } else { + // Before activation and without protectors, foreign reads are fine. + // That's the entire point of 2-phase borrows. + res + }, Active => if protected { + // We wrote, someone else reads -- that's bad. + // (If this is initialized, this move-to-protected will mean insta-UB.) Disabled } else { + // We don't want to disable here to allow read-read reordering: it is crucial + // that the foreign read does not invalidate future reads through this tag. Frozen }, - non_writeable @ (Frozen | Disabled) => non_writeable, }) } From 44fa4cdf94473b292625c259d49f2bcf2b325886 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 20 Aug 2023 16:31:10 +0200 Subject: [PATCH 2/7] pin a version of serde without intransparent unreproducible binary blobs --- src/tools/miri/Cargo.lock | 1 + src/tools/miri/Cargo.toml | 2 ++ src/tools/miri/cargo-miri/Cargo.toml | 3 ++- src/tools/miri/test-cargo-miri/Cargo.toml | 4 +++- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index 4232d7fda78f1..d7a9acd419ffa 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -443,6 +443,7 @@ dependencies = [ "rand", "regex", "rustc_version", + "serde", "smallvec", "ui_test", ] diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index a625e1696e143..2ad89cdde532a 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -41,6 +41,8 @@ rustc_version = "0.4" # Features chosen to match those required by env_logger, to avoid rebuilds regex = { version = "1.5.5", default-features = false, features = ["perf", "std"] } lazy_static = "1.4.0" +# Pin a version of serde without intransparent unreproducible binary blobs. +serde = { version = "1.0, < 1.0.172", features = ["derive"] } [package.metadata.rust-analyzer] # This crate uses #[feature(rustc_private)]. diff --git a/src/tools/miri/cargo-miri/Cargo.toml b/src/tools/miri/cargo-miri/Cargo.toml index cfa5ac5281df0..87bf4a1626f9a 100644 --- a/src/tools/miri/cargo-miri/Cargo.toml +++ b/src/tools/miri/cargo-miri/Cargo.toml @@ -22,7 +22,8 @@ rustc-build-sysroot = "0.4.1" # Enable some feature flags that dev-dependencies need but dependencies # do not. This makes `./miri install` after `./miri build` faster. -serde = { version = "*", features = ["derive"] } +# Pin a version of serde without intransparent unreproducible binary blobs. +serde = { version = "1.0, < 1.0.172", features = ["derive"] } [build-dependencies] rustc_tools_util = "0.3" diff --git a/src/tools/miri/test-cargo-miri/Cargo.toml b/src/tools/miri/test-cargo-miri/Cargo.toml index 37c996de6623c..ea44b3f64b671 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.toml +++ b/src/tools/miri/test-cargo-miri/Cargo.toml @@ -20,7 +20,9 @@ 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 -serde_derive = "1.0" # not actually used, but exercises some unique code path (`--extern` .so file) +# Not actually used, but exercises some unique code path (`--extern` .so file). +# Pin a version without intransparent unreproducible binary blobs. +serde_derive = "=1.0.152" [build-dependencies] autocfg = "1" From c05fb4cbf99eea385c4c051fbbc1f652e37a9b08 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Mon, 21 Aug 2023 05:29:21 +0000 Subject: [PATCH 3/7] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 30123b92f6caa..d96e7040aaf73 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -656ee47db32e882fb02913f6204e09cc7a41a50e +c40cfcf0494ff7506e753e750adb00eeea839f9c From 5356aa43044e6e142985ff169bd14423220f34ff Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Mon, 21 Aug 2023 05:39:33 +0000 Subject: [PATCH 4/7] fmt --- src/tools/miri/src/shims/intrinsics/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/src/shims/intrinsics/mod.rs b/src/tools/miri/src/shims/intrinsics/mod.rs index ea6ea5a8474e9..8a84b4f51b7f2 100644 --- a/src/tools/miri/src/shims/intrinsics/mod.rs +++ b/src/tools/miri/src/shims/intrinsics/mod.rs @@ -44,7 +44,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { "the program aborted execution".to_owned() )) } - _ => {}, + _ => {} } // All remaining supported intrinsics have a return place. From 5615562b68ae3985f3dab0f9ac87f92eceb31bab Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 21 Aug 2023 09:37:54 +0200 Subject: [PATCH 5/7] update recommended RA config --- src/tools/miri/CONTRIBUTING.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index b67e7103fd095..60ba2cd2346e1 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -165,16 +165,17 @@ to `.vscode/settings.json` in your local Miri clone: { "rust-analyzer.rustc.source": "discover", "rust-analyzer.linkedProjects": [ - "./Cargo.toml", - "./cargo-miri/Cargo.toml" + "Cargo.toml", + "cargo-miri/Cargo.toml", + "miri-script/Cargo.toml", ], - "rust-analyzer.checkOnSave.overrideCommand": [ + "rust-analyzer.check.overrideCommand": [ "env", "MIRI_AUTO_OPS=no", "./miri", "cargo", "clippy", // make this `check` when working with a locally built rustc - "--message-format=json" + "--message-format=json", ], // Contrary to what the name suggests, this also affects proc macros. "rust-analyzer.cargo.buildScripts.overrideCommand": [ From 2f6ffa923e83c31226e7d0e88e45609bf05564e4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 21 Aug 2023 09:38:06 +0200 Subject: [PATCH 6/7] fix MIRI_AUTO_OPS not having an effect any more --- src/tools/miri/README.md | 4 ++-- src/tools/miri/miri-script/src/commands.rs | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 4483ae242d5e9..06fe668354ac2 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -458,8 +458,8 @@ Some native rustc `-Z` flags are also very relevant for Miri: Moreover, Miri recognizes some environment variables: * `MIRI_AUTO_OPS` indicates whether the automatic execution of rustfmt, clippy and toolchain setup - should be skipped. If it is set to any value, they are skipped. This is used for avoiding infinite - recursion in `./miri` and to allow automated IDE actions to avoid the auto ops. + should be skipped. If it is set to `no`, they are skipped. This is used to allow automated IDE + actions to avoid the auto ops. * `MIRI_LOG`, `MIRI_BACKTRACE` control logging and backtrace printing during Miri executions, also [see "Testing the Miri driver" in `CONTRIBUTING.md`][testing-miri]. * `MIRIFLAGS` (recognized by `cargo miri` and the test suite) defines extra diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index ed78f80c02335..ebaef1fd475eb 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -57,6 +57,10 @@ impl MiriEnv { impl Command { fn auto_actions() -> Result<()> { + if env::var_os("MIRI_AUTO_OPS").is_some_and(|x| x == "no") { + return Ok(()); + } + let miri_dir = miri_dir()?; let auto_everything = path!(miri_dir / ".auto-everything").exists(); let auto_toolchain = auto_everything || path!(miri_dir / ".auto-toolchain").exists(); @@ -78,6 +82,7 @@ impl Command { } pub fn exec(self) -> Result<()> { + // First, and crucially only once, run the auto-actions -- but not for all commands. match &self { Command::Install { .. } | Command::Build { .. } @@ -93,6 +98,7 @@ impl Command { | Command::Bench { .. } | Command::RustcPush { .. } => {} } + // Then run the actual command. match self { Command::Install { flags } => Self::install(flags), Command::Build { flags } => Self::build(flags), From 9a3cb9e5e499b5e1231c777a8b73b9e2312a7dcf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 21 Aug 2023 12:09:20 +0200 Subject: [PATCH 7/7] update lockfile --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 266f34e69edb8..e46a1f387ee1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2334,6 +2334,7 @@ dependencies = [ "rand", "regex", "rustc_version", + "serde", "smallvec", "ui_test", ]