From 02a1532090b39f77577059f13677e3231a6712b4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 11 Apr 2024 10:48:51 +0200 Subject: [PATCH] Miri function identity hack: account for possible inlining --- .../rustc_middle/src/mir/interpret/mod.rs | 21 +++++++++++++------ .../miri/tests/pass/function_pointers.rs | 3 ++- .../miri/tests/pass/issues/issue-91636.rs | 1 + 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_middle/src/mir/interpret/mod.rs b/compiler/rustc_middle/src/mir/interpret/mod.rs index 6275942bafe65..f333cda7ce824 100644 --- a/compiler/rustc_middle/src/mir/interpret/mod.rs +++ b/compiler/rustc_middle/src/mir/interpret/mod.rs @@ -126,6 +126,7 @@ use std::num::NonZero; use std::sync::atomic::{AtomicU32, Ordering}; use rustc_ast::LitKind; +use rustc_attr::InlineAttr; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::{HashMapExt, Lock}; use rustc_data_structures::tiny_list::TinyList; @@ -555,16 +556,24 @@ impl<'tcx> TyCtxt<'tcx> { pub fn reserve_and_set_fn_alloc(self, instance: Instance<'tcx>) -> AllocId { // Functions cannot be identified by pointers, as asm-equal functions can get deduplicated // by the linker (we set the "unnamed_addr" attribute for LLVM) and functions can be - // duplicated across crates. - // We thus generate a new `AllocId` for every mention of a function. This means that - // `main as fn() == main as fn()` is false, while `let x = main as fn(); x == x` is true. - // However, formatting code relies on function identity (see #58320), so we only do - // this for generic functions. Lifetime parameters are ignored. + // duplicated across crates. We thus generate a new `AllocId` for every mention of a + // function. This means that `main as fn() == main as fn()` is false, while `let x = main as + // fn(); x == x` is true. However, as a quality-of-life feature it can be useful to identify + // certain functions uniquely, e.g. for backtraces. So we identify whether codegen will + // actually emit duplicate functions. It does that when they have non-lifetime generics, or + // when they can be inlined. All other functions are given a unique address. + // This is not a stable guarantee! The `inline` attribute is a hint and cannot be relied + // upon for anything. But if we don't do this, backtraces look terrible and we risk breaking + // the `USIZE_MARKER` hack in the formatting machinery. let is_generic = instance .args .into_iter() .any(|kind| !matches!(kind.unpack(), GenericArgKind::Lifetime(_))); - if is_generic { + let can_be_inlined = match self.codegen_fn_attrs(instance.def_id()).inline { + InlineAttr::Never => false, + _ => true, + }; + if is_generic || can_be_inlined { // Get a fresh ID. let mut alloc_map = self.alloc_map.lock(); let id = alloc_map.reserve(); diff --git a/src/tools/miri/tests/pass/function_pointers.rs b/src/tools/miri/tests/pass/function_pointers.rs index 36679b7180a64..2aa3ebf2dd0b4 100644 --- a/src/tools/miri/tests/pass/function_pointers.rs +++ b/src/tools/miri/tests/pass/function_pointers.rs @@ -23,6 +23,7 @@ fn h(i: i32, j: i32) -> i32 { j * i * 7 } +#[inline(never)] fn i() -> i32 { 73 } @@ -77,7 +78,7 @@ fn main() { assert_eq!(indirect_mut3(h), 210); assert_eq!(indirect_once3(h), 210); // Check that `i` always has the same address. This is not guaranteed - // but Miri currently uses a fixed address for monomorphic functions. + // but Miri currently uses a fixed address for non-inlineable monomorphic functions. assert!(return_fn_ptr(i) == i); assert!(return_fn_ptr(i) as unsafe fn() -> i32 == i as fn() -> i32 as unsafe fn() -> i32); // Miri gives different addresses to different reifications of a generic function. diff --git a/src/tools/miri/tests/pass/issues/issue-91636.rs b/src/tools/miri/tests/pass/issues/issue-91636.rs index 21000bb68d2bc..00370165812f5 100644 --- a/src/tools/miri/tests/pass/issues/issue-91636.rs +++ b/src/tools/miri/tests/pass/issues/issue-91636.rs @@ -10,6 +10,7 @@ impl Function { } } +#[inline(never)] fn dummy(_: &str) {} fn main() {