From 457fda1701e191d9ff439d9d01ee650e6bbefee6 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 20 Jun 2024 10:49:40 +1000 Subject: [PATCH] coverage: Detach `#[coverage(..)]` from codegen attribute handling --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 17 ------------ .../src/middle/codegen_fn_attrs.rs | 5 +--- compiler/rustc_middle/src/query/mod.rs | 8 ++++++ .../rustc_mir_transform/src/coverage/query.rs | 27 ++++++++++++++++++- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index fb71cdaa8ff..d224695d1f2 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -124,22 +124,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { .emit(); } } - sym::coverage => { - let inner = attr.meta_item_list(); - match inner.as_deref() { - Some([item]) if item.has_name(sym::off) => { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE; - } - Some([item]) if item.has_name(sym::on) => { - // Allow #[coverage(on)] for being explicit, maybe also in future to enable - // coverage on a smaller scope within an excluded larger scope. - } - Some(_) | None => { - tcx.dcx() - .span_delayed_bug(attr.span, "unexpected value of coverage attribute"); - } - } - } sym::rustc_std_internal_symbol => { codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL } @@ -584,7 +568,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE; codegen_fn_attrs.inline = InlineAttr::Never; } diff --git a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs index 3fa5054baed..c8f0d0795a3 100644 --- a/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs +++ b/compiler/rustc_middle/src/middle/codegen_fn_attrs.rs @@ -87,10 +87,7 @@ impl CodegenFnAttrFlags: u32 { /// #[cmse_nonsecure_entry]: with a TrustZone-M extension, declare a /// function as an entry function from Non-Secure code. const CMSE_NONSECURE_ENTRY = 1 << 13; - /// `#[coverage(off)]`: indicates that the function should be ignored by - /// the MIR `InstrumentCoverage` pass and not added to the coverage map - /// during codegen. - const NO_COVERAGE = 1 << 14; + // (Bit 14 was used for `#[coverage(off)]`, but is now unused.) /// `#[used(linker)]`: /// indicates that neither LLVM nor the linker will eliminate this function. const USED_LINKER = 1 << 15; diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index c5afecffb07..320e21e85da 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -572,6 +572,14 @@ separate_provide_extern } + /// Checks for `#[coverage(off)]` or `#[coverage(on)]`. + /// + /// Returns `false` if `#[coverage(off)]` was found, or `true` if + /// either `#[coverage(on)]` or no coverage attribute was found. + query coverage_attr_on(key: LocalDefId) -> bool { + desc { |tcx| "checking for `#[coverage(..)]` on `{}`", tcx.def_path_str(key) } + } + /// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass /// (for compiler option `-Cinstrument-coverage`), after MIR optimizations /// have had a chance to potentially remove some of them. diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 25744009be8..14dcb7ef424 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -6,11 +6,13 @@ use rustc_middle::ty::{self, TyCtxt}; use rustc_middle::util::Providers; use rustc_span::def_id::LocalDefId; +use rustc_span::sym; /// Registers query/hook implementations related to coverage. pub(crate) fn provide(providers: &mut Providers) { providers.hooks.is_eligible_for_coverage = |TyCtxtAt { tcx, .. }, def_id| is_eligible_for_coverage(tcx, def_id); + providers.queries.coverage_attr_on = coverage_attr_on; providers.queries.coverage_ids_info = coverage_ids_info; } @@ -38,7 +40,12 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { return false; } - if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NO_COVERAGE) { + if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NAKED) { + trace!("InstrumentCoverage skipped for {def_id:?} (`#[naked]`)"); + return false; + } + + if !tcx.coverage_attr_on(def_id) { trace!("InstrumentCoverage skipped for {def_id:?} (`#[coverage(off)]`)"); return false; } @@ -46,6 +53,24 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { true } +/// Query implementation for `coverage_attr_on`. +fn coverage_attr_on(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { + if let Some(attr) = tcx.get_attr(def_id, sym::coverage) { + match attr.meta_item_list().as_deref() { + Some([item]) if item.has_name(sym::off) => return false, + Some([item]) if item.has_name(sym::on) => return true, + Some(_) | None => { + // Other possibilities should have been rejected by `rustc_parse::validate_attr`. + tcx.dcx().span_bug(attr.span, "unexpected value of coverage attribute"); + } + } + } + + // We didn't see an explicit coverage attribute, so + // allow coverage instrumentation by default. + true +} + /// Query implementation for `coverage_ids_info`. fn coverage_ids_info<'tcx>( tcx: TyCtxt<'tcx>,