From 73dc95dad19e2fa513ea8ac2b76231474398c8ec Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 9 Dec 2024 08:52:23 +0100
Subject: [PATCH] interpret: clean up deduplicating allocation functions
---
.../rustc_const_eval/src/interpret/place.rs | 44 +++++-------
.../src/util/caller_location.rs | 9 +--
compiler/rustc_middle/src/ty/context.rs | 2 +-
src/tools/miri/src/shims/backtrace.rs | 10 +--
src/tools/miri/src/shims/panic.rs | 5 +-
.../tests/pass/backtrace/backtrace-api-v0.rs | 69 -------------------
.../pass/backtrace/backtrace-api-v0.stderr | 18 -----
.../pass/backtrace/backtrace-api-v0.stdout | 5 --
8 files changed, 24 insertions(+), 138 deletions(-)
delete mode 100644 src/tools/miri/tests/pass/backtrace/backtrace-api-v0.rs
delete mode 100644 src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stderr
delete mode 100644 src/tools/miri/tests/pass/backtrace/backtrace-api-v0.stdout
diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs
index f54a932e1b6f5..810e9356b26c4 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 9bf16d4fe1673..6593547cd2383 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 d4835bb07f689..2841470d24878 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 64bd546458d8a..c7b399228bfb4 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 722c3a2f0c599..93479540009ea 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 3fff7921aff77..0000000000000
--- 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 9849a1aa74ea0..0000000000000
--- 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 8c1bc5c353e9f..0000000000000
--- 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)