From 7c4ac71ad1bef9b1e4cb5a3fd10d0b2ef7b418c4 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 11 Dec 2024 14:28:55 +1100 Subject: coverage: Extract function metadata handling to a `covfun` submodule --- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 174 +----------------- .../src/coverageinfo/mapgen/covfun.rs | 198 +++++++++++++++++++++ 2 files changed, 206 insertions(+), 166 deletions(-) create mode 100644 compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index a6c3caf9e2b..5da7848b39c 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -1,4 +1,3 @@ -use std::ffi::CString; use std::iter; use itertools::Itertools as _; @@ -9,21 +8,21 @@ use rustc_codegen_ssa::traits::{ use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_index::IndexVec; -use rustc_middle::mir::coverage::MappingKind; use rustc_middle::ty::{self, TyCtxt}; use rustc_middle::{bug, mir}; use rustc_session::RemapFileNameExt; use rustc_session::config::RemapPathScopeComponents; use rustc_span::def_id::DefIdSet; use rustc_span::{Span, Symbol}; -use rustc_target::spec::HasTargetSpec; use tracing::debug; use crate::common::CodegenCx; +use crate::coverageinfo::llvm_cov; use crate::coverageinfo::map_data::FunctionCoverage; -use crate::coverageinfo::{ffi, llvm_cov}; use crate::llvm; +mod covfun; + /// Generates and exports the coverage map, which is embedded in special /// linker sections in the final binary. /// @@ -88,38 +87,13 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { // Encode coverage mappings and generate function records for (instance, function_coverage) in function_coverage_map { - debug!("Generate function coverage for {}, {:?}", cx.codegen_unit.name(), instance); - - let mangled_function_name = tcx.symbol_name(instance).name; - let source_hash = function_coverage.source_hash(); - let is_used = function_coverage.is_used(); - - let coverage_mapping_buffer = - encode_mappings_for_function(tcx, &global_file_table, &function_coverage); - - if coverage_mapping_buffer.is_empty() { - if function_coverage.is_used() { - bug!( - "A used function should have had coverage mapping data but did not: {}", - mangled_function_name - ); - } else { - debug!("unused function had no coverage mapping data: {}", mangled_function_name); - continue; - } - } - - if !is_used { - unused_function_names.push(mangled_function_name); - } - - generate_covfun_record( + covfun::prepare_and_generate_covfun_record( cx, - mangled_function_name, - source_hash, + &global_file_table, filenames_ref, - coverage_mapping_buffer, - is_used, + &mut unused_function_names, + instance, + &function_coverage, ); } @@ -236,83 +210,6 @@ fn span_file_name(tcx: TyCtxt<'_>, span: Span) -> Symbol { Symbol::intern(&name) } -/// Using the expressions and counter regions collected for a single function, -/// generate the variable-sized payload of its corresponding `__llvm_covfun` -/// entry. The payload is returned as a vector of bytes. -/// -/// Newly-encountered filenames will be added to the global file table. -fn encode_mappings_for_function( - tcx: TyCtxt<'_>, - global_file_table: &GlobalFileTable, - function_coverage: &FunctionCoverage<'_>, -) -> Vec { - let counter_regions = function_coverage.counter_regions(); - if counter_regions.is_empty() { - return Vec::new(); - } - - let expressions = function_coverage.counter_expressions().collect::>(); - - let mut virtual_file_mapping = VirtualFileMapping::default(); - let mut code_regions = vec![]; - let mut branch_regions = vec![]; - let mut mcdc_branch_regions = vec![]; - let mut mcdc_decision_regions = vec![]; - - // Currently a function's mappings must all be in the same file as its body span. - let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span); - - // Look up the global file ID for that filename. - let global_file_id = global_file_table.global_file_id_for_file_name(file_name); - - // Associate that global file ID with a local file ID for this function. - let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id); - debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'"); - - // For each counter/region pair in this function+file, convert it to a - // form suitable for FFI. - for (mapping_kind, region) in counter_regions { - debug!("Adding counter {mapping_kind:?} to map for {region:?}"); - let span = ffi::CoverageSpan::from_source_region(local_file_id, region); - match mapping_kind { - MappingKind::Code(term) => { - code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) }); - } - MappingKind::Branch { true_term, false_term } => { - branch_regions.push(ffi::BranchRegion { - span, - true_counter: ffi::Counter::from_term(true_term), - false_counter: ffi::Counter::from_term(false_term), - }); - } - MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => { - mcdc_branch_regions.push(ffi::MCDCBranchRegion { - span, - true_counter: ffi::Counter::from_term(true_term), - false_counter: ffi::Counter::from_term(false_term), - mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params), - }); - } - MappingKind::MCDCDecision(mcdc_decision_params) => { - mcdc_decision_regions.push(ffi::MCDCDecisionRegion { - span, - mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params), - }); - } - } - } - - // Encode the function's coverage mappings into a buffer. - llvm_cov::write_function_mappings_to_buffer( - &virtual_file_mapping.into_vec(), - &expressions, - &code_regions, - &branch_regions, - &mcdc_branch_regions, - &mcdc_decision_regions, - ) -} - /// Generates the contents of the covmap record for this CGU, which mostly /// consists of a header and a list of filenames. The record is then stored /// as a global variable in the `__llvm_covmap` section. @@ -350,61 +247,6 @@ fn generate_covmap_record<'ll>( cx.add_used_global(llglobal); } -/// Generates the contents of the covfun record for this function, which -/// contains the function's coverage mapping data. The record is then stored -/// as a global variable in the `__llvm_covfun` section. -fn generate_covfun_record( - cx: &CodegenCx<'_, '_>, - mangled_function_name: &str, - source_hash: u64, - filenames_ref: u64, - coverage_mapping_buffer: Vec, - is_used: bool, -) { - // Concatenate the encoded coverage mappings - let coverage_mapping_size = coverage_mapping_buffer.len(); - let coverage_mapping_val = cx.const_bytes(&coverage_mapping_buffer); - - let func_name_hash = llvm_cov::hash_bytes(mangled_function_name.as_bytes()); - let func_name_hash_val = cx.const_u64(func_name_hash); - let coverage_mapping_size_val = cx.const_u32(coverage_mapping_size as u32); - let source_hash_val = cx.const_u64(source_hash); - let filenames_ref_val = cx.const_u64(filenames_ref); - let func_record_val = cx.const_struct( - &[ - func_name_hash_val, - coverage_mapping_size_val, - source_hash_val, - filenames_ref_val, - coverage_mapping_val, - ], - /*packed=*/ true, - ); - - // Choose a variable name to hold this function's covfun data. - // Functions that are used have a suffix ("u") to distinguish them from - // unused copies of the same function (from different CGUs), so that if a - // linker sees both it won't discard the used copy's data. - let func_record_var_name = - CString::new(format!("__covrec_{:X}{}", func_name_hash, if is_used { "u" } else { "" })) - .unwrap(); - debug!("function record var name: {:?}", func_record_var_name); - - let llglobal = llvm::add_global(cx.llmod, cx.val_ty(func_record_val), &func_record_var_name); - llvm::set_initializer(llglobal, func_record_val); - llvm::set_global_constant(llglobal, true); - llvm::set_linkage(llglobal, llvm::Linkage::LinkOnceODRLinkage); - llvm::set_visibility(llglobal, llvm::Visibility::Hidden); - llvm::set_section(llglobal, cx.covfun_section_name()); - // LLVM's coverage mapping format specifies 8-byte alignment for items in this section. - // - llvm::set_alignment(llglobal, Align::EIGHT); - if cx.target_spec().supports_comdat() { - llvm::set_comdat(cx.llmod, llglobal, &func_record_var_name); - } - cx.add_used_global(llglobal); -} - /// Each CGU will normally only emit coverage metadata for the functions that it actually generates. /// But since we don't want unused functions to disappear from coverage reports, we also scan for /// functions that were instrumented but are not participating in codegen. diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs new file mode 100644 index 00000000000..aaa6bbcdfc9 --- /dev/null +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -0,0 +1,198 @@ +//! For each function that was instrumented for coverage, we need to embed its +//! corresponding coverage mapping metadata inside the `__llvm_covfun`[^win] +//! linker section of the final binary. +//! +//! [^win]: On Windows the section name is `.lcovfun`. + +use std::ffi::CString; + +use rustc_abi::Align; +use rustc_codegen_ssa::traits::{ + BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods, +}; +use rustc_middle::bug; +use rustc_middle::mir::coverage::MappingKind; +use rustc_middle::ty::{Instance, TyCtxt}; +use rustc_target::spec::HasTargetSpec; +use tracing::debug; + +use crate::common::CodegenCx; +use crate::coverageinfo::map_data::FunctionCoverage; +use crate::coverageinfo::mapgen::{GlobalFileTable, VirtualFileMapping, span_file_name}; +use crate::coverageinfo::{ffi, llvm_cov}; +use crate::llvm; + +pub(crate) fn prepare_and_generate_covfun_record<'ll, 'tcx>( + cx: &CodegenCx<'ll, 'tcx>, + global_file_table: &GlobalFileTable, + filenames_ref: u64, + unused_function_names: &mut Vec<&'tcx str>, + instance: Instance<'tcx>, + function_coverage: &FunctionCoverage<'tcx>, +) { + let tcx = cx.tcx; + + let mangled_function_name = tcx.symbol_name(instance).name; + let source_hash = function_coverage.source_hash(); + let is_used = function_coverage.is_used(); + + let coverage_mapping_buffer = + encode_mappings_for_function(tcx, global_file_table, function_coverage); + + if coverage_mapping_buffer.is_empty() { + if function_coverage.is_used() { + bug!( + "A used function should have had coverage mapping data but did not: {}", + mangled_function_name + ); + } else { + debug!("unused function had no coverage mapping data: {}", mangled_function_name); + return; + } + } + + if !is_used { + unused_function_names.push(mangled_function_name); + } + + generate_covfun_record( + cx, + mangled_function_name, + source_hash, + filenames_ref, + coverage_mapping_buffer, + is_used, + ); +} + +/// Using the expressions and counter regions collected for a single function, +/// generate the variable-sized payload of its corresponding `__llvm_covfun` +/// entry. The payload is returned as a vector of bytes. +/// +/// Newly-encountered filenames will be added to the global file table. +fn encode_mappings_for_function( + tcx: TyCtxt<'_>, + global_file_table: &GlobalFileTable, + function_coverage: &FunctionCoverage<'_>, +) -> Vec { + let counter_regions = function_coverage.counter_regions(); + if counter_regions.is_empty() { + return Vec::new(); + } + + let expressions = function_coverage.counter_expressions().collect::>(); + + let mut virtual_file_mapping = VirtualFileMapping::default(); + let mut code_regions = vec![]; + let mut branch_regions = vec![]; + let mut mcdc_branch_regions = vec![]; + let mut mcdc_decision_regions = vec![]; + + // Currently a function's mappings must all be in the same file as its body span. + let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span); + + // Look up the global file ID for that filename. + let global_file_id = global_file_table.global_file_id_for_file_name(file_name); + + // Associate that global file ID with a local file ID for this function. + let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id); + debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'"); + + // For each counter/region pair in this function+file, convert it to a + // form suitable for FFI. + for (mapping_kind, region) in counter_regions { + debug!("Adding counter {mapping_kind:?} to map for {region:?}"); + let span = ffi::CoverageSpan::from_source_region(local_file_id, region); + match mapping_kind { + MappingKind::Code(term) => { + code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) }); + } + MappingKind::Branch { true_term, false_term } => { + branch_regions.push(ffi::BranchRegion { + span, + true_counter: ffi::Counter::from_term(true_term), + false_counter: ffi::Counter::from_term(false_term), + }); + } + MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => { + mcdc_branch_regions.push(ffi::MCDCBranchRegion { + span, + true_counter: ffi::Counter::from_term(true_term), + false_counter: ffi::Counter::from_term(false_term), + mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params), + }); + } + MappingKind::MCDCDecision(mcdc_decision_params) => { + mcdc_decision_regions.push(ffi::MCDCDecisionRegion { + span, + mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params), + }); + } + } + } + + // Encode the function's coverage mappings into a buffer. + llvm_cov::write_function_mappings_to_buffer( + &virtual_file_mapping.into_vec(), + &expressions, + &code_regions, + &branch_regions, + &mcdc_branch_regions, + &mcdc_decision_regions, + ) +} + +/// Generates the contents of the covfun record for this function, which +/// contains the function's coverage mapping data. The record is then stored +/// as a global variable in the `__llvm_covfun` section. +fn generate_covfun_record( + cx: &CodegenCx<'_, '_>, + mangled_function_name: &str, + source_hash: u64, + filenames_ref: u64, + coverage_mapping_buffer: Vec, + is_used: bool, +) { + // Concatenate the encoded coverage mappings + let coverage_mapping_size = coverage_mapping_buffer.len(); + let coverage_mapping_val = cx.const_bytes(&coverage_mapping_buffer); + + let func_name_hash = llvm_cov::hash_bytes(mangled_function_name.as_bytes()); + let func_name_hash_val = cx.const_u64(func_name_hash); + let coverage_mapping_size_val = cx.const_u32(coverage_mapping_size as u32); + let source_hash_val = cx.const_u64(source_hash); + let filenames_ref_val = cx.const_u64(filenames_ref); + let func_record_val = cx.const_struct( + &[ + func_name_hash_val, + coverage_mapping_size_val, + source_hash_val, + filenames_ref_val, + coverage_mapping_val, + ], + /*packed=*/ true, + ); + + // Choose a variable name to hold this function's covfun data. + // Functions that are used have a suffix ("u") to distinguish them from + // unused copies of the same function (from different CGUs), so that if a + // linker sees both it won't discard the used copy's data. + let func_record_var_name = + CString::new(format!("__covrec_{:X}{}", func_name_hash, if is_used { "u" } else { "" })) + .unwrap(); + debug!("function record var name: {:?}", func_record_var_name); + + let llglobal = llvm::add_global(cx.llmod, cx.val_ty(func_record_val), &func_record_var_name); + llvm::set_initializer(llglobal, func_record_val); + llvm::set_global_constant(llglobal, true); + llvm::set_linkage(llglobal, llvm::Linkage::LinkOnceODRLinkage); + llvm::set_visibility(llglobal, llvm::Visibility::Hidden); + llvm::set_section(llglobal, cx.covfun_section_name()); + // LLVM's coverage mapping format specifies 8-byte alignment for items in this section. + // + llvm::set_alignment(llglobal, Align::EIGHT); + if cx.target_spec().supports_comdat() { + llvm::set_comdat(cx.llmod, llglobal, &func_record_var_name); + } + cx.add_used_global(llglobal); +} -- cgit 1.4.1-3-g733a5 From 6a8c016266a6b7514ff8284dc6d8b056e34b9399 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 11 Dec 2024 15:03:31 +1100 Subject: coverage: Reify `CovfunRecord` as an intermediate step --- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 22 ++++---- .../src/coverageinfo/mapgen/covfun.rs | 59 ++++++++++++---------- 2 files changed, 45 insertions(+), 36 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 5da7848b39c..39fb0aeed07 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -19,6 +19,7 @@ use tracing::debug; use crate::common::CodegenCx; use crate::coverageinfo::llvm_cov; use crate::coverageinfo::map_data::FunctionCoverage; +use crate::coverageinfo::mapgen::covfun::prepare_covfun_record; use crate::llvm; mod covfun; @@ -85,16 +86,17 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { let mut unused_function_names = Vec::new(); - // Encode coverage mappings and generate function records - for (instance, function_coverage) in function_coverage_map { - covfun::prepare_and_generate_covfun_record( - cx, - &global_file_table, - filenames_ref, - &mut unused_function_names, - instance, - &function_coverage, - ); + let covfun_records = function_coverage_map + .into_iter() + .filter_map(|(instance, function_coverage)| { + prepare_covfun_record(tcx, &global_file_table, instance, &function_coverage) + }) + .collect::>(); + + for covfun in &covfun_records { + unused_function_names.extend(covfun.mangled_function_name_if_unused()); + + covfun::generate_covfun_record(cx, filenames_ref, covfun) } // For unused functions, we need to take their mangled names and store them diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index aaa6bbcdfc9..a68b026a4c0 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -22,16 +22,30 @@ use crate::coverageinfo::mapgen::{GlobalFileTable, VirtualFileMapping, span_file use crate::coverageinfo::{ffi, llvm_cov}; use crate::llvm; -pub(crate) fn prepare_and_generate_covfun_record<'ll, 'tcx>( - cx: &CodegenCx<'ll, 'tcx>, +/// Intermediate coverage metadata for a single function, used to help build +/// the final record that will be embedded in the `__llvm_covfun` section. +#[derive(Debug)] +pub(crate) struct CovfunRecord<'tcx> { + mangled_function_name: &'tcx str, + source_hash: u64, + is_used: bool, + coverage_mapping_buffer: Vec, +} + +impl<'tcx> CovfunRecord<'tcx> { + /// FIXME(Zalathar): Make this the responsibility of the code that determines + /// which functions are unused. + pub(crate) fn mangled_function_name_if_unused(&self) -> Option<&'tcx str> { + (!self.is_used).then_some(self.mangled_function_name) + } +} + +pub(crate) fn prepare_covfun_record<'tcx>( + tcx: TyCtxt<'tcx>, global_file_table: &GlobalFileTable, - filenames_ref: u64, - unused_function_names: &mut Vec<&'tcx str>, instance: Instance<'tcx>, function_coverage: &FunctionCoverage<'tcx>, -) { - let tcx = cx.tcx; - +) -> Option> { let mangled_function_name = tcx.symbol_name(instance).name; let source_hash = function_coverage.source_hash(); let is_used = function_coverage.is_used(); @@ -47,22 +61,11 @@ pub(crate) fn prepare_and_generate_covfun_record<'ll, 'tcx>( ); } else { debug!("unused function had no coverage mapping data: {}", mangled_function_name); - return; + return None; } } - if !is_used { - unused_function_names.push(mangled_function_name); - } - - generate_covfun_record( - cx, - mangled_function_name, - source_hash, - filenames_ref, - coverage_mapping_buffer, - is_used, - ); + Some(CovfunRecord { mangled_function_name, source_hash, is_used, coverage_mapping_buffer }) } /// Using the expressions and counter regions collected for a single function, @@ -145,14 +148,18 @@ fn encode_mappings_for_function( /// Generates the contents of the covfun record for this function, which /// contains the function's coverage mapping data. The record is then stored /// as a global variable in the `__llvm_covfun` section. -fn generate_covfun_record( - cx: &CodegenCx<'_, '_>, - mangled_function_name: &str, - source_hash: u64, +pub(crate) fn generate_covfun_record<'tcx>( + cx: &CodegenCx<'_, 'tcx>, filenames_ref: u64, - coverage_mapping_buffer: Vec, - is_used: bool, + covfun: &CovfunRecord<'tcx>, ) { + let &CovfunRecord { + mangled_function_name, + source_hash, + is_used, + ref coverage_mapping_buffer, // Previously-encoded coverage mappings + } = covfun; + // Concatenate the encoded coverage mappings let coverage_mapping_size = coverage_mapping_buffer.len(); let coverage_mapping_val = cx.const_bytes(&coverage_mapping_buffer); -- cgit 1.4.1-3-g733a5 From 512f3fdebe72532a435238435f0e16eff61fbf38 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 11 Dec 2024 15:12:23 +1100 Subject: coverage: Only generate a CGU's covmap record if it has covfun records --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 39fb0aeed07..f7c3193a449 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -80,10 +80,6 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { let filenames_val = cx.const_bytes(&filenames_buffer); let filenames_ref = llvm_cov::hash_bytes(&filenames_buffer); - // Generate the coverage map header, which contains the filenames used by - // this CGU's coverage mappings, and store it in a well-known global. - generate_covmap_record(cx, covmap_version, filenames_size, filenames_val); - let mut unused_function_names = Vec::new(); let covfun_records = function_coverage_map @@ -93,6 +89,15 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { }) .collect::>(); + // If there are no covfun records for this CGU, don't generate a covmap record. + // Emitting a covmap record without any covfun records causes `llvm-cov` to + // fail when generating coverage reports, and if there are no covfun records + // then the covmap record isn't useful anyway. + // This should prevent a repeat of . + if covfun_records.is_empty() { + return; + } + for covfun in &covfun_records { unused_function_names.extend(covfun.mangled_function_name_if_unused()); @@ -117,6 +122,11 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { llvm::set_linkage(array, llvm::Linkage::InternalLinkage); llvm::set_initializer(array, initializer); } + + // Generate the coverage map header, which contains the filenames used by + // this CGU's coverage mappings, and store it in a well-known global. + // (This is skipped if we returned early due to having no covfun records.) + generate_covmap_record(cx, covmap_version, filenames_size, filenames_val); } /// Maps "global" (per-CGU) file ID numbers to their underlying filenames. -- cgit 1.4.1-3-g733a5 From 3f3a9bf7f50afbfde72cd0b9479323dddc84fe6d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 11 Dec 2024 15:41:02 +1100 Subject: coverage: Store intermediate region tables in `CovfunRecord` This defers the call to `llvm_cov::write_function_mappings_to_buffer` until just before its enclosing global variable is created. --- .../rustc_codegen_llvm/src/coverageinfo/ffi.rs | 28 +++++++ .../src/coverageinfo/llvm_cov.rs | 7 +- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 10 +-- .../src/coverageinfo/mapgen/covfun.rs | 87 ++++++++++------------ 4 files changed, 77 insertions(+), 55 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index a6e07ea2a60..1f133141c18 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -152,6 +152,34 @@ impl CoverageSpan { } } +/// Holds tables of the various region types in one struct. +/// +/// Don't pass this struct across FFI; pass the individual region tables as +/// pointer/length pairs instead. +/// +/// Each field name has a `_regions` suffix for improved readability after +/// exhaustive destructing, which ensures that all region types are handled. +#[derive(Clone, Debug, Default)] +pub(crate) struct Regions { + pub(crate) code_regions: Vec, + pub(crate) branch_regions: Vec, + pub(crate) mcdc_branch_regions: Vec, + pub(crate) mcdc_decision_regions: Vec, +} + +impl Regions { + /// Returns true if none of this structure's tables contain any regions. + pub(crate) fn has_no_regions(&self) -> bool { + let Self { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } = + self; + + code_regions.is_empty() + && branch_regions.is_empty() + && mcdc_branch_regions.is_empty() + && mcdc_decision_regions.is_empty() + } +} + /// Must match the layout of `LLVMRustCoverageCodeRegion`. #[derive(Clone, Debug)] #[repr(C)] diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs index 99c2d12b261..086cf1f44a0 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs @@ -62,11 +62,10 @@ pub(crate) fn write_filenames_to_buffer<'a>( pub(crate) fn write_function_mappings_to_buffer( virtual_file_mapping: &[u32], expressions: &[ffi::CounterExpression], - code_regions: &[ffi::CodeRegion], - branch_regions: &[ffi::BranchRegion], - mcdc_branch_regions: &[ffi::MCDCBranchRegion], - mcdc_decision_regions: &[ffi::MCDCDecisionRegion], + regions: &ffi::Regions, ) -> Vec { + let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } = + regions; llvm::build_byte_buffer(|buffer| unsafe { llvm::LLVMRustCoverageWriteFunctionMappingsToBuffer( virtual_file_mapping.as_ptr(), diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index f7c3193a449..a573a37beb3 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -194,7 +194,7 @@ rustc_index::newtype_index! { /// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU) /// file IDs. -#[derive(Default)] +#[derive(Debug, Default)] struct VirtualFileMapping { local_to_global: IndexVec, global_to_local: FxIndexMap, @@ -208,10 +208,10 @@ impl VirtualFileMapping { .or_insert_with(|| self.local_to_global.push(global_file_id)) } - fn into_vec(self) -> Vec { - // This conversion should be optimized away to ~zero overhead. - // In any case, it's probably not hot enough to worry about. - self.local_to_global.into_iter().map(|global| global.as_u32()).collect() + fn to_vec(&self) -> Vec { + // This clone could be avoided by transmuting `&[GlobalFileId]` to `&[u32]`, + // but it isn't hot or expensive enough to justify the extra unsafety. + self.local_to_global.iter().map(|&global| GlobalFileId::as_u32(global)).collect() } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index a68b026a4c0..530e6827f55 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -29,7 +29,10 @@ pub(crate) struct CovfunRecord<'tcx> { mangled_function_name: &'tcx str, source_hash: u64, is_used: bool, - coverage_mapping_buffer: Vec, + + virtual_file_mapping: VirtualFileMapping, + expressions: Vec, + regions: ffi::Regions, } impl<'tcx> CovfunRecord<'tcx> { @@ -46,51 +49,41 @@ pub(crate) fn prepare_covfun_record<'tcx>( instance: Instance<'tcx>, function_coverage: &FunctionCoverage<'tcx>, ) -> Option> { - let mangled_function_name = tcx.symbol_name(instance).name; - let source_hash = function_coverage.source_hash(); - let is_used = function_coverage.is_used(); - - let coverage_mapping_buffer = - encode_mappings_for_function(tcx, global_file_table, function_coverage); - - if coverage_mapping_buffer.is_empty() { - if function_coverage.is_used() { - bug!( - "A used function should have had coverage mapping data but did not: {}", - mangled_function_name - ); + let mut covfun = CovfunRecord { + mangled_function_name: tcx.symbol_name(instance).name, + source_hash: function_coverage.source_hash(), + is_used: function_coverage.is_used(), + virtual_file_mapping: VirtualFileMapping::default(), + expressions: function_coverage.counter_expressions().collect::>(), + regions: ffi::Regions::default(), + }; + + fill_region_tables(tcx, global_file_table, function_coverage, &mut covfun); + + if covfun.regions.has_no_regions() { + if covfun.is_used { + bug!("a used function should have had coverage mapping data but did not: {covfun:?}"); } else { - debug!("unused function had no coverage mapping data: {}", mangled_function_name); + debug!(?covfun, "unused function had no coverage mapping data"); return None; } } - Some(CovfunRecord { mangled_function_name, source_hash, is_used, coverage_mapping_buffer }) + Some(covfun) } -/// Using the expressions and counter regions collected for a single function, -/// generate the variable-sized payload of its corresponding `__llvm_covfun` -/// entry. The payload is returned as a vector of bytes. -/// -/// Newly-encountered filenames will be added to the global file table. -fn encode_mappings_for_function( - tcx: TyCtxt<'_>, +/// Populates the mapping region tables in the current function's covfun record. +fn fill_region_tables<'tcx>( + tcx: TyCtxt<'tcx>, global_file_table: &GlobalFileTable, - function_coverage: &FunctionCoverage<'_>, -) -> Vec { + function_coverage: &FunctionCoverage<'tcx>, + covfun: &mut CovfunRecord<'tcx>, +) { let counter_regions = function_coverage.counter_regions(); if counter_regions.is_empty() { - return Vec::new(); + return; } - let expressions = function_coverage.counter_expressions().collect::>(); - - let mut virtual_file_mapping = VirtualFileMapping::default(); - let mut code_regions = vec![]; - let mut branch_regions = vec![]; - let mut mcdc_branch_regions = vec![]; - let mut mcdc_decision_regions = vec![]; - // Currently a function's mappings must all be in the same file as its body span. let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span); @@ -98,9 +91,12 @@ fn encode_mappings_for_function( let global_file_id = global_file_table.global_file_id_for_file_name(file_name); // Associate that global file ID with a local file ID for this function. - let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id); + let local_file_id = covfun.virtual_file_mapping.local_id_for_global(global_file_id); debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'"); + let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } = + &mut covfun.regions; + // For each counter/region pair in this function+file, convert it to a // form suitable for FFI. for (mapping_kind, region) in counter_regions { @@ -133,16 +129,6 @@ fn encode_mappings_for_function( } } } - - // Encode the function's coverage mappings into a buffer. - llvm_cov::write_function_mappings_to_buffer( - &virtual_file_mapping.into_vec(), - &expressions, - &code_regions, - &branch_regions, - &mcdc_branch_regions, - &mcdc_decision_regions, - ) } /// Generates the contents of the covfun record for this function, which @@ -157,9 +143,18 @@ pub(crate) fn generate_covfun_record<'tcx>( mangled_function_name, source_hash, is_used, - ref coverage_mapping_buffer, // Previously-encoded coverage mappings + ref virtual_file_mapping, + ref expressions, + ref regions, } = covfun; + // Encode the function's coverage mappings into a buffer. + let coverage_mapping_buffer = llvm_cov::write_function_mappings_to_buffer( + &virtual_file_mapping.to_vec(), + expressions, + regions, + ); + // Concatenate the encoded coverage mappings let coverage_mapping_size = coverage_mapping_buffer.len(); let coverage_mapping_val = cx.const_bytes(&coverage_mapping_buffer); -- cgit 1.4.1-3-g733a5 From de53fe245d7f937640bef68a4eb3622b43c39674 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 12 Dec 2024 18:33:33 +1100 Subject: coverage: Tidy up creation of covmap records --- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 60 ++++++++++------------ 1 file changed, 27 insertions(+), 33 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index a573a37beb3..7c7903ce842 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -75,9 +75,6 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { // Encode all filenames referenced by coverage mappings in this CGU. let filenames_buffer = global_file_table.make_filenames_buffer(tcx); - - let filenames_size = filenames_buffer.len(); - let filenames_val = cx.const_bytes(&filenames_buffer); let filenames_ref = llvm_cov::hash_bytes(&filenames_buffer); let mut unused_function_names = Vec::new(); @@ -126,7 +123,7 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { // Generate the coverage map header, which contains the filenames used by // this CGU's coverage mappings, and store it in a well-known global. // (This is skipped if we returned early due to having no covfun records.) - generate_covmap_record(cx, covmap_version, filenames_size, filenames_val); + generate_covmap_record(cx, covmap_version, &filenames_buffer); } /// Maps "global" (per-CGU) file ID numbers to their underlying filenames. @@ -225,38 +222,35 @@ fn span_file_name(tcx: TyCtxt<'_>, span: Span) -> Symbol { /// Generates the contents of the covmap record for this CGU, which mostly /// consists of a header and a list of filenames. The record is then stored /// as a global variable in the `__llvm_covmap` section. -fn generate_covmap_record<'ll>( - cx: &CodegenCx<'ll, '_>, - version: u32, - filenames_size: usize, - filenames_val: &'ll llvm::Value, -) { - debug!("cov map: filenames_size = {}, 0-based version = {}", filenames_size, version); - - // Create the coverage data header (Note, fields 0 and 2 are now always zero, - // as of `llvm::coverage::CovMapVersion::Version4`.) - let zero_was_n_records_val = cx.const_u32(0); - let filenames_size_val = cx.const_u32(filenames_size as u32); - let zero_was_coverage_size_val = cx.const_u32(0); - let version_val = cx.const_u32(version); - let cov_data_header_val = cx.const_struct( - &[zero_was_n_records_val, filenames_size_val, zero_was_coverage_size_val, version_val], - /*packed=*/ false, +fn generate_covmap_record<'ll>(cx: &CodegenCx<'ll, '_>, version: u32, filenames_buffer: &[u8]) { + // A covmap record consists of four target-endian u32 values, followed by + // the encoded filenames table. Two of the header fields are unused in + // modern versions of the LLVM coverage mapping format, and are always 0. + // + // See also `src/llvm-project/clang/lib/CodeGen/CoverageMappingGen.cpp`. + let covmap_header = cx.const_struct( + &[ + cx.const_u32(0), // (unused) + cx.const_u32(filenames_buffer.len() as u32), + cx.const_u32(0), // (unused) + cx.const_u32(version), + ], + /* packed */ false, ); - - // Create the complete LLVM coverage data value to add to the LLVM IR - let covmap_data = - cx.const_struct(&[cov_data_header_val, filenames_val], /*packed=*/ false); - - let llglobal = llvm::add_global(cx.llmod, cx.val_ty(covmap_data), &llvm_cov::covmap_var_name()); - llvm::set_initializer(llglobal, covmap_data); - llvm::set_global_constant(llglobal, true); - llvm::set_linkage(llglobal, llvm::Linkage::PrivateLinkage); - llvm::set_section(llglobal, &llvm_cov::covmap_section_name(cx.llmod)); + let covmap_record = cx + .const_struct(&[covmap_header, cx.const_bytes(filenames_buffer)], /* packed */ false); + + let covmap_global = + llvm::add_global(cx.llmod, cx.val_ty(covmap_record), &llvm_cov::covmap_var_name()); + llvm::set_initializer(covmap_global, covmap_record); + llvm::set_global_constant(covmap_global, true); + llvm::set_linkage(covmap_global, llvm::Linkage::PrivateLinkage); + llvm::set_section(covmap_global, &llvm_cov::covmap_section_name(cx.llmod)); // LLVM's coverage mapping format specifies 8-byte alignment for items in this section. // - llvm::set_alignment(llglobal, Align::EIGHT); - cx.add_used_global(llglobal); + llvm::set_alignment(covmap_global, Align::EIGHT); + + cx.add_used_global(covmap_global); } /// Each CGU will normally only emit coverage metadata for the functions that it actually generates. -- cgit 1.4.1-3-g733a5 From 5f5745beb06cac3f0d0cabf1d8edd1ffa53a9b55 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 12 Dec 2024 20:28:10 +1100 Subject: coverage: Tidy up creation of covfun records --- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 7 ++- .../src/coverageinfo/mapgen/covfun.rs | 59 +++++++++++----------- 2 files changed, 34 insertions(+), 32 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 7c7903ce842..4f2af732527 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -75,7 +75,10 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { // Encode all filenames referenced by coverage mappings in this CGU. let filenames_buffer = global_file_table.make_filenames_buffer(tcx); - let filenames_ref = llvm_cov::hash_bytes(&filenames_buffer); + // The `llvm-cov` tool uses this hash to associate each covfun record with + // its corresponding filenames table, since the final binary will typically + // contain multiple covmap records from different compilation units. + let filenames_hash = llvm_cov::hash_bytes(&filenames_buffer); let mut unused_function_names = Vec::new(); @@ -98,7 +101,7 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { for covfun in &covfun_records { unused_function_names.extend(covfun.mangled_function_name_if_unused()); - covfun::generate_covfun_record(cx, filenames_ref, covfun) + covfun::generate_covfun_record(cx, filenames_hash, covfun) } // For unused functions, we need to take their mangled names and store them diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index 530e6827f55..33e7a0f2f20 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -136,7 +136,7 @@ fn fill_region_tables<'tcx>( /// as a global variable in the `__llvm_covfun` section. pub(crate) fn generate_covfun_record<'tcx>( cx: &CodegenCx<'_, 'tcx>, - filenames_ref: u64, + filenames_hash: u64, covfun: &CovfunRecord<'tcx>, ) { let &CovfunRecord { @@ -155,46 +155,45 @@ pub(crate) fn generate_covfun_record<'tcx>( regions, ); - // Concatenate the encoded coverage mappings - let coverage_mapping_size = coverage_mapping_buffer.len(); - let coverage_mapping_val = cx.const_bytes(&coverage_mapping_buffer); - + // A covfun record consists of four target-endian integers, followed by the + // encoded mapping data in bytes. Note that the length field is 32 bits. + // + // See also `src/llvm-project/clang/lib/CodeGen/CoverageMappingGen.cpp` and + // `COVMAP_V3` in `src/llvm-project/llvm/include/llvm/ProfileData/InstrProfData.inc`. let func_name_hash = llvm_cov::hash_bytes(mangled_function_name.as_bytes()); - let func_name_hash_val = cx.const_u64(func_name_hash); - let coverage_mapping_size_val = cx.const_u32(coverage_mapping_size as u32); - let source_hash_val = cx.const_u64(source_hash); - let filenames_ref_val = cx.const_u64(filenames_ref); - let func_record_val = cx.const_struct( + let covfun_record = cx.const_struct( &[ - func_name_hash_val, - coverage_mapping_size_val, - source_hash_val, - filenames_ref_val, - coverage_mapping_val, + cx.const_u64(func_name_hash), + cx.const_u32(coverage_mapping_buffer.len() as u32), + cx.const_u64(source_hash), + cx.const_u64(filenames_hash), + cx.const_bytes(&coverage_mapping_buffer), ], - /*packed=*/ true, + // This struct needs to be packed, so that the 32-bit length field + // doesn't have unexpected padding. + true, ); // Choose a variable name to hold this function's covfun data. // Functions that are used have a suffix ("u") to distinguish them from // unused copies of the same function (from different CGUs), so that if a // linker sees both it won't discard the used copy's data. - let func_record_var_name = - CString::new(format!("__covrec_{:X}{}", func_name_hash, if is_used { "u" } else { "" })) - .unwrap(); - debug!("function record var name: {:?}", func_record_var_name); - - let llglobal = llvm::add_global(cx.llmod, cx.val_ty(func_record_val), &func_record_var_name); - llvm::set_initializer(llglobal, func_record_val); - llvm::set_global_constant(llglobal, true); - llvm::set_linkage(llglobal, llvm::Linkage::LinkOnceODRLinkage); - llvm::set_visibility(llglobal, llvm::Visibility::Hidden); - llvm::set_section(llglobal, cx.covfun_section_name()); + let u = if is_used { "u" } else { "" }; + let covfun_var_name = CString::new(format!("__covrec_{func_name_hash:X}{u}")).unwrap(); + debug!("function record var name: {covfun_var_name:?}"); + + let covfun_global = llvm::add_global(cx.llmod, cx.val_ty(covfun_record), &covfun_var_name); + llvm::set_initializer(covfun_global, covfun_record); + llvm::set_global_constant(covfun_global, true); + llvm::set_linkage(covfun_global, llvm::Linkage::LinkOnceODRLinkage); + llvm::set_visibility(covfun_global, llvm::Visibility::Hidden); + llvm::set_section(covfun_global, cx.covfun_section_name()); // LLVM's coverage mapping format specifies 8-byte alignment for items in this section. // - llvm::set_alignment(llglobal, Align::EIGHT); + llvm::set_alignment(covfun_global, Align::EIGHT); if cx.target_spec().supports_comdat() { - llvm::set_comdat(cx.llmod, llglobal, &func_record_var_name); + llvm::set_comdat(cx.llmod, covfun_global, &covfun_var_name); } - cx.add_used_global(llglobal); + + cx.add_used_global(covfun_global); } -- cgit 1.4.1-3-g733a5 From fe412af4fce43aa5222b8940e068da3a19188e4b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 15 Dec 2024 00:34:06 +1100 Subject: coverage: Use `is_eligible_for_coverage` to filter unused functions The checks in `is_eligible_for_coverage` include `is_fn_like`, but will also exclude various function-like things that cannot possibly have coverage instrumentation. --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 4f2af732527..c57946cc317 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -271,16 +271,15 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { let usage = prepare_usage_sets(tcx); let is_unused_fn = |def_id: LocalDefId| -> bool { - let def_id = def_id.to_def_id(); - - // To be eligible for "unused function" mappings, a definition must: - // - Be function-like + // Usage sets expect `DefId`, so convert from `LocalDefId`. + let d: DefId = LocalDefId::to_def_id(def_id); + // To be potentially eligible for "unused function" mappings, a definition must: + // - Be eligible for coverage instrumentation // - Not participate directly in codegen (or have lost all its coverage statements) // - Not have any coverage statements inlined into codegenned functions - tcx.def_kind(def_id).is_fn_like() - && (!usage.all_mono_items.contains(&def_id) - || usage.missing_own_coverage.contains(&def_id)) - && !usage.used_via_inlining.contains(&def_id) + tcx.is_eligible_for_coverage(def_id) + && (!usage.all_mono_items.contains(&d) || usage.missing_own_coverage.contains(&d)) + && !usage.used_via_inlining.contains(&d) }; // Scan for unused functions that were instrumented for coverage. -- cgit 1.4.1-3-g733a5 From 154fae1e8d825955ce83ae4c7d21a03db406e56c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 13 Dec 2024 22:40:45 +1100 Subject: coverage: Build the global file table on the fly --- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 58 ++++++++++------------ .../src/coverageinfo/mapgen/covfun.rs | 4 +- tests/coverage/unused_mod.cov-map | 8 +-- 3 files changed, 33 insertions(+), 37 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index c57946cc317..0c7ad300a8b 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -1,6 +1,6 @@ use std::iter; -use itertools::Itertools as _; +use itertools::Itertools; use rustc_abi::Align; use rustc_codegen_ssa::traits::{ BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods, @@ -8,8 +8,8 @@ use rustc_codegen_ssa::traits::{ use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_index::IndexVec; +use rustc_middle::mir; use rustc_middle::ty::{self, TyCtxt}; -use rustc_middle::{bug, mir}; use rustc_session::RemapFileNameExt; use rustc_session::config::RemapPathScopeComponents; use rustc_span::def_id::DefIdSet; @@ -67,25 +67,21 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { return; } - let all_file_names = function_coverage_map - .iter() - .map(|(_, fn_cov)| fn_cov.function_coverage_info.body_span) - .map(|span| span_file_name(tcx, span)); - let global_file_table = GlobalFileTable::new(all_file_names); - - // Encode all filenames referenced by coverage mappings in this CGU. - let filenames_buffer = global_file_table.make_filenames_buffer(tcx); - // The `llvm-cov` tool uses this hash to associate each covfun record with - // its corresponding filenames table, since the final binary will typically - // contain multiple covmap records from different compilation units. - let filenames_hash = llvm_cov::hash_bytes(&filenames_buffer); - - let mut unused_function_names = Vec::new(); + // The order of entries in this global file table needs to be deterministic, + // and ideally should also be independent of the details of stable-hashing, + // because coverage tests snapshots (`.cov-map`) can observe the order and + // would need to be re-blessed if it changes. As long as those requirements + // are satisfied, the order can be arbitrary. + let mut global_file_table = GlobalFileTable::new(); let covfun_records = function_coverage_map .into_iter() + // Sort by symbol name, so that the global file table is built in an + // order that doesn't depend on the stable-hash-based order in which + // instances were visited during codegen. + .sorted_by_cached_key(|&(instance, _)| tcx.symbol_name(instance).name) .filter_map(|(instance, function_coverage)| { - prepare_covfun_record(tcx, &global_file_table, instance, &function_coverage) + prepare_covfun_record(tcx, &mut global_file_table, instance, &function_coverage) }) .collect::>(); @@ -98,6 +94,15 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { return; } + // Encode all filenames referenced by coverage mappings in this CGU. + let filenames_buffer = global_file_table.make_filenames_buffer(tcx); + // The `llvm-cov` tool uses this hash to associate each covfun record with + // its corresponding filenames table, since the final binary will typically + // contain multiple covmap records from different compilation units. + let filenames_hash = llvm_cov::hash_bytes(&filenames_buffer); + + let mut unused_function_names = vec![]; + for covfun in &covfun_records { unused_function_names.extend(covfun.mangled_function_name_if_unused()); @@ -137,22 +142,13 @@ struct GlobalFileTable { } impl GlobalFileTable { - fn new(all_file_names: impl IntoIterator) -> Self { - // Collect all of the filenames into a set. Filenames usually come in - // contiguous runs, so we can dedup adjacent ones to save work. - let mut raw_file_table = all_file_names.into_iter().dedup().collect::>(); - - // Sort the file table by its actual string values, not the arbitrary - // ordering of its symbols. - raw_file_table.sort_unstable_by(|a, b| a.as_str().cmp(b.as_str())); - - Self { raw_file_table } + fn new() -> Self { + Self { raw_file_table: FxIndexSet::default() } } - fn global_file_id_for_file_name(&self, file_name: Symbol) -> GlobalFileId { - let raw_id = self.raw_file_table.get_index_of(&file_name).unwrap_or_else(|| { - bug!("file name not found in prepared global file table: {file_name}"); - }); + fn global_file_id_for_file_name(&mut self, file_name: Symbol) -> GlobalFileId { + // Ensure the given file has a table entry, and get its index. + let (raw_id, _) = self.raw_file_table.insert_full(file_name); // The raw file table doesn't include an entry for the working dir // (which has ID 0), so add 1 to get the correct ID. GlobalFileId::from_usize(raw_id + 1) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index 33e7a0f2f20..6a151d3c2b0 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -45,7 +45,7 @@ impl<'tcx> CovfunRecord<'tcx> { pub(crate) fn prepare_covfun_record<'tcx>( tcx: TyCtxt<'tcx>, - global_file_table: &GlobalFileTable, + global_file_table: &mut GlobalFileTable, instance: Instance<'tcx>, function_coverage: &FunctionCoverage<'tcx>, ) -> Option> { @@ -75,7 +75,7 @@ pub(crate) fn prepare_covfun_record<'tcx>( /// Populates the mapping region tables in the current function's covfun record. fn fill_region_tables<'tcx>( tcx: TyCtxt<'tcx>, - global_file_table: &GlobalFileTable, + global_file_table: &mut GlobalFileTable, function_coverage: &FunctionCoverage<'tcx>, covfun: &mut CovfunRecord<'tcx>, ) { diff --git a/tests/coverage/unused_mod.cov-map b/tests/coverage/unused_mod.cov-map index af10906fa3c..5e8b69fcdba 100644 --- a/tests/coverage/unused_mod.cov-map +++ b/tests/coverage/unused_mod.cov-map @@ -1,16 +1,16 @@ Function name: unused_mod::main -Raw bytes (9): 0x[01, 02, 00, 01, 01, 04, 01, 02, 02] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 04, 01, 02, 02] Number of files: 1 -- file 0 => global file 2 +- file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 4, 1) to (start + 2, 2) Highest counter ID seen: c0 Function name: unused_mod::unused_module::never_called_function (unused) -Raw bytes (9): 0x[01, 01, 00, 01, 00, 02, 01, 02, 02] +Raw bytes (9): 0x[01, 02, 00, 01, 00, 02, 01, 02, 02] Number of files: 1 -- file 0 => global file 1 +- file 0 => global file 2 Number of expressions: 0 Number of file 0 mappings: 1 - Code(Zero) at (prev + 2, 1) to (start + 2, 2) -- cgit 1.4.1-3-g733a5 From 252276a53d05f49523e6eac992b5f49e9815db0c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 12 Dec 2024 21:36:20 +1100 Subject: coverage: Pull expression conversion out of `map_data.rs` --- .../src/coverageinfo/map_data.rs | 32 +-------------- .../src/coverageinfo/mapgen/covfun.rs | 45 +++++++++++++++++++++- 2 files changed, 44 insertions(+), 33 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index c5d1ebdfe7c..5b05f0867b9 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -1,11 +1,7 @@ -use rustc_data_structures::captures::Captures; use rustc_middle::mir::coverage::{ - CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping, MappingKind, Op, - SourceRegion, + CovTerm, CoverageIdsInfo, FunctionCoverageInfo, Mapping, MappingKind, SourceRegion, }; -use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind}; - pub(crate) struct FunctionCoverage<'tcx> { pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo, /// If `None`, the corresponding function is unused. @@ -35,28 +31,6 @@ impl<'tcx> FunctionCoverage<'tcx> { if self.is_used() { self.function_coverage_info.function_source_hash } else { 0 } } - /// Convert this function's coverage expression data into a form that can be - /// passed through FFI to LLVM. - pub(crate) fn counter_expressions( - &self, - ) -> impl Iterator + ExactSizeIterator + Captures<'_> { - // We know that LLVM will optimize out any unused expressions before - // producing the final coverage map, so there's no need to do the same - // thing on the Rust side unless we're confident we can do much better. - // (See `CounterExpressionsMinimizer` in `CoverageMappingWriter.cpp`.) - - self.function_coverage_info.expressions.iter().map(move |&Expression { lhs, op, rhs }| { - CounterExpression { - lhs: self.counter_for_term(lhs), - kind: match op { - Op::Add => ExprKind::Add, - Op::Subtract => ExprKind::Subtract, - }, - rhs: self.counter_for_term(rhs), - } - }) - } - /// Converts this function's coverage mappings into an intermediate form /// that will be used by `mapgen` when preparing for FFI. pub(crate) fn counter_regions( @@ -70,10 +44,6 @@ impl<'tcx> FunctionCoverage<'tcx> { }) } - fn counter_for_term(&self, term: CovTerm) -> Counter { - if self.is_zero_term(term) { Counter::ZERO } else { Counter::from_term(term) } - } - fn is_zero_term(&self, term: CovTerm) -> bool { match self.ids_info { Some(ids_info) => ids_info.is_zero_term(term), diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index 6a151d3c2b0..e71c15e7856 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -11,7 +11,9 @@ use rustc_codegen_ssa::traits::{ BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods, }; use rustc_middle::bug; -use rustc_middle::mir::coverage::MappingKind; +use rustc_middle::mir::coverage::{ + CoverageIdsInfo, Expression, FunctionCoverageInfo, MappingKind, Op, +}; use rustc_middle::ty::{Instance, TyCtxt}; use rustc_target::spec::HasTargetSpec; use tracing::debug; @@ -49,12 +51,17 @@ pub(crate) fn prepare_covfun_record<'tcx>( instance: Instance<'tcx>, function_coverage: &FunctionCoverage<'tcx>, ) -> Option> { + let fn_cov_info = tcx.instance_mir(instance.def).function_coverage_info.as_deref()?; + let ids_info = tcx.coverage_ids_info(instance.def); + + let expressions = prepare_expressions(fn_cov_info, ids_info, function_coverage.is_used()); + let mut covfun = CovfunRecord { mangled_function_name: tcx.symbol_name(instance).name, source_hash: function_coverage.source_hash(), is_used: function_coverage.is_used(), virtual_file_mapping: VirtualFileMapping::default(), - expressions: function_coverage.counter_expressions().collect::>(), + expressions, regions: ffi::Regions::default(), }; @@ -72,6 +79,40 @@ pub(crate) fn prepare_covfun_record<'tcx>( Some(covfun) } +/// Convert the function's coverage-counter expressions into a form suitable for FFI. +fn prepare_expressions( + fn_cov_info: &FunctionCoverageInfo, + ids_info: &CoverageIdsInfo, + is_used: bool, +) -> Vec { + // If any counters or expressions were removed by MIR opts, replace their + // terms with zero. + let counter_for_term = |term| { + if !is_used || ids_info.is_zero_term(term) { + ffi::Counter::ZERO + } else { + ffi::Counter::from_term(term) + } + }; + + // We know that LLVM will optimize out any unused expressions before + // producing the final coverage map, so there's no need to do the same + // thing on the Rust side unless we're confident we can do much better. + // (See `CounterExpressionsMinimizer` in `CoverageMappingWriter.cpp`.) + fn_cov_info + .expressions + .iter() + .map(move |&Expression { lhs, op, rhs }| ffi::CounterExpression { + lhs: counter_for_term(lhs), + kind: match op { + Op::Add => ffi::ExprKind::Add, + Op::Subtract => ffi::ExprKind::Subtract, + }, + rhs: counter_for_term(rhs), + }) + .collect::>() +} + /// Populates the mapping region tables in the current function's covfun record. fn fill_region_tables<'tcx>( tcx: TyCtxt<'tcx>, -- cgit 1.4.1-3-g733a5 From 527f8127bbde4d189ee292dee0d6070550ec0ba6 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 12 Dec 2024 21:46:34 +1100 Subject: coverage: Pull region conversion out of `map_data.rs` --- .../src/coverageinfo/map_data.rs | 25 +-------------------- .../src/coverageinfo/mapgen/covfun.rs | 26 +++++++++++----------- 2 files changed, 14 insertions(+), 37 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index 5b05f0867b9..e1b2a1b87bb 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -1,6 +1,4 @@ -use rustc_middle::mir::coverage::{ - CovTerm, CoverageIdsInfo, FunctionCoverageInfo, Mapping, MappingKind, SourceRegion, -}; +use rustc_middle::mir::coverage::{CoverageIdsInfo, FunctionCoverageInfo}; pub(crate) struct FunctionCoverage<'tcx> { pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo, @@ -30,25 +28,4 @@ impl<'tcx> FunctionCoverage<'tcx> { pub(crate) fn source_hash(&self) -> u64 { if self.is_used() { self.function_coverage_info.function_source_hash } else { 0 } } - - /// Converts this function's coverage mappings into an intermediate form - /// that will be used by `mapgen` when preparing for FFI. - pub(crate) fn counter_regions( - &self, - ) -> impl Iterator + ExactSizeIterator { - self.function_coverage_info.mappings.iter().map(move |mapping| { - let Mapping { kind, source_region } = mapping; - let kind = - kind.map_terms(|term| if self.is_zero_term(term) { CovTerm::Zero } else { term }); - (kind, source_region) - }) - } - - fn is_zero_term(&self, term: CovTerm) -> bool { - match self.ids_info { - Some(ids_info) => ids_info.is_zero_term(term), - // This function is unused, so all coverage counters/expressions are zero. - None => true, - } - } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index e71c15e7856..fa0af8415e7 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -12,7 +12,7 @@ use rustc_codegen_ssa::traits::{ }; use rustc_middle::bug; use rustc_middle::mir::coverage::{ - CoverageIdsInfo, Expression, FunctionCoverageInfo, MappingKind, Op, + CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping, MappingKind, Op, }; use rustc_middle::ty::{Instance, TyCtxt}; use rustc_target::spec::HasTargetSpec; @@ -65,7 +65,7 @@ pub(crate) fn prepare_covfun_record<'tcx>( regions: ffi::Regions::default(), }; - fill_region_tables(tcx, global_file_table, function_coverage, &mut covfun); + fill_region_tables(tcx, global_file_table, fn_cov_info, ids_info, &mut covfun); if covfun.regions.has_no_regions() { if covfun.is_used { @@ -117,16 +117,12 @@ fn prepare_expressions( fn fill_region_tables<'tcx>( tcx: TyCtxt<'tcx>, global_file_table: &mut GlobalFileTable, - function_coverage: &FunctionCoverage<'tcx>, + fn_cov_info: &'tcx FunctionCoverageInfo, + ids_info: &'tcx CoverageIdsInfo, covfun: &mut CovfunRecord<'tcx>, ) { - let counter_regions = function_coverage.counter_regions(); - if counter_regions.is_empty() { - return; - } - // Currently a function's mappings must all be in the same file as its body span. - let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span); + let file_name = span_file_name(tcx, fn_cov_info.body_span); // Look up the global file ID for that filename. let global_file_id = global_file_table.global_file_id_for_file_name(file_name); @@ -140,10 +136,14 @@ fn fill_region_tables<'tcx>( // For each counter/region pair in this function+file, convert it to a // form suitable for FFI. - for (mapping_kind, region) in counter_regions { - debug!("Adding counter {mapping_kind:?} to map for {region:?}"); - let span = ffi::CoverageSpan::from_source_region(local_file_id, region); - match mapping_kind { + let is_zero_term = |term| !covfun.is_used || ids_info.is_zero_term(term); + for Mapping { kind, ref source_region } in &fn_cov_info.mappings { + // If the mapping refers to counters/expressions that were removed by + // MIR opts, replace those occurrences with zero. + let kind = kind.map_terms(|term| if is_zero_term(term) { CovTerm::Zero } else { term }); + + let span = ffi::CoverageSpan::from_source_region(local_file_id, source_region); + match kind { MappingKind::Code(term) => { code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) }); } -- cgit 1.4.1-3-g733a5 From d34c365eb0c40d907daf42fff42b7b6ebdc314ab Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 14 Dec 2024 23:14:19 +1100 Subject: coverage: Pull function source hash out of `map_data.rs` --- compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs | 7 +------ compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 3 ++- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs | 9 ++++----- 3 files changed, 7 insertions(+), 12 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index e1b2a1b87bb..261a014c3d1 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -1,6 +1,7 @@ use rustc_middle::mir::coverage::{CoverageIdsInfo, FunctionCoverageInfo}; pub(crate) struct FunctionCoverage<'tcx> { + #[expect(unused)] // This whole file gets deleted later in the same PR. pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo, /// If `None`, the corresponding function is unused. ids_info: Option<&'tcx CoverageIdsInfo>, @@ -22,10 +23,4 @@ impl<'tcx> FunctionCoverage<'tcx> { pub(crate) fn is_used(&self) -> bool { self.ids_info.is_some() } - - /// Return the source hash, generated from the HIR node structure, and used to indicate whether - /// or not the source code structure changed between different compilations. - pub(crate) fn source_hash(&self) -> u64 { - if self.is_used() { self.function_coverage_info.function_source_hash } else { 0 } - } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 0c7ad300a8b..9bc33967044 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -81,7 +81,8 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { // instances were visited during codegen. .sorted_by_cached_key(|&(instance, _)| tcx.symbol_name(instance).name) .filter_map(|(instance, function_coverage)| { - prepare_covfun_record(tcx, &mut global_file_table, instance, &function_coverage) + let is_used = function_coverage.is_used(); + prepare_covfun_record(tcx, &mut global_file_table, instance, is_used) }) .collect::>(); diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index fa0af8415e7..8e853f057be 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -19,7 +19,6 @@ use rustc_target::spec::HasTargetSpec; use tracing::debug; use crate::common::CodegenCx; -use crate::coverageinfo::map_data::FunctionCoverage; use crate::coverageinfo::mapgen::{GlobalFileTable, VirtualFileMapping, span_file_name}; use crate::coverageinfo::{ffi, llvm_cov}; use crate::llvm; @@ -49,17 +48,17 @@ pub(crate) fn prepare_covfun_record<'tcx>( tcx: TyCtxt<'tcx>, global_file_table: &mut GlobalFileTable, instance: Instance<'tcx>, - function_coverage: &FunctionCoverage<'tcx>, + is_used: bool, ) -> Option> { let fn_cov_info = tcx.instance_mir(instance.def).function_coverage_info.as_deref()?; let ids_info = tcx.coverage_ids_info(instance.def); - let expressions = prepare_expressions(fn_cov_info, ids_info, function_coverage.is_used()); + let expressions = prepare_expressions(fn_cov_info, ids_info, is_used); let mut covfun = CovfunRecord { mangled_function_name: tcx.symbol_name(instance).name, - source_hash: function_coverage.source_hash(), - is_used: function_coverage.is_used(), + source_hash: if is_used { fn_cov_info.function_source_hash } else { 0 }, + is_used, virtual_file_mapping: VirtualFileMapping::default(), expressions, regions: ffi::Regions::default(), -- cgit 1.4.1-3-g733a5 From 541d4e85d90b4c0416014c3620d3f3c9771bd476 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 14 Dec 2024 22:48:12 +1100 Subject: coverage: Track used functions in a set instead of a map This patch dismantles what was left of `FunctionCoverage` in `map_data.rs`, replaces `function_coverage_map` with a set, and overhauls how we prepare covfun records for unused functions. --- .../src/coverageinfo/map_data.rs | 26 ------- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 82 +++++++++------------- .../rustc_codegen_llvm/src/coverageinfo/mod.rs | 19 ++--- 3 files changed, 37 insertions(+), 90 deletions(-) delete mode 100644 compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs deleted file mode 100644 index 261a014c3d1..00000000000 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ /dev/null @@ -1,26 +0,0 @@ -use rustc_middle::mir::coverage::{CoverageIdsInfo, FunctionCoverageInfo}; - -pub(crate) struct FunctionCoverage<'tcx> { - #[expect(unused)] // This whole file gets deleted later in the same PR. - pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo, - /// If `None`, the corresponding function is unused. - ids_info: Option<&'tcx CoverageIdsInfo>, -} - -impl<'tcx> FunctionCoverage<'tcx> { - pub(crate) fn new_used( - function_coverage_info: &'tcx FunctionCoverageInfo, - ids_info: &'tcx CoverageIdsInfo, - ) -> Self { - Self { function_coverage_info, ids_info: Some(ids_info) } - } - - pub(crate) fn new_unused(function_coverage_info: &'tcx FunctionCoverageInfo) -> Self { - Self { function_coverage_info, ids_info: None } - } - - /// Returns true for a used (called) function, and false for an unused function. - pub(crate) fn is_used(&self) -> bool { - self.ids_info.is_some() - } -} diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 9bc33967044..ca334286200 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -18,7 +18,6 @@ use tracing::debug; use crate::common::CodegenCx; use crate::coverageinfo::llvm_cov; -use crate::coverageinfo::map_data::FunctionCoverage; use crate::coverageinfo::mapgen::covfun::prepare_covfun_record; use crate::llvm; @@ -49,23 +48,11 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { debug!("Generating coverage map for CodegenUnit: `{}`", cx.codegen_unit.name()); - // In order to show that unused functions have coverage counts of zero (0), LLVM requires the - // functions exist. Generate synthetic functions with a (required) single counter, and add the - // MIR `Coverage` code regions to the `function_coverage_map`, before calling - // `ctx.take_function_coverage_map()`. - if cx.codegen_unit.is_code_coverage_dead_code_cgu() { - add_unused_functions(cx); - } - // FIXME(#132395): Can this be none even when coverage is enabled? - let function_coverage_map = match cx.coverage_cx { - Some(ref cx) => cx.take_function_coverage_map(), + let instances_used = match cx.coverage_cx { + Some(ref cx) => cx.instances_used.borrow(), None => return, }; - if function_coverage_map.is_empty() { - // This CGU has no functions with coverage instrumentation. - return; - } // The order of entries in this global file table needs to be deterministic, // and ideally should also be independent of the details of stable-hashing, @@ -74,18 +61,27 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { // are satisfied, the order can be arbitrary. let mut global_file_table = GlobalFileTable::new(); - let covfun_records = function_coverage_map - .into_iter() + let mut covfun_records = instances_used + .iter() + .copied() // Sort by symbol name, so that the global file table is built in an // order that doesn't depend on the stable-hash-based order in which // instances were visited during codegen. - .sorted_by_cached_key(|&(instance, _)| tcx.symbol_name(instance).name) - .filter_map(|(instance, function_coverage)| { - let is_used = function_coverage.is_used(); - prepare_covfun_record(tcx, &mut global_file_table, instance, is_used) - }) + .sorted_by_cached_key(|&instance| tcx.symbol_name(instance).name) + .filter_map(|instance| prepare_covfun_record(tcx, &mut global_file_table, instance, true)) .collect::>(); + // In a single designated CGU, also prepare covfun records for functions + // in this crate that were instrumented for coverage, but are unused. + if cx.codegen_unit.is_code_coverage_dead_code_cgu() { + let mut unused_instances = gather_unused_function_instances(cx); + // Sort the unused instances by symbol name, for the same reason as the used ones. + unused_instances.sort_by_cached_key(|&instance| tcx.symbol_name(instance).name); + covfun_records.extend(unused_instances.into_iter().filter_map(|instance| { + prepare_covfun_record(tcx, &mut global_file_table, instance, false) + })); + } + // If there are no covfun records for this CGU, don't generate a covmap record. // Emitting a covmap record without any covfun records causes `llvm-cov` to // fail when generating coverage reports, and if there are no covfun records @@ -261,7 +257,7 @@ fn generate_covmap_record<'ll>(cx: &CodegenCx<'ll, '_>, version: u32, filenames_ /// coverage map (in a single designated CGU) so that we still emit coverage mappings for them. /// We also end up adding their symbol names to a special global array that LLVM will include in /// its embedded coverage data. -fn add_unused_functions(cx: &CodegenCx<'_, '_>) { +fn gather_unused_function_instances<'tcx>(cx: &CodegenCx<'_, 'tcx>) -> Vec> { assert!(cx.codegen_unit.is_code_coverage_dead_code_cgu()); let tcx = cx.tcx; @@ -279,20 +275,17 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) { && !usage.used_via_inlining.contains(&d) }; - // Scan for unused functions that were instrumented for coverage. - for def_id in tcx.mir_keys(()).iter().copied().filter(|&def_id| is_unused_fn(def_id)) { - // Get the coverage info from MIR, skipping functions that were never instrumented. - let body = tcx.optimized_mir(def_id); - let Some(function_coverage_info) = body.function_coverage_info.as_deref() else { continue }; + // FIXME(#79651): Consider trying to filter out dummy instantiations of + // unused generic functions from library crates, because they can produce + // "unused instantiation" in coverage reports even when they are actually + // used by some downstream crate in the same binary. - // FIXME(79651): Consider trying to filter out dummy instantiations of - // unused generic functions from library crates, because they can produce - // "unused instantiation" in coverage reports even when they are actually - // used by some downstream crate in the same binary. - - debug!("generating unused fn: {def_id:?}"); - add_unused_function_coverage(cx, def_id, function_coverage_info); - } + tcx.mir_keys(()) + .iter() + .copied() + .filter(|&def_id| is_unused_fn(def_id)) + .map(|def_id| make_dummy_instance(tcx, def_id)) + .collect::>() } struct UsageSets<'tcx> { @@ -357,16 +350,11 @@ fn prepare_usage_sets<'tcx>(tcx: TyCtxt<'tcx>) -> UsageSets<'tcx> { UsageSets { all_mono_items, used_via_inlining, missing_own_coverage } } -fn add_unused_function_coverage<'tcx>( - cx: &CodegenCx<'_, 'tcx>, - def_id: LocalDefId, - function_coverage_info: &'tcx mir::coverage::FunctionCoverageInfo, -) { - let tcx = cx.tcx; - let def_id = def_id.to_def_id(); +fn make_dummy_instance<'tcx>(tcx: TyCtxt<'tcx>, local_def_id: LocalDefId) -> ty::Instance<'tcx> { + let def_id = local_def_id.to_def_id(); // Make a dummy instance that fills in all generics with placeholders. - let instance = ty::Instance::new( + ty::Instance::new( def_id, ty::GenericArgs::for_item(tcx, def_id, |param, _| { if let ty::GenericParamDefKind::Lifetime = param.kind { @@ -375,9 +363,5 @@ fn add_unused_function_coverage<'tcx>( tcx.mk_param_from_def(param) } }), - ); - - // An unused function's mappings will all be rewritten to map to zero. - let function_coverage = FunctionCoverage::new_unused(function_coverage_info); - cx.coverage_cx().function_coverage_map.borrow_mut().insert(instance, function_coverage); + ) } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 82b6677e203..7311cd9d230 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -5,7 +5,7 @@ use rustc_abi::Size; use rustc_codegen_ssa::traits::{ BuilderMethods, ConstCodegenMethods, CoverageInfoBuilderMethods, MiscCodegenMethods, }; -use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; +use rustc_data_structures::fx::{FxHashMap, FxIndexSet}; use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::ty::Instance; use rustc_middle::ty::layout::HasTyCtxt; @@ -13,18 +13,16 @@ use tracing::{debug, instrument}; use crate::builder::Builder; use crate::common::CodegenCx; -use crate::coverageinfo::map_data::FunctionCoverage; use crate::llvm; pub(crate) mod ffi; mod llvm_cov; -pub(crate) mod map_data; mod mapgen; /// Extra per-CGU context/state needed for coverage instrumentation. pub(crate) struct CguCoverageContext<'ll, 'tcx> { /// Coverage data for each instrumented function identified by DefId. - pub(crate) function_coverage_map: RefCell, FunctionCoverage<'tcx>>>, + pub(crate) instances_used: RefCell>>, pub(crate) pgo_func_name_var_map: RefCell, &'ll llvm::Value>>, pub(crate) mcdc_condition_bitmap_map: RefCell, Vec<&'ll llvm::Value>>>, @@ -34,17 +32,13 @@ pub(crate) struct CguCoverageContext<'ll, 'tcx> { impl<'ll, 'tcx> CguCoverageContext<'ll, 'tcx> { pub(crate) fn new() -> Self { Self { - function_coverage_map: Default::default(), + instances_used: RefCell::>::default(), pgo_func_name_var_map: Default::default(), mcdc_condition_bitmap_map: Default::default(), covfun_section_name: Default::default(), } } - fn take_function_coverage_map(&self) -> FxIndexMap, FunctionCoverage<'tcx>> { - self.function_coverage_map.replace(FxIndexMap::default()) - } - /// LLVM use a temp value to record evaluated mcdc test vector of each decision, which is /// called condition bitmap. In order to handle nested decisions, several condition bitmaps can /// be allocated for a function body. These values are named `mcdc.addr.{i}` and are a 32-bit @@ -157,12 +151,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { // Mark the instance as used in this CGU, for coverage purposes. // This includes functions that were not partitioned into this CGU, // but were MIR-inlined into one of this CGU's functions. - coverage_cx.function_coverage_map.borrow_mut().entry(instance).or_insert_with(|| { - FunctionCoverage::new_used( - function_coverage_info, - bx.tcx.coverage_ids_info(instance.def), - ) - }); + coverage_cx.instances_used.borrow_mut().insert(instance); match *kind { CoverageKind::SpanMarker | CoverageKind::BlockMarker { .. } => unreachable!( -- cgit 1.4.1-3-g733a5 From d416cead5a92d6a054d0e4d18a4b3a965f35334d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 18 Dec 2024 18:59:27 +1100 Subject: coverage: Rename some FFI fields from `span` to `cov_span` This will avoid confusion with actual `Span` spans. --- compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs | 8 ++++---- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs | 11 ++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 1f133141c18..4cad09b3a2d 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -184,7 +184,7 @@ impl Regions { #[derive(Clone, Debug)] #[repr(C)] pub(crate) struct CodeRegion { - pub(crate) span: CoverageSpan, + pub(crate) cov_span: CoverageSpan, pub(crate) counter: Counter, } @@ -192,7 +192,7 @@ pub(crate) struct CodeRegion { #[derive(Clone, Debug)] #[repr(C)] pub(crate) struct BranchRegion { - pub(crate) span: CoverageSpan, + pub(crate) cov_span: CoverageSpan, pub(crate) true_counter: Counter, pub(crate) false_counter: Counter, } @@ -201,7 +201,7 @@ pub(crate) struct BranchRegion { #[derive(Clone, Debug)] #[repr(C)] pub(crate) struct MCDCBranchRegion { - pub(crate) span: CoverageSpan, + pub(crate) cov_span: CoverageSpan, pub(crate) true_counter: Counter, pub(crate) false_counter: Counter, pub(crate) mcdc_branch_params: mcdc::BranchParameters, @@ -211,6 +211,6 @@ pub(crate) struct MCDCBranchRegion { #[derive(Clone, Debug)] #[repr(C)] pub(crate) struct MCDCDecisionRegion { - pub(crate) span: CoverageSpan, + pub(crate) cov_span: CoverageSpan, pub(crate) mcdc_decision_params: mcdc::DecisionParameters, } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index 8e853f057be..f9e9555e8bd 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -141,21 +141,22 @@ fn fill_region_tables<'tcx>( // MIR opts, replace those occurrences with zero. let kind = kind.map_terms(|term| if is_zero_term(term) { CovTerm::Zero } else { term }); - let span = ffi::CoverageSpan::from_source_region(local_file_id, source_region); + let cov_span = ffi::CoverageSpan::from_source_region(local_file_id, source_region); match kind { MappingKind::Code(term) => { - code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) }); + code_regions + .push(ffi::CodeRegion { cov_span, counter: ffi::Counter::from_term(term) }); } MappingKind::Branch { true_term, false_term } => { branch_regions.push(ffi::BranchRegion { - span, + cov_span, true_counter: ffi::Counter::from_term(true_term), false_counter: ffi::Counter::from_term(false_term), }); } MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => { mcdc_branch_regions.push(ffi::MCDCBranchRegion { - span, + cov_span, true_counter: ffi::Counter::from_term(true_term), false_counter: ffi::Counter::from_term(false_term), mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params), @@ -163,7 +164,7 @@ fn fill_region_tables<'tcx>( } MappingKind::MCDCDecision(mcdc_decision_params) => { mcdc_decision_regions.push(ffi::MCDCDecisionRegion { - span, + cov_span, mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params), }); } -- cgit 1.4.1-3-g733a5 From c3780e1d227d7544209e2cd722ac4fa35ac61b56 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 18 Dec 2024 21:23:48 +1100 Subject: coverage: Quietly skip functions that end up having no mappings In codegen, a used function with `FunctionCoverageInfo` but no mappings has historically indicated a bug. However, that will no longer be the case after moving some fallible span-processing steps into codegen. --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index f9e9555e8bd..dca1779d821 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -10,7 +10,6 @@ use rustc_abi::Align; use rustc_codegen_ssa::traits::{ BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods, }; -use rustc_middle::bug; use rustc_middle::mir::coverage::{ CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping, MappingKind, Op, }; @@ -67,12 +66,8 @@ pub(crate) fn prepare_covfun_record<'tcx>( fill_region_tables(tcx, global_file_table, fn_cov_info, ids_info, &mut covfun); if covfun.regions.has_no_regions() { - if covfun.is_used { - bug!("a used function should have had coverage mapping data but did not: {covfun:?}"); - } else { - debug!(?covfun, "unused function had no coverage mapping data"); - return None; - } + debug!(?covfun, "function has no mappings to embed; skipping"); + return None; } Some(covfun) -- cgit 1.4.1-3-g733a5 From 34ed51cb837d1dd755b9b97798a41c8cd0358ca6 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 18 Dec 2024 21:00:51 +1100 Subject: coverage: Store coverage source regions as `Span` until codegen --- .../rustc_codegen_llvm/src/coverageinfo/ffi.rs | 28 +--- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 3 +- .../src/coverageinfo/mapgen/covfun.rs | 14 +- .../src/coverageinfo/mapgen/spans.rs | 126 ++++++++++++++++ compiler/rustc_codegen_llvm/src/lib.rs | 1 + compiler/rustc_middle/src/mir/coverage.rs | 18 +-- compiler/rustc_middle/src/mir/pretty.rs | 4 +- compiler/rustc_mir_transform/src/coverage/mod.rs | 168 +++------------------ .../branch_match_arms.main.InstrumentCoverage.diff | 12 +- ...instrument_coverage.bar.InstrumentCoverage.diff | 2 +- ...nstrument_coverage.main.InstrumentCoverage.diff | 10 +- ..._coverage_cleanup.main.CleanupPostBorrowck.diff | 10 +- ...t_coverage_cleanup.main.InstrumentCoverage.diff | 10 +- 13 files changed, 189 insertions(+), 217 deletions(-) create mode 100644 compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 4cad09b3a2d..b617f4d37f5 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -1,6 +1,4 @@ -use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId, SourceRegion}; - -use crate::coverageinfo::mapgen::LocalFileId; +use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId}; /// Must match the layout of `LLVMRustCounterKind`. #[derive(Copy, Clone, Debug)] @@ -126,30 +124,16 @@ pub(crate) struct CoverageSpan { /// Local index into the function's local-to-global file ID table. /// The value at that index is itself an index into the coverage filename /// table in the CGU's `__llvm_covmap` section. - file_id: u32, + pub(crate) file_id: u32, /// 1-based starting line of the source code span. - start_line: u32, + pub(crate) start_line: u32, /// 1-based starting column of the source code span. - start_col: u32, + pub(crate) start_col: u32, /// 1-based ending line of the source code span. - end_line: u32, + pub(crate) end_line: u32, /// 1-based ending column of the source code span. High bit must be unset. - end_col: u32, -} - -impl CoverageSpan { - pub(crate) fn from_source_region( - local_file_id: LocalFileId, - code_region: &SourceRegion, - ) -> Self { - let file_id = local_file_id.as_u32(); - let &SourceRegion { start_line, start_col, end_line, end_col } = code_region; - // Internally, LLVM uses the high bit of `end_col` to distinguish between - // code regions and gap regions, so it can't be used by the column number. - assert!(end_col & (1u32 << 31) == 0, "high bit of `end_col` must be unset: {end_col:#X}"); - Self { file_id, start_line, start_col, end_line, end_col } - } + pub(crate) end_col: u32, } /// Holds tables of the various region types in one struct. diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index ca334286200..7d3bb4f93f6 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -22,6 +22,7 @@ use crate::coverageinfo::mapgen::covfun::prepare_covfun_record; use crate::llvm; mod covfun; +mod spans; /// Generates and exports the coverage map, which is embedded in special /// linker sections in the final binary. @@ -182,7 +183,7 @@ rustc_index::newtype_index! { /// An index into a function's list of global file IDs. That underlying list /// of local-to-global mappings will be embedded in the function's record in /// the `__llvm_covfun` linker section. - pub(crate) struct LocalFileId {} + struct LocalFileId {} } /// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index dca1779d821..cde7a0ebc6d 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -14,11 +14,12 @@ use rustc_middle::mir::coverage::{ CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping, MappingKind, Op, }; use rustc_middle::ty::{Instance, TyCtxt}; +use rustc_span::Span; use rustc_target::spec::HasTargetSpec; use tracing::debug; use crate::common::CodegenCx; -use crate::coverageinfo::mapgen::{GlobalFileTable, VirtualFileMapping, span_file_name}; +use crate::coverageinfo::mapgen::{GlobalFileTable, VirtualFileMapping, span_file_name, spans}; use crate::coverageinfo::{ffi, llvm_cov}; use crate::llvm; @@ -117,6 +118,8 @@ fn fill_region_tables<'tcx>( ) { // Currently a function's mappings must all be in the same file as its body span. let file_name = span_file_name(tcx, fn_cov_info.body_span); + let source_map = tcx.sess.source_map(); + let source_file = source_map.lookup_source_file(fn_cov_info.body_span.lo()); // Look up the global file ID for that filename. let global_file_id = global_file_table.global_file_id_for_file_name(file_name); @@ -128,15 +131,20 @@ fn fill_region_tables<'tcx>( let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } = &mut covfun.regions; + let make_cov_span = |span: Span| { + spans::make_coverage_span(local_file_id, source_map, fn_cov_info, &source_file, span) + }; + // For each counter/region pair in this function+file, convert it to a // form suitable for FFI. let is_zero_term = |term| !covfun.is_used || ids_info.is_zero_term(term); - for Mapping { kind, ref source_region } in &fn_cov_info.mappings { + for &Mapping { ref kind, span } in &fn_cov_info.mappings { // If the mapping refers to counters/expressions that were removed by // MIR opts, replace those occurrences with zero. let kind = kind.map_terms(|term| if is_zero_term(term) { CovTerm::Zero } else { term }); - let cov_span = ffi::CoverageSpan::from_source_region(local_file_id, source_region); + let Some(cov_span) = make_cov_span(span) else { continue }; + match kind { MappingKind::Code(term) => { code_regions diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs new file mode 100644 index 00000000000..6d1d91340c2 --- /dev/null +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs @@ -0,0 +1,126 @@ +use rustc_middle::mir::coverage::FunctionCoverageInfo; +use rustc_span::source_map::SourceMap; +use rustc_span::{BytePos, Pos, SourceFile, Span}; +use tracing::debug; + +use crate::coverageinfo::ffi; +use crate::coverageinfo::mapgen::LocalFileId; + +/// Converts the span into its start line and column, and end line and column. +/// +/// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by +/// the compiler, these column numbers are denoted in **bytes**, because that's what +/// LLVM's `llvm-cov` tool expects to see in coverage maps. +/// +/// Returns `None` if the conversion failed for some reason. This shouldn't happen, +/// but it's hard to rule out entirely (especially in the presence of complex macros +/// or other expansions), and if it does happen then skipping a span or function is +/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid. +pub(crate) fn make_coverage_span( + file_id: LocalFileId, + source_map: &SourceMap, + fn_cov_info: &FunctionCoverageInfo, + file: &SourceFile, + span: Span, +) -> Option { + let span = ensure_non_empty_span(source_map, fn_cov_info, span)?; + + let lo = span.lo(); + let hi = span.hi(); + + // Column numbers need to be in bytes, so we can't use the more convenient + // `SourceMap` methods for looking up file coordinates. + let line_and_byte_column = |pos: BytePos| -> Option<(usize, usize)> { + let rpos = file.relative_position(pos); + let line_index = file.lookup_line(rpos)?; + let line_start = file.lines()[line_index]; + // Line numbers and column numbers are 1-based, so add 1 to each. + Some((line_index + 1, (rpos - line_start).to_usize() + 1)) + }; + + let (mut start_line, start_col) = line_and_byte_column(lo)?; + let (mut end_line, end_col) = line_and_byte_column(hi)?; + + // Apply an offset so that code in doctests has correct line numbers. + // FIXME(#79417): Currently we have no way to offset doctest _columns_. + start_line = source_map.doctest_offset_line(&file.name, start_line); + end_line = source_map.doctest_offset_line(&file.name, end_line); + + check_coverage_span(ffi::CoverageSpan { + file_id: file_id.as_u32(), + start_line: start_line as u32, + start_col: start_col as u32, + end_line: end_line as u32, + end_col: end_col as u32, + }) +} + +fn ensure_non_empty_span( + source_map: &SourceMap, + fn_cov_info: &FunctionCoverageInfo, + span: Span, +) -> Option { + if !span.is_empty() { + return Some(span); + } + + let lo = span.lo(); + let hi = span.hi(); + + // The span is empty, so try to expand it to cover an adjacent '{' or '}', + // but only within the bounds of the body span. + let try_next = hi < fn_cov_info.body_span.hi(); + let try_prev = fn_cov_info.body_span.lo() < lo; + if !(try_next || try_prev) { + return None; + } + + source_map + .span_to_source(span, |src, start, end| try { + // Adjusting span endpoints by `BytePos(1)` is normally a bug, + // but in this case we have specifically checked that the character + // we're skipping over is one of two specific ASCII characters, so + // adjusting by exactly 1 byte is correct. + if try_next && src.as_bytes()[end] == b'{' { + Some(span.with_hi(hi + BytePos(1))) + } else if try_prev && src.as_bytes()[start - 1] == b'}' { + Some(span.with_lo(lo - BytePos(1))) + } else { + None + } + }) + .ok()? +} + +/// If `llvm-cov` sees a source region that is improperly ordered (end < start), +/// it will immediately exit with a fatal error. To prevent that from happening, +/// discard regions that are improperly ordered, or might be interpreted in a +/// way that makes them improperly ordered. +fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option { + let ffi::CoverageSpan { file_id: _, start_line, start_col, end_line, end_col } = cov_span; + + // Line/column coordinates are supposed to be 1-based. If we ever emit + // coordinates of 0, `llvm-cov` might misinterpret them. + let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0); + // Coverage mappings use the high bit of `end_col` to indicate that a + // region is actually a "gap" region, so make sure it's unset. + let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0; + // If a region is improperly ordered (end < start), `llvm-cov` will exit + // with a fatal error, which is inconvenient for users and hard to debug. + let is_ordered = (start_line, start_col) <= (end_line, end_col); + + if all_nonzero && end_col_has_high_bit_unset && is_ordered { + Some(cov_span) + } else { + debug!( + ?cov_span, + ?all_nonzero, + ?end_col_has_high_bit_unset, + ?is_ordered, + "Skipping source region that would be misinterpreted or rejected by LLVM" + ); + // If this happens in a debug build, ICE to make it easier to notice. + debug_assert!(false, "Improper source region: {cov_span:?}"); + None + } +} diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index b079eb8fe0c..0de0c6a7a89 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -17,6 +17,7 @@ #![feature(iter_intersperse)] #![feature(let_chains)] #![feature(rustdoc_internals)] +#![feature(try_blocks)] #![warn(unreachable_pub)] // tidy-alphabetical-end diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 962176290df..29f26180c97 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -154,22 +154,6 @@ impl Debug for CoverageKind { } } -#[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq, Eq, PartialOrd, Ord)] -#[derive(TypeFoldable, TypeVisitable)] -pub struct SourceRegion { - pub start_line: u32, - pub start_col: u32, - pub end_line: u32, - pub end_col: u32, -} - -impl Debug for SourceRegion { - fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { - let &Self { start_line, start_col, end_line, end_col } = self; - write!(fmt, "{start_line}:{start_col} - {end_line}:{end_col}") - } -} - #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)] #[derive(TyEncodable, TyDecodable, TypeFoldable, TypeVisitable)] pub enum Op { @@ -231,7 +215,7 @@ impl MappingKind { #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct Mapping { pub kind: MappingKind, - pub source_region: SourceRegion, + pub span: Span, } /// Stores per-function coverage information attached to a `mir::Body`, diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 2bfcd0a6227..ece468947c2 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -603,8 +603,8 @@ fn write_function_coverage_info( for (id, expression) in expressions.iter_enumerated() { writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?; } - for coverage::Mapping { kind, source_region } in mappings { - writeln!(w, "{INDENT}coverage {kind:?} => {source_region:?};")?; + for coverage::Mapping { kind, span } in mappings { + writeln!(w, "{INDENT}coverage {kind:?} => {span:?};")?; } writeln!(w)?; diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 241c3c79114..83e7ff99639 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -13,16 +13,15 @@ use rustc_hir::intravisit::{Visitor, walk_expr}; use rustc_middle::hir::map::Map; use rustc_middle::hir::nested_filter; use rustc_middle::mir::coverage::{ - CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, SourceRegion, + CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, }; use rustc_middle::mir::{ self, BasicBlock, BasicBlockData, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, }; use rustc_middle::ty::TyCtxt; +use rustc_span::Span; use rustc_span::def_id::LocalDefId; -use rustc_span::source_map::SourceMap; -use rustc_span::{BytePos, Pos, SourceFile, Span}; use tracing::{debug, debug_span, trace}; use crate::coverage::counters::{CoverageCounters, Site}; @@ -97,7 +96,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: let coverage_counters = CoverageCounters::make_bcb_counters(&basic_coverage_blocks, &bcbs_with_counter_mappings); - let mappings = create_mappings(tcx, &hir_info, &extracted_mappings, &coverage_counters); + let mappings = create_mappings(&extracted_mappings, &coverage_counters); if mappings.is_empty() { // No spans could be converted into valid mappings, so skip this function. debug!("no spans could be converted into valid mappings; skipping"); @@ -136,18 +135,12 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: /// /// Precondition: All BCBs corresponding to those spans have been given /// coverage counters. -fn create_mappings<'tcx>( - tcx: TyCtxt<'tcx>, - hir_info: &ExtractedHirInfo, +fn create_mappings( extracted_mappings: &ExtractedMappings, coverage_counters: &CoverageCounters, ) -> Vec { - let source_map = tcx.sess.source_map(); - let file = source_map.lookup_source_file(hir_info.body_span.lo()); - let term_for_bcb = |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters"); - let region_for_span = |span: Span| make_source_region(source_map, hir_info, &file, span); // Fully destructure the mappings struct to make sure we don't miss any kinds. let ExtractedMappings { @@ -160,22 +153,20 @@ fn create_mappings<'tcx>( } = extracted_mappings; let mut mappings = Vec::new(); - mappings.extend(code_mappings.iter().filter_map( + mappings.extend(code_mappings.iter().map( // Ordinary code mappings are the simplest kind. |&mappings::CodeMapping { span, bcb }| { - let source_region = region_for_span(span)?; let kind = MappingKind::Code(term_for_bcb(bcb)); - Some(Mapping { kind, source_region }) + Mapping { kind, span } }, )); - mappings.extend(branch_pairs.iter().filter_map( + mappings.extend(branch_pairs.iter().map( |&mappings::BranchPair { span, true_bcb, false_bcb }| { let true_term = term_for_bcb(true_bcb); let false_term = term_for_bcb(false_bcb); let kind = MappingKind::Branch { true_term, false_term }; - let source_region = region_for_span(span)?; - Some(Mapping { kind, source_region }) + Mapping { kind, span } }, )); @@ -183,7 +174,7 @@ fn create_mappings<'tcx>( |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters"); // MCDC branch mappings are appended with their decisions in case decisions were ignored. - mappings.extend(mcdc_degraded_branches.iter().filter_map( + mappings.extend(mcdc_degraded_branches.iter().map( |&mappings::MCDCBranch { span, true_bcb, @@ -192,10 +183,9 @@ fn create_mappings<'tcx>( true_index: _, false_index: _, }| { - let source_region = region_for_span(span)?; let true_term = term_for_bcb(true_bcb); let false_term = term_for_bcb(false_bcb); - Some(Mapping { kind: MappingKind::Branch { true_term, false_term }, source_region }) + Mapping { kind: MappingKind::Branch { true_term, false_term }, span } }, )); @@ -203,7 +193,7 @@ fn create_mappings<'tcx>( let num_conditions = branches.len() as u16; let conditions = branches .into_iter() - .filter_map( + .map( |&mappings::MCDCBranch { span, true_bcb, @@ -212,32 +202,28 @@ fn create_mappings<'tcx>( true_index: _, false_index: _, }| { - let source_region = region_for_span(span)?; let true_term = term_for_bcb(true_bcb); let false_term = term_for_bcb(false_bcb); - Some(Mapping { + Mapping { kind: MappingKind::MCDCBranch { true_term, false_term, mcdc_params: condition_info, }, - source_region, - }) + span, + } }, ) .collect::>(); - if conditions.len() == num_conditions as usize - && let Some(source_region) = region_for_span(decision.span) - { + if conditions.len() == num_conditions as usize { // LLVM requires end index for counter mapping regions. let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx: (decision.bitmap_idx + decision.num_test_vectors) as u32, num_conditions, }); - mappings.extend( - std::iter::once(Mapping { kind, source_region }).chain(conditions.into_iter()), - ); + let span = decision.span; + mappings.extend(std::iter::once(Mapping { kind, span }).chain(conditions.into_iter())); } else { mappings.extend(conditions.into_iter().map(|mapping| { let MappingKind::MCDCBranch { true_term, false_term, mcdc_params: _ } = @@ -245,10 +231,7 @@ fn create_mappings<'tcx>( else { unreachable!("all mappings here are MCDCBranch as shown above"); }; - Mapping { - kind: MappingKind::Branch { true_term, false_term }, - source_region: mapping.source_region, - } + Mapping { kind: MappingKind::Branch { true_term, false_term }, span: mapping.span } })) } } @@ -391,121 +374,6 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb data.statements.insert(0, statement); } -fn ensure_non_empty_span( - source_map: &SourceMap, - hir_info: &ExtractedHirInfo, - span: Span, -) -> Option { - if !span.is_empty() { - return Some(span); - } - - let lo = span.lo(); - let hi = span.hi(); - - // The span is empty, so try to expand it to cover an adjacent '{' or '}', - // but only within the bounds of the body span. - let try_next = hi < hir_info.body_span.hi(); - let try_prev = hir_info.body_span.lo() < lo; - if !(try_next || try_prev) { - return None; - } - - source_map - .span_to_source(span, |src, start, end| try { - // We're only checking for specific ASCII characters, so we don't - // have to worry about multi-byte code points. - if try_next && src.as_bytes()[end] == b'{' { - Some(span.with_hi(hi + BytePos(1))) - } else if try_prev && src.as_bytes()[start - 1] == b'}' { - Some(span.with_lo(lo - BytePos(1))) - } else { - None - } - }) - .ok()? -} - -/// Converts the span into its start line and column, and end line and column. -/// -/// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by -/// the compiler, these column numbers are denoted in **bytes**, because that's what -/// LLVM's `llvm-cov` tool expects to see in coverage maps. -/// -/// Returns `None` if the conversion failed for some reason. This shouldn't happen, -/// but it's hard to rule out entirely (especially in the presence of complex macros -/// or other expansions), and if it does happen then skipping a span or function is -/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid. -fn make_source_region( - source_map: &SourceMap, - hir_info: &ExtractedHirInfo, - file: &SourceFile, - span: Span, -) -> Option { - let span = ensure_non_empty_span(source_map, hir_info, span)?; - - let lo = span.lo(); - let hi = span.hi(); - - // Column numbers need to be in bytes, so we can't use the more convenient - // `SourceMap` methods for looking up file coordinates. - let line_and_byte_column = |pos: BytePos| -> Option<(usize, usize)> { - let rpos = file.relative_position(pos); - let line_index = file.lookup_line(rpos)?; - let line_start = file.lines()[line_index]; - // Line numbers and column numbers are 1-based, so add 1 to each. - Some((line_index + 1, (rpos - line_start).to_usize() + 1)) - }; - - let (mut start_line, start_col) = line_and_byte_column(lo)?; - let (mut end_line, end_col) = line_and_byte_column(hi)?; - - // Apply an offset so that code in doctests has correct line numbers. - // FIXME(#79417): Currently we have no way to offset doctest _columns_. - start_line = source_map.doctest_offset_line(&file.name, start_line); - end_line = source_map.doctest_offset_line(&file.name, end_line); - - check_source_region(SourceRegion { - start_line: start_line as u32, - start_col: start_col as u32, - end_line: end_line as u32, - end_col: end_col as u32, - }) -} - -/// If `llvm-cov` sees a source region that is improperly ordered (end < start), -/// it will immediately exit with a fatal error. To prevent that from happening, -/// discard regions that are improperly ordered, or might be interpreted in a -/// way that makes them improperly ordered. -fn check_source_region(source_region: SourceRegion) -> Option { - let SourceRegion { start_line, start_col, end_line, end_col } = source_region; - - // Line/column coordinates are supposed to be 1-based. If we ever emit - // coordinates of 0, `llvm-cov` might misinterpret them. - let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0); - // Coverage mappings use the high bit of `end_col` to indicate that a - // region is actually a "gap" region, so make sure it's unset. - let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0; - // If a region is improperly ordered (end < start), `llvm-cov` will exit - // with a fatal error, which is inconvenient for users and hard to debug. - let is_ordered = (start_line, start_col) <= (end_line, end_col); - - if all_nonzero && end_col_has_high_bit_unset && is_ordered { - Some(source_region) - } else { - debug!( - ?source_region, - ?all_nonzero, - ?end_col_has_high_bit_unset, - ?is_ordered, - "Skipping source region that would be misinterpreted or rejected by LLVM" - ); - // If this happens in a debug build, ICE to make it easier to notice. - debug_assert!(false, "Improper source region: {source_region:?}"); - None - } -} - /// Function information extracted from HIR by the coverage instrumentor. #[derive(Debug)] struct ExtractedHirInfo { diff --git a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff index f0d128ec02e..69ef6016d25 100644 --- a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff @@ -30,12 +30,12 @@ + coverage ExpressionId(0) => Expression { lhs: Counter(1), op: Add, rhs: Counter(2) }; + coverage ExpressionId(1) => Expression { lhs: Expression(0), op: Add, rhs: Counter(3) }; + coverage ExpressionId(2) => Expression { lhs: Counter(0), op: Subtract, rhs: Expression(1) }; -+ coverage Code(Counter(0)) => 13:1 - 14:21; -+ coverage Code(Counter(1)) => 15:17 - 15:33; -+ coverage Code(Counter(2)) => 16:17 - 16:33; -+ coverage Code(Counter(3)) => 17:17 - 17:33; -+ coverage Code(Expression(2)) => 18:17 - 18:33; -+ coverage Code(Counter(0)) => 20:1 - 20:2; ++ coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:13:1: 14:21 (#0); ++ coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:15:17: 15:33 (#0); ++ coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:16:17: 16:33 (#0); ++ coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:17:17: 17:33 (#0); ++ coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:18:17: 18:33 (#0); ++ coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:20:2: 20:2 (#0); + bb0: { + Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff index 2efb1fd0a17..148ff86354b 100644 --- a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff @@ -5,7 +5,7 @@ let mut _0: bool; + coverage body span: $DIR/instrument_coverage.rs:19:18: 21:2 (#0) -+ coverage Code(Counter(0)) => 19:1 - 21:2; ++ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:19:1: 21:2 (#0); + bb0: { + Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff index a179824d6c7..b480d1ac13a 100644 --- a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff @@ -9,11 +9,11 @@ + coverage body span: $DIR/instrument_coverage.rs:10:11: 16:2 (#0) + coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Add, rhs: Counter(1) }; -+ coverage Code(Counter(0)) => 10:1 - 10:11; -+ coverage Code(Expression(0)) => 12:12 - 12:17; -+ coverage Code(Counter(0)) => 13:13 - 13:18; -+ coverage Code(Counter(1)) => 14:9 - 14:10; -+ coverage Code(Counter(0)) => 16:1 - 16:2; ++ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:10:1: 10:11 (#0); ++ coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:12:12: 12:17 (#0); ++ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:13:13: 13:18 (#0); ++ coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:14:10: 14:10 (#0); ++ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:16:2: 16:2 (#0); + bb0: { + Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff index 082539369f7..2c7ec6e85eb 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff @@ -9,11 +9,11 @@ coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#0) coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) }; - coverage Code(Counter(0)) => 13:1 - 14:36; - coverage Code(Expression(0)) => 14:37 - 14:39; - coverage Code(Counter(1)) => 14:38 - 14:39; - coverage Code(Counter(0)) => 15:1 - 15:2; - coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36; + coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0); + coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); + coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); + coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); + coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); bb0: { Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff index 8635818c6a7..c08265eb0e9 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff @@ -9,11 +9,11 @@ + coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#0) + coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) }; -+ coverage Code(Counter(0)) => 13:1 - 14:36; -+ coverage Code(Expression(0)) => 14:37 - 14:39; -+ coverage Code(Counter(1)) => 14:38 - 14:39; -+ coverage Code(Counter(0)) => 15:1 - 15:2; -+ coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36; ++ coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0); ++ coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); ++ coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); ++ coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); ++ coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); + bb0: { + Coverage::CounterIncrement(0); -- cgit 1.4.1-3-g733a5 From 837a25dd41a1cc58cee093159a01f73719f330e8 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 18 Dec 2024 21:19:30 +1100 Subject: coverage: Identify source files by ID, not by interned filename --- .../src/coverageinfo/llvm_cov.rs | 5 +- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 57 +++++++++++----------- .../src/coverageinfo/mapgen/covfun.rs | 8 ++- 3 files changed, 33 insertions(+), 37 deletions(-) (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs index 086cf1f44a0..2cd7fa3225a 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs @@ -40,11 +40,10 @@ pub(crate) fn create_pgo_func_name_var<'ll>( } } -pub(crate) fn write_filenames_to_buffer<'a>( - filenames: impl IntoIterator, -) -> Vec { +pub(crate) fn write_filenames_to_buffer(filenames: &[impl AsRef]) -> Vec { let (pointers, lengths) = filenames .into_iter() + .map(AsRef::as_ref) .map(|s: &str| (s.as_c_char_ptr(), s.len())) .unzip::<_, _, Vec<_>, Vec<_>>(); diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 7d3bb4f93f6..b3ad2a0e409 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -1,11 +1,11 @@ -use std::iter; +use std::sync::Arc; use itertools::Itertools; use rustc_abi::Align; use rustc_codegen_ssa::traits::{ BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods, }; -use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; +use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_index::IndexVec; use rustc_middle::mir; @@ -13,7 +13,7 @@ use rustc_middle::ty::{self, TyCtxt}; use rustc_session::RemapFileNameExt; use rustc_session::config::RemapPathScopeComponents; use rustc_span::def_id::DefIdSet; -use rustc_span::{Span, Symbol}; +use rustc_span::{SourceFile, StableSourceFileId}; use tracing::debug; use crate::common::CodegenCx; @@ -132,45 +132,51 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { generate_covmap_record(cx, covmap_version, &filenames_buffer); } -/// Maps "global" (per-CGU) file ID numbers to their underlying filenames. +/// Maps "global" (per-CGU) file ID numbers to their underlying source files. struct GlobalFileTable { - /// This "raw" table doesn't include the working dir, so a filename's + /// This "raw" table doesn't include the working dir, so a file's /// global ID is its index in this set **plus one**. - raw_file_table: FxIndexSet, + raw_file_table: FxIndexMap>, } impl GlobalFileTable { fn new() -> Self { - Self { raw_file_table: FxIndexSet::default() } + Self { raw_file_table: FxIndexMap::default() } } - fn global_file_id_for_file_name(&mut self, file_name: Symbol) -> GlobalFileId { + fn global_file_id_for_file(&mut self, file: &Arc) -> GlobalFileId { // Ensure the given file has a table entry, and get its index. - let (raw_id, _) = self.raw_file_table.insert_full(file_name); + let entry = self.raw_file_table.entry(file.stable_id); + let raw_id = entry.index(); + entry.or_insert_with(|| Arc::clone(file)); + // The raw file table doesn't include an entry for the working dir // (which has ID 0), so add 1 to get the correct ID. GlobalFileId::from_usize(raw_id + 1) } fn make_filenames_buffer(&self, tcx: TyCtxt<'_>) -> Vec { + let mut table = Vec::with_capacity(self.raw_file_table.len() + 1); + // LLVM Coverage Mapping Format version 6 (zero-based encoded as 5) // requires setting the first filename to the compilation directory. // Since rustc generates coverage maps with relative paths, the // compilation directory can be combined with the relative paths // to get absolute paths, if needed. - use rustc_session::RemapFileNameExt; - use rustc_session::config::RemapPathScopeComponents; - let working_dir: &str = &tcx - .sess - .opts - .working_dir - .for_scope(tcx.sess, RemapPathScopeComponents::MACRO) - .to_string_lossy(); - - // Insert the working dir at index 0, before the other filenames. - let filenames = - iter::once(working_dir).chain(self.raw_file_table.iter().map(Symbol::as_str)); - llvm_cov::write_filenames_to_buffer(filenames) + table.push( + tcx.sess + .opts + .working_dir + .for_scope(tcx.sess, RemapPathScopeComponents::MACRO) + .to_string_lossy(), + ); + + // Add the regular entries after the base directory. + table.extend(self.raw_file_table.values().map(|file| { + file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy() + })); + + llvm_cov::write_filenames_to_buffer(&table) } } @@ -209,13 +215,6 @@ impl VirtualFileMapping { } } -fn span_file_name(tcx: TyCtxt<'_>, span: Span) -> Symbol { - let source_file = tcx.sess.source_map().lookup_source_file(span.lo()); - let name = - source_file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy(); - Symbol::intern(&name) -} - /// Generates the contents of the covmap record for this CGU, which mostly /// consists of a header and a list of filenames. The record is then stored /// as a global variable in the `__llvm_covmap` section. diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index cde7a0ebc6d..6ce6626f290 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -19,7 +19,7 @@ use rustc_target::spec::HasTargetSpec; use tracing::debug; use crate::common::CodegenCx; -use crate::coverageinfo::mapgen::{GlobalFileTable, VirtualFileMapping, span_file_name, spans}; +use crate::coverageinfo::mapgen::{GlobalFileTable, VirtualFileMapping, spans}; use crate::coverageinfo::{ffi, llvm_cov}; use crate::llvm; @@ -117,16 +117,14 @@ fn fill_region_tables<'tcx>( covfun: &mut CovfunRecord<'tcx>, ) { // Currently a function's mappings must all be in the same file as its body span. - let file_name = span_file_name(tcx, fn_cov_info.body_span); let source_map = tcx.sess.source_map(); let source_file = source_map.lookup_source_file(fn_cov_info.body_span.lo()); - // Look up the global file ID for that filename. - let global_file_id = global_file_table.global_file_id_for_file_name(file_name); + // Look up the global file ID for that file. + let global_file_id = global_file_table.global_file_id_for_file(&source_file); // Associate that global file ID with a local file ID for this function. let local_file_id = covfun.virtual_file_mapping.local_id_for_global(global_file_id); - debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'"); let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } = &mut covfun.regions; -- cgit 1.4.1-3-g733a5 From aced4dcf1047aab08b769b34fae9d4ba7de87f04 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 19 Dec 2024 21:53:10 +1100 Subject: coverage: Add a synthetic test for when all spans are discarded --- .../src/coverageinfo/mapgen/covfun.rs | 11 ++++++++++ compiler/rustc_interface/src/tests.rs | 6 +++++- compiler/rustc_session/src/config.rs | 10 +++++++-- compiler/rustc_session/src/options.rs | 1 + compiler/rustc_session/src/session.rs | 5 +++++ tests/coverage/auxiliary/discard_all_helper.rs | 6 ++++++ tests/coverage/discard-all-issue-133606.coverage | 7 +++++++ tests/coverage/discard-all-issue-133606.rs | 24 ++++++++++++++++++++++ 8 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 tests/coverage/auxiliary/discard_all_helper.rs create mode 100644 tests/coverage/discard-all-issue-133606.coverage create mode 100644 tests/coverage/discard-all-issue-133606.rs (limited to 'compiler/rustc_codegen_llvm/src/coverageinfo') diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index 6ce6626f290..5428d776f41 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -132,6 +132,7 @@ fn fill_region_tables<'tcx>( let make_cov_span = |span: Span| { spans::make_coverage_span(local_file_id, source_map, fn_cov_info, &source_file, span) }; + let discard_all = tcx.sess.coverage_discard_all_spans_in_codegen(); // For each counter/region pair in this function+file, convert it to a // form suitable for FFI. @@ -141,7 +142,17 @@ fn fill_region_tables<'tcx>( // MIR opts, replace those occurrences with zero. let kind = kind.map_terms(|term| if is_zero_term(term) { CovTerm::Zero } else { term }); + // Convert the `Span` into coordinates that we can pass to LLVM, or + // discard the span if conversion fails. In rare, cases _all_ of a + // function's spans are discarded, and the rest of coverage codegen + // needs to handle that gracefully to avoid a repeat of #133606. + // We don't have a good test case for triggering that organically, so + // instead we set `-Zcoverage-options=discard-all-spans-in-codegen` + // to force it to occur. let Some(cov_span) = make_cov_span(span) else { continue }; + if discard_all { + continue; + } match kind { MappingKind::Code(term) => { diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index d103f7f45e2..9ad69039914 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -766,7 +766,11 @@ fn test_unstable_options_tracking_hash() { }) ); tracked!(codegen_backend, Some("abc".to_string())); - tracked!(coverage_options, CoverageOptions { level: CoverageLevel::Mcdc, no_mir_spans: true }); + tracked!(coverage_options, CoverageOptions { + level: CoverageLevel::Mcdc, + no_mir_spans: true, + discard_all_spans_in_codegen: true + }); tracked!(crate_attr, vec!["abc".to_string()]); tracked!(cross_crate_inline_threshold, InliningThreshold::Always); tracked!(debug_info_for_profiling, true); diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 4784a4d1953..047e920e688 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -147,18 +147,24 @@ pub enum InstrumentCoverage { Yes, } -/// Individual flag values controlled by `-Z coverage-options`. +/// Individual flag values controlled by `-Zcoverage-options`. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Default)] pub struct CoverageOptions { pub level: CoverageLevel, - /// `-Z coverage-options=no-mir-spans`: Don't extract block coverage spans + /// `-Zcoverage-options=no-mir-spans`: Don't extract block coverage spans /// from MIR statements/terminators, making it easier to inspect/debug /// branch and MC/DC coverage mappings. /// /// For internal debugging only. If other code changes would make it hard /// to keep supporting this flag, remove it. pub no_mir_spans: bool, + + /// `-Zcoverage-options=discard-all-spans-in-codegen`: During codgen, + /// discard all coverage spans as though they were invalid. Needed by + /// regression tests for #133606, because we don't have an easy way to + /// reproduce it from actual source code. + pub discard_all_spans_in_codegen: bool, } /// Controls whether branch coverage or MC/DC coverage is enabled. diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 3873a203bd3..3772a4a08af 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1037,6 +1037,7 @@ pub mod parse { "condition" => slot.level = CoverageLevel::Condition, "mcdc" => slot.level = CoverageLevel::Mcdc, "no-mir-spans" => slot.no_mir_spans = true, + "discard-all-spans-in-codegen" => slot.discard_all_spans_in_codegen = true, _ => return false, } } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 9f6106f9cfb..54bb4622963 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -352,6 +352,11 @@ impl Session { self.opts.unstable_opts.coverage_options.no_mir_spans } + /// True if `-Zcoverage-options=discard-all-spans-in-codegen` was passed. + pub fn coverage_discard_all_spans_in_codegen(&self) -> bool { + self.opts.unstable_opts.coverage_options.discard_all_spans_in_codegen + } + pub fn is_sanitizer_cfi_enabled(&self) -> bool { self.opts.unstable_opts.sanitizer.contains(SanitizerSet::CFI) } diff --git a/tests/coverage/auxiliary/discard_all_helper.rs b/tests/coverage/auxiliary/discard_all_helper.rs new file mode 100644 index 00000000000..9290bac7e53 --- /dev/null +++ b/tests/coverage/auxiliary/discard_all_helper.rs @@ -0,0 +1,6 @@ +//@ edition: 2021 + +// Force this function to be generated in its home crate, so that it ends up +// with normal coverage metadata. +#[inline(never)] +pub fn external_function() {} diff --git a/tests/coverage/discard-all-issue-133606.coverage b/tests/coverage/discard-all-issue-133606.coverage new file mode 100644 index 00000000000..8f5a32f6d70 --- /dev/null +++ b/tests/coverage/discard-all-issue-133606.coverage @@ -0,0 +1,7 @@ + LL| |//@ edition: 2021 + LL| | + LL| |// Force this function to be generated in its home crate, so that it ends up + LL| |// with normal coverage metadata. + LL| |#[inline(never)] + LL| 1|pub fn external_function() {} + diff --git a/tests/coverage/discard-all-issue-133606.rs b/tests/coverage/discard-all-issue-133606.rs new file mode 100644 index 00000000000..91c278b2bb8 --- /dev/null +++ b/tests/coverage/discard-all-issue-133606.rs @@ -0,0 +1,24 @@ +//! Regression test for . +//! +//! In rare cases, all of a function's coverage spans are discarded at a late +//! stage during codegen. When that happens, the subsequent code needs to take +//! special care to avoid emitting coverage metadata that would cause `llvm-cov` +//! to fail with a fatal error. +//! +//! We currently don't know of a concise way to reproduce that scenario with +//! ordinary Rust source code, so instead we set a special testing-only flag to +//! force it to occur. + +//@ edition: 2021 +//@ compile-flags: -Zcoverage-options=discard-all-spans-in-codegen + +// The `llvm-cov` tool will complain if the test binary ends up having no +// coverage metadata at all. To prevent that, we also link to instrumented +// code in an auxiliary crate that doesn't have the special flag set. + +//@ aux-build: discard_all_helper.rs +extern crate discard_all_helper; + +fn main() { + discard_all_helper::external_function(); +} -- cgit 1.4.1-3-g733a5