From d416cead5a92d6a054d0e4d18a4b3a965f35334d Mon Sep 17 00:00:00 2001
From: Zalathar <Zalathar@users.noreply.github.com>
Date: Wed, 18 Dec 2024 18:59:27 +1100
Subject: [PATCH 1/5] 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 ++++----
.../src/coverageinfo/mapgen/covfun.rs | 11 ++++++-----
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
index 1f133141c18fd..4cad09b3a2d26 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 8e853f057bedf..f9e9555e8bd70 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),
});
}
From c3780e1d227d7544209e2cd722ac4fa35ac61b56 Mon Sep 17 00:00:00 2001
From: Zalathar <Zalathar@users.noreply.github.com>
Date: Wed, 18 Dec 2024 21:23:48 +1100
Subject: [PATCH 2/5] 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.
---
.../rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs
index f9e9555e8bd70..dca1779d82148 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)
From 34ed51cb837d1dd755b9b97798a41c8cd0358ca6 Mon Sep 17 00:00:00 2001
From: Zalathar <Zalathar@users.noreply.github.com>
Date: Wed, 18 Dec 2024 21:00:51 +1100
Subject: [PATCH 3/5] coverage: Store coverage source regions as `Span` until
codegen
---
.../src/coverageinfo/ffi.rs | 28 +--
.../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 +-
.../rustc_mir_transform/src/coverage/mod.rs | 168 ++----------------
...ch_match_arms.main.InstrumentCoverage.diff | 12 +-
...ument_coverage.bar.InstrumentCoverage.diff | 2 +-
...ment_coverage.main.InstrumentCoverage.diff | 10 +-
...rage_cleanup.main.CleanupPostBorrowck.diff | 10 +-
...erage_cleanup.main.InstrumentCoverage.diff | 10 +-
13 files changed, 189 insertions(+), 217 deletions(-)
create mode 100644 compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
index 4cad09b3a2d26..b617f4d37f5bf 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 ca33428620052..7d3bb4f93f6f6 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 dca1779d82148..cde7a0ebc6d60 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 0000000000000..6d1d91340c295
--- /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<ffi::CoverageSpan> {
+ 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<Span> {
+ 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<ffi::CoverageSpan> {
+ 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 b079eb8fe0cc2..0de0c6a7a89ec 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 962176290df32..29f26180c973f 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 2bfcd0a62274d..ece468947c25d 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 241c3c79114ac..83e7ff99639d1 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<Mapping> {
- 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::<Vec<_>>();
- 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<Span> {
- 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<SourceRegion> {
- 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<SourceRegion> {
- 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 f0d128ec02ee0..69ef6016d2580 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 2efb1fd0a17a3..148ff86354b57 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 a179824d6c736..b480d1ac13a76 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 082539369f707..2c7ec6e85eb0a 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 8635818c6a7a1..c08265eb0e953 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);
From 837a25dd41a1cc58cee093159a01f73719f330e8 Mon Sep 17 00:00:00 2001
From: Zalathar <Zalathar@users.noreply.github.com>
Date: Wed, 18 Dec 2024 21:19:30 +1100
Subject: [PATCH 4/5] coverage: Identify source files by ID, not by interned
filename
---
.../src/coverageinfo/llvm_cov.rs | 5 +-
.../src/coverageinfo/mapgen.rs | 57 +++++++++----------
.../src/coverageinfo/mapgen/covfun.rs | 8 +--
3 files changed, 33 insertions(+), 37 deletions(-)
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs
index 086cf1f44a03e..2cd7fa3225acd 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<Item = &'a str>,
-) -> Vec<u8> {
+pub(crate) fn write_filenames_to_buffer(filenames: &[impl AsRef<str>]) -> Vec<u8> {
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 7d3bb4f93f6f6..b3ad2a0e4098f 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<Symbol>,
+ raw_file_table: FxIndexMap<StableSourceFileId, Arc<SourceFile>>,
}
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<SourceFile>) -> 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<u8> {
+ 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 cde7a0ebc6d60..6ce6626f290bf 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;
From aced4dcf1047aab08b769b34fae9d4ba7de87f04 Mon Sep 17 00:00:00 2001
From: Zalathar <Zalathar@users.noreply.github.com>
Date: Thu, 19 Dec 2024 21:53:10 +1100
Subject: [PATCH 5/5] 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 ++++
.../coverage/auxiliary/discard_all_helper.rs | 6 +++++
.../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
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs
index 6ce6626f290bf..5428d776f41e8 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 d103f7f45e26c..9ad690399140e 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 4784a4d1953df..047e920e6883b 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 3873a203bd36c..3772a4a08af01 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 9f6106f9cfb7e..54bb462296358 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 0000000000000..9290bac7e535e
--- /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 0000000000000..8f5a32f6d7080
--- /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 0000000000000..91c278b2bb84a
--- /dev/null
+++ b/tests/coverage/discard-all-issue-133606.rs
@@ -0,0 +1,24 @@
+//! Regression test for <https://github.com/rust-lang/rust/issues/133606>.
+//!
+//! 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();
+}