Auto merge of #121936 - RalfJung:miri, r=RalfJung

Miri subtree update

r? `@ghost`

Author: bors

src/tools/miri/README.md

@@ -412,6 +412,8 @@ to Miri failing to detect cases of undefined behavior in a program.
   The default is to search for and remove unreachable provenance once every `10000` basic blocks. Setting
   this to `0` disables the garbage collector, which causes some programs to have explosive memory
   usage and/or super-linear runtime.
+* `-Zmiri-track-alloc-accesses` show not only allocation and free events for tracked allocations,
+  but also reads and writes.
 * `-Zmiri-track-alloc-id=<id1>,<id2>,...` shows a backtrace when the given allocations are
   being allocated or freed.  This helps in debugging memory leaks and
   use after free bugs. Specifying this argument multiple times does not overwrite the previous
@@ -588,6 +590,7 @@ Definite bugs found:
 * [Dropping with unaligned pointers in `vec::IntoIter`](https://github.com/rust-lang/rust/pull/106084)
 * [Deallocating with the wrong layout in new specializations for in-place `Iterator::collect`](https://github.com/rust-lang/rust/pull/118460)
 * [Incorrect offset computation for highly-aligned types in `portable-atomic-util`](https://github.com/taiki-e/portable-atomic/pull/138)
+* [Occasional memory leak in `std::mpsc` channels](https://github.com/rust-lang/rust/issues/121582) (original code in [crossbeam](https://github.com/crossbeam-rs/crossbeam/pull/1084))
 
 Violations of [Stacked Borrows] found that are likely bugs (but Stacked Borrows is currently just an experiment):
 

src/tools/miri/miri-script/src/commands.rs

@@ -356,11 +356,17 @@ impl Command {
             .unwrap_or_else(|_| "0".into())
             .parse()
             .context("failed to parse MIRI_SEED_START")?;
-        let seed_count: u64 = env::var("MIRI_SEEDS")
-            .unwrap_or_else(|_| "256".into())
-            .parse()
-            .context("failed to parse MIRI_SEEDS")?;
-        let seed_end = seed_start + seed_count;
+        let seed_end: u64 = match (env::var("MIRI_SEEDS"), env::var("MIRI_SEED_END")) {
+            (Ok(_), Ok(_)) => bail!("Only one of MIRI_SEEDS and MIRI_SEED_END may be set"),
+            (Ok(seeds), Err(_)) =>
+                seed_start + seeds.parse::<u64>().context("failed to parse MIRI_SEEDS")?,
+            (Err(_), Ok(seed_end)) => seed_end.parse().context("failed to parse MIRI_SEED_END")?,
+            (Err(_), Err(_)) => seed_start + 256,
+        };
+        if seed_end <= seed_start {
+            bail!("the end of the seed range must be larger than the start.");
+        }
+
         let Some((command_name, trailing_args)) = command.split_first() else {
             bail!("expected many-seeds command to be non-empty");
         };

src/tools/miri/miri-script/src/main.rs

@@ -113,8 +113,9 @@ sysroot, to prevent conflicts with other toolchains.
 ./miri many-seeds <command>:
 Runs <command> over and over again with different seeds for Miri. The MIRIFLAGS
 variable is set to its original value appended with ` -Zmiri-seed=$SEED` for
-many different seeds. The MIRI_SEEDS variable controls how many seeds are being
-tried; MIRI_SEED_START controls the first seed to try.
+many different seeds. MIRI_SEED_START controls the first seed to try (default: 0).
+MIRI_SEEDS controls how many seeds are being tried (default: 256);
+alternatively, MIRI_SEED_END controls the end of the (exclusive) seed range to try.
 
 ./miri bench <benches>:
 Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed.

src/tools/miri/miri.bat

@@ -2,7 +2,10 @@
 :: Windows will not execute the bash script, and select this.
 @echo off
 set MIRI_SCRIPT_TARGET_DIR=%0\..\miri-script\target
-cargo build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml
+
+:: If any other steps are added, the "|| exit /b" must be appended to early
+:: return from the script. If not, it will continue execution.
+cargo build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml || exit /b
 
 :: Forwards all arguments to this file to the executable.
 :: We invoke the binary directly to avoid going through rustup, which would set some extra

src/tools/miri/rust-version

@@ -1 +1 @@
-c5f69bdd5173a948e0131f934fa7c4cbf5e0b55f
+1a1876c9790f168fb51afa335a7ba3e6fc267d75

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

@@ -534,6 +534,8 @@ fn main() {
                     ),
             };
             miri_config.tracked_alloc_ids.extend(ids);
+        } else if arg == "-Zmiri-track-alloc-accesses" {
+            miri_config.track_alloc_accesses = true;
         } else if let Some(param) = arg.strip_prefix("-Zmiri-compare-exchange-weak-failure-rate=") {
             let rate = match param.parse::<f64>() {
                 Ok(rate) if rate >= 0.0 && rate <= 1.0 => rate,

src/tools/miri/src/borrow_tracker/mod.rs

@@ -122,13 +122,6 @@ impl VisitProvenance for GlobalStateInner {
 /// We need interior mutable access to the global state.
 pub type GlobalState = RefCell<GlobalStateInner>;
 
-/// Indicates which kind of access is being performed.
-#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
-pub enum AccessKind {
-    Read,
-    Write,
-}
-
 impl fmt::Display for AccessKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
@@ -384,7 +377,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             if matches!(kind, AllocKind::LiveData) {
                 let alloc_extra = this.get_alloc_extra(*alloc_id)?; // can still fail for `extern static`
                 let alloc_borrow_tracker = &alloc_extra.borrow_tracker.as_ref().unwrap();
-                alloc_borrow_tracker.release_protector(&this.machine, borrow_tracker, *tag)?;
+                alloc_borrow_tracker.release_protector(
+                    &this.machine,
+                    borrow_tracker,
+                    *tag,
+                    *alloc_id,
+                )?;
             }
         }
         borrow_tracker.borrow_mut().end_call(&frame.extra);
@@ -498,10 +496,12 @@ impl AllocState {
         machine: &MiriMachine<'_, 'tcx>,
         global: &GlobalState,
         tag: BorTag,
+        alloc_id: AllocId, // diagnostics
     ) -> InterpResult<'tcx> {
         match self {
             AllocState::StackedBorrows(_sb) => Ok(()),
-            AllocState::TreeBorrows(tb) => tb.borrow_mut().release_protector(machine, global, tag),
+            AllocState::TreeBorrows(tb) =>
+                tb.borrow_mut().release_protector(machine, global, tag, alloc_id),
         }
     }
 }

src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs

@@ -5,7 +5,7 @@ use rustc_data_structures::fx::FxHashSet;
 use rustc_span::{Span, SpanData};
 use rustc_target::abi::Size;
 
-use crate::borrow_tracker::{AccessKind, GlobalStateInner, ProtectorKind};
+use crate::borrow_tracker::{GlobalStateInner, ProtectorKind};
 use crate::*;
 
 /// Error reporting

src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

@@ -16,7 +16,7 @@ use rustc_target::abi::{Abi, Size};
 
 use crate::borrow_tracker::{
     stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder},
-    AccessKind, GlobalStateInner, ProtectorKind,
+    GlobalStateInner, ProtectorKind,
 };
 use crate::*;
 

src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs

@@ -9,7 +9,7 @@ use crate::borrow_tracker::tree_borrows::{
     tree::LocationState,
     unimap::UniIndex,
 };
-use crate::borrow_tracker::{AccessKind, ProtectorKind};
+use crate::borrow_tracker::ProtectorKind;
 use crate::*;
 
 /// Cause of an access: either a real access or one
@@ -278,6 +278,8 @@ impl History {
 pub(super) struct TbError<'node> {
     /// What failure occurred.
     pub error_kind: TransitionError,
+    /// The allocation in which the error is happening.
+    pub alloc_id: AllocId,
     /// The offset (into the allocation) at which the conflict occurred.
     pub error_offset: u64,
     /// The tag on which the error was triggered.
@@ -300,7 +302,11 @@ impl TbError<'_> {
         let accessed = self.accessed_info;
         let conflicting = self.conflicting_info;
         let accessed_is_conflicting = accessed.tag == conflicting.tag;
-        let title = format!("{cause} through {accessed} is forbidden");
+        let title = format!(
+            "{cause} through {accessed} at {alloc_id:?}[{offset:#x}] is forbidden",
+            alloc_id = self.alloc_id,
+            offset = self.error_offset
+        );
         let (title, details, conflicting_tag_name) = match self.error_kind {
             ChildAccessForbidden(perm) => {
                 let conflicting_tag_name =

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

@@ -1,6 +1,3 @@
-use rustc_target::abi::{Abi, Size};
-
-use crate::borrow_tracker::{AccessKind, GlobalState, GlobalStateInner, ProtectorKind};
 use rustc_middle::{
     mir::{Mutability, RetagKind},
     ty::{
@@ -10,7 +7,9 @@ use rustc_middle::{
     },
 };
 use rustc_span::def_id::DefId;
+use rustc_target::abi::{Abi, Size};
 
+use crate::borrow_tracker::{GlobalState, GlobalStateInner, ProtectorKind};
 use crate::*;
 
 pub mod diagnostics;
@@ -70,6 +69,7 @@ impl<'tcx> Tree {
             tag,
             Some(range),
             global,
+            alloc_id,
             span,
             diagnostics::AccessCause::Explicit(access_kind),
         )
@@ -78,7 +78,7 @@ impl<'tcx> Tree {
     /// Check that this pointer has permission to deallocate this range.
     pub fn before_memory_deallocation(
         &mut self,
-        _alloc_id: AllocId,
+        alloc_id: AllocId,
         prov: ProvenanceExtra,
         range: AllocRange,
         machine: &MiriMachine<'_, 'tcx>,
@@ -91,7 +91,7 @@ impl<'tcx> Tree {
         };
         let global = machine.borrow_tracker.as_ref().unwrap();
         let span = machine.current_span();
-        self.dealloc(tag, range, global, span)
+        self.dealloc(tag, range, global, alloc_id, span)
     }
 
     pub fn expose_tag(&mut self, _tag: BorTag) {
@@ -109,13 +109,15 @@ impl<'tcx> Tree {
         machine: &MiriMachine<'_, 'tcx>,
         global: &GlobalState,
         tag: BorTag,
+        alloc_id: AllocId, // diagnostics
     ) -> InterpResult<'tcx> {
         let span = machine.current_span();
         self.perform_access(
             AccessKind::Read,
             tag,
             None, // no specified range because it occurs on the entire allocation
             global,
+            alloc_id,
             span,
             diagnostics::AccessCause::FnExit,
         )
@@ -211,7 +213,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
             let global = this.machine.borrow_tracker.as_ref().unwrap().borrow();
             let ty = place.layout.ty;
             if global.tracked_pointer_tags.contains(&new_tag) {
-                let kind_str = format!("{new_perm:?} (pointee type {ty})");
+                let kind_str = format!("initial state {} (pointee type {ty})", new_perm.initial_state);
                 this.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(
                     new_tag.inner(),
                     Some(kind_str),
@@ -299,6 +301,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
             orig_tag,
             Some(range),
             this.machine.borrow_tracker.as_ref().unwrap(),
+            alloc_id,
             this.machine.current_span(),
             diagnostics::AccessCause::Reborrow,
         )?;

src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs

@@ -3,7 +3,7 @@ use std::fmt;
 
 use crate::borrow_tracker::tree_borrows::diagnostics::TransitionError;
 use crate::borrow_tracker::tree_borrows::tree::AccessRelatedness;
-use crate::borrow_tracker::AccessKind;
+use crate::AccessKind;
 
 /// The activation states of a pointer.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]

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

@@ -24,7 +24,7 @@ use crate::borrow_tracker::tree_borrows::{
     unimap::{UniEntry, UniIndex, UniKeyMap, UniValMap},
     Permission,
 };
-use crate::borrow_tracker::{AccessKind, GlobalState, ProtectorKind};
+use crate::borrow_tracker::{GlobalState, ProtectorKind};
 use crate::*;
 
 mod tests;
@@ -516,13 +516,15 @@ impl<'tcx> Tree {
         tag: BorTag,
         access_range: AllocRange,
         global: &GlobalState,
-        span: Span, // diagnostics
+        alloc_id: AllocId, // diagnostics
+        span: Span,        // diagnostics
     ) -> InterpResult<'tcx> {
         self.perform_access(
             AccessKind::Write,
             tag,
             Some(access_range),
             global,
+            alloc_id,
             span,
             diagnostics::AccessCause::Dealloc,
         )?;
@@ -545,6 +547,7 @@ impl<'tcx> Tree {
                         TbError {
                             conflicting_info,
                             access_cause: diagnostics::AccessCause::Dealloc,
+                            alloc_id,
                             error_offset: perms_range.start,
                             error_kind,
                             accessed_info,
@@ -576,6 +579,7 @@ impl<'tcx> Tree {
         tag: BorTag,
         access_range: Option<AllocRange>,
         global: &GlobalState,
+        alloc_id: AllocId,                      // diagnostics
         span: Span,                             // diagnostics
         access_cause: diagnostics::AccessCause, // diagnostics
     ) -> InterpResult<'tcx> {
@@ -628,6 +632,7 @@ impl<'tcx> Tree {
             TbError {
                 conflicting_info,
                 access_cause,
+                alloc_id,
                 error_offset: perms_range.start,
                 error_kind,
                 accessed_info,

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

@@ -812,6 +812,7 @@ impl VClockAlloc {
                 | MiriMemoryKind::Miri
                 | MiriMemoryKind::C
                 | MiriMemoryKind::WinHeap
+                | MiriMemoryKind::WinLocal
                 | MiriMemoryKind::Mmap,
             )
             | MemoryKind::Stack => {
@@ -820,7 +821,8 @@ impl VClockAlloc {
                 alloc_timestamp.span = current_span;
                 (alloc_timestamp, alloc_index)
             }
-            // Other global memory should trace races but be allocated at the 0 timestamp.
+            // Other global memory should trace races but be allocated at the 0 timestamp
+            // (conceptually they are allocated before everything).
             MemoryKind::Machine(
                 MiriMemoryKind::Global
                 | MiriMemoryKind::Machine
@@ -1673,8 +1675,8 @@ impl GlobalState {
         vector: VectorIdx,
     ) -> String {
         let thread = self.vector_info.borrow()[vector];
-        let thread_name = thread_mgr.get_thread_name(thread);
-        format!("thread `{}`", String::from_utf8_lossy(thread_name))
+        let thread_name = thread_mgr.get_thread_display_name(thread);
+        format!("thread `{thread_name}`")
     }
 
     /// Acquire a lock, express that the previous call of

src/tools/miri/src/concurrency/thread.rs

@@ -160,9 +160,18 @@ pub type StackEmptyCallback<'mir, 'tcx> =
     Box<dyn FnMut(&mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx, Poll<()>>>;
 
 impl<'mir, 'tcx> Thread<'mir, 'tcx> {
-    /// Get the name of the current thread, or `<unnamed>` if it was not set.
-    fn thread_name(&self) -> &[u8] {
-        if let Some(ref thread_name) = self.thread_name { thread_name } else { b"<unnamed>" }
+    /// Get the name of the current thread if it was set.
+    fn thread_name(&self) -> Option<&[u8]> {
+        self.thread_name.as_deref()
+    }
+
+    /// Get the name of the current thread for display purposes; will include thread ID if not set.
+    fn thread_display_name(&self, id: ThreadId) -> String {
+        if let Some(ref thread_name) = self.thread_name {
+            String::from_utf8_lossy(thread_name).into_owned()
+        } else {
+            format!("unnamed-{}", id.index())
+        }
     }
 
     /// Return the top user-relevant frame, if there is one.
@@ -205,7 +214,7 @@ impl<'mir, 'tcx> std::fmt::Debug for Thread<'mir, 'tcx> {
         write!(
             f,
             "{}({:?}, {:?})",
-            String::from_utf8_lossy(self.thread_name()),
+            String::from_utf8_lossy(self.thread_name().unwrap_or(b"<unnamed>")),
             self.state,
             self.join_status
         )
@@ -572,10 +581,14 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
     }
 
     /// Get the name of the given thread.
-    pub fn get_thread_name(&self, thread: ThreadId) -> &[u8] {
+    pub fn get_thread_name(&self, thread: ThreadId) -> Option<&[u8]> {
         self.threads[thread].thread_name()
     }
 
+    pub fn get_thread_display_name(&self, thread: ThreadId) -> String {
+        self.threads[thread].thread_display_name(thread)
+    }
+
     /// Put the thread into the blocked state.
     fn block_thread(&mut self, thread: ThreadId) {
         let state = &mut self.threads[thread].state;
@@ -969,18 +982,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     }
 
     #[inline]
-    fn set_thread_name_wide(&mut self, thread: ThreadId, new_thread_name: &[u16]) {
-        let this = self.eval_context_mut();
-
-        // The Windows `GetThreadDescription` shim to get the thread name isn't implemented, so being lossy is okay.
-        // This is only read by diagnostics, which already use `from_utf8_lossy`.
-        this.machine
-            .threads
-            .set_thread_name(thread, String::from_utf16_lossy(new_thread_name).into_bytes());
-    }
-
-    #[inline]
-    fn get_thread_name<'c>(&'c self, thread: ThreadId) -> &'c [u8]
+    fn get_thread_name<'c>(&'c self, thread: ThreadId) -> Option<&[u8]>
     where
         'mir: 'c,
     {