Auto merge of rust-lang#2368 - RalfJung:debug, r=oli-obk

Make "./miri {build,run,test}" use debug assertions but "./miri install" not

This makes `./miri run`/`./miri test` use the full set of debug assertions (including the rather expensive ones that check consistency of the Stacked Borrows cache), but `./miri install` installs a Miri *without* those debug assertions.

That's the same behavior as cargo, and helps catch Miri bugs with the test suite while making installed Miri usable for larger runs.

Author: bors

Cargo.toml

@@ -54,6 +54,7 @@ harness = false
 
 [features]
 default = ["stack-cache"]
-# Will be enabled on CI via `--all-features`.
-expensive-debug-assertions = []
 stack-cache = []
+
+[profile.dev]
+opt-level = 2 # because it's too slow otherwise

ci.sh

@@ -5,11 +5,11 @@ set -x
 # Determine configuration
 export RUSTFLAGS="-D warnings"
 export CARGO_INCREMENTAL=0
-export CARGO_EXTRA_FLAGS="--all-features" # in particular, expensive-debug-assertions
+export CARGO_EXTRA_FLAGS="--all-features"
 
 # Prepare
 echo "Build and install miri"
-CARGO_EXTRA_FLAGS="" ./miri install # implicitly locked -- and the *installed* Miri does *not* get the expensive-debug-assertions feature
+./miri install # implicitly locked
 ./miri build --all-targets --locked # the build that all the `./miri test` below will use
 echo
 

miri

@@ -6,7 +6,8 @@ USAGE=$(cat <<"EOF"
 ./miri install <flags>:
 Installs the miri driver and cargo-miri. <flags> are passed to `cargo
 install`. Sets up the rpath such that the installed binary should work in any
-working directory.
+working directory. However, the rustup toolchain when invoking `cargo miri`
+needs to be the same one used for `./miri install`.
 
 ./miri build <flags>:
 Just build miri. <flags> are passed to `cargo build`.
@@ -22,10 +23,6 @@ to the final `cargo test` invocation.
 Build miri, set up a sysroot and then run the driver with the given <flags>.
 (Also respects MIRIFLAGS environment variable.)
 
-The commands above also exist in a "-debug" variant (e.g. "./miri run-debug
-<flags>") which uses debug builds instead of release builds, for faster build
-times and slower execution times.
-
 ./miri fmt <flags>:
 Format all sources and tests. <flags> are passed to `rustfmt`.
 
@@ -99,40 +96,21 @@ fi
 
 # Prepare flags for cargo and rustc.
 CARGO="cargo +$TOOLCHAIN"
-if [ -z "$CARGO_INCREMENTAL" ]; then
-    # Default CARGO_INCREMENTAL to 1.
-    export CARGO_INCREMENTAL=1
-fi
 if [ -z "$CARGO_TARGET_DIR" ]; then
     # Share target dir between `miri` and `cargo-miri`.
     export CARGO_TARGET_DIR="$MIRIDIR/target"
 fi
 # We set the rpath so that Miri finds the private rustc libraries it needs.
-# We enable debug-assertions to get tracing.
-# We enable line-only debuginfo for backtraces.
-export RUSTFLAGS="-C link-args=-Wl,-rpath,$LIBDIR -C debug-assertions -C debuginfo=1 $RUSTFLAGS"
-# Determine flags passed to all cargo invocations.
-# This is a bit more annoying that one would hope due to
-# <https://github.com/rust-lang/cargo/issues/6992>.
-case "$COMMAND" in
-*-debug)
-    CARGO_INSTALL_FLAGS="--target $TARGET --debug $CARGO_EXTRA_FLAGS"
-    CARGO_BUILD_FLAGS="--target $TARGET $CARGO_EXTRA_FLAGS"
-    ;;
-*)
-    CARGO_INSTALL_FLAGS="--target $TARGET $CARGO_EXTRA_FLAGS"
-    CARGO_BUILD_FLAGS="--target $TARGET --release $CARGO_EXTRA_FLAGS"
-    ;;
-esac
+export RUSTFLAGS="-C link-args=-Wl,-rpath,$LIBDIR $RUSTFLAGS"
 
 ## Helper functions
 
 # 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_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -- miri setup "$@"
+    $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_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -q -- miri setup --print-sysroot "$@")"
+    export MIRI_SYSROOT="$($CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -q -- miri setup --print-sysroot "$@")"
 }
 
 # Prepare and set MIRI_SYSROOT. Respects `MIRI_TEST_TARGET` and takes into account
@@ -154,37 +132,35 @@ find_sysroot() {
 
 # Run command.
 case "$COMMAND" in
-install|install-debug)
+install)
     # "--locked" to respect the Cargo.lock file if it exists,
     # "--offline" to avoid querying the registry (for yanked packages).
-    $CARGO install $CARGO_INSTALL_FLAGS --path "$MIRIDIR" --force --locked --offline "$@"
-    $CARGO install $CARGO_INSTALL_FLAGS --path "$MIRIDIR"/cargo-miri --force --locked --offline "$@"
+    $CARGO install $CARGO_EXTRA_FLAGS --path "$MIRIDIR" --force --locked --offline "$@"
+    $CARGO install $CARGO_EXTRA_FLAGS --path "$MIRIDIR"/cargo-miri --force --locked --offline "$@"
     ;;
-check|check-debug)
+check)
     # Check, and let caller control flags.
-    $CARGO check $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml --all-targets "$@"
-    $CARGO check $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@"
+    $CARGO check $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml --all-targets "$@"
+    $CARGO check $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@"
     ;;
-build|build-debug)
+build)
     # Build, and let caller control flags.
