Skip to content

Commit 4e901b6

Browse files
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 #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 #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 #142721 (comment), 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>
2 parents bdf3757 + 14f4700 commit 4e901b6

File tree

4 files changed

+41
-19
lines changed

4 files changed

+41
-19
lines changed

compiler/rustc_const_eval/src/interpret/call.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::borrow::Cow;
66
use either::{Left, Right};
77
use rustc_abi::{self as abi, ExternAbi, FieldIdx, Integer, VariantIdx};
88
use rustc_hir::def_id::DefId;
9-
use rustc_middle::ty::layout::{FnAbiOf, IntegerExt, TyAndLayout};
9+
use rustc_middle::ty::layout::{IntegerExt, TyAndLayout};
1010
use rustc_middle::ty::{self, AdtDef, Instance, Ty, VariantDef};
1111
use rustc_middle::{bug, mir, span_bug};
1212
use rustc_span::sym;

compiler/rustc_const_eval/src/interpret/eval_context.rs

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ use rustc_hir::def_id::DefId;
77
use rustc_middle::mir::interpret::{ErrorHandled, InvalidMetaKind, ReportedErrorInfo};
88
use rustc_middle::query::TyCtxtAt;
99
use rustc_middle::ty::layout::{
10-
self, FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOf, LayoutOfHelpers,
11-
TyAndLayout,
10+
self, FnAbiError, FnAbiOf, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOf,
11+
LayoutOfHelpers, TyAndLayout,
1212
};
1313
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeFoldable, TypingEnv, Variance};
1414
use rustc_middle::{mir, span_bug};
@@ -92,20 +92,6 @@ impl<'tcx, M: Machine<'tcx>> LayoutOfHelpers<'tcx> for InterpCx<'tcx, M> {
9292
}
9393
}
9494

95-
impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
96-
/// This inherent method takes priority over the trait method with the same name in LayoutOf,
97-
/// and allows wrapping the actual [LayoutOf::layout_of] with a tracing span.
98-
/// See [LayoutOf::layout_of] for the original documentation.
99-
#[inline(always)]
100-
pub fn layout_of(
101-
&self,
102-
ty: Ty<'tcx>,
103-
) -> <InterpCx<'tcx, M> as LayoutOfHelpers<'tcx>>::LayoutOfResult {
104-
let _span = enter_trace_span!(M, "InterpCx::layout_of", "ty = {:?}", ty.kind());
105-
LayoutOf::layout_of(self, ty)
106-
}
107-
}
108-
10995
impl<'tcx, M: Machine<'tcx>> FnAbiOfHelpers<'tcx> for InterpCx<'tcx, M> {
11096
type FnAbiOfResult = Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, InterpErrorKind<'tcx>>;
11197

@@ -121,6 +107,43 @@ impl<'tcx, M: Machine<'tcx>> FnAbiOfHelpers<'tcx> for InterpCx<'tcx, M> {
121107
}
122108
}
123109

110+
impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
111+
/// This inherent method takes priority over the trait method with the same name in LayoutOf,
112+
/// and allows wrapping the actual [LayoutOf::layout_of] with a tracing span.
113+
/// See [LayoutOf::layout_of] for the original documentation.
114+
#[inline(always)]
115+
pub fn layout_of(&self, ty: Ty<'tcx>) -> <Self as LayoutOfHelpers<'tcx>>::LayoutOfResult {
116+
let _span = enter_trace_span!(M, "InterpCx::layout_of", ty = ?ty.kind());
117+
LayoutOf::layout_of(self, ty)
118+
}
119+
120+
/// This inherent method takes priority over the trait method with the same name in FnAbiOf,
121+
/// and allows wrapping the actual [FnAbiOf::fn_abi_of_fn_ptr] with a tracing span.
122+
/// See [FnAbiOf::fn_abi_of_fn_ptr] for the original documentation.
123+
#[inline(always)]
124+
pub fn fn_abi_of_fn_ptr(
125+
&self,
126+
sig: ty::PolyFnSig<'tcx>,
127+
extra_args: &'tcx ty::List<Ty<'tcx>>,
128+
) -> <Self as FnAbiOfHelpers<'tcx>>::FnAbiOfResult {
129+
let _span = enter_trace_span!(M, "InterpCx::fn_abi_of_fn_ptr", ?sig, ?extra_args);
130+
FnAbiOf::fn_abi_of_fn_ptr(self, sig, extra_args)
131+
}
132+
133+
/// This inherent method takes priority over the trait method with the same name in FnAbiOf,
134+
/// and allows wrapping the actual [FnAbiOf::fn_abi_of_instance] with a tracing span.
135+
/// See [FnAbiOf::fn_abi_of_instance] for the original documentation.
136+
#[inline(always)]
137+
pub fn fn_abi_of_instance(
138+
&self,
139+
instance: ty::Instance<'tcx>,
140+
extra_args: &'tcx ty::List<Ty<'tcx>>,
141+
) -> <Self as FnAbiOfHelpers<'tcx>>::FnAbiOfResult {
142+
let _span = enter_trace_span!(M, "InterpCx::fn_abi_of_instance", ?instance, ?extra_args);
143+
FnAbiOf::fn_abi_of_instance(self, instance, extra_args)
144+
}
145+
}
146+
124147
/// Test if it is valid for a MIR assignment to assign `src`-typed place to `dest`-typed value.
125148
/// This test should be symmetric, as it is primarily about layout compatibility.
126149
pub(super) fn mir_assign_valid_types<'tcx>(

compiler/rustc_const_eval/src/interpret/step.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
use either::Either;
66
use rustc_abi::{FIRST_VARIANT, FieldIdx};
77
use rustc_index::IndexSlice;
8-
use rustc_middle::ty::layout::FnAbiOf;
98
use rustc_middle::ty::{self, Instance, Ty};
109
use rustc_middle::{bug, mir, span_bug};
1110
use rustc_span::source_map::Spanned;

src/tools/miri/src/helpers.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_index::IndexVec;
1313
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
1414
use rustc_middle::middle::dependency_format::Linkage;
1515
use rustc_middle::middle::exported_symbols::ExportedSymbol;
16-
use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, MaybeResult, TyAndLayout};
16+
use rustc_middle::ty::layout::{LayoutOf, MaybeResult, TyAndLayout};
1717
use rustc_middle::ty::{self, Binder, FloatTy, FnSig, IntTy, Ty, TyCtxt, UintTy};
1818
use rustc_session::config::CrateType;
1919
use rustc_span::{Span, Symbol};

0 commit comments

Comments
 (0)