Rollup merge of #134055 - RalfJung:interpret-alloc-dedup, r=oli-obk

interpret: clean up deduplicating allocation functions

The "align" and "kind" arguments would be largely ignored in the "dedup" case, so let's move that to entirely separate function.

Let's also remove support for old-style miri_resolve_frame while we are at it. The docs have already said for a while that this must be set to 1.

8 files changed, 24 insertions, 138 deletions

diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs
index f54a932e1b6..810e9356b26 100644
--- a/compiler/rustc_const_eval/src/interpret/place.rs
+++ b/compiler/rustc_const_eval/src/interpret/place.rs
@@ -5,8 +5,7 @@
 use std::assert_matches::assert_matches;
 
 use either::{Either, Left, Right};
-use rustc_abi::{Align, BackendRepr, HasDataLayout, Size};
-use rustc_ast::Mutability;
+use rustc_abi::{BackendRepr, HasDataLayout, Size};
 use rustc_middle::ty::Ty;
 use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
 use rustc_middle::{bug, mir, span_bug};
@@ -1018,40 +1017,31 @@ where
         self.allocate_dyn(layout, kind, MemPlaceMeta::None)
     }
 
-    /// Allocates a sequence of bytes in the interpreter's memory.
-    /// For immutable allocations, uses deduplication to reuse existing memory.
-    /// For mutable allocations, creates a new unique allocation.
-    pub fn allocate_bytes(
+    /// Allocates a sequence of bytes in the interpreter's memory with alignment 1.
+    /// This is allocated in immutable global memory and deduplicated.
+    pub fn allocate_bytes_dedup(
         &mut self,
         bytes: &[u8],
-        align: Align,
-        kind: MemoryKind<M::MemoryKind>,
-        mutbl: Mutability,
     ) -> InterpResult<'tcx, Pointer<M::Provenance>> {
-        // Use cache for immutable strings.
-        if mutbl.is_not() {
-            // Use dedup'd allocation function.
-            let salt = M::get_global_alloc_salt(self, None);
-            let id = self.tcx.allocate_bytes_dedup(bytes, salt);
-
-            // Turn untagged "global" pointers (obtained via `tcx`) into the machine pointer to the allocation.
-            M::adjust_alloc_root_pointer(&self, Pointer::from(id), Some(kind))
-        } else {
-            // Allocate new memory for mutable data.
-            self.allocate_bytes_ptr(bytes, align, kind, mutbl)
-        }
+        let salt = M::get_global_alloc_salt(self, None);
+        let id = self.tcx.allocate_bytes_dedup(bytes, salt);
+
+        // Turn untagged "global" pointers (obtained via `tcx`) into the machine pointer to the allocation.
+        M::adjust_alloc_root_pointer(
+            &self,
+            Pointer::from(id),
+            M::GLOBAL_KIND.map(MemoryKind::Machine),
+        )
     }
 
