Merge branch 'master' into int-function-args-returns

Author: Ellen Arteca

README.md

@@ -385,6 +385,9 @@ to Miri failing to detect cases of undefined behavior in a program.
   happening and where in your code would be a good place to look for it.
   Specifying this argument multiple times does not overwrite the previous
   values, instead it appends its values to the list. Listing a tag multiple times has no effect.
+* `-Zmiri-track-weak-memory-loads` shows a backtrace when weak memory emulation returns an outdated
+  value from a load. This can help diagnose problems that disappear under
+  `-Zmiri-disable-weak-memory-emulation`.
 
 [function ABI]: https://doc.rust-lang.org/reference/items/functions.html#extern-function-qualifier
 
@@ -414,10 +417,9 @@ Moreover, Miri recognizes some environment variables:
   checkout. Note that changing files in that directory does not automatically
   trigger a re-build of the standard library; you have to clear the Miri build
   cache manually (on Linux, `rm -rf ~/.cache/miri`).
-* `MIRI_SYSROOT` (recognized by `cargo miri` and the test suite) indicates the
-  sysroot to use. Only set this if you do not want to use the automatically
-  created sysroot. (The `miri` driver sysroot is controlled via the `--sysroot`
-  flag instead.)
+* `MIRI_SYSROOT` (recognized by `cargo miri` and the Miri driver) indicates the sysroot to use. When
+  using `cargo miri`, only set this if you do not want to use the automatically created sysroot. For
+  directly invoking the Miri driver, this variable (or a `--sysroot` flag) is mandatory.
 * `MIRI_TEST_TARGET` (recognized by the test suite and the `./miri` script) indicates which target
   architecture to test against.  `miri` and `cargo miri` accept the `--target` flag for the same
   purpose.
@@ -453,6 +455,8 @@ binaries, and as such worth documenting:
   crate currently being compiled.
 * `MIRI_VERBOSE` when set to any value tells the various `cargo-miri` phases to
   perform verbose logging.
+* `MIRI_HOST_SYSROOT` is set by bootstrap to tell `cargo-miri` which sysroot to use for *host*
+  operations.
 
 [testing-miri]: CONTRIBUTING.md#testing-the-miri-driver
 

cargo-miri/bin.rs

@@ -21,6 +21,7 @@ function run_tests {
     echo "Testing host architecture"
   fi
 
+  ## ui test suite
   ./miri test --locked
   if [ -z "${MIRI_TEST_TARGET+exists}" ]; then
     # Only for host architecture: tests with optimizations (`-O` is what cargo passes, but crank MIR
@@ -30,17 +31,28 @@ function run_tests {
     MIRIFLAGS="-O -Zmir-opt-level=4" MIRI_SKIP_UI_CHECKS=1 ./miri test --locked -- tests/{pass,panic}
   fi
 
+  ## test-cargo-miri
   # On Windows, there is always "python", not "python3" or "python2".
   if command -v python3 > /dev/null; then
     PYTHON=python3
   else
     PYTHON=python
   fi
-
-  # "miri test" has built the sysroot for us, now this should pass without
-  # any interactive questions.
+  # Some environment setup that attempts to confuse the heck out of cargo-miri.
+  if [ "$HOST_TARGET" = x86_64-unknown-linux-gnu ]; then
+    # These act up on Windows (`which miri` produces a filename that does not exist?!?),
+    # so let's do this only on Linux. Also makes sure things work without these set.
+    export RUSTC=$(which rustc)
+    export MIRI=$(which miri)
+  fi
+  mkdir -p .cargo
+  echo 'build.rustc-wrapper = "thisdoesnotexist"' > .cargo/config.toml
+  # Run the actual test
   ${PYTHON} test-cargo-miri/run-test.py
   echo
+  # Clean up
+  unset RUSTC MIRI
+  rm -rf .cargo
 
   # Ensure that our benchmarks all work, on the host at least.
   if [ -z "${MIRI_TEST_TARGET+exists}" ]; then

ci.sh

@@ -131,10 +131,11 @@ export RUSTFLAGS="-C link-args=-Wl,-rpath,$LIBDIR $RUSTFLAGS"
 
 # Build a sysroot and set MIRI_SYSROOT to use it. Arguments are passed to `cargo miri setup`.
 build_sysroot() {
-    # Build once, for the user to see.
-    $CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -- miri setup "$@"
-    # Call again, to just set env var.
-    export MIRI_SYSROOT="$($CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -q -- miri setup --print-sysroot "$@")"
+    if ! MIRI_SYSROOT="$($CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -q -- miri setup --print-sysroot "$@")"; then
+        echo "'cargo miri setup' failed"
+        exit 1
+    fi
+    export MIRI_SYSROOT
 }
 
 # Prepare and set MIRI_SYSROOT. Respects `MIRI_TEST_TARGET` and takes into account
@@ -204,7 +205,7 @@ run)
     $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml
     find_sysroot
     # Then run the actual command.
-    exec $CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml -- --sysroot "$MIRI_SYSROOT" $MIRIFLAGS "$@"
+    exec $CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml -- $MIRIFLAGS "$@"
     ;;
 fmt)
     find "$MIRIDIR" -not \( -name target -prune \) -name '*.rs' \

miri

@@ -1 +1 @@
-a7468c60f8dbf5feb23ad840b174d7e57113a846
+c11207ec89b856164bba03b8ecfe07b0b234ed21

rust-version

@@ -27,7 +27,7 @@ use rustc_middle::{
     },
     ty::{query::ExternProviders, TyCtxt},
 };
-use rustc_session::{search_paths::PathKind, CtfeBacktrace};
+use rustc_session::{config::CrateType, search_paths::PathKind, CtfeBacktrace};
 
 use miri::{BacktraceStyle, ProvenanceMode};
 
@@ -60,6 +60,10 @@ impl rustc_driver::Callbacks for MiriCompilerCalls {
 
         queries.global_ctxt().unwrap().peek_mut().enter(|tcx| {
             init_late_loggers(tcx);
+            if !tcx.sess.crate_types().contains(&CrateType::Executable) {
+                tcx.sess.fatal("miri only makes sense on bin crates");
+            }
+
             let (entry_def_id, entry_type) = if let Some(entry_def) = tcx.entry_fn(()) {
                 entry_def
             } else {
@@ -204,9 +208,9 @@ fn init_late_loggers(tcx: TyCtxt<'_>) {
     }
 }
 
-/// Returns the "default sysroot" that Miri will use if no `--sysroot` flag is set.
+/// Returns the "default sysroot" that Miri will use for host things if no `--sysroot` flag is set.
 /// Should be a compile-time constant.
-fn compile_time_sysroot() -> Option<String> {
+fn host_sysroot() -> Option<String> {
     if option_env!("RUSTC_STAGE").is_some() {
         // This is being built as part of rustc, and gets shipped with rustup.
         // We can rely on the sysroot computation in librustc_session.
@@ -227,7 +231,7 @@ fn compile_time_sysroot() -> Option<String> {
                 if toolchain_runtime != toolchain {
                     show_error(format!(
                         "This Miri got built with local toolchain `{toolchain}`, but now is being run under a different toolchain. \n\
-                    Make sure to run Miri in the toolchain it got built with, e.g. via `cargo +{toolchain} miri`."
+                        Make sure to run Miri in the toolchain it got built with, e.g. via `cargo +{toolchain} miri`."
                     ));
                 }
             }
@@ -246,25 +250,42 @@ fn compile_time_sysroot() -> Option<String> {
 /// Execute a compiler with the given CLI arguments and callbacks.
 fn run_compiler(
     mut args: Vec<String>,
+    target_crate: bool,
     callbacks: &mut (dyn rustc_driver::Callbacks + Send),
-    insert_default_args: bool,
 ) -> ! {
     // Make sure we use the right default sysroot. The default sysroot is wrong,
     // because `get_or_default_sysroot` in `librustc_session` bases that on `current_exe`.
     //
-    // Make sure we always call `compile_time_sysroot` as that also does some sanity-checks
-    // of the environment we were built in.
-    // FIXME: Ideally we'd turn a bad build env into a compile-time error via CTFE or so.
-    if let Some(sysroot) = compile_time_sysroot() {
-        let sysroot_flag = "--sysroot";
-        if !args.iter().any(|e| e == sysroot_flag) {
+    // Make sure we always call `host_sysroot` as that also does some sanity-checks
+    // of the environment we were built in and whether it matches what we are running in.
+    let host_default_sysroot = host_sysroot();
+    // Now see if we even need to set something.
+    let sysroot_flag = "--sysroot";
+    if !args.iter().any(|e| e == sysroot_flag) {
+        // No sysroot was set, let's see if we have a custom default we want to configure.
+        let default_sysroot = if target_crate {
+            // Using the built-in default here would be plain wrong, so we *require*
+            // the env var to make sure things make sense.
+            Some(env::var("MIRI_SYSROOT").unwrap_or_else(|_| {
+                show_error(format!(
+                    "Miri was invoked in 'target' mode without `MIRI_SYSROOT` or `--sysroot` being set"
+                ))
+            }))
+        } else {
+            host_default_sysroot
+        };
+        if let Some(sysroot) = default_sysroot {
             // We need to overwrite the default that librustc_session would compute.
             args.push(sysroot_flag.to_owned());
             args.push(sysroot);
         }
     }
 
-    if insert_default_args {
+    // Don't insert `MIRI_DEFAULT_ARGS`, in particular, `--cfg=miri`, if we are building
+    // a "host" crate. That may cause procedural macros (and probably build scripts) to
+    // depend on Miri-only symbols, such as `miri_resolve_frame`:
+    // https://github.com/rust-lang/miri/issues/1760
+    if target_crate {
         // Some options have different defaults in Miri than in plain rustc; apply those by making
         // them the first arguments after the binary name (but later arguments can overwrite them).
         args.splice(1..1, miri::MIRI_DEFAULT_ARGS.iter().map(ToString::to_string));
@@ -302,13 +323,8 @@ fn main() {
         // We cannot use `rustc_driver::main` as we need to adjust the CLI arguments.
         run_compiler(
             env::args().collect(),
+            target_crate,
             &mut MiriBeRustCompilerCalls { target_crate },
-            // Don't insert `MIRI_DEFAULT_ARGS`, in particular, `--cfg=miri`, if we are building
-            // a "host" crate. That may cause procedural macros (and probably build scripts) to
-            // depend on Miri-only symbols, such as `miri_resolve_frame`:
-            // https://github.com/rust-lang/miri/issues/1760
-            #[rustfmt::skip]
-            /* insert_default_args: */ target_crate,
         )
     }
 
@@ -358,6 +374,8 @@ fn main() {
             miri_config.isolated_op = miri::IsolatedOp::Allow;
         } else if arg == "-Zmiri-disable-weak-memory-emulation" {
             miri_config.weak_memory_emulation = false;
+        } else if arg == "-Zmiri-track-weak-memory-loads" {
+            miri_config.track_outdated_loads = true;
         } else if let Some(param) = arg.strip_prefix("-Zmiri-isolation-error=") {
             if matches!(isolation_enabled, Some(false)) {
                 panic!("-Zmiri-isolation-error cannot be used along with -Zmiri-disable-isolation");
@@ -513,9 +531,5 @@ fn main() {
 
     debug!("rustc arguments: {:?}", rustc_args);
     debug!("crate arguments: {:?}", miri_config.args);
-    run_compiler(
-        rustc_args,
-        &mut MiriCompilerCalls { miri_config },
-        /* insert_default_args: */ true,
-    )
+    run_compiler(rustc_args, /* target_crate: */ true, &mut MiriCompilerCalls { miri_config })
 }

src/bin/miri.rs

@@ -1187,12 +1187,15 @@ pub struct GlobalState {
 
     /// The timestamp of last SC write performed by each thread
     last_sc_write: RefCell<VClock>,
+
+    /// Track when an outdated (weak memory) load happens.
+    pub track_outdated_loads: bool,
 }
 
 impl GlobalState {
     /// Create a new global state, setup with just thread-id=0
     /// advanced to timestamp = 1.
-    pub fn new() -> Self {
+    pub fn new(config: &MiriConfig) -> Self {
         let mut global_state = GlobalState {
             multi_threaded: Cell::new(false),
             ongoing_action_data_race_free: Cell::new(false),
@@ -1203,6 +1206,7 @@ impl GlobalState {
             terminated_threads: RefCell::new(FxHashMap::default()),
             last_sc_fence: RefCell::new(VClock::default()),
             last_sc_write: RefCell::new(VClock::default()),
+            track_outdated_loads: config.track_outdated_loads,
         };
 
         // Setup the main-thread since it is not explicitly created:

src/concurrency/data_race.rs

@@ -82,10 +82,7 @@ use rustc_const_eval::interpret::{
 };
 use rustc_data_structures::fx::FxHashMap;
 
-use crate::{
-    AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd, Provenance, ThreadManager, VClock, VTimestamp,
-    VectorIdx,
-};
+use crate::*;
 
 use super::{
     data_race::{GlobalState as DataRaceState, ThreadClockSet},
@@ -113,6 +110,13 @@ pub(super) struct StoreBuffer {
     buffer: VecDeque<StoreElement>,
 }
 
+/// Whether a load returned the latest value or not.
+#[derive(PartialEq, Eq)]
+enum LoadRecency {
+    Latest,
+    Outdated,
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 struct StoreElement {
     /// The identifier of the vector index, corresponding to a thread
@@ -254,11 +258,11 @@ impl<'mir, 'tcx: 'mir> StoreBuffer {
         is_seqcst: bool,
         rng: &mut (impl rand::Rng + ?Sized),
         validate: impl FnOnce() -> InterpResult<'tcx>,
-    ) -> InterpResult<'tcx, ScalarMaybeUninit<Provenance>> {
+    ) -> InterpResult<'tcx, (ScalarMaybeUninit<Provenance>, LoadRecency)> {
         // Having a live borrow to store_buffer while calling validate_atomic_load is fine
         // because the race detector doesn't touch store_buffer
 
-        let store_elem = {
+        let (store_elem, recency) = {
             // The `clocks` we got here must be dropped before calling validate_atomic_load
             // as the race detector will update it
             let (.., clocks) = global.current_thread_state(thread_mgr);
@@ -274,7 +278,7 @@ impl<'mir, 'tcx: 'mir> StoreBuffer {
 
         let (index, clocks) = global.current_thread_state(thread_mgr);
         let loaded = store_elem.load_impl(index, &clocks);
-        Ok(loaded)
+        Ok((loaded, recency))
     }
 
     fn buffered_write(
@@ -296,7 +300,7 @@ impl<'mir, 'tcx: 'mir> StoreBuffer {
         is_seqcst: bool,
         clocks: &ThreadClockSet,
         rng: &mut R,
-    ) -> &StoreElement {
+    ) -> (&StoreElement, LoadRecency) {
         use rand::seq::IteratorRandom;
         let mut found_sc = false;
         // FIXME: we want an inclusive take_while (stops after a false predicate, but
@@ -359,9 +363,12 @@ impl<'mir, 'tcx: 'mir> StoreBuffer {
                 }
             });
 
-        candidates
-            .choose(rng)
-            .expect("store buffer cannot be empty, an element is populated on construction")
+        let chosen = candidates.choose(rng).expect("store buffer cannot be empty");
+        if std::ptr::eq(chosen, self.buffer.back().expect("store buffer cannot be empty")) {
+            (chosen, LoadRecency::Latest)
+        } else {
+            (chosen, LoadRecency::Outdated)
+        }
     }
 
     /// ATOMIC STORE IMPL in the paper (except we don't need the location's vector clock)
@@ -499,13 +506,16 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>:
                     alloc_range(base_offset, place.layout.size),
                     latest_in_mo,
                 )?;
-                let loaded = buffer.buffered_read(
+                let (loaded, recency) = buffer.buffered_read(
                     global,
                     &this.machine.threads,
                     atomic == AtomicReadOrd::SeqCst,
                     &mut *rng,
                     validate,
                 )?;
+                if global.track_outdated_loads && recency == LoadRecency::Outdated {
+                    register_diagnostic(NonHaltingDiagnostic::WeakMemoryOutdatedLoad);
+                }
 
                 return Ok(loaded);
             }

src/concurrency/weak_memory.rs

@@ -74,6 +74,7 @@ pub enum NonHaltingDiagnostic {
     Int2Ptr {
         details: bool,
     },
+    WeakMemoryOutdatedLoad,
 }
 
 /// Level of Miri specific diagnostics
@@ -474,6 +475,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                         format!("progress report: current operation being executed is here"),
                     Int2Ptr { .. } =>
                         format!("integer-to-pointer cast"),
+                    WeakMemoryOutdatedLoad =>
+                        format!("weak memory emulation: outdated value returned from load"),
                 };
 
                 let (title, diag_level) = match e {
@@ -485,7 +488,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     | CreatedCallId(..)
                     | CreatedAlloc(..)
                     | FreedAlloc(..)
-                    | ProgressReport => ("tracking was triggered", DiagLevel::Note),
+                    | ProgressReport
+                    | WeakMemoryOutdatedLoad =>
+                        ("tracking was triggered", DiagLevel::Note),
                 };
 
                 let helps = match e {

src/diagnostics.rs

@@ -103,6 +103,8 @@ pub struct MiriConfig {
     pub data_race_detector: bool,
     /// Determine if weak memory emulation should be enabled. Requires data race detection to be enabled
     pub weak_memory_emulation: bool,
+    /// Track when an outdated (weak memory) load happens.
+    pub track_outdated_loads: bool,
     /// Rate of spurious failures for compare_exchange_weak atomic operations,
     /// between 0.0 and 1.0, defaulting to 0.8 (80% chance of failure).
     pub cmpxchg_weak_failure_rate: f64,
@@ -147,6 +149,7 @@ impl Default for MiriConfig {
             tracked_alloc_ids: HashSet::default(),
             data_race_detector: true,
             weak_memory_emulation: true,
+            track_outdated_loads: false,
             cmpxchg_weak_failure_rate: 0.8, // 80%
             measureme_out: None,
             panic_on_unsupported: false,