Auto merge of rust-lang#131727 - RalfJung:miri-sync, r=RalfJung

Miri subtree update

r? `@ghost`

Author: bors

Cargo.lock

@@ -377,7 +377,7 @@ dependencies = [
  "cargo_metadata",
  "directories",
  "rustc-build-sysroot",
- "rustc_tools_util",
+ "rustc_tools_util 0.4.0",
  "rustc_version",
  "serde",
  "serde_json",
@@ -552,7 +552,7 @@ dependencies = [
  "parking_lot",
  "quote",
  "regex",
- "rustc_tools_util",
+ "rustc_tools_util 0.3.0",
  "serde",
  "serde_json",
  "syn 2.0.79",
@@ -4465,6 +4465,12 @@ version = "0.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "8ba09476327c4b70ccefb6180f046ef588c26a24cf5d269a9feba316eb4f029f"
 
+[[package]]
+name = "rustc_tools_util"
+version = "0.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3316159ab19e19d1065ecc49278e87f767a9dae9fae80348d2b4d4fa4ae02d4d"
+
 [[package]]
 name = "rustc_trait_selection"
 version = "0.0.0"

src/tools/miri/CONTRIBUTING.md

@@ -45,6 +45,14 @@ 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!
 
+### Managing the review state
+
+Most PRs bounce back and forth between the reviewer and the author several times, so it is good to
+keep track of who is expected to take the next step. We are using the `S-waiting-for-review` and
+`S-waiting-for-author` labels for that. If a reviewer asked you to do some changes and you think
+they are all taken care of, post a comment saying `@rustbot ready` to mark a PR as ready for the
+next round of review.
+
 ## Preparing the build environment
 
 Miri heavily relies on internal and unstable rustc interfaces to execute MIR,
@@ -195,48 +203,37 @@ installed (`cargo install hyperfine`).
 
 ## Configuring `rust-analyzer`
 
-To configure `rust-analyzer` and VS Code for working on Miri, save the following
-to `.vscode/settings.json` in your local Miri clone:
-
-```json
-{
-    "rust-analyzer.rustc.source": "discover",
-    "rust-analyzer.linkedProjects": [
-        "Cargo.toml",
-        "cargo-miri/Cargo.toml",
-        "miri-script/Cargo.toml",
-    ],
-    "rust-analyzer.check.invocationLocation": "root",
-    "rust-analyzer.check.invocationStrategy": "once",
-    "rust-analyzer.check.overrideCommand": [
-        "env",
-        "MIRI_AUTO_OPS=no",
-        "./miri",
-        "clippy", // make this `check` when working with a locally built rustc
-        "--message-format=json",
-    ],
-    // Contrary to what the name suggests, this also affects proc macros.
-    "rust-analyzer.cargo.buildScripts.invocationLocation": "root",
-    "rust-analyzer.cargo.buildScripts.invocationStrategy": "once",
-    "rust-analyzer.cargo.buildScripts.overrideCommand": [
-        "env",
-        "MIRI_AUTO_OPS=no",
-        "./miri",
-        "check",
-        "--message-format=json",
-    ],
-}
-```
+To configure `rust-analyzer` and the IDE for working on Miri, copy one of the provided
+configuration files according to the instructions below. You can also set up a symbolic
+link to keep the configuration in sync with our recommendations.
+
+### Visual Studio Code
+
+Copy [`etc/rust_analyzer_vscode.json`] to `.vscode/settings.json` in the project root directory.
+
+[`etc/rust_analyzer_vscode.json`]: https://github.com/rust-lang/miri/blob/master/etc/rust_analyzer_vscode.json
+
+### Helix
+
+Copy [`etc/rust_analyzer_helix.toml`] to `.helix/languages.toml` in the project root directory.
+
+Since working on Miri requires a custom toolchain, and Helix requires the language server
+to be installed with the toolchain, you have to run `./miri toolchain -c rust-analyzer`
+when installing the Miri toolchain. Alternatively, set the `RUSTUP_TOOLCHAIN` environment variable according to
+[the documentation](https://rust-analyzer.github.io/manual.html#toolchain).
+
+[`etc/rust_analyzer_helix.toml`]: https://github.com/rust-lang/miri/blob/master/etc/rust_analyzer_helix.toml
+
+### Advanced configuration
 
-> #### Note
->
-> If you are [building Miri with a locally built rustc][], set
-> `rust-analyzer.rustcSource` to the relative path from your Miri clone to the
-> root `Cargo.toml` of the locally built rustc. For example, the path might look
-> like `../rust/Cargo.toml`.
+If you are building Miri with a locally built rustc, set
+`rust-analyzer.rustcSource` to the relative path from your Miri clone to the
+root `Cargo.toml` of the locally built rustc. For example, the path might look
+like `../rust/Cargo.toml`. In addition to that, replace `clippy` by `check`
+in the `rust-analyzer.check.overrideCommand` setting.
 
 See the rustc-dev-guide's docs on ["Configuring `rust-analyzer` for `rustc`"][rdg-r-a]
-for more information about configuring VS Code and `rust-analyzer`.
+for more information about configuring the IDE and `rust-analyzer`.
 
 [rdg-r-a]: https://rustc-dev-guide.rust-lang.org/building/suggested.html#configuring-rust-analyzer-for-rustc
 

src/tools/miri/README.md

@@ -290,7 +290,7 @@ environment variable. We first document the most relevant and most commonly used
 * `-Zmiri-compare-exchange-weak-failure-rate=<rate>` changes the failure rate of
   `compare_exchange_weak` operations. The default is `0.8` (so 4 out of 5 weak ops will fail).
   You can change it to any value between `0.0` and `1.0`, where `1.0` means it
-  will always fail and `0.0` means it will never fail. Note than setting it to
+  will always fail and `0.0` means it will never fail. Note that setting it to
   `1.0` will likely cause hangs, since it means programs using
   `compare_exchange_weak` cannot make progress.
 * `-Zmiri-disable-isolation` disables host isolation.  As a consequence,
@@ -392,11 +392,6 @@ to Miri failing to detect cases of undefined behavior in a program.
   but reports to the program that it did actually write. This is useful when you
   are not interested in the actual program's output, but only want to see Miri's
   errors and warnings.
-* `-Zmiri-panic-on-unsupported` will makes some forms of unsupported functionality,
-  such as FFI and unsupported syscalls, panic within the context of the emulated
-  application instead of raising an error within the context of Miri (and halting
-  execution). Note that code might not expect these operations to ever panic, so
-  this flag can lead to strange (mis)behavior.
 * `-Zmiri-recursive-validation` is a *highly experimental* flag that makes validity checking
   recurse below references.
 * `-Zmiri-retag-fields[=<all|none|scalar>]` controls when Stacked Borrows retagging recurses into

src/tools/miri/cargo-miri/Cargo.lock

@@ -1,6 +1,6 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
-version = 3
+version = 4
 
 [[package]]
 name = "anyhow"
@@ -202,9 +202,9 @@ dependencies = [
 
 [[package]]
 name = "rustc_tools_util"
-version = "0.3.0"
+version = "0.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8ba09476327c4b70ccefb6180f046ef588c26a24cf5d269a9feba316eb4f029f"
+checksum = "3316159ab19e19d1065ecc49278e87f767a9dae9fae80348d2b4d4fa4ae02d4d"
 
 [[package]]
 name = "rustc_version"

src/tools/miri/cargo-miri/Cargo.toml

@@ -26,4 +26,4 @@ rustc-build-sysroot = "0.5.4"
 serde = { version = "1.0.185", features = ["derive"] }
 
 [build-dependencies]
-rustc_tools_util = "0.3"
+rustc_tools_util = "0.4"

src/tools/miri/ci/ci.sh

@@ -150,10 +150,10 @@ case $HOST_TARGET in
     # Partially supported targets (tier 2)
     BASIC="empty_main integer heap_alloc libc-mem vec string btreemap" # ensures we have the basics: pre-main code, system allocator
     UNIX="hello panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there
-    TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX time hashmap random threadname pthread fs
-    TEST_TARGET=i686-unknown-freebsd   run_tests_minimal $BASIC $UNIX time hashmap random threadname pthread fs
-    TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls
-    TEST_TARGET=x86_64-pc-solaris      run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls
+    TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX time hashmap random threadname pthread fs libc-pipe
+    TEST_TARGET=i686-unknown-freebsd   run_tests_minimal $BASIC $UNIX time hashmap random threadname pthread fs libc-pipe
+    TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls libc-pipe
+    TEST_TARGET=x86_64-pc-solaris      run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls libc-pipe
     TEST_TARGET=aarch64-linux-android  run_tests_minimal $BASIC $UNIX time hashmap pthread --skip threadname
     TEST_TARGET=wasm32-wasip2          run_tests_minimal $BASIC wasm
     TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std empty_main wasm # this target doesn't really have std

src/tools/miri/etc/rust_analyzer_helix.toml

@@ -0,0 +1,32 @@
+[language-server.rust-analyzer.config.rustc]
+source = "discover"
+
+[language-server.rust-analyzer.config]
+linkedProjects = [
+    "Cargo.toml",
+    "cargo-miri/Cargo.toml",
+    "miri-script/Cargo.toml",
+]
+
+[language-server.rust-analyzer.config.check]
+invocationLocation = "root"
+invocationStrategy = "once"
+overrideCommand = [
+    "env",
+    "MIRI_AUTO_OPS=no",
+    "./miri",
+    "clippy", # make this `check` when working with a locally built rustc
+    "--message-format=json",
+]
+
+# Contrary to what the name suggests, this also affects proc macros.
+[language-server.rust-analyzer.config.buildScripts]
+invocationLocation = "root"
+invocationStrategy = "once"
+overrideCommand = [
+    "env",
+    "MIRI_AUTO_OPS=no",
+    "./miri",
+    "check",
+    "--message-format=json",
+]

src/tools/miri/etc/rust_analyzer_vscode.json

@@ -0,0 +1,27 @@
+{
+    "rust-analyzer.rustc.source": "discover",
+    "rust-analyzer.linkedProjects": [
+        "Cargo.toml",
+        "cargo-miri/Cargo.toml",
+        "miri-script/Cargo.toml",
+    ],
+    "rust-analyzer.check.invocationLocation": "root",
+    "rust-analyzer.check.invocationStrategy": "once",
+    "rust-analyzer.check.overrideCommand": [
+        "env",
+        "MIRI_AUTO_OPS=no",
+        "./miri",
+        "clippy", // make this `check` when working with a locally built rustc
+        "--message-format=json",
+    ],
+    // Contrary to what the name suggests, this also affects proc macros.
+    "rust-analyzer.cargo.buildScripts.invocationLocation": "root",
+    "rust-analyzer.cargo.buildScripts.invocationStrategy": "once",
+    "rust-analyzer.cargo.buildScripts.overrideCommand": [
+        "env",
+        "MIRI_AUTO_OPS=no",
+        "./miri",
+        "check",
+        "--message-format=json",
+    ],
+}

src/tools/miri/rust-version

@@ -1 +1 @@
-7067e4aee45c18cfa1c6af3bf79bd097684fb294
+17a19e684cdf3ca088af8b4da6a6209d128913f4

src/tools/miri/src/alloc_addresses/mod.rs

@@ -453,7 +453,7 @@ impl<'tcx> MiriMachine<'tcx> {
         let thread = self.threads.active_thread();
         global_state.reuse.add_addr(rng, addr, size, align, kind, thread, || {
             if let Some(data_race) = &self.data_race {
-                data_race.release_clock(&self.threads).clone()
+                data_race.release_clock(&self.threads, |clock| clock.clone())
             } else {
                 VClock::default()
             }

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

@@ -530,8 +530,6 @@ fn main() {
         } else if arg == "-Zmiri-ignore-leaks" {
             miri_config.ignore_leaks = true;
             miri_config.collect_leak_backtraces = false;
-        } else if arg == "-Zmiri-panic-on-unsupported" {
-            miri_config.panic_on_unsupported = true;
         } else if arg == "-Zmiri-strict-provenance" {
             miri_config.provenance_mode = ProvenanceMode::Strict;
         } else if arg == "-Zmiri-permissive-provenance" {

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

@@ -828,15 +828,14 @@ pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> {
         }
     }
 
-    /// Returns the `release` clock of the current thread.
+    /// Calls the callback with the "release" clock of the current thread.
     /// Other threads can acquire this clock in the future to establish synchronization
     /// with this program point.
-    fn release_clock<'a>(&'a self) -> Option<Ref<'a, VClock>>
-    where
-        'tcx: 'a,
-    {
+    ///
+    /// The closure will only be invoked if data race handling is on.
+    fn release_clock<R>(&self, callback: impl FnOnce(&VClock) -> R) -> Option<R> {
         let this = self.eval_context_ref();
-        Some(this.machine.data_race.as_ref()?.release_clock(&this.machine.threads))
+        Some(this.machine.data_race.as_ref()?.release_clock(&this.machine.threads, callback))
     }
 
     /// Acquire the given clock into the current thread, establishing synchronization with
@@ -1728,7 +1727,7 @@ impl GlobalState {
         let current_index = self.active_thread_index(thread_mgr);
 
         // Store the terminaion clock.
-        let terminaion_clock = self.release_clock(thread_mgr).clone();
+        let terminaion_clock = self.release_clock(thread_mgr, |clock| clock.clone());
         self.thread_info.get_mut()[current_thread].termination_vector_clock =
             Some(terminaion_clock);
 
@@ -1778,21 +1777,23 @@ impl GlobalState {
         clocks.clock.join(clock);
     }
 
-    /// Returns the `release` clock of the current thread.
+    /// Calls the given closure with the "release" clock of the current thread.
     /// Other threads can acquire this clock in the future to establish synchronization
     /// with this program point.
-    pub fn release_clock<'tcx>(&self, threads: &ThreadManager<'tcx>) -> Ref<'_, VClock> {
+    pub fn release_clock<'tcx, R>(
+        &self,
+        threads: &ThreadManager<'tcx>,
+        callback: impl FnOnce(&VClock) -> R,
+    ) -> R {
         let thread = threads.active_thread();
         let span = threads.active_thread_ref().current_span();
-        // We increment the clock each time this happens, to ensure no two releases
-        // can be confused with each other.
         let (index, mut clocks) = self.thread_state_mut(thread);
+        let r = callback(&clocks.clock);
+        // Increment the clock, so that all following events cannot be confused with anything that
+        // occurred before the release. Crucially, the callback is invoked on the *old* clock!
         clocks.increment_clock(index, span);
-        drop(clocks);
-        // To return a read-only view, we need to release the RefCell
-        // and borrow it again.
-        let (_index, clocks) = self.thread_state(thread);
-        Ref::map(clocks, |c| &c.clock)
+
+        r
     }
 
     fn thread_index(&self, thread: ThreadId) -> VectorIdx {

src/tools/miri/src/concurrency/init_once.rs

@@ -2,7 +2,7 @@ use std::collections::VecDeque;
 
 use rustc_index::Idx;
 
-use super::sync::EvalContextExtPriv as _;
+use super::thread::DynUnblockCallback;
 use super::vector_clock::VClock;
 use crate::*;
 
@@ -27,22 +27,6 @@ pub(super) struct InitOnce {
 
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
-    fn init_once_get_or_create_id(
-        &mut self,
-        lock: &MPlaceTy<'tcx>,
-        offset: u64,
-    ) -> InterpResult<'tcx, InitOnceId> {
-        let this = self.eval_context_mut();
-        this.get_or_create_id(
-            lock,
-            offset,
-            |ecx| &mut ecx.machine.sync.init_onces,
-            |_| interp_ok(Default::default()),
-        )?
-        .ok_or_else(|| err_ub_format!("init_once has invalid ID"))
-        .into()
-    }
-
     #[inline]
     fn init_once_status(&mut self, id: InitOnceId) -> InitOnceStatus {
         let this = self.eval_context_ref();
@@ -51,11 +35,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
     /// Put the thread into the queue waiting for the initialization.
     #[inline]
-    fn init_once_enqueue_and_block(
-        &mut self,
-        id: InitOnceId,
-        callback: impl UnblockCallback<'tcx> + 'tcx,
-    ) {
+    fn init_once_enqueue_and_block(&mut self, id: InitOnceId, callback: DynUnblockCallback<'tcx>) {
         let this = self.eval_context_mut();
         let thread = this.active_thread();
         let init_once = &mut this.machine.sync.init_onces[id];
@@ -93,7 +73,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         // Each complete happens-before the end of the wait
         if let Some(data_race) = &this.machine.data_race {
-            init_once.clock.clone_from(&data_race.release_clock(&this.machine.threads));
+            data_race
+                .release_clock(&this.machine.threads, |clock| init_once.clock.clone_from(clock));
         }
 
         // Wake up everyone.
@@ -119,7 +100,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         // Each complete happens-before the end of the wait
         if let Some(data_race) = &this.machine.data_race {
-            init_once.clock.clone_from(&data_race.release_clock(&this.machine.threads));
+            data_race
+                .release_clock(&this.machine.threads, |clock| init_once.clock.clone_from(clock));
         }
 
         // Wake up one waiting thread, so they can go ahead and try to init this.