Auto merge of #140436 - RalfJung:miri-sync, r=RalfJung

Miri subtree update

r? `@ghost`

Author: bors

src/tools/miri/.github/workflows/ci.yml

@@ -89,41 +89,16 @@ jobs:
             # Check if all jobs that we depend on (in the needs array) were successful.
             jq --exit-status 'all(.result == "success")' <<< '${{ toJson(needs) }}'
 
-  cron-fail-notify:
-    name: cronjob failure notification
+  cron-rustc-pull:
+    name: automatic pull from rustc
     runs-on: ubuntu-latest
     permissions:
         # The cronjob needs to be able to push to the repo...
         contents: write
         # ... and create a PR.
         pull-requests: write
-    needs: [build, style, coverage]
-    if: ${{ github.event_name == 'schedule' && failure() }}
+    if: ${{ github.event_name == 'schedule' }}
     steps:
-      # Send a Zulip notification
-      - name: Install zulip-send
-        run: pip3 install zulip
-      - name: Send Zulip notification
-        env:
-          ZULIP_BOT_EMAIL: ${{ secrets.ZULIP_BOT_EMAIL }}
-          ZULIP_API_TOKEN: ${{ secrets.ZULIP_API_TOKEN }}
-        run: |
-          ~/.local/bin/zulip-send --user $ZULIP_BOT_EMAIL --api-key $ZULIP_API_TOKEN --site https://rust-lang.zulipchat.com \
-            --stream miri --subject "Miri Build Failure ($(date -u +%Y-%m))" \
-            --message 'Dear @*T-miri*,
-
-          It would appear that the [Miri cron job build]('"https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"') failed.
-
-          This likely means that rustc changed the miri directory and
-          we now need to do a [`./miri rustc-pull`](https://github.com/rust-lang/miri/blob/master/CONTRIBUTING.md#importing-changes-from-the-rustc-repo).
-
-          Would you mind investigating this issue?
-
-          Thanks in advance!
-          Sincerely,
-          The Miri Cronjobs Bot'
-
-      # Attempt to auto-sync with rustc
       - uses: actions/checkout@v4
         with:
           fetch-depth: 256 # get a bit more of the history
@@ -143,18 +118,45 @@ jobs:
         run: |
           ./miri toolchain
           ./miri fmt --check || (./miri fmt && git commit -am "fmt")
-      - name: Push changes to a branch
+      - name: Push changes to a branch and create PR
         run: |
+          # `git diff --exit-code` "succeeds" if the diff is empty.
+          if git diff --exit-code HEAD^; then echo "Nothing changed in rustc, skipping PR"; exit 0; fi
+          # The diff is non-empty, create a PR.
           BRANCH="rustup-$(date -u +%Y-%m-%d)"
           git switch -c $BRANCH
           git push -u origin $BRANCH
-      - name: Create Pull Request
-        run: |
-          PR=$(gh pr create -B master --title 'Automatic Rustup' --body 'Please close and re-open this PR to trigger CI, then enable auto-merge.')
-          ~/.local/bin/zulip-send --user $ZULIP_BOT_EMAIL --api-key $ZULIP_API_TOKEN --site https://rust-lang.zulipchat.com \
-            --stream miri --subject "Miri Build Failure ($(date -u +%Y-%m))" \
-            --message "A PR doing a rustc-pull [has been automatically created]($PR) for your convenience."
+          gh pr create -B master --title 'Automatic Rustup' --body 'Please close and re-open this PR to trigger CI, then enable auto-merge.'
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           ZULIP_BOT_EMAIL: ${{ secrets.ZULIP_BOT_EMAIL }}
           ZULIP_API_TOKEN: ${{ secrets.ZULIP_API_TOKEN }}
+
+  cron-fail-notify:
+    name: cronjob failure notification
+    runs-on: ubuntu-latest
+    needs: [build, style, coverage]
+    if: ${{ github.event_name == 'schedule' && failure() }}
+    steps:
+      # Send a Zulip notification
+      - name: Install zulip-send
+        run: pip3 install zulip
+      - name: Send Zulip notification
+        env:
+          ZULIP_BOT_EMAIL: ${{ secrets.ZULIP_BOT_EMAIL }}
+          ZULIP_API_TOKEN: ${{ secrets.ZULIP_API_TOKEN }}
+        run: |
+          ~/.local/bin/zulip-send --user $ZULIP_BOT_EMAIL --api-key $ZULIP_API_TOKEN --site https://rust-lang.zulipchat.com \
+            --stream miri --subject "Miri Build Failure ($(date -u +%Y-%m))" \
+            --message 'Dear @*T-miri*,
+
+          It would appear that the [Miri cron job build]('"https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"') failed.
+
+          This likely means that rustc changed the miri directory and
+          we now need to do a [`./miri rustc-pull`](https://github.com/rust-lang/miri/blob/master/CONTRIBUTING.md#importing-changes-from-the-rustc-repo).
+
+          Would you mind investigating this issue?
+
+          Thanks in advance!
+          Sincerely,
+          The Miri Cronjobs Bot'

src/tools/miri/README.md

@@ -489,7 +489,7 @@ Miri knows where it is supposed to start execution:
 
 ```rust
 #[cfg(miri)]
-#[no_mangle]
+#[unsafe(no_mangle)]
 fn miri_start(argc: isize, argv: *const *const u8) -> isize {
     // Call the actual start function that your project implements, based on your target's conventions.
 }

src/tools/miri/bench-cargo-miri/mse/src/main.rs

@@ -13,7 +13,7 @@ fn read_i16(buffer: &[u8], index: usize) -> i16 {
     const SIZE: usize = size_of::<i16>();
     let mut bytes: [u8; SIZE] = [0u8; SIZE];
     bytes.copy_from_slice(&buffer[(index * SIZE)..(index * SIZE + SIZE)]);
-    unsafe { std::mem::transmute(bytes) }
+    i16::from_ne_bytes(bytes)
 }
 
 fn mse(samples: usize, frame_buf: &[i16], buf_ref: &[u8]) -> f64 {

src/tools/miri/rust-version

@@ -1 +1 @@
-1bc56185ee257ed829a0aea7abdc3b03c5fed887
+1b8ab72680f36e783af84c1a3c4f8508572bd9f9

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

@@ -107,47 +107,6 @@ fn align_addr(addr: u64, align: u64) -> u64 {
 
 impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
 trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
-    // Returns the exposed `AllocId` that corresponds to the specified addr,
-    // or `None` if the addr is out of bounds
-    fn alloc_id_from_addr(&self, addr: u64, size: i64) -> Option<AllocId> {
-        let this = self.eval_context_ref();
-        let global_state = this.machine.alloc_addresses.borrow();
-        assert!(global_state.provenance_mode != ProvenanceMode::Strict);
-
-        // We always search the allocation to the right of this address. So if the size is structly
-        // negative, we have to search for `addr-1` instead.
-        let addr = if size >= 0 { addr } else { addr.saturating_sub(1) };
-        let pos = global_state.int_to_ptr_map.binary_search_by_key(&addr, |(addr, _)| *addr);
-
-        // Determine the in-bounds provenance for this pointer.
-        let alloc_id = match pos {
-            Ok(pos) => Some(global_state.int_to_ptr_map[pos].1),
-            Err(0) => None,
-            Err(pos) => {
-                // This is the largest of the addresses smaller than `int`,
-                // i.e. the greatest lower bound (glb)
-                let (glb, alloc_id) = global_state.int_to_ptr_map[pos - 1];
-                // This never overflows because `addr >= glb`
-                let offset = addr - glb;
-                // We require this to be strict in-bounds of the allocation. This arm is only
-                // entered for addresses that are not the base address, so even zero-sized
-                // allocations will get recognized at their base address -- but all other
-                // allocations will *not* be recognized at their "end" address.
-                let size = this.get_alloc_info(alloc_id).size;
-                if offset < size.bytes() { Some(alloc_id) } else { None }
-            }
-        }?;
-
-        // We only use this provenance if it has been exposed.
-        if global_state.exposed.contains(&alloc_id) {
-            // This must still be live, since we remove allocations from `int_to_ptr_map` when they get freed.
-            debug_assert!(this.is_alloc_live(alloc_id));
-            Some(alloc_id)
-        } else {
-            None
-        }
-    }
-
     fn addr_from_alloc_id_uncached(
         &self,
         global_state: &mut GlobalStateInner,
@@ -242,11 +201,65 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
             interp_ok(base_addr)
         }
     }
+}
 
+impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
+pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
+    // Returns the `AllocId` that corresponds to the specified addr,
+    // or `None` if the addr is out of bounds.
+    // Setting `only_exposed_allocations` selects whether only exposed allocations are considered.
+    fn alloc_id_from_addr(
+        &self,
+        addr: u64,
+        size: i64,
+        only_exposed_allocations: bool,
+    ) -> Option<AllocId> {
+        let this = self.eval_context_ref();
+        let global_state = this.machine.alloc_addresses.borrow();
+        assert!(global_state.provenance_mode != ProvenanceMode::Strict);
+
+        // We always search the allocation to the right of this address. So if the size is strictly
+        // negative, we have to search for `addr-1` instead.
+        let addr = if size >= 0 { addr } else { addr.saturating_sub(1) };
+        let pos = global_state.int_to_ptr_map.binary_search_by_key(&addr, |(addr, _)| *addr);
+
+        // Determine the in-bounds provenance for this pointer.
+        let alloc_id = match pos {
+            Ok(pos) => Some(global_state.int_to_ptr_map[pos].1),
+            Err(0) => None,
+            Err(pos) => {
+                // This is the largest of the addresses smaller than `int`,
+                // i.e. the greatest lower bound (glb)
+                let (glb, alloc_id) = global_state.int_to_ptr_map[pos - 1];
+                // This never overflows because `addr >= glb`
+                let offset = addr - glb;
+                // We require this to be strict in-bounds of the allocation. This arm is only
+                // entered for addresses that are not the base address, so even zero-sized
+                // allocations will get recognized at their base address -- but all other
+                // allocations will *not* be recognized at their "end" address.
+                let size = this.get_alloc_info(alloc_id).size;
+                if offset < size.bytes() { Some(alloc_id) } else { None }
+            }
+        }?;
+
+        // We only use this provenance if it has been exposed, or if the caller requested also non-exposed allocations
+        if !only_exposed_allocations || global_state.exposed.contains(&alloc_id) {
+            // This must still be live, since we remove allocations from `int_to_ptr_map` when they get freed.
+            debug_assert!(this.is_alloc_live(alloc_id));
+            Some(alloc_id)
+        } else {
+            None
+        }
+    }
+
+    /// Returns the base address of an allocation, or an error if no base address could be found
+    ///
+    /// # Panics
+    /// If `memory_kind = None` and the `alloc_id` is not cached, meaning that the first call to this function per `alloc_id` must get the `memory_kind`.
     fn addr_from_alloc_id(
         &self,
         alloc_id: AllocId,
-        memory_kind: MemoryKind,
+        memory_kind: Option<MemoryKind>,
     ) -> InterpResult<'tcx, u64> {
         let this = self.eval_context_ref();
         let mut global_state = this.machine.alloc_addresses.borrow_mut();
@@ -256,8 +269,10 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
             Some(&addr) => interp_ok(addr),
             None => {
                 // First time we're looking for the absolute address of this allocation.
+                let memory_kind =
+                    memory_kind.expect("memory_kind is required since alloc_id is not cached");
                 let base_addr =
-                    self.addr_from_alloc_id_uncached(global_state, alloc_id, memory_kind)?;
+                    this.addr_from_alloc_id_uncached(global_state, alloc_id, memory_kind)?;
                 trace!("Assigning base address {:#x} to allocation {:?}", base_addr, alloc_id);
 
                 // Store address in cache.
@@ -283,10 +298,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
             }
         }
     }
-}
 
-impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
-pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     fn expose_provenance(&self, provenance: Provenance) -> InterpResult<'tcx> {
         let this = self.eval_context_ref();
         let mut global_state = this.machine.alloc_addresses.borrow_mut();
@@ -365,7 +377,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let alloc_id = prov.alloc_id();
 
         // Get a pointer to the beginning of this allocation.
-        let base_addr = this.addr_from_alloc_id(alloc_id, kind)?;
+        let base_addr = this.addr_from_alloc_id(alloc_id, Some(kind))?;
         let base_ptr = interpret::Pointer::new(
             Provenance::Concrete { alloc_id, tag },
             Size::from_bytes(base_addr),
@@ -388,7 +400,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             // In native lib mode, MiriAllocBytes for global allocations are handled via `prepared_alloc_bytes`.
             // This additional call ensures that some `MiriAllocBytes` are always prepared, just in case
             // this function gets called before the first time `addr_from_alloc_id` gets called.
-            this.addr_from_alloc_id(id, MiriMemoryKind::Global.into())?;
+            this.addr_from_alloc_id(id, Some(MiriMemoryKind::Global.into()))?;
             // The memory we need here will have already been allocated during an earlier call to
             // `addr_from_alloc_id` for this allocation. So don't create a new `MiriAllocBytes` here, instead
             // fetch the previously prepared bytes from `prepared_alloc_bytes`.
@@ -423,7 +435,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             alloc_id
         } else {
             // A wildcard pointer.
-            this.alloc_id_from_addr(addr.bytes(), size)?
+            let only_exposed_allocations = true;
+            this.alloc_id_from_addr(addr.bytes(), size, only_exposed_allocations)?
         };
 
         // This cannot fail: since we already have a pointer with that provenance, adjust_alloc_root_pointer

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

@@ -106,7 +106,7 @@ fn entry_fn(tcx: TyCtxt<'_>) -> (DefId, MiriEntryFnType) {
         } else {
             tcx.dcx().fatal(
                 "`miri_start` must have the following signature:\n\
-                        fn miri_start(argc: isize, argv: *const *const u8) -> isize",
+                fn miri_start(argc: isize, argv: *const *const u8) -> isize",
             );
         }
     } else {
@@ -115,7 +115,7 @@ fn entry_fn(tcx: TyCtxt<'_>) -> (DefId, MiriEntryFnType) {
             Alternatively, you can export a `miri_start` function:\n\
             \n\
             #[cfg(miri)]\n\
-            #[no_mangle]\n\
+            #[unsafe(no_mangle)]\n\
             fn miri_start(argc: isize, argv: *const *const u8) -> isize {\
             \n    // Call the actual start function that your project implements, based on your target's conventions.\n\
             }"

src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs

@@ -61,8 +61,7 @@ fn all_read_accesses_commute() {
                 // ... and produce the same final result.
                 assert_eq!(
                     loc12, loc21,
-                    "Read accesses {:?} followed by {:?} do not commute !",
-                    rel1, rel2
+                    "Read accesses {rel1:?} followed by {rel2:?} do not commute !"
                 );
             }
         }
@@ -674,8 +673,8 @@ mod spurious_read {
         fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
             let (x, y) = self.retag_permissions();
             write!(f, "{}; ", self.xy_rel)?;
-            write!(f, "y: ({}); ", y,)?;
-            write!(f, "retag x ({}); ", x)?;
+            write!(f, "y: ({y}); ")?;
+            write!(f, "retag x ({x}); ")?;
 
             write!(f, "<arbitrary code>; <spurious read x>;")?;
             Ok(())
@@ -730,17 +729,17 @@ mod spurious_read {
                         // protector.
                         final_source
                             .distinguishable::</*X*/ AllowRet, /*Y*/ AllowRet>(&final_target)
-                            .then_some(format!("{}", final_target))
+                            .then_some(format!("{final_target}"))
                     } else {
                         Some(format!("UB"))
                     }
                 };
                 if let Some(final_target) = distinguishable {
                     eprintln!(
-                        "For pattern '{}', inserting a spurious read through x makes the final state '{}' instead of '{}' which is observable",
-                        pat, final_target, final_source
+                        "For pattern '{pat}', inserting a spurious read through x makes the final state '{final_target}' \
+                        instead of '{final_source}' which is observable"
                     );
-                    eprintln!("  (arbitrary code instanciated with '{}')", opaque);
+                    eprintln!("  (arbitrary code instanciated with '{opaque}')");
                     err += 1;
                     // We found an instanciation of the opaque code that makes this Pattern
                     // fail, we don't really need to check the rest.

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

@@ -381,7 +381,7 @@ impl AccessType {
         });
 
         if let Some(ty) = ty {
-            msg.push_str(&format!(" of type `{}`", ty));
+            msg.push_str(&format!(" of type `{ty}`"));
         }
 
         msg

src/tools/miri/src/helpers.rs

@@ -1017,7 +1017,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     }
 
     /// Check that the given `caller_fn_abi` matches the expected ABI described by
-    /// `callee_abi`, `callee_input_tys`, `callee_output_ty`, and the return the list of
+    /// `callee_abi`, `callee_input_tys`, `callee_output_ty`, and then returns the list of
     /// arguments.
     fn check_shim_abi<'a, const N: usize>(
         &mut self,

src/tools/miri/src/machine.rs

@@ -721,9 +721,8 @@ impl<'tcx> MiriMachine<'tcx> {
                 // Check if host target == the session target.
                 if host_triple != target_triple {
                     panic!(
-                        "calling external C functions in linked .so file requires host and target to be the same: host={}, target={}",
-                        host_triple,
-                        target_triple,
+                        "calling native C functions in linked .so file requires host and target to be the same: \
+                        host={host_triple}, target={target_triple}",
                     );
                 }
                 // Note: it is the user's responsibility to provide a correct SO file.