Auto merge of rust-lang#103571 - RalfJung:miri, r=RalfJung

update Miri

r? `@ghost`

README.md

@@ -538,15 +538,23 @@ extern "Rust" {
     fn miri_start_panic(payload: *mut u8) -> !;
 
     /// Miri-provided extern function to get the internal unique identifier for the allocation that a pointer
-    /// points to. This is only useful as an input to `miri_print_stacks`, and it is a separate call because
+    /// points to. If this pointer is invalid (not pointing to an allocation), interpretation will abort.
+    ///
+    /// This is only useful as an input to `miri_print_borrow_stacks`, and it is a separate call because
     /// getting a pointer to an allocation at runtime can change the borrow stacks in the allocation.
+    /// This function should be considered unstable. It exists only to support `miri_print_borrow_stacks` and so
+    /// inherits all of its instability.
     fn miri_get_alloc_id(ptr: *const ()) -> u64;
 
     /// Miri-provided extern function to print (from the interpreter, not the program) the contents of all
-    /// borrow stacks in an allocation. The format of what this emits is unstable and may change at any time.
-    /// In particular, users should be aware that Miri will periodically attempt to garbage collect the
-    /// contents of all stacks. Callers of this function may wish to pass `-Zmiri-tag-gc=0` to disable the GC.
-    fn miri_print_stacks(alloc_id: u64);
+    /// borrow stacks in an allocation. The leftmost tag is the bottom of the stack.
+    /// The format of what this emits is unstable and may change at any time. In particular, users should be
+    /// aware that Miri will periodically attempt to garbage collect the contents of all stacks. Callers of
+    /// this function may wish to pass `-Zmiri-tag-gc=0` to disable the GC.
+    ///
+    /// This function is extremely unstable. At any time the format of its output may change, its signature may
+    /// change, or it may be removed entirely.
+    fn miri_print_borrow_stacks(alloc_id: u64);
 
     /// Miri-provided extern function to print (from the interpreter, not the
     /// program) the contents of a section of program memory, as bytes. Bytes

build.rs

@@ -4,5 +4,5 @@ fn main() {
     // Re-export the TARGET environment variable so it can
     // be accessed by miri.
     let target = std::env::var("TARGET").unwrap();
-    println!("cargo:rustc-env=TARGET={}", target);
+    println!("cargo:rustc-env=TARGET={target}");
 }

cargo-miri/src/phases.rs

@@ -34,7 +34,7 @@ Examples:
 "#;
 
 fn show_help() {
-    println!("{}", CARGO_MIRI_HELP);
+    println!("{CARGO_MIRI_HELP}");
 }
 
 fn show_version() {
@@ -52,7 +52,7 @@ fn forward_patched_extern_arg(args: &mut impl Iterator<Item = String>, cmd: &mut
     let path = args.next().expect("`--extern` should be followed by a filename");
     if let Some(lib) = path.strip_suffix(".rlib") {
         // If this is an rlib, make it an rmeta.
-        cmd.arg(format!("{}.rmeta", lib));
+        cmd.arg(format!("{lib}.rmeta"));
     } else {
         // Some other extern file (e.g. a `.so`). Forward unchanged.
         cmd.arg(path);
@@ -336,7 +336,7 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
                     "[cargo-miri rustc inside rustdoc] captured input:\n{}",
                     std::str::from_utf8(&env.stdin).unwrap()
                 );
-                eprintln!("[cargo-miri rustc inside rustdoc] going to run:\n{:?}", cmd);
+                eprintln!("[cargo-miri rustc inside rustdoc] going to run:\n{cmd:?}");
             }
 
             exec_with_pipe(cmd, &env.stdin, format!("{}.stdin", out_filename("", "").display()));
@@ -374,7 +374,7 @@ pub fn phase_rustc(mut args: impl Iterator<Item = String>, phase: RustcPhase) {
                         val.push("metadata");
                     }
                 }
-                cmd.arg(format!("{}={}", emit_flag, val.join(",")));
+                cmd.arg(format!("{emit_flag}={}", val.join(",")));
             } else if arg == "--extern" {
                 // Patch `--extern` filenames, since Cargo sometimes passes stub `.rlib` files:
                 // https://github.com/rust-lang/miri/issues/1705
@@ -535,7 +535,7 @@ pub fn phase_runner(mut binary_args: impl Iterator<Item = String>, phase: Runner
     // Run it.
     debug_cmd("[cargo-miri runner]", verbose, &cmd);
     match phase {
-        RunnerPhase::Rustdoc => exec_with_pipe(cmd, &info.stdin, format!("{}.stdin", binary)),
+        RunnerPhase::Rustdoc => exec_with_pipe(cmd, &info.stdin, format!("{binary}.stdin")),
         RunnerPhase::Cargo => exec(cmd),
     }
 }

cargo-miri/src/util.rs

@@ -83,7 +83,7 @@ pub fn escape_for_toml(s: &str) -> String {
     // We want to surround this string in quotes `"`. So we first escape all quotes,
     // and also all backslashes (that are used to escape quotes).
     let s = s.replace('\\', r#"\\"#).replace('"', r#"\""#);
-    format!("\"{}\"", s)
+    format!("\"{s}\"")
 }
 
 /// Returns the path to the `miri` binary
@@ -175,7 +175,7 @@ pub fn ask_to_run(mut cmd: Command, ask: bool, text: &str) {
     let is_ci = env::var_os("CI").is_some() || env::var_os("TF_BUILD").is_some();
     if ask && !is_ci {
         let mut buf = String::new();
-        print!("I will run `{:?}` to {}. Proceed? [Y/n] ", cmd, text);
+        print!("I will run `{cmd:?}` to {text}. Proceed? [Y/n] ");
         io::stdout().flush().unwrap();
         io::stdin().read_line(&mut buf).unwrap();
         match buf.trim().to_lowercase().as_ref() {
@@ -185,10 +185,10 @@ pub fn ask_to_run(mut cmd: Command, ask: bool, text: &str) {
             a => show_error!("invalid answer `{}`", a),
         };
     } else {
-        eprintln!("Running `{:?}` to {}.", cmd, text);
+        eprintln!("Running `{cmd:?}` to {text}.");
     }
 
-    if cmd.status().unwrap_or_else(|_| panic!("failed to execute {:?}", cmd)).success().not() {
+    if cmd.status().unwrap_or_else(|_| panic!("failed to execute {cmd:?}")).success().not() {
         show_error!("failed to {}", text);
     }
 }
@@ -276,12 +276,12 @@ pub fn debug_cmd(prefix: &str, verbose: usize, cmd: &Command) {
         // Print only what has been changed for this `cmd`.
         for (var, val) in cmd.get_envs() {
             if let Some(val) = val {
-                writeln!(out, "{}={:?} \\", var.to_string_lossy(), val).unwrap();
+                writeln!(out, "{}={val:?} \\", var.to_string_lossy()).unwrap();
             } else {
                 writeln!(out, "--unset={}", var.to_string_lossy()).unwrap();
             }
         }
     }
     write!(out, "{cmd:?}").unwrap();
-    eprintln!("{}", out);
+    eprintln!("{out}");
 }

rust-version

@@ -1 +1 @@
-b1ab3b738ac718da74cd4aa0bb7f362d0adbdf84
+85d089b41e2a0c0f07ab34f6c5a7c451389f25e6

src/bin/miri.rs

@@ -192,7 +192,7 @@ fn init_late_loggers(tcx: TyCtxt<'_>) {
             if log::Level::from_str(&var).is_ok() {
                 env::set_var(
                     "RUSTC_LOG",
-                    &format!(
+                    format!(
                         "rustc_middle::mir::interpret={0},rustc_const_eval::interpret={0}",
                         var
                     ),
@@ -243,7 +243,7 @@ fn host_sysroot() -> Option<String> {
                     )
                 }
             }
-            format!("{}/toolchains/{}", home, toolchain)
+            format!("{home}/toolchains/{toolchain}")
         }
         _ => option_env!("RUST_SYSROOT")
             .unwrap_or_else(|| {
@@ -330,7 +330,7 @@ fn main() {
         } else if crate_kind == "host" {
             false
         } else {
-            panic!("invalid `MIRI_BE_RUSTC` value: {:?}", crate_kind)
+            panic!("invalid `MIRI_BE_RUSTC` value: {crate_kind:?}")
         };
 
         // We cannot use `rustc_driver::main` as we need to adjust the CLI arguments.

src/concurrency/thread.rs

@@ -870,6 +870,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         this.machine.threads.active_thread_stack_mut()
     }
 
+    /// Set the name of the current thread. The buffer must not include the null terminator.
     #[inline]
     fn set_thread_name(&mut self, thread: ThreadId, new_thread_name: Vec<u8>) {
         let this = self.eval_context_mut();

src/concurrency/vector_clock.rs

@@ -399,7 +399,7 @@ mod tests {
 
         //Test partial_cmp
         let compare = l.partial_cmp(&r);
-        assert_eq!(compare, o, "Invalid comparison\n l: {:?}\n r: {:?}", l, r);
+        assert_eq!(compare, o, "Invalid comparison\n l: {l:?}\n r: {r:?}");
         let alt_compare = r.partial_cmp(&l);
         assert_eq!(
             alt_compare,

src/diagnostics.rs

@@ -263,7 +263,7 @@ pub fn report_error<'tcx, 'mir>(
     msg.insert(0, e.to_string());
     report_msg(
         DiagLevel::Error,
-        &if let Some(title) = title { format!("{}: {}", title, msg[0]) } else { msg[0].clone() },
+        &if let Some(title) = title { format!("{title}: {}", msg[0]) } else { msg[0].clone() },
         msg,
         vec![],
         helps,

src/helpers.rs

@@ -1,6 +1,7 @@
 pub mod convert;
 
 use std::cmp;
+use std::iter;
 use std::mem;
 use std::num::NonZeroUsize;
 use std::time::Duration;
@@ -107,7 +108,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     /// Gets an instance for a path.
     fn resolve_path(&self, path: &[&str]) -> ty::Instance<'tcx> {
         self.try_resolve_path(path)
-            .unwrap_or_else(|| panic!("failed to find required Rust item: {:?}", path))
+            .unwrap_or_else(|| panic!("failed to find required Rust item: {path:?}"))
     }
 
     /// Evaluates the scalar at the specified path. Returns Some(val)
@@ -505,7 +506,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             RejectOpWith::WarningWithoutBacktrace => {
                 this.tcx
                     .sess
-                    .warn(&format!("{} was made to return an error due to isolation", op_name));
+                    .warn(format!("{op_name} was made to return an error due to isolation"));
                 Ok(())
             }
             RejectOpWith::Warning => {
@@ -735,6 +736,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         })
     }
 
+    /// Read a sequence of bytes until the first null terminator.
     fn read_c_str<'a>(&'a self, ptr: Pointer<Option<Provenance>>) -> InterpResult<'tcx, &'a [u8]>
     where
         'tcx: 'a,
@@ -761,6 +763,30 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         this.read_bytes_ptr_strip_provenance(ptr, len)
     }
 
+    /// Helper function to write a sequence of bytes with an added null-terminator, which is what
+    /// the Unix APIs usually handle. This function returns `Ok((false, length))` without trying
+    /// to write if `size` is not large enough to fit the contents of `c_str` plus a null
+    /// terminator. It returns `Ok((true, length))` if the writing process was successful. The
+    /// string length returned does include the null terminator.
+    fn write_c_str(
+        &mut self,
+        c_str: &[u8],
+        ptr: Pointer<Option<Provenance>>,
+        size: u64,
+    ) -> InterpResult<'tcx, (bool, u64)> {
+        // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null
+        // terminator to memory using the `ptr` pointer would cause an out-of-bounds access.
+        let string_length = u64::try_from(c_str.len()).unwrap();
+        let string_length = string_length.checked_add(1).unwrap();
+        if size < string_length {
+            return Ok((false, string_length));
+        }
+        self.eval_context_mut()
+            .write_bytes_ptr(ptr, c_str.iter().copied().chain(iter::once(0u8)))?;
+        Ok((true, string_length))
+    }
+
+    /// Read a sequence of u16 until the first null terminator.
     fn read_wide_str(&self, mut ptr: Pointer<Option<Provenance>>) -> InterpResult<'tcx, Vec<u16>> {
         let this = self.eval_context_ref();
         let size2 = Size::from_bytes(2);
@@ -783,6 +809,39 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         Ok(wchars)
     }
 
+    /// Helper function to write a sequence of u16 with an added 0x0000-terminator, which is what
+    /// the Windows APIs usually handle. This function returns `Ok((false, length))` without trying
+    /// to write if `size` is not large enough to fit the contents of `os_string` plus a null
+    /// terminator. It returns `Ok((true, length))` if the writing process was successful. The
+    /// string length returned does include the null terminator. Length is measured in units of
+    /// `u16.`
+    fn write_wide_str(
+        &mut self,
+        wide_str: &[u16],
+        ptr: Pointer<Option<Provenance>>,
+        size: u64,
+    ) -> InterpResult<'tcx, (bool, u64)> {
+        // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required
+        // 0x0000 terminator to memory would cause an out-of-bounds access.
+        let string_length = u64::try_from(wide_str.len()).unwrap();
+        let string_length = string_length.checked_add(1).unwrap();
+        if size < string_length {
+            return Ok((false, string_length));
+        }
+
+        // Store the UTF-16 string.
+        let size2 = Size::from_bytes(2);
+        let this = self.eval_context_mut();
+        let mut alloc = this
+            .get_ptr_alloc_mut(ptr, size2 * string_length, Align::from_bytes(2).unwrap())?
+            .unwrap(); // not a ZST, so we will get a result
+        for (offset, wchar) in wide_str.iter().copied().chain(iter::once(0x0000)).enumerate() {
+            let offset = u64::try_from(offset).unwrap();
+            alloc.write_scalar(alloc_range(size2 * offset, size2), Scalar::from_u16(wchar))?;
+        }
+        Ok((true, string_length))
+    }
+
     /// Check that the ABI is what we expect.
     fn check_abi<'a>(&self, abi: Abi, exp_abi: Abi) -> InterpResult<'a, ()> {
         if self.eval_context_ref().machine.enforce_abi && abi != exp_abi {

src/machine.rs

@@ -191,12 +191,12 @@ impl interpret::Provenance for Provenance {
             Provenance::Concrete { alloc_id, sb } => {
                 // Forward `alternate` flag to `alloc_id` printing.
                 if f.alternate() {
-                    write!(f, "[{:#?}]", alloc_id)?;
+                    write!(f, "[{alloc_id:#?}]")?;
                 } else {
-                    write!(f, "[{:?}]", alloc_id)?;
+                    write!(f, "[{alloc_id:?}]")?;
                 }
                 // Print Stacked Borrows tag.
-                write!(f, "{:?}", sb)?;
+                write!(f, "{sb:?}")?;
             }
             Provenance::Wildcard => {
                 write!(f, "[wildcard]")?;

src/range_map.rs

@@ -40,7 +40,7 @@ impl<T> RangeMap<T> {
         let mut left = 0usize; // inclusive
         let mut right = self.v.len(); // exclusive
         loop {
-            debug_assert!(left < right, "find_offset: offset {} is out-of-bounds", offset);
+            debug_assert!(left < right, "find_offset: offset {offset} is out-of-bounds");
             let candidate = left.checked_add(right).unwrap() / 2;
             let elem = &self.v[candidate];
             if offset < elem.range.start {

src/shims/foreign_items.rs

@@ -321,7 +321,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     return Ok(Some(body));
                 }
 
-                this.handle_unsupported(format!("can't call foreign function: {}", link_name))?;
+                this.handle_unsupported(format!("can't call foreign function: {link_name}"))?;
                 return Ok(None);
             }
         }
@@ -420,10 +420,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "miri_get_alloc_id" => {
                 let [ptr] = this.check_shim(abi, Abi::Rust, link_name, args)?;
                 let ptr = this.read_pointer(ptr)?;
-                let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr)?;
+                let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr).map_err(|_e| {
+                    err_machine_stop!(TerminationInfo::Abort(
+                        format!("pointer passed to miri_get_alloc_id must not be dangling, got {ptr:?}")
+                    ))
+                })?;
                 this.write_scalar(Scalar::from_u64(alloc_id.0.get()), dest)?;
             }
-            "miri_print_stacks" => {
+            "miri_print_borrow_stacks" => {
                 let [id] = this.check_shim(abi, Abi::Rust, link_name, args)?;
                 let id = this.read_scalar(id)?.to_u64()?;
                 if let Some(id) = std::num::NonZeroU64::new(id) {

src/shims/os_str.rs

@@ -1,6 +1,5 @@
 use std::borrow::Cow;
 use std::ffi::{OsStr, OsString};
-use std::iter;
 use std::path::{Path, PathBuf};
 
 #[cfg(unix)]
@@ -9,7 +8,6 @@ use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::os::windows::ffi::{OsStrExt, OsStringExt};
 
 use rustc_middle::ty::layout::LayoutOf;
-use rustc_target::abi::{Align, Size};
 
 use crate::*;
 
@@ -100,16 +98,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         size: u64,
     ) -> InterpResult<'tcx, (bool, u64)> {
         let bytes = os_str_to_bytes(os_str)?;
-        // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null
-        // terminator to memory using the `ptr` pointer would cause an out-of-bounds access.
-        let string_length = u64::try_from(bytes.len()).unwrap();
-        let string_length = string_length.checked_add(1).unwrap();
-        if size < string_length {
-            return Ok((false, string_length));
-        }
-        self.eval_context_mut()
-            .write_bytes_ptr(ptr, bytes.iter().copied().chain(iter::once(0u8)))?;
-        Ok((true, string_length))
+        self.eval_context_mut().write_c_str(bytes, ptr, size)
     }
 
     /// Helper function to write an OsStr as a 0x0000-terminated u16-sequence, which is what
@@ -140,25 +129,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         }
 
         let u16_vec = os_str_to_u16vec(os_str)?;
-        // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required
-        // 0x0000 terminator to memory would cause an out-of-bounds access.
-        let string_length = u64::try_from(u16_vec.len()).unwrap();
-        let string_length = string_length.checked_add(1).unwrap();
-        if size < string_length {
-            return Ok((false, string_length));
-        }
-
-        // Store the UTF-16 string.
-        let size2 = Size::from_bytes(2);
-        let this = self.eval_context_mut();
-        let mut alloc = this
-            .get_ptr_alloc_mut(ptr, size2 * string_length, Align::from_bytes(2).unwrap())?
-            .unwrap(); // not a ZST, so we will get a result
-        for (offset, wchar) in u16_vec.into_iter().chain(iter::once(0x0000)).enumerate() {
-            let offset = u64::try_from(offset).unwrap();
-            alloc.write_scalar(alloc_range(size2 * offset, size2), Scalar::from_u16(wchar))?;
-        }
-        Ok((true, string_length))
+        self.eval_context_mut().write_wide_str(&u16_vec, ptr, size)
     }
 
     /// Allocate enough memory to store the given `OsStr` as a null-terminated sequence of bytes.