Rustup

Cargo.toml

@@ -21,7 +21,6 @@ doctest = false # and no doc tests
 getrandom = { version = "0.2", features = ["std"] }
 env_logger = "0.9"
 log = "0.4"
-shell-escape = "0.1.4"
 rand = "0.8"
 smallvec = "1.7"
 

miri

@@ -48,10 +48,10 @@ Update and activate the rustup toolchain 'miri' to the commit given in the
 `rustup-toolchain-install-master` must be installed for this to work. Any extra
 flags are passed to `rustup-toolchain-install-master`.
 
-./miri rustc-pull:
-Pull and merge Miri changes from the rustc repo. The fetched commit is stored in
-the `rust-version` file, so the next `./miri toolchain` will install the rustc
-we just pulled.
+./miri rustc-pull <commit>:
+Pull and merge Miri changes from the rustc repo. Defaults to fetching the latest
+rustc commit. The fetched commit is stored in the `rust-version` file, so the
+next `./miri toolchain` will install the rustc that just got pulled.
 
 ./miri rustc-push <github user> <branch>:
 Push Miri changes back to the rustc repo. This will pull a copy of the rustc
@@ -113,18 +113,17 @@ toolchain)
     ;;
 rustc-pull)
     cd "$MIRIDIR"
-    FETCH_COMMIT=$(git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1)
-    # We can't pull from a commit with josh
-    # (https://github.com/josh-project/josh/issues/1034), so we just hope that
-    # nothing gets merged into rustc *during* this pull.
-    git fetch http://localhost:8000/rust-lang/rust.git$JOSH_FILTER.git master
-    # Just verify that `master` didn't move.
-    if [[ $FETCH_COMMIT != $(git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1) ]]; then
-        echo "Looks like something got merged into Rust *while we were pulling*. Aborting. Please try again."
+    FETCH_COMMIT="$1"
+    if [ -z "$FETCH_COMMIT" ]; then
+        FETCH_COMMIT=$(git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1)
     fi
-    echo "$FETCH_COMMIT" > rust-version # do this *before* merging as merging will fail in case of conflicts
+    # Update rust-version file. As a separate commit, since making it part of
+    # the merge has confused the heck out of josh in the past.
+    echo "$FETCH_COMMIT" > rust-version
+    git commit rust-version -m "Preparing for merge from rustc"
+    # Fetch given rustc commit and note down which one that was
+    git fetch http://localhost:8000/rust-lang/rust.git@$FETCH_COMMIT$JOSH_FILTER.git
     git merge FETCH_HEAD --no-ff -m "Merge from rustc"
-    git commit rust-version --amend -m "Merge from rustc"
     exit 0
     ;;
 rustc-push)
@@ -157,11 +156,22 @@ rustc-push)
     fi
     git fetch https://github.com/rust-lang/rust $BASE
     git push https://github.com/$USER/rust $BASE:refs/heads/$BRANCH -f
+    echo
     # Do the actual push.
     cd "$MIRIDIR"
     echo "Pushing Miri changes..."
     git push http://localhost:8000/$USER/rust.git$JOSH_FILTER.git HEAD:$BRANCH
-    exit 0
+    # Do a round-trip check to make sure the push worked as expected.
+    echo
+    git fetch http://localhost:8000/$USER/rust.git@$JOSH_FILTER.git $BRANCH &>/dev/null
+    if [[ $(git rev-parse HEAD) != $(git rev-parse FETCH_HEAD) ]]; then
+        echo "ERROR: Josh created a non-roundtrip push! Do NOT merge this into rustc!"
+        exit 1
+    else
+        echo "Confirmed that the push round-trips back to Miri properly. Please create a rustc PR:"
+        echo "    https://github.com/$USER/rust/pull/new/$BRANCH"
+        exit 0
+    fi
     ;;
 many-seeds)
     for SEED in $({ echo obase=16; seq 0 255; } | bc); do

rust-version

@@ -1 +1 @@
-101e1822c3e54e63996c8aaa014d55716f3937eb
+9340e5c1b9dee53fd32a18f7bfb54faabfe00b7b

src/stacked_borrows/mod.rs

@@ -14,7 +14,6 @@ use rustc_middle::mir::RetagKind;
 use rustc_middle::ty::{
     self,
     layout::{HasParamEnv, LayoutOf},
-    Ty,
 };
 use rustc_target::abi::Abi;
 use rustc_target::abi::Size;
@@ -983,28 +982,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let mut visitor = RetagVisitor { ecx: this, kind, retag_cause, retag_fields };
         return visitor.visit_value(place);
 