-    /// Allocates a string in the interpreter's memory with metadata for length.
-    /// Uses `allocate_bytes` internally but adds string-specific metadata handling.
-    pub fn allocate_str(
+    /// Allocates a string in the interpreter's memory, returning it as a (wide) place.
+    /// This is allocated in immutable global memory and deduplicated.
+    pub fn allocate_str_dedup(
         &mut self,
         str: &str,
-        kind: MemoryKind<M::MemoryKind>,
-        mutbl: Mutability,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
         let bytes = str.as_bytes();
-        let ptr = self.allocate_bytes(bytes, Align::ONE, kind, mutbl)?;
+        let ptr = self.allocate_bytes_dedup(bytes)?;
 
         // Create length metadata for the string.
         let meta = Scalar::from_target_usize(u64::try_from(bytes.len()).unwrap(), self);
diff --git a/compiler/rustc_const_eval/src/util/caller_location.rs b/compiler/rustc_const_eval/src/util/caller_location.rs
index 9bf16d4fe16..6593547cd23 100644
--- a/compiler/rustc_const_eval/src/util/caller_location.rs
+++ b/compiler/rustc_const_eval/src/util/caller_location.rs
@@ -1,7 +1,7 @@
 use rustc_hir::LangItem;
 use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::layout::LayoutOf;
-use rustc_middle::ty::{self, Mutability};
+use rustc_middle::ty::{self};
 use rustc_middle::{bug, mir};
 use rustc_span::symbol::Symbol;
 use tracing::trace;
@@ -20,12 +20,9 @@ fn alloc_caller_location<'tcx>(
     // This can fail if rustc runs out of memory right here. Trying to emit an error would be
     // pointless, since that would require allocating more memory than these short strings.
     let file = if loc_details.file {
-        ecx.allocate_str(filename.as_str(), MemoryKind::CallerLocation, Mutability::Not).unwrap()
+        ecx.allocate_str_dedup(filename.as_str()).unwrap()
     } else {
-        // FIXME: This creates a new allocation each time. It might be preferable to
-        // perform this allocation only once, and re-use the `MPlaceTy`.
-        // See https://github.com/rust-lang/rust/pull/89920#discussion_r730012398
-        ecx.allocate_str("<redacted>", MemoryKind::CallerLocation, Mutability::Not).unwrap()
+        ecx.allocate_str_dedup("<redacted>").unwrap()
     };
     let file = file.map_provenance(CtfeProvenance::as_immutable);
     let line = if loc_details.line { Scalar::from_u32(line) } else { Scalar::from_u32(0) };
diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs
index d4835bb07f6..2841470d248 100644
--- a/compiler/rustc_middle/src/ty/context.rs
+++ b/compiler/rustc_middle/src/ty/context.rs
@@ -1479,7 +1479,7 @@ impl<'tcx> TyCtxt<'tcx> {
         self.mk_adt_def_from_data(ty::AdtDefData::new(self, did, kind, variants, repr))
     }
 
-    /// Allocates a read-only byte or string literal for `mir::interpret`.
+    /// Allocates a read-only byte or string literal for `mir::interpret` with alignment 1.
     /// Returns the same `AllocId` if called again with the same bytes.
     pub fn allocate_bytes_dedup(self, bytes: &[u8], salt: usize) -> interpret::AllocId {
         // Create an allocation that just contains these bytes.
diff --git a/src/tools/miri/src/shims/backtrace.rs b/src/tools/miri/src/shims/backtrace.rs
index 64bd546458d..c7b399228bf 100644
--- a/src/tools/miri/src/shims/backtrace.rs
+++ b/src/tools/miri/src/shims/backtrace.rs
@@ -1,5 +1,4 @@
 use rustc_abi::{ExternAbi, Size};
-use rustc_ast::ast::Mutability;
 use rustc_middle::ty::layout::LayoutOf as _;
 use rustc_middle::ty::{self, Instance, Ty};
 use rustc_span::{BytePos, Loc, Symbol, hygiene};
@@ -179,14 +178,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         match flags {
             0 => {
-                // These are "mutable" allocations as we consider them to be owned by the callee.
-                let name_alloc =
-                    this.allocate_str(&name, MiriMemoryKind::Rust.into(), Mutability::Mut)?;
-                let filename_alloc =
-                    this.allocate_str(&filename, MiriMemoryKind::Rust.into(), Mutability::Mut)?;
-
-                this.write_immediate(name_alloc.to_ref(this), &this.project_field(dest, 0)?)?;
-                this.write_immediate(filename_alloc.to_ref(this), &this.project_field(dest, 1)?)?;
+                throw_unsup_format!("miri_resolve_frame: v0 is not supported any more");
             }
             1 => {
                 this.write_scalar(
diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs
index 722c3a2f0c5..93479540009 100644
--- a/src/tools/miri/src/shims/panic.rs
+++ b/src/tools/miri/src/shims/panic.rs
@@ -12,7 +12,6 @@
 //!   metadata we remembered when pushing said frame.
 
 use rustc_abi::ExternAbi;
-use rustc_ast::Mutability;
 use rustc_middle::{mir, ty};
 use rustc_target::spec::PanicStrategy;
 
@@ -161,7 +160,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let this = self.eval_context_mut();
 
         // First arg: message.
-        let msg = this.allocate_str(msg, MiriMemoryKind::Machine.into(), Mutability::Not)?;
+        let msg = this.allocate_str_dedup(msg)?;
 
         // Call the lang item.
         let panic = this.tcx.lang_items().panic_fn().unwrap();
@@ -180,7 +179,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let this = self.eval_context_mut();
 
         // First arg: message.
-        let msg = this.allocate_str(msg, MiriMemoryKind::Machine.into(), Mutability::Not)?;
+        let msg = this.allocate_str_dedup(msg)?;
 
         // Call the lang item.
         let panic = this.tcx.lang_items().panic_nounwind().unwrap();
diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-api-v0.rs b/src/tools/miri/tests/pass/backtrace/backtrace-api-v0.rs
deleted file mode 100644
index 3fff7921aff..00000000000
--- a/src/tools/miri/tests/pass/backtrace/backtrace-api-v0.rs
+++ /dev/null
@@ -1,69 +0,0 @@
-//@normalize-stderr-test: "::<.*>" -> ""
-
-#[inline(never)]
-fn func_a() -> Box<[*mut ()]> {
-    func_b::<u8>()
-}
-#[inline(never)]
-fn func_b<T>() -> Box<[*mut ()]> {
-    func_c()
-}
-
-macro_rules! invoke_func_d {
-    () => {
-        func_d()
-    };
-}
-
-#[inline(never)]
-fn func_c() -> Box<[*mut ()]> {
-    invoke_func_d!()
-}
-#[inline(never)]
-fn func_d() -> Box<[*mut ()]> {
-    unsafe { miri_get_backtrace(0) }
-}
-
-fn main() {
-    let mut seen_main = false;
-    let frames = func_a();
-    for frame in frames.iter() {
-        let miri_frame = unsafe { miri_resolve_frame(*frame, 0) };
-        let name = String::from_utf8(miri_frame.name.into()).unwrap();
-        let filename = String::from_utf8(miri_frame.filename.into()).unwrap();
-
-        if name == "func_a" {
-            assert_eq!(func_a as *mut (), miri_frame.fn_ptr);
-        }
-
-        // Print every frame to stderr.
-        let out = format!("{}:{}:{} ({})", filename, miri_frame.lineno, miri_frame.colno, name);
-        eprintln!("{}", out);
-        // Print the 'main' frame (and everything before it) to stdout, skipping
-        // the printing of internal (and possibly fragile) libstd frames.
-        // Stdout is less normalized so we see more, but it also means we can print less
-        // as platform differences would lead to test suite failures.
-        if !seen_main {
-            println!("{}", out);
-            seen_main = name == "main";
-        }
-    }
-}
-
-// This goes at the bottom of the file so that we can change it
-// without disturbing line numbers of the functions in the backtrace.
-
-extern "Rust" {
-    fn miri_get_backtrace(flags: u64) -> Box<[*mut ()]>;
-    fn miri_resolve_frame(ptr: *mut (), flags: u64) -> MiriFrame;
-}
-
-#[derive(Debug)]
-#[repr(C)]
-struct MiriFrame {
-    name: Box<[u8]>,
-    filename: Box<[u8]>,
-    lineno: u32,
-    colno: u32,
-    fn_ptr: *mut (),
-}
diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stderr b/src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stderr
deleted file mode 100644
index 9849a1aa74e..00000000000
--- a/src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stderr
+++ /dev/null
@@ -1,18 +0,0 @@
-tests/pass/backtrace/backtrace-api-v0.rs:LL:CC (func_d)
-tests/pass/backtrace/backtrace-api-v0.rs:LL:CC (func_c)
-tests/pass/backtrace/backtrace-api-v0.rs:LL:CC (func_b)
-tests/pass/backtrace/backtrace-api-v0.rs:LL:CC (func_a)
-tests/pass/backtrace/backtrace-api-v0.rs:LL:CC (main)
-RUSTLIB/core/src/ops/function.rs:LL:CC (<fn() as std::ops::FnOnce<()>>::call_once - shim(fn()))
-RUSTLIB/std/src/sys/backtrace.rs:LL:CC (std::sys::backtrace::__rust_begin_short_backtrace)
-RUSTLIB/std/src/rt.rs:LL:CC (std::rt::lang_start::{closure#0})
-RUSTLIB/core/src/ops/function.rs:LL:CC (std::ops::function::impls::call_once)
-RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try::do_call)
-RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try)
-RUSTLIB/std/src/panic.rs:LL:CC (std::panic::catch_unwind)
-RUSTLIB/std/src/rt.rs:LL:CC (std::rt::lang_start_internal::{closure#1})
-RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try::do_call)
-RUSTLIB/std/src/panicking.rs:LL:CC (std::panicking::r#try)
-RUSTLIB/std/src/panic.rs:LL:CC (std::panic::catch_unwind)
-RUSTLIB/std/src/rt.rs:LL:CC (std::rt::lang_start_internal)
-RUSTLIB/std/src/rt.rs:LL:CC (std::rt::lang_start)
diff --git a/src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stdout b/src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stdout
deleted file mode 100644
index 8c1bc5c353e..00000000000
--- a/src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stdout
+++ /dev/null
@@ -1,5 +0,0 @@
-tests/pass/backtrace/backtrace-api-v0.rs:24:14 (func_d)
-tests/pass/backtrace/backtrace-api-v0.rs:14:9 (func_c)
-tests/pass/backtrace/backtrace-api-v0.rs:9:5 (func_b::<u8>)
-tests/pass/backtrace/backtrace-api-v0.rs:5:5 (func_a)
-tests/pass/backtrace/backtrace-api-v0.rs:29:18 (main)