coverage: Remove unhelpful code for handling multiple files per function

Functions currently can't have mappings in multiple files, and if that ever
changes (e.g. to properly support expansion regions), this code will need to be
completely overhauled anyway.
This commit is contained in:
Zalathar 2024-11-04 14:53:52 +11:00
parent 3f9c54caf0
commit 996bdabc2a
11 changed files with 96 additions and 108 deletions

View File

@ -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}");

View File

@ -6,7 +6,6 @@ use rustc_middle::mir::coverage::{
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 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
}
pub(crate) struct FunctionCoverage<'tcx> {
function_coverage_info: &'tcx FunctionCoverageInfo,
pub(crate) function_coverage_info: &'tcx FunctionCoverageInfo,
is_used: bool,
counters_seen: BitSet<CounterId>,
@ -199,11 +198,6 @@ impl<'tcx> FunctionCoverage<'tcx> {
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<Item = Symbol> + 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(

View File

@ -11,8 +11,10 @@ use rustc_index::IndexVec;
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::<Vec<_>>();
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 @@ 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)
}
/// 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<u8> {
@ -254,14 +266,10 @@ 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.
// 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.
@ -270,13 +278,12 @@ fn encode_mappings_for_function(
// 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 {
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) });
code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) });
}
MappingKind::Branch { true_term, false_term } => {
branch_regions.push(ffi::BranchRegion {
@ -296,14 +303,11 @@ fn encode_mappings_for_function(
MappingKind::MCDCDecision(mcdc_decision_params) => {
mcdc_decision_regions.push(ffi::MCDCDecisionRegion {
span,
mcdc_decision_params: ffi::mcdc::DecisionParameters::from(
mcdc_decision_params,
),
mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params),
});
}
}
}
}
// Encode the function's coverage mappings into a buffer.
llvm::build_byte_buffer(|buffer| {

View File

@ -4,7 +4,7 @@ use std::fmt::{self, Debug, Formatter};
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 @@ impl Debug for CoverageKind {
#[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<ExpressionId, Expression>,

View File

@ -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:?};")?;
}

View File

@ -22,7 +22,7 @@ use rustc_middle::mir::{
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<Mapping> {
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<SourceRegion> {
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<SourceRegion> {
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.

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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);