diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index feac97f3e2b..a73ee0a3e07 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -138,7 +138,7 @@ pub(crate) struct CoverageSpan { impl CoverageSpan { pub(crate) fn from_source_region(file_id: u32, code_region: &SourceRegion) -> Self { - let &SourceRegion { file_name: _, start_line, start_col, end_line, end_col } = code_region; + 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}"); diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index 5ed640b840e..e3c0df27883 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -6,7 +6,6 @@ SourceRegion, }; use rustc_middle::ty::Instance; -use rustc_span::Symbol; use tracing::{debug, instrument}; use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind}; @@ -180,7 +179,7 @@ pub(crate) fn into_finished(self) -> FunctionCoverage<'tcx> { } pub(crate) struct FunctionCoverage<'tcx> { - function_coverage_info: &'tcx FunctionCoverageInfo, + pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo, is_used: bool, counters_seen: BitSet, @@ -199,11 +198,6 @@ pub(crate) fn source_hash(&self) -> u64 { if self.is_used { self.function_coverage_info.function_source_hash } else { 0 } } - /// Returns an iterator over all filenames used by this function's mappings. - pub(crate) fn all_file_names(&self) -> impl Iterator + Captures<'_> { - self.function_coverage_info.mappings.iter().map(|mapping| mapping.source_region.file_name) - } - /// Convert this function's coverage expression data into a form that can be /// passed through FFI to LLVM. pub(crate) fn counter_expressions( diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 9effcaede1c..488ce620746 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -11,8 +11,10 @@ use rustc_middle::mir::coverage::MappingKind; use rustc_middle::ty::{self, TyCtxt}; use rustc_middle::{bug, mir}; -use rustc_span::Symbol; +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; @@ -69,8 +71,10 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { .map(|(instance, function_coverage)| (instance, function_coverage.into_finished())) .collect::>(); - let all_file_names = - function_coverage_entries.iter().flat_map(|(_, fn_cov)| fn_cov.all_file_names()); + let all_file_names = function_coverage_entries + .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. @@ -95,7 +99,7 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { let is_used = function_coverage.is_used(); let coverage_mapping_buffer = - encode_mappings_for_function(&global_file_table, &function_coverage); + encode_mappings_for_function(tcx, &global_file_table, &function_coverage); if coverage_mapping_buffer.is_empty() { if function_coverage.is_used() { @@ -232,12 +236,20 @@ fn into_vec(self) -> Vec { } } +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) +} + /// 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 { @@ -254,53 +266,45 @@ fn encode_mappings_for_function( let mut mcdc_branch_regions = vec![]; let mut mcdc_decision_regions = vec![]; - // Group mappings into runs with the same filename, preserving the order - // yielded by `FunctionCoverage`. - // Prepare file IDs for each filename, and prepare the mapping data so that - // we can pass it through FFI to LLVM. - for (file_name, counter_regions_for_file) in - &counter_regions.group_by(|(_, region)| region.file_name) - { - // Look up the global file ID for this filename. - let global_file_id = global_file_table.global_file_id_for_file_name(file_name); + // 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); - // 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:?}'"); + // Look up the global file ID for that filename. + let global_file_id = global_file_table.global_file_id_for_file_name(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_for_file { - debug!("Adding counter {mapping_kind:?} to map for {region:?}"); - let span = ffi::CoverageSpan::from_source_region(local_file_id.as_u32(), 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, - ), - }); - } + // 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.as_u32(), 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), + }); } } } diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 4a876dc1228..11a4e7f89a7 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -4,7 +4,7 @@ use rustc_index::IndexVec; use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable}; -use rustc_span::{Span, Symbol}; +use rustc_span::Span; rustc_index::newtype_index! { /// Used by [`CoverageKind::BlockMarker`] to mark blocks during THIR-to-MIR @@ -158,7 +158,6 @@ fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { #[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq, Eq, PartialOrd, Ord)] #[derive(TypeFoldable, TypeVisitable)] pub struct SourceRegion { - pub file_name: Symbol, pub start_line: u32, pub start_col: u32, pub end_line: u32, @@ -167,11 +166,8 @@ pub struct SourceRegion { impl Debug for SourceRegion { fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { - write!( - fmt, - "{}:{}:{} - {}:{}", - self.file_name, self.start_line, self.start_col, self.end_line, self.end_col - ) + let &Self { start_line, start_col, end_line, end_col } = self; + write!(fmt, "{start_line}:{start_col} - {end_line}:{end_col}") } } @@ -246,6 +242,7 @@ pub struct Mapping { #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct FunctionCoverageInfo { pub function_source_hash: u64, + pub body_span: Span, pub num_counters: usize, pub mcdc_bitmap_bits: usize, pub expressions: IndexVec, diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index d0f93c19e44..f0e2b7a376c 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -596,8 +596,10 @@ fn write_function_coverage_info( function_coverage_info: &coverage::FunctionCoverageInfo, w: &mut dyn io::Write, ) -> io::Result<()> { - let coverage::FunctionCoverageInfo { expressions, mappings, .. } = function_coverage_info; + let coverage::FunctionCoverageInfo { body_span, expressions, mappings, .. } = + function_coverage_info; + writeln!(w, "{INDENT}coverage body span: {body_span:?}")?; for (id, expression) in expressions.iter_enumerated() { writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?; } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index bc698e9ebf6..39ad1fd23b2 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -22,7 +22,7 @@ use rustc_middle::ty::TyCtxt; use rustc_span::def_id::LocalDefId; use rustc_span::source_map::SourceMap; -use rustc_span::{BytePos, Pos, RelativeBytePos, Span, Symbol}; +use rustc_span::{BytePos, Pos, RelativeBytePos, SourceFile, Span}; use tracing::{debug, debug_span, trace}; use crate::coverage::counters::{CounterIncrementSite, CoverageCounters}; @@ -122,6 +122,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { function_source_hash: hir_info.function_source_hash, + body_span: hir_info.body_span, num_counters: coverage_counters.num_counters(), mcdc_bitmap_bits: extracted_mappings.mcdc_bitmap_bits, expressions: coverage_counters.into_expressions(), @@ -142,19 +143,11 @@ fn create_mappings<'tcx>( coverage_counters: &CoverageCounters, ) -> Vec { let source_map = tcx.sess.source_map(); - let body_span = hir_info.body_span; - - let source_file = source_map.lookup_source_file(body_span.lo()); - - use rustc_session::RemapFileNameExt; - use rustc_session::config::RemapPathScopeComponents; - let file_name = Symbol::intern( - &source_file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy(), - ); + 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_name, span); + 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 { @@ -398,7 +391,7 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb data.statements.insert(0, statement); } -/// Convert the Span into its file name, start line and column, and end line and column. +/// 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 @@ -411,18 +404,12 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb fn make_source_region( source_map: &SourceMap, hir_info: &ExtractedHirInfo, - file_name: Symbol, + file: &SourceFile, span: Span, ) -> Option { let lo = span.lo(); let hi = span.hi(); - let file = source_map.lookup_source_file(lo); - if !file.contains(hi) { - debug!(?span, ?file, ?lo, ?hi, "span crosses multiple files; skipping"); - return None; - } - // Column numbers need to be in bytes, so we can't use the more convenient // `SourceMap` methods for looking up file coordinates. let rpos_and_line_and_byte_column = |pos: BytePos| -> Option<(RelativeBytePos, usize, usize)> { @@ -465,7 +452,6 @@ fn make_source_region( end_line = source_map.doctest_offset_line(&file.name, end_line); check_source_region(SourceRegion { - file_name, start_line: start_line as u32, start_col: start_col as u32, end_line: end_line as u32, @@ -478,7 +464,7 @@ fn make_source_region( /// 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 { file_name: _, start_line, start_col, end_line, end_col } = source_region; + 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. 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 eeeac70b5b8..cbb11d50f79 100644 --- a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff @@ -26,18 +26,19 @@ debug a => _9; } ++ coverage body span: $DIR/branch_match_arms.rs:14:11: 21:2 (#0) + 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 ExpressionId(3) => Expression { lhs: Counter(3), op: Add, rhs: Counter(2) }; + coverage ExpressionId(4) => Expression { lhs: Expression(3), op: Add, rhs: Counter(1) }; + coverage ExpressionId(5) => Expression { lhs: Expression(4), op: Add, rhs: Expression(2) }; -+ coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:14:1 - 15:21; -+ coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:16:17 - 16:33; -+ coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:17:17 - 17:33; -+ coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:18:17 - 18:33; -+ coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:19:17 - 19:33; -+ coverage Code(Expression(5)) => $DIR/branch_match_arms.rs:21:1 - 21:2; ++ coverage Code(Counter(0)) => 14:1 - 15:21; ++ coverage Code(Counter(3)) => 16:17 - 16:33; ++ coverage Code(Counter(2)) => 17:17 - 17:33; ++ coverage Code(Counter(1)) => 18:17 - 18:33; ++ coverage Code(Expression(2)) => 19:17 - 19:33; ++ coverage Code(Expression(5)) => 21:1 - 21:2; + 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 01b01ea5c17..2efb1fd0a17 100644 --- a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff @@ -4,7 +4,8 @@ fn bar() -> bool { let mut _0: bool; -+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:19:1 - 21:2; ++ coverage body span: $DIR/instrument_coverage.rs:19:18: 21:2 (#0) ++ coverage Code(Counter(0)) => 19:1 - 21:2; + 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 a594c44c316..510aa07f32d 100644 --- a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff @@ -7,12 +7,13 @@ let mut _2: bool; let mut _3: !; ++ 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)) => $DIR/instrument_coverage.rs:10:1 - 10:11; -+ coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:12:12 - 12:17; -+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:13:13 - 13:18; -+ coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:14:10 - 14:11; -+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:16:1 - 16:2; ++ 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:10 - 14:11; ++ coverage Code(Counter(0)) => 16:1 - 16:2; + 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 efb1559baf5..23cdb3d7717 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff @@ -7,12 +7,13 @@ coverage branch { true: BlockMarkerId(0), false: BlockMarkerId(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0) + 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)) => $DIR/instrument_coverage_cleanup.rs:13:1 - 14:36; - coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37 - 14:39; - coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39 - 14:40; - coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:1 - 15:2; - coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8 - 14:36; + coverage Code(Counter(0)) => 13:1 - 14:36; + coverage Code(Expression(0)) => 14:37 - 14:39; + coverage Code(Counter(1)) => 14:39 - 14:40; + coverage Code(Counter(0)) => 15:1 - 15:2; + coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36; 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 a0fe9a5c05c..f6aa42f3eee 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff @@ -7,12 +7,13 @@ coverage branch { true: BlockMarkerId(0), false: BlockMarkerId(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0) ++ 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)) => $DIR/instrument_coverage_cleanup.rs:13:1 - 14:36; -+ coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37 - 14:39; -+ coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39 - 14:40; -+ coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:1 - 15:2; -+ coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8 - 14:36; ++ coverage Code(Counter(0)) => 13:1 - 14:36; ++ coverage Code(Expression(0)) => 14:37 - 14:39; ++ coverage Code(Counter(1)) => 14:39 - 14:40; ++ coverage Code(Counter(0)) => 15:1 - 15:2; ++ coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36; + bb0: { + Coverage::CounterIncrement(0);