-        // Determine mutability and whether to add a protector.
-        // Cannot use `builtin_deref` because that reports *immutable* for `Box`,
-        // making it useless.
-        fn qualify(ty: Ty<'_>, kind: RetagKind) -> Option<(RefKind, bool)> {
-            match ty.kind() {
-                // References are simple.
-                ty::Ref(_, _, Mutability::Mut) =>
-                    Some((
-                        RefKind::Unique { two_phase: kind == RetagKind::TwoPhase },
-                        kind == RetagKind::FnEntry,
-                    )),
-                ty::Ref(_, _, Mutability::Not) =>
-                    Some((RefKind::Shared, kind == RetagKind::FnEntry)),
-                // Raw pointers need to be enabled.
-                ty::RawPtr(tym) if kind == RetagKind::Raw =>
-                    Some((RefKind::Raw { mutable: tym.mutbl == Mutability::Mut }, false)),
-                // Boxes are handled separately due to that allocator situation,
-                // see the visitor below.
-                _ => None,
-            }
-        }
-
         // The actual visitor.
         struct RetagVisitor<'ecx, 'mir, 'tcx> {
             ecx: &'ecx mut MiriInterpCx<'mir, 'tcx>,
@@ -1057,34 +1034,58 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     return Ok(());
                 }
 
-                let recurse_for_fields = || {
-                    match self.retag_fields {
-                        RetagFields::No => false,
-                        RetagFields::Yes => true,
-                        RetagFields::OnlyScalar => {
-                            // Matching `ArgAbi::new` at the time of writing, only fields of
-                            // `Scalar` and `ScalarPair` ABI are considered.
-                            matches!(place.layout.abi, Abi::Scalar(..) | Abi::ScalarPair(..))
+                // Check the type of this value to see what to do with it (retag, or recurse).
+                match place.layout.ty.kind() {
+                    ty::Ref(_, _, mutbl) => {
+                        let ref_kind = match mutbl {
+                            Mutability::Mut =>
+                                RefKind::Unique { two_phase: self.kind == RetagKind::TwoPhase },
+                            Mutability::Not => RefKind::Shared,
+                        };
+                        self.retag_place(
+                            place,
+                            ref_kind,
+                            self.retag_cause,
+                            /*protector*/ self.kind == RetagKind::FnEntry,
+                        )?;
+                    }
+                    ty::RawPtr(tym) => {
+                        // We definitely do *not* want to recurse into raw pointers -- wide raw
+                        // pointers have fields, and for dyn Trait pointees those can have reference
+                        // type!
+                        if self.kind == RetagKind::Raw {
+                            // Raw pointers need to be enabled.
+                            self.retag_place(
+                                place,
+                                RefKind::Raw { mutable: tym.mutbl == Mutability::Mut },
+                                self.retag_cause,
+                                /*protector*/ false,
+                            )?;
+                        }
+                    }
+                    _ if place.layout.ty.ty_adt_def().is_some_and(|adt| adt.is_box()) => {
+                        // Recurse for boxes, they require some tricky handling and will end up in `visit_box` above.
+                        // (Yes this means we technically also recursively retag the allocator itself
+                        // even if field retagging is not enabled. *shrug*)
+                        self.walk_value(place)?;
+                    }
+                    _ => {
+                        // Not a reference/pointer/box. Only recurse if configured appropriately.
+                        let recurse = match self.retag_fields {
+                            RetagFields::No => false,
+                            RetagFields::Yes => true,
+                            RetagFields::OnlyScalar => {
+                                // Matching `ArgAbi::new` at the time of writing, only fields of
+                                // `Scalar` and `ScalarPair` ABI are considered.
+                                matches!(place.layout.abi, Abi::Scalar(..) | Abi::ScalarPair(..))
+                            }
+                        };
+                        if recurse {
+                            self.walk_value(place)?;
                         }
                     }
-                };
-
-                if let Some((ref_kind, protector)) = qualify(place.layout.ty, self.kind) {
-                    self.retag_place(place, ref_kind, self.retag_cause, protector)?;
-                } else if matches!(place.layout.ty.kind(), ty::RawPtr(..)) {
-                    // Wide raw pointers *do* have fields and their types are strange.
-                    // vtables have a type like `&[*const (); 3]` or so!
-                    // Do *not* recurse into them.
-                    // (No need to worry about wide references, those always "qualify". And Boxes
-                    // are handles specially by the visitor anyway.)
-                } else if recurse_for_fields()
-                    || place.layout.ty.ty_adt_def().is_some_and(|adt| adt.is_box())
-                {
-                    // Recurse deeper. Need to always recurse for `Box` to even hit `visit_box`.
-                    // (Yes this means we technically also recursively retag the allocator itself
-                    // even if field retagging is not enabled. *shrug*)
-                    self.walk_value(place)?;
                 }
+
                 Ok(())
             }
         }