From 5adf145bc7fa4961781470ae43c5f08010c6a31d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 18 Nov 2023 17:51:17 +1100 Subject: [PATCH 1/8] coverage: Assert that the instrumentor never sees promoted MIR --- compiler/rustc_mir_transform/src/coverage/mod.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 796150f931581..01aa81f3bd2a2 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -39,15 +39,9 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage { fn run_pass(&self, tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) { let mir_source = mir_body.source; - // If the InstrumentCoverage pass is called on promoted MIRs, skip them. - // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 - if mir_source.promoted.is_some() { - trace!( - "InstrumentCoverage skipped for {:?} (already promoted for Miri evaluation)", - mir_source.def_id() - ); - return; - } + // This pass runs after MIR promotion, but before promoted MIR starts to + // be transformed, so it should never see promoted MIR. + assert!(mir_source.promoted.is_none()); let is_fn_like = tcx.hir().get_by_def_id(mir_source.def_id().expect_local()).fn_kind().is_some(); From 3babb38075f15fa2717f756586eb261e119c6b78 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 24 Nov 2023 16:08:05 +1100 Subject: [PATCH 2/8] coverage: Extract `is_eligible_for_coverage` --- .../rustc_mir_transform/src/coverage/mod.rs | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 01aa81f3bd2a2..7c95a3e3afb05 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -22,7 +22,7 @@ use rustc_middle::mir::{ TerminatorKind, }; use rustc_middle::ty::TyCtxt; -use rustc_span::def_id::DefId; +use rustc_span::def_id::{DefId, LocalDefId}; use rustc_span::source_map::SourceMap; use rustc_span::{ExpnKind, SourceFile, Span, Symbol}; @@ -43,18 +43,9 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage { // be transformed, so it should never see promoted MIR. assert!(mir_source.promoted.is_none()); - let is_fn_like = - tcx.hir().get_by_def_id(mir_source.def_id().expect_local()).fn_kind().is_some(); - - // Only instrument functions, methods, and closures (not constants since they are evaluated - // at compile time by Miri). - // FIXME(#73156): Handle source code coverage in const eval, but note, if and when const - // expressions get coverage spans, we will probably have to "carve out" space for const - // expressions from coverage spans in enclosing MIR's, like we do for closures. (That might - // be tricky if const expressions have no corresponding statements in the enclosing MIR. - // Closures are carved out by their initial `Assign` statement.) - if !is_fn_like { - trace!("InstrumentCoverage skipped for {:?} (not an fn-like)", mir_source.def_id()); + let def_id = mir_source.def_id().expect_local(); + if !is_eligible_for_coverage(tcx, def_id) { + trace!("InstrumentCoverage skipped for {def_id:?} (not eligible)"); return; } @@ -66,14 +57,9 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage { _ => {} } - let codegen_fn_attrs = tcx.codegen_fn_attrs(mir_source.def_id()); - if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) { - return; - } - - trace!("InstrumentCoverage starting for {:?}", mir_source.def_id()); + trace!("InstrumentCoverage starting for {def_id:?}"); Instrumentor::new(tcx, mir_body).inject_counters(); - trace!("InstrumentCoverage done for {:?}", mir_source.def_id()); + trace!("InstrumentCoverage done for {def_id:?}"); } } @@ -319,6 +305,28 @@ fn make_code_region( } } +fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { + let is_fn_like = tcx.hir().get_by_def_id(def_id).fn_kind().is_some(); + + // Only instrument functions, methods, and closures (not constants since they are evaluated + // at compile time by Miri). + // FIXME(#73156): Handle source code coverage in const eval, but note, if and when const + // expressions get coverage spans, we will probably have to "carve out" space for const + // expressions from coverage spans in enclosing MIR's, like we do for closures. (That might + // be tricky if const expressions have no corresponding statements in the enclosing MIR. + // Closures are carved out by their initial `Assign` statement.) + if !is_fn_like { + return false; + } + + let codegen_fn_attrs = tcx.codegen_fn_attrs(def_id); + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) { + return false; + } + + true +} + fn fn_sig_and_body( tcx: TyCtxt<'_>, def_id: DefId, From 6421752e2c2e709e7107c359f451a8be1fe8dfb2 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 24 Nov 2023 16:15:54 +1100 Subject: [PATCH 3/8] coverage: Extract helper for getting HIR info for coverage --- compiler/rustc_middle/src/mir/coverage.rs | 14 ++++- .../rustc_mir_transform/src/coverage/mod.rs | 55 ++++++++++--------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index ec5edceb26997..3d6d6ca03372e 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -2,7 +2,7 @@ use rustc_index::IndexVec; use rustc_macros::HashStable; -use rustc_span::Symbol; +use rustc_span::{Span, Symbol}; use std::fmt::{self, Debug, Formatter}; @@ -184,3 +184,15 @@ pub struct FunctionCoverageInfo { pub expressions: IndexVec, pub mappings: Vec, } + +/// Coverage information captured from HIR/THIR during MIR building. +/// +/// A MIR body that does not have this information cannot be instrumented for +/// coverage. +#[derive(Clone, Debug)] +#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] +pub struct HirInfo { + pub function_source_hash: u64, + pub fn_sig_span: Span, + pub body_span: Span, +} diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 7c95a3e3afb05..e02b4d7575546 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -22,7 +22,7 @@ use rustc_middle::mir::{ TerminatorKind, }; use rustc_middle::ty::TyCtxt; -use rustc_span::def_id::{DefId, LocalDefId}; +use rustc_span::def_id::LocalDefId; use rustc_span::source_map::SourceMap; use rustc_span::{ExpnKind, SourceFile, Span, Symbol}; @@ -77,29 +77,15 @@ struct Instrumentor<'a, 'tcx> { impl<'a, 'tcx> Instrumentor<'a, 'tcx> { fn new(tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self { let source_map = tcx.sess.source_map(); - let def_id = mir_body.source.def_id(); - let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id); - let body_span = get_body_span(tcx, hir_body, mir_body); + let def_id = mir_body.source.def_id().expect_local(); + let mir::coverage::HirInfo { function_source_hash, fn_sig_span, body_span, .. } = + make_coverage_hir_info(tcx, def_id); let source_file = source_map.lookup_source_file(body_span.lo()); - let fn_sig_span = match some_fn_sig.filter(|fn_sig| { - fn_sig.span.eq_ctxt(body_span) - && Lrc::ptr_eq(&source_file, &source_map.lookup_source_file(fn_sig.span.lo())) - }) { - Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()), - None => body_span.shrink_to_lo(), - }; - - debug!( - "instrumenting {}: {:?}, fn sig span: {:?}, body span: {:?}", - if tcx.is_closure(def_id) { "closure" } else { "function" }, - def_id, - fn_sig_span, - body_span - ); - let function_source_hash = hash_mir_source(tcx, hir_body); + debug!(?fn_sig_span, ?body_span, "instrumenting {def_id:?}",); + let basic_coverage_blocks = CoverageGraph::from_mir(mir_body); let coverage_counters = CoverageCounters::new(&basic_coverage_blocks); @@ -327,13 +313,33 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { true } +fn make_coverage_hir_info(tcx: TyCtxt<'_>, def_id: LocalDefId) -> mir::coverage::HirInfo { + let source_map = tcx.sess.source_map(); + let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id); + + let body_span = get_body_span(tcx, hir_body, def_id); + + let source_file = source_map.lookup_source_file(body_span.lo()); + let fn_sig_span = match some_fn_sig.filter(|fn_sig| { + fn_sig.span.eq_ctxt(body_span) + && Lrc::ptr_eq(&source_file, &source_map.lookup_source_file(fn_sig.span.lo())) + }) { + Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()), + None => body_span.shrink_to_lo(), + }; + + let function_source_hash = hash_mir_source(tcx, hir_body); + + mir::coverage::HirInfo { function_source_hash, fn_sig_span, body_span } +} + fn fn_sig_and_body( tcx: TyCtxt<'_>, - def_id: DefId, + def_id: LocalDefId, ) -> (Option<&rustc_hir::FnSig<'_>>, &rustc_hir::Body<'_>) { // FIXME(#79625): Consider improving MIR to provide the information needed, to avoid going back // to HIR for it. - let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); + let hir_node = tcx.hir().get_by_def_id(def_id); let (_, fn_body_id) = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); (hir_node.fn_sig(), tcx.hir().body(fn_body_id)) @@ -342,12 +348,11 @@ fn fn_sig_and_body( fn get_body_span<'tcx>( tcx: TyCtxt<'tcx>, hir_body: &rustc_hir::Body<'tcx>, - mir_body: &mut mir::Body<'tcx>, + def_id: LocalDefId, ) -> Span { let mut body_span = hir_body.value.span; - let def_id = mir_body.source.def_id(); - if tcx.is_closure(def_id) { + if tcx.is_closure(def_id.to_def_id()) { // If the MIR function is a closure, and if the closure body span // starts from a macro, but it's content is not in that macro, try // to find a non-macro callsite, and instrument the spans there From 623ef3782ecf3332f1e77901ad0425baf30bc132 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 24 Nov 2023 16:21:25 +1100 Subject: [PATCH 4/8] coverage: Simplify `make_coverage_hir_info` --- .../rustc_mir_transform/src/coverage/mod.rs | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index e02b4d7575546..a82593732c997 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -314,21 +314,27 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { } fn make_coverage_hir_info(tcx: TyCtxt<'_>, def_id: LocalDefId) -> mir::coverage::HirInfo { - let source_map = tcx.sess.source_map(); - let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id); + let (maybe_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id); + let function_source_hash = hash_mir_source(tcx, hir_body); let body_span = get_body_span(tcx, hir_body, def_id); - let source_file = source_map.lookup_source_file(body_span.lo()); - let fn_sig_span = match some_fn_sig.filter(|fn_sig| { - fn_sig.span.eq_ctxt(body_span) - && Lrc::ptr_eq(&source_file, &source_map.lookup_source_file(fn_sig.span.lo())) - }) { - Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()), - None => body_span.shrink_to_lo(), + let spans_are_compatible = { + let source_map = tcx.sess.source_map(); + |a: Span, b: Span| { + a.eq_ctxt(b) + && source_map.lookup_source_file_idx(a.lo()) + == source_map.lookup_source_file_idx(b.lo()) + } }; - let function_source_hash = hash_mir_source(tcx, hir_body); + let fn_sig_span = if let Some(fn_sig) = maybe_fn_sig + && spans_are_compatible(fn_sig.span, body_span) + { + fn_sig.span.with_hi(body_span.lo()) + } else { + body_span.shrink_to_lo() + }; mir::coverage::HirInfo { function_source_hash, fn_sig_span, body_span } } From 3dbe59ebcd3fc721fec6a7288becdc33e5af84dd Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 24 Nov 2023 16:28:09 +1100 Subject: [PATCH 5/8] coverage: Check that the fn sig span is before the body span If the function signature span comes after the body span (due to macro expansion), expanding it towards the start of the body span would produce nonsense. --- compiler/rustc_mir_transform/src/coverage/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index a82593732c997..3d65ba55f7e11 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -330,6 +330,7 @@ fn make_coverage_hir_info(tcx: TyCtxt<'_>, def_id: LocalDefId) -> mir::coverage: let fn_sig_span = if let Some(fn_sig) = maybe_fn_sig && spans_are_compatible(fn_sig.span, body_span) + && fn_sig.span.lo() <= body_span.lo() { fn_sig.span.with_hi(body_span.lo()) } else { From 65606bfe611677e1b270d6bf029bcfa030c0e5cd Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 24 Nov 2023 16:45:31 +1100 Subject: [PATCH 6/8] coverage: Collect HIR info during MIR building The coverage instrumentor pass operates on MIR, but it needs access to some source-related information that is normally not present in MIR. Historically, that information was obtained by looking back at HIR during MIR transformation. This patch arranges for the necessary information to be collected ahead of time during MIR building instead. --- .../src/transform/promote_consts.rs | 1 + compiler/rustc_middle/src/mir/mod.rs | 10 ++ .../rustc_mir_build/src/build/coverageinfo.rs | 117 ++++++++++++++++++ .../rustc_mir_build/src/build/custom/mod.rs | 1 + compiler/rustc_mir_build/src/build/mod.rs | 9 ++ .../rustc_mir_transform/src/coverage/mod.rs | 117 ++---------------- compiler/rustc_mir_transform/src/shim.rs | 1 + 7 files changed, 148 insertions(+), 108 deletions(-) create mode 100644 compiler/rustc_mir_build/src/build/coverageinfo.rs diff --git a/compiler/rustc_const_eval/src/transform/promote_consts.rs b/compiler/rustc_const_eval/src/transform/promote_consts.rs index 8b2ea2dc21dd1..dae1fa60c986d 100644 --- a/compiler/rustc_const_eval/src/transform/promote_consts.rs +++ b/compiler/rustc_const_eval/src/transform/promote_consts.rs @@ -971,6 +971,7 @@ pub fn promote_candidates<'tcx>( body.span, body.coroutine_kind(), body.tainted_by_errors, + None, ); promoted.phase = MirPhase::Analysis(AnalysisPhase::Initial); diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 1e5a7401c6f94..e7a6ba72bf469 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -346,6 +346,13 @@ pub struct Body<'tcx> { pub tainted_by_errors: Option, + /// Extra information about this function's source code captured during MIR + /// building, for use by coverage instrumentation. + /// + /// If `-Cinstrument-coverage` is not active, or if an individual function + /// is not eligible for coverage, then this should always be `None`. + pub coverage_hir_info: Option>, + /// Per-function coverage information added by the `InstrumentCoverage` /// pass, to be used in conjunction with the coverage statements injected /// into this body's blocks. @@ -367,6 +374,7 @@ impl<'tcx> Body<'tcx> { span: Span, coroutine_kind: Option, tainted_by_errors: Option, + coverage_hir_info: Option>, ) -> Self { // We need `arg_count` locals, and one for the return place. assert!( @@ -400,6 +408,7 @@ impl<'tcx> Body<'tcx> { is_polymorphic: false, injection_phase: None, tainted_by_errors, + coverage_hir_info, function_coverage_info: None, }; body.is_polymorphic = body.has_non_region_param(); @@ -429,6 +438,7 @@ impl<'tcx> Body<'tcx> { is_polymorphic: false, injection_phase: None, tainted_by_errors: None, + coverage_hir_info: None, function_coverage_info: None, }; body.is_polymorphic = body.has_non_region_param(); diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs new file mode 100644 index 0000000000000..bbf41e72df959 --- /dev/null +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -0,0 +1,117 @@ +use rustc_middle::hir; +use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; +use rustc_middle::mir; +use rustc_middle::ty::TyCtxt; +use rustc_span::def_id::LocalDefId; +use rustc_span::{ExpnKind, Span}; + +/// If the given item is eligible for coverage instrumentation, collect relevant +/// HIR information that will be needed by the instrumentor pass. +pub(crate) fn make_coverage_hir_info_if_eligible( + tcx: TyCtxt<'_>, + def_id: LocalDefId, +) -> Option> { + assert!(tcx.sess.instrument_coverage()); + + is_eligible_for_coverage(tcx, def_id).then(|| Box::new(make_coverage_hir_info(tcx, def_id))) +} + +fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { + let is_fn_like = tcx.hir().get_by_def_id(def_id).fn_kind().is_some(); + + // Only instrument functions, methods, and closures (not constants since they are evaluated + // at compile time by Miri). + // FIXME(#73156): Handle source code coverage in const eval, but note, if and when const + // expressions get coverage spans, we will probably have to "carve out" space for const + // expressions from coverage spans in enclosing MIR's, like we do for closures. (That might + // be tricky if const expressions have no corresponding statements in the enclosing MIR. + // Closures are carved out by their initial `Assign` statement.) + if !is_fn_like { + return false; + } + + let codegen_fn_attrs = tcx.codegen_fn_attrs(def_id); + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) { + return false; + } + + true +} + +fn make_coverage_hir_info(tcx: TyCtxt<'_>, def_id: LocalDefId) -> mir::coverage::HirInfo { + let (maybe_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id); + + let function_source_hash = hash_mir_source(tcx, hir_body); + let body_span = get_body_span(tcx, hir_body, def_id); + + let spans_are_compatible = { + let source_map = tcx.sess.source_map(); + |a: Span, b: Span| { + a.eq_ctxt(b) + && source_map.lookup_source_file_idx(a.lo()) + == source_map.lookup_source_file_idx(b.lo()) + } + }; + + let fn_sig_span = if let Some(fn_sig) = maybe_fn_sig + && spans_are_compatible(fn_sig.span, body_span) + && fn_sig.span.lo() <= body_span.lo() + { + fn_sig.span.with_hi(body_span.lo()) + } else { + body_span.shrink_to_lo() + }; + + mir::coverage::HirInfo { function_source_hash, fn_sig_span, body_span } +} + +fn fn_sig_and_body( + tcx: TyCtxt<'_>, + def_id: LocalDefId, +) -> (Option<&rustc_hir::FnSig<'_>>, &rustc_hir::Body<'_>) { + // FIXME(#79625): Consider improving MIR to provide the information needed, to avoid going back + // to HIR for it. + let hir_node = tcx.hir().get_by_def_id(def_id); + let (_, fn_body_id) = + hir::map::associated_body(hir_node).expect("HIR node is a function with body"); + (hir_node.fn_sig(), tcx.hir().body(fn_body_id)) +} + +fn get_body_span<'tcx>( + tcx: TyCtxt<'tcx>, + hir_body: &rustc_hir::Body<'tcx>, + def_id: LocalDefId, +) -> Span { + let mut body_span = hir_body.value.span; + + if tcx.is_closure(def_id.to_def_id()) { + // If the MIR function is a closure, and if the closure body span + // starts from a macro, but it's content is not in that macro, try + // to find a non-macro callsite, and instrument the spans there + // instead. + loop { + let expn_data = body_span.ctxt().outer_expn_data(); + if expn_data.is_root() { + break; + } + if let ExpnKind::Macro { .. } = expn_data.kind { + body_span = expn_data.call_site; + } else { + break; + } + } + } + + body_span +} + +fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { + // FIXME(cjgillot) Stop hashing HIR manually here. + let owner = hir_body.id().hir_id.owner; + tcx.hir_owner_nodes(owner) + .unwrap() + .opt_hash_including_bodies + .unwrap() + .to_smaller_hash() + .as_u64() +} diff --git a/compiler/rustc_mir_build/src/build/custom/mod.rs b/compiler/rustc_mir_build/src/build/custom/mod.rs index 8c68a58d4062d..816bc11e804ef 100644 --- a/compiler/rustc_mir_build/src/build/custom/mod.rs +++ b/compiler/rustc_mir_build/src/build/custom/mod.rs @@ -60,6 +60,7 @@ pub(super) fn build_custom_mir<'tcx>( tainted_by_errors: None, injection_phase: None, pass_count: 0, + coverage_hir_info: None, function_coverage_info: None, }; diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 19b6496b102ef..dd57b989f1b45 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -693,6 +693,7 @@ fn construct_error(tcx: TyCtxt<'_>, def_id: LocalDefId, guar: ErrorGuaranteed) - span, coroutine_kind, Some(guar), + None, ); body.coroutine.as_mut().map(|gen| gen.yield_ty = yield_ty); @@ -775,6 +776,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + let coverage_hir_info = if self.tcx.sess.instrument_coverage() { + coverageinfo::make_coverage_hir_info_if_eligible(self.tcx, self.def_id) + } else { + None + }; + Body::new( MirSource::item(self.def_id.to_def_id()), self.cfg.basic_blocks, @@ -786,6 +793,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.fn_span, self.coroutine_kind, None, + coverage_hir_info, ) } @@ -1056,6 +1064,7 @@ pub(crate) fn parse_float_into_scalar( mod block; mod cfg; +mod coverageinfo; mod custom; mod expr; mod matches; diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 3d65ba55f7e11..465dfe8eafb14 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -14,17 +14,14 @@ use self::spans::CoverageSpans; use crate::MirPass; use rustc_data_structures::sync::Lrc; -use rustc_middle::hir; -use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::coverage::*; use rustc_middle::mir::{ self, BasicBlock, BasicBlockData, Coverage, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, }; use rustc_middle::ty::TyCtxt; -use rustc_span::def_id::LocalDefId; use rustc_span::source_map::SourceMap; -use rustc_span::{ExpnKind, SourceFile, Span, Symbol}; +use rustc_span::{SourceFile, Span, Symbol}; /// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected /// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen @@ -43,8 +40,10 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage { // be transformed, so it should never see promoted MIR. assert!(mir_source.promoted.is_none()); - let def_id = mir_source.def_id().expect_local(); - if !is_eligible_for_coverage(tcx, def_id) { + let def_id = mir_source.def_id(); + if mir_body.coverage_hir_info.is_none() { + // If we didn't capture HIR info during MIR building, this MIR + // wasn't eligible for coverage instrumentation, so skip it. trace!("InstrumentCoverage skipped for {def_id:?} (not eligible)"); return; } @@ -79,8 +78,10 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { let source_map = tcx.sess.source_map(); let def_id = mir_body.source.def_id().expect_local(); - let mir::coverage::HirInfo { function_source_hash, fn_sig_span, body_span, .. } = - make_coverage_hir_info(tcx, def_id); + let &mir::coverage::HirInfo { function_source_hash, fn_sig_span, body_span, .. } = mir_body + .coverage_hir_info + .as_deref() + .expect("functions without HIR info have already been skipped"); let source_file = source_map.lookup_source_file(body_span.lo()); @@ -290,103 +291,3 @@ fn make_code_region( end_col: end_col as u32, } } - -fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { - let is_fn_like = tcx.hir().get_by_def_id(def_id).fn_kind().is_some(); - - // Only instrument functions, methods, and closures (not constants since they are evaluated - // at compile time by Miri). - // FIXME(#73156): Handle source code coverage in const eval, but note, if and when const - // expressions get coverage spans, we will probably have to "carve out" space for const - // expressions from coverage spans in enclosing MIR's, like we do for closures. (That might - // be tricky if const expressions have no corresponding statements in the enclosing MIR. - // Closures are carved out by their initial `Assign` statement.) - if !is_fn_like { - return false; - } - - let codegen_fn_attrs = tcx.codegen_fn_attrs(def_id); - if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) { - return false; - } - - true -} - -fn make_coverage_hir_info(tcx: TyCtxt<'_>, def_id: LocalDefId) -> mir::coverage::HirInfo { - let (maybe_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id); - - let function_source_hash = hash_mir_source(tcx, hir_body); - let body_span = get_body_span(tcx, hir_body, def_id); - - let spans_are_compatible = { - let source_map = tcx.sess.source_map(); - |a: Span, b: Span| { - a.eq_ctxt(b) - && source_map.lookup_source_file_idx(a.lo()) - == source_map.lookup_source_file_idx(b.lo()) - } - }; - - let fn_sig_span = if let Some(fn_sig) = maybe_fn_sig - && spans_are_compatible(fn_sig.span, body_span) - && fn_sig.span.lo() <= body_span.lo() - { - fn_sig.span.with_hi(body_span.lo()) - } else { - body_span.shrink_to_lo() - }; - - mir::coverage::HirInfo { function_source_hash, fn_sig_span, body_span } -} - -fn fn_sig_and_body( - tcx: TyCtxt<'_>, - def_id: LocalDefId, -) -> (Option<&rustc_hir::FnSig<'_>>, &rustc_hir::Body<'_>) { - // FIXME(#79625): Consider improving MIR to provide the information needed, to avoid going back - // to HIR for it. - let hir_node = tcx.hir().get_by_def_id(def_id); - let (_, fn_body_id) = - hir::map::associated_body(hir_node).expect("HIR node is a function with body"); - (hir_node.fn_sig(), tcx.hir().body(fn_body_id)) -} - -fn get_body_span<'tcx>( - tcx: TyCtxt<'tcx>, - hir_body: &rustc_hir::Body<'tcx>, - def_id: LocalDefId, -) -> Span { - let mut body_span = hir_body.value.span; - - if tcx.is_closure(def_id.to_def_id()) { - // If the MIR function is a closure, and if the closure body span - // starts from a macro, but it's content is not in that macro, try - // to find a non-macro callsite, and instrument the spans there - // instead. - loop { - let expn_data = body_span.ctxt().outer_expn_data(); - if expn_data.is_root() { - break; - } - if let ExpnKind::Macro { .. } = expn_data.kind { - body_span = expn_data.call_site; - } else { - break; - } - } - } - - body_span -} - -fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { - // FIXME(cjgillot) Stop hashing HIR manually here. - let owner = hir_body.id().hir_id.owner; - tcx.hir_owner_nodes(owner) - .unwrap() - .opt_hash_including_bodies - .unwrap() - .to_smaller_hash() - .as_u64() -} diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index fba73d5195b76..0781a347c777d 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -283,6 +283,7 @@ fn new_body<'tcx>( None, // FIXME(compiler-errors): is this correct? None, + None, ) } From 8b62d93859ffe47dc476b5a90200f63bb823b302 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 24 Nov 2023 17:10:01 +1100 Subject: [PATCH 7/8] coverage: Remove some FIXMEs that are now fixed --- compiler/rustc_mir_build/src/build/coverageinfo.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index bbf41e72df959..63d1571e2afb4 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -69,8 +69,6 @@ fn fn_sig_and_body( tcx: TyCtxt<'_>, def_id: LocalDefId, ) -> (Option<&rustc_hir::FnSig<'_>>, &rustc_hir::Body<'_>) { - // FIXME(#79625): Consider improving MIR to provide the information needed, to avoid going back - // to HIR for it. let hir_node = tcx.hir().get_by_def_id(def_id); let (_, fn_body_id) = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); @@ -106,7 +104,6 @@ fn get_body_span<'tcx>( } fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { - // FIXME(cjgillot) Stop hashing HIR manually here. let owner = hir_body.id().hir_id.owner; tcx.hir_owner_nodes(owner) .unwrap() From 2106aaaa181170e1320d75e2437818eebda927ef Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 4 Dec 2023 12:35:14 +1100 Subject: [PATCH 8/8] coverage: Don't use HIR to determine whether a def is fn-like --- compiler/rustc_mir_build/src/build/coverageinfo.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 63d1571e2afb4..fe7763e5ef2b8 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -17,8 +17,6 @@ pub(crate) fn make_coverage_hir_info_if_eligible( } fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { - let is_fn_like = tcx.hir().get_by_def_id(def_id).fn_kind().is_some(); - // Only instrument functions, methods, and closures (not constants since they are evaluated // at compile time by Miri). // FIXME(#73156): Handle source code coverage in const eval, but note, if and when const @@ -26,7 +24,7 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { // expressions from coverage spans in enclosing MIR's, like we do for closures. (That might // be tricky if const expressions have no corresponding statements in the enclosing MIR. // Closures are carved out by their initial `Assign` statement.) - if !is_fn_like { + if !tcx.def_kind(def_id).is_fn_like() { return false; }