-    $CARGO build $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@"
-    $CARGO build $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@"
+    $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@"
+    $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@"
     ;;
-test|test-debug|bless|bless-debug)
+test|bless)
     # First build and get a sysroot.
-    $CARGO build $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml
+    $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml
     find_sysroot
-    case "$COMMAND" in
-    bless|bless-debug)
+    if [ "$COMMAND" = "bless" ]; then
         export MIRI_BLESS="Gesundheit"
-        ;;
-    esac
+    fi
     # Then test, and let caller control flags.
     # Only in root project and ui_test as `cargo-miri` has no tests.
-    $CARGO test $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@"
-    $CARGO test $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/ui_test/Cargo.toml "$@"
+    $CARGO test $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@"
+    $CARGO test $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/ui_test/Cargo.toml "$@"
     ;;
-run|run-debug)
+run)
     # Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so
     # that we set the MIRI_SYSROOT up the right way.
     FOUND_TARGET_OPT=0
@@ -202,19 +178,19 @@ run|run-debug)
         MIRIFLAGS="$MIRIFLAGS --target $MIRI_TEST_TARGET"
     fi
     # First build and get a sysroot.
-    $CARGO build $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml
+    $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml
     find_sysroot
     # Then run the actual command.
-    exec $CARGO run $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml -- --sysroot "$MIRI_SYSROOT" $MIRIFLAGS "$@"
+    exec $CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml -- --sysroot "$MIRI_SYSROOT" $MIRIFLAGS "$@"
     ;;
 fmt)
     find "$MIRIDIR" -not \( -name target -prune \) -name '*.rs' \
         | xargs rustfmt +$TOOLCHAIN --edition=2021 --config-path "$MIRIDIR/rustfmt.toml" "$@"
     ;;
 clippy)
-    $CARGO clippy $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml --all-targets "$@"
-    $CARGO clippy $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/ui_test/Cargo.toml --all-targets "$@"
-    $CARGO clippy $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@"
+    $CARGO clippy $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml --all-targets "$@"
+    $CARGO clippy $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/ui_test/Cargo.toml --all-targets "$@"
+    $CARGO clippy $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@"
     ;;
 *)
     if [ -n "$COMMAND" ]; then

src/concurrency/range_object_map.rs

@@ -117,11 +117,11 @@ impl<T> RangeObjectMap<T> {
         self.v.insert(pos, Elem { range, data });
         // If we aren't the first element, then our start must be greater than the preivous element's end
         if pos > 0 {
-            debug_assert!(self.v[pos - 1].range.end() <= range.start);
+            assert!(self.v[pos - 1].range.end() <= range.start);
         }
         // If we aren't the last element, then our end must be smaller than next element's start
         if pos < self.v.len() - 1 {
-            debug_assert!(range.end() <= self.v[pos + 1].range.start);
+            assert!(range.end() <= self.v[pos + 1].range.start);
         }
     }
 

src/range_map.rs

@@ -77,6 +77,10 @@ impl<T> RangeMap<T> {
         };
         // The first offset that is not included any more.
         let end = offset + len;
+        assert!(
+            end <= self.v.last().unwrap().range.end,
+            "iterating beyond the bounds of this RangeMap"
+        );
         slice
             .iter()
             .take_while(move |elem| elem.range.start < end)
@@ -279,4 +283,18 @@ mod tests {
         assert_eq!(map.v.len(), 5);
         assert_eq!(to_vec(&map, 10, 10), vec![23, 42, 23, 23, 23, 19, 19, 19, 19, 19]);
     }
+
+    #[test]
+    #[should_panic]
+    fn out_of_range_iter_mut() {
+        let mut map = RangeMap::<i32>::new(Size::from_bytes(20), -1);
+        let _ = map.iter_mut(Size::from_bytes(11), Size::from_bytes(11));
+    }
+
+    #[test]
+    #[should_panic]
+    fn out_of_range_iter() {
+        let map = RangeMap::<i32>::new(Size::from_bytes(20), -1);
+        let _ = map.iter(Size::from_bytes(11), Size::from_bytes(11));
+    }
 }

src/stacked_borrows/stack.rs

@@ -82,7 +82,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(feature = "expensive-debug-assertions")]
+    #[cfg(debug_assertions)]
     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.
@@ -115,7 +115,7 @@ impl<'tcx> Stack {
         tag: SbTagExtra,
         exposed_tags: &FxHashSet<SbTag>,
     ) -> Result<Option<usize>, ()> {
-        #[cfg(feature = "expensive-debug-assertions")]
+        #[cfg(debug_assertions)]
         self.verify_cache_consistency();
 
         let SbTagExtra::Concrete(tag) = tag else {
@@ -247,7 +247,7 @@ impl<'tcx> Stack {
         // This primes the cache for the next access, which is almost always the just-added tag.
         self.cache.add(new_idx, new);
 
-        #[cfg(feature = "expensive-debug-assertions")]
+        #[cfg(debug_assertions)]
         self.verify_cache_consistency();
     }
 
@@ -325,7 +325,7 @@ impl<'tcx> Stack {
             self.unique_range.end = self.unique_range.end.min(disable_start + 1);
         }
 
-        #[cfg(feature = "expensive-debug-assertions")]
+        #[cfg(debug_assertions)]
         self.verify_cache_consistency();
 
         Ok(())
@@ -380,7 +380,7 @@ impl<'tcx> Stack {
             self.unique_range = 0..0;
         }
 
-        #[cfg(feature = "expensive-debug-assertions")]
+        #[cfg(debug_assertions)]
         self.verify_cache_consistency();
         Ok(())
     }