about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2025-07-16 17:06:43 +0200
committerGitHub <noreply@github.com>2025-07-16 17:06:43 +0200
commit2a8f97910a794f852b893213e059cab2ca12da7e (patch)
treea2c2b3e63722d7fda9d2e06665f59c4b45101f0c
parent19470bc9c55b03d774658f457f98d197f3663c4f (diff)
parent14f47009cc5b02cd8c28ca2cc9c1fd58aeeef25b (diff)
downloadrust-2a8f97910a794f852b893213e059cab2ca12da7e.tar.gz
rust-2a8f97910a794f852b893213e059cab2ca12da7e.zip
Rollup merge of #143968 - Stypox:tracing-FnAbiOf, r=RalfJung
Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`

This PR adds tracing to the `InterpCx::fn_abi_of_instance`/`::fn_abi_of_fn_ptr` functions by shadowing `FnAbiOf`'s trait methods with inherent methods on `InterpCx`, like done in rust-lang/rust#142721. The reason why I am targeting these two functions is because they are used for Miri interpretation, and they make a `layout_of` query down the line without passing through the `layout_of` that was traced in rust-lang/rust#142721.

There are other places where `layout_of` is called without being traced (see the analysis below), but that's because the `Machine` used there is not `MiriMachine` but rather `CompileTimeMachine` which does not implement `enter_trace_span()`. But after discussing with ```````@RalfJung``````` we agreed that the const-eval part should not be traced together with Miri, that's why I am ignoring the other places where `layout_of` is called.

r? ```````@RalfJung```````

<details><summary>Analysis of the places where <code>layout_of</code> is called</summary>

I did some analysis for https://github.com/rust-lang/rust/pull/142721#discussion_r2171494841, and these are all the places where the query `tcx.layout_of` is called (directly or indirectly) outside of a traced `InterpCx::layout_of` while a program is being interpreted by Miri:

```
adjust_for_rust_scalar  at ./compiler/rustc_ty_utils/src/abi.rs:302:35
{closure#2}             at ./compiler/rustc_ty_utils/src/abi.rs:522:25
eval_body_using_ecx<>   at ./compiler/rustc_const_eval/src/const_eval/eval_queries.rs:49:22
{closure#1}<>           at ./compiler/rustc_const_eval/src/interpret/operand.rs:851:76
{closure#0}<>           at ./compiler/rustc_const_eval/src/interpret/stack.rs:612:18
size_and_align          at ./compiler/rustc_middle/src/mir/interpret/mod.rs:387:38
```

I got these by:
- patching rustc with this patch that adds a span to the `layout_of` query which prints the backtrace:
[layout_of_other_places.diff.txt](https://github.com/user-attachments/files/21235523/layout_of_other_places.diff.txt)
- adding this to my bootstrap.toml to have debug symbols inside the Miri binary: `rust.debuginfo-level = "line-tables-only"` and also `build.tool.miri.features = ["tracing"]`
- obtaining a trace file with `MIRI_TRACING=1 ./x.py run miri --stage 1 --warnings warn --args src/tools/miri/tests/pass/hello.rs` (note: maybe using a file different than "src/tools/miri/tests/pass/hello.rs" would lead to more places where layout_of is called?)
-  running this query in Perfetto to select all `layout_of` spans that have as a direct parent a span named "frame" (as opposed to the parent being `InterpCx::layout_of`) and extract their backtrace: `select args.string_value from slice left join args on slice.arg_set_id = args.id where slice.name = "tcx.layout_of" and slice.parent_id in (select slice2.id from slice as slice2 where slice2.name = "frame") group by args.string_value`
- exporting the data as `.tsv` and processing that file through this Python script. It finds the first path in the backtraces where "layout" isn't mentioned, which imo is a good heuristic to not consider `layout_of` wrappers/friends as call places, but rather go down the backtrace until an actual call place is reached. [layout_of_other_places.py.txt](https://github.com/user-attachments/files/21235529/layout_of_other_places.py.txt)

</details>
-rw-r--r--compiler/rustc_const_eval/src/interpret/call.rs2
-rw-r--r--compiler/rustc_const_eval/src/interpret/eval_context.rs55
-rw-r--r--compiler/rustc_const_eval/src/interpret/step.rs1
-rw-r--r--src/tools/miri/src/helpers.rs2
4 files changed, 41 insertions, 19 deletions
diff --git a/compiler/rustc_const_eval/src/interpret/call.rs b/compiler/rustc_const_eval/src/interpret/call.rs
index ad3e02580f3..1503f3bcd99 100644
--- a/compiler/rustc_const_eval/src/interpret/call.rs
+++ b/compiler/rustc_const_eval/src/interpret/call.rs
@@ -6,7 +6,7 @@ use std::borrow::Cow;
 use either::{Left, Right};
 use rustc_abi::{self as abi, ExternAbi, FieldIdx, Integer, VariantIdx};
 use rustc_hir::def_id::DefId;
-use rustc_middle::ty::layout::{FnAbiOf, IntegerExt, TyAndLayout};
+use rustc_middle::ty::layout::{IntegerExt, TyAndLayout};
 use rustc_middle::ty::{self, AdtDef, Instance, Ty, VariantDef};
 use rustc_middle::{bug, mir, span_bug};
 use rustc_span::sym;
diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs
index 41fc8d47cd3..11e7706fe60 100644
--- a/compiler/rustc_const_eval/src/interpret/eval_context.rs
+++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs
@@ -7,8 +7,8 @@ use rustc_hir::def_id::DefId;
 use rustc_middle::mir::interpret::{ErrorHandled, InvalidMetaKind, ReportedErrorInfo};
 use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::layout::{
-    self, FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOf, LayoutOfHelpers,
-    TyAndLayout,
+    self, FnAbiError, FnAbiOf, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOf,
+    LayoutOfHelpers, TyAndLayout,
 };
 use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeFoldable, TypingEnv, Variance};
 use rustc_middle::{mir, span_bug};
@@ -92,20 +92,6 @@ impl<'tcx, M: Machine<'tcx>> LayoutOfHelpers<'tcx> for InterpCx<'tcx, M> {
     }
 }
 
-impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
-    /// This inherent method takes priority over the trait method with the same name in LayoutOf,
-    /// and allows wrapping the actual [LayoutOf::layout_of] with a tracing span.
-    /// See [LayoutOf::layout_of] for the original documentation.
-    #[inline(always)]
-    pub fn layout_of(
-        &self,
-        ty: Ty<'tcx>,
-    ) -> <InterpCx<'tcx, M> as LayoutOfHelpers<'tcx>>::LayoutOfResult {
-        let _span = enter_trace_span!(M, "InterpCx::layout_of", "ty = {:?}", ty.kind());
-        LayoutOf::layout_of(self, ty)
-    }
-}
-
 impl<'tcx, M: Machine<'tcx>> FnAbiOfHelpers<'tcx> for InterpCx<'tcx, M> {
     type FnAbiOfResult = Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, InterpErrorKind<'tcx>>;
 
@@ -121,6 +107,43 @@ impl<'tcx, M: Machine<'tcx>> FnAbiOfHelpers<'tcx> for InterpCx<'tcx, M> {
     }
 }
 
+impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
+    /// This inherent method takes priority over the trait method with the same name in LayoutOf,
+    /// and allows wrapping the actual [LayoutOf::layout_of] with a tracing span.
+    /// See [LayoutOf::layout_of] for the original documentation.
+    #[inline(always)]
+    pub fn layout_of(&self, ty: Ty<'tcx>) -> <Self as LayoutOfHelpers<'tcx>>::LayoutOfResult {
+        let _span = enter_trace_span!(M, "InterpCx::layout_of", ty = ?ty.kind());
+        LayoutOf::layout_of(self, ty)
+    }
+
+    /// This inherent method takes priority over the trait method with the same name in FnAbiOf,
+    /// and allows wrapping the actual [FnAbiOf::fn_abi_of_fn_ptr] with a tracing span.
+    /// See [FnAbiOf::fn_abi_of_fn_ptr] for the original documentation.
+    #[inline(always)]
+    pub fn fn_abi_of_fn_ptr(
+        &self,
+        sig: ty::PolyFnSig<'tcx>,
+        extra_args: &'tcx ty::List<Ty<'tcx>>,
+    ) -> <Self as FnAbiOfHelpers<'tcx>>::FnAbiOfResult {
+        let _span = enter_trace_span!(M, "InterpCx::fn_abi_of_fn_ptr", ?sig, ?extra_args);
+        FnAbiOf::fn_abi_of_fn_ptr(self, sig, extra_args)
+    }
+
+    /// This inherent method takes priority over the trait method with the same name in FnAbiOf,
+    /// and allows wrapping the actual [FnAbiOf::fn_abi_of_instance] with a tracing span.
+    /// See [FnAbiOf::fn_abi_of_instance] for the original documentation.
+    #[inline(always)]
+    pub fn fn_abi_of_instance(
+        &self,
+        instance: ty::Instance<'tcx>,
+        extra_args: &'tcx ty::List<Ty<'tcx>>,
+    ) -> <Self as FnAbiOfHelpers<'tcx>>::FnAbiOfResult {
+        let _span = enter_trace_span!(M, "InterpCx::fn_abi_of_instance", ?instance, ?extra_args);
+        FnAbiOf::fn_abi_of_instance(self, instance, extra_args)
+    }
+}
+
 /// Test if it is valid for a MIR assignment to assign `src`-typed place to `dest`-typed value.
 /// This test should be symmetric, as it is primarily about layout compatibility.
 pub(super) fn mir_assign_valid_types<'tcx>(
diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs
index 833fcc38817..629dcc3523c 100644
--- a/compiler/rustc_const_eval/src/interpret/step.rs
+++ b/compiler/rustc_const_eval/src/interpret/step.rs
@@ -5,7 +5,6 @@
 use either::Either;
 use rustc_abi::{FIRST_VARIANT, FieldIdx};
 use rustc_index::IndexSlice;
-use rustc_middle::ty::layout::FnAbiOf;
 use rustc_middle::ty::{self, Instance, Ty};
 use rustc_middle::{bug, mir, span_bug};
 use rustc_span::source_map::Spanned;
diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs
index c150dc16b07..f2283f0902b 100644
--- a/src/tools/miri/src/helpers.rs
+++ b/src/tools/miri/src/helpers.rs
@@ -13,7 +13,7 @@ use rustc_index::IndexVec;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::middle::dependency_format::Linkage;
 use rustc_middle::middle::exported_symbols::ExportedSymbol;
-use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, MaybeResult, TyAndLayout};
+use rustc_middle::ty::layout::{LayoutOf, MaybeResult, TyAndLayout};
 use rustc_middle::ty::{self, Binder, FloatTy, FnSig, IntTy, Ty, TyCtxt, UintTy};
 use rustc_session::config::CrateType;
 use rustc_span::{Span, Symbol};