From 7c4ac71ad1bef9b1e4cb5a3fd10d0b2ef7b418c4 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 11 Dec 2024 14:28:55 +1100 Subject: [PATCH 1/4] coverage: Extract function metadata handling to a `covfun` submodule --- .../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 diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index a6c3caf9e2b51..5da7848b39c09 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 0000000000000..aaa6bbcdfc914 --- /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); +} From 6a8c016266a6b7514ff8284dc6d8b056e34b9399 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 11 Dec 2024 15:03:31 +1100 Subject: [PATCH 2/4] coverage: Reify `CovfunRecord` as an intermediate step --- .../src/coverageinfo/mapgen.rs | 22 +++---- .../src/coverageinfo/mapgen/covfun.rs | 59 +++++++++++-------- 2 files changed, 45 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 5da7848b39c09..39fb0aeed0714 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 aaa6bbcdfc914..a68b026a4c06e 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); From 2393a06b449ee41c06b6051561ff44c8a5be9a1f Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 11 Dec 2024 15:12:23 +1100 Subject: [PATCH 3/4] coverage: Only generate a CGU's covmap record if it has covfun records --- .../rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 14 ++++++++++---- tests/codegen/instrument-coverage/testprog.rs | 6 +++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 39fb0aeed0714..3037672801dee 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,12 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { }) .collect::>(); + // If there are no covfun records for this CGU, don't generate a covmap record. + // 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 +119,10 @@ 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. + generate_covmap_record(cx, covmap_version, filenames_size, filenames_val); } /// Maps "global" (per-CGU) file ID numbers to their underlying filenames. diff --git a/tests/codegen/instrument-coverage/testprog.rs b/tests/codegen/instrument-coverage/testprog.rs index 655fe779fca6c..aec526122b8e0 100644 --- a/tests/codegen/instrument-coverage/testprog.rs +++ b/tests/codegen/instrument-coverage/testprog.rs @@ -73,12 +73,12 @@ fn main() { // WIN: $__llvm_profile_runtime_user = comdat any -// CHECK: @__llvm_coverage_mapping = private constant -// CHECK-SAME: section "[[INSTR_PROF_COVMAP]]", align 8 - // CHECK: @__covrec_{{[A-F0-9]+}}u = linkonce_odr hidden constant // CHECK-SAME: section "[[INSTR_PROF_COVFUN]]"[[COMDAT_IF_SUPPORTED]], align 8 +// CHECK: @__llvm_coverage_mapping = private constant +// CHECK-SAME: section "[[INSTR_PROF_COVMAP]]", align 8 + // WIN: @__llvm_profile_runtime = external{{.*}}global i32 // CHECK: @__profc__R{{[a-zA-Z0-9_]+}}testprog14will_be_called = {{private|internal}} global From ec7eb7ed3828b94eafd1d6fe03e1093e9b669e67 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 11 Dec 2024 15:41:02 +1100 Subject: [PATCH 4/4] 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. --- .../src/coverageinfo/ffi.rs | 28 ++++++ .../src/coverageinfo/llvm_cov.rs | 7 +- .../src/coverageinfo/mapgen.rs | 8 +- .../src/coverageinfo/mapgen/covfun.rs | 87 +++++++++---------- 4 files changed, 75 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index a6e07ea2a60ec..1f133141c18fd 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 99c2d12b2612d..086cf1f44a03e 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 3037672801dee..47e1131452f53 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -190,7 +190,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, @@ -204,10 +204,8 @@ 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 { + 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 a68b026a4c06e..530e6827f55d3 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);