bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Revert "Rollup merge of #133418 - Zalathar:spans, r=jieyouxu"

Browse source 
This reverts commit adf9b5fcd1, reversing
changes made to af1ca153d4.

Reverting due to <https://github.com/rust-lang/rust/issues/133606>.
This commit is contained in:
Zalathar 2024-11-29 14:52:41 +11:00
parent d53f0b1d8e
commit 9461f4296f
13 changed files with 284 additions and 250 deletions
Download patch file Download diff file Expand all files Collapse all files

36
compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
View file

@ -1,4 +1,6 @@
use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId}; use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId, SourceRegion};
use crate::coverageinfo::mapgen::LocalFileId;
/// Must match the layout of `LLVMRustCounterKind`. /// Must match the layout of `LLVMRustCounterKind`.
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug)]
@ -124,23 +126,37 @@ pub(crate) struct CoverageSpan {
/// Local index into the function's local-to-global file ID table. /// 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 /// The value at that index is itself an index into the coverage filename
/// table in the CGU's `__llvm_covmap` section. /// table in the CGU's `__llvm_covmap` section.
pub(crate) file_id: u32, file_id: u32,
/// 1-based starting line of the source code span. /// 1-based starting line of the source code span.
pub(crate) start_line: u32, start_line: u32,
/// 1-based starting column of the source code span. /// 1-based starting column of the source code span.
pub(crate) start_col: u32, start_col: u32,
/// 1-based ending line of the source code span. /// 1-based ending line of the source code span.
pub(crate) end_line: u32, end_line: u32,
/// 1-based ending column of the source code span. High bit must be unset. /// 1-based ending column of the source code span. High bit must be unset.
pub(crate) end_col: u32, 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 }
}
} }
/// Must match the layout of `LLVMRustCoverageCodeRegion`. /// Must match the layout of `LLVMRustCoverageCodeRegion`.
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[repr(C)] #[repr(C)]
pub(crate) struct CodeRegion { pub(crate) struct CodeRegion {
pub(crate) cov_span: CoverageSpan, pub(crate) span: CoverageSpan,
pub(crate) counter: Counter, pub(crate) counter: Counter,
} }
@ -148,7 +164,7 @@ pub(crate) struct CodeRegion {
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[repr(C)] #[repr(C)]
pub(crate) struct BranchRegion { pub(crate) struct BranchRegion {
pub(crate) cov_span: CoverageSpan, pub(crate) span: CoverageSpan,
pub(crate) true_counter: Counter, pub(crate) true_counter: Counter,
pub(crate) false_counter: Counter, pub(crate) false_counter: Counter,
} }
@ -157,7 +173,7 @@ pub(crate) struct BranchRegion {
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[repr(C)] #[repr(C)]
pub(crate) struct MCDCBranchRegion { pub(crate) struct MCDCBranchRegion {
pub(crate) cov_span: CoverageSpan, pub(crate) span: CoverageSpan,
pub(crate) true_counter: Counter, pub(crate) true_counter: Counter,
pub(crate) false_counter: Counter, pub(crate) false_counter: Counter,
pub(crate) mcdc_branch_params: mcdc::BranchParameters, pub(crate) mcdc_branch_params: mcdc::BranchParameters,
@ -167,6 +183,6 @@ pub(crate) struct MCDCBranchRegion {
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[repr(C)] #[repr(C)]
pub(crate) struct MCDCDecisionRegion { pub(crate) struct MCDCDecisionRegion {
pub(crate) cov_span: CoverageSpan, pub(crate) span: CoverageSpan,
pub(crate) mcdc_decision_params: mcdc::DecisionParameters, pub(crate) mcdc_decision_params: mcdc::DecisionParameters,
} }

14
compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
View file

@ -3,9 +3,9 @@ use rustc_data_structures::fx::FxIndexSet;
use rustc_index::bit_set::BitSet; use rustc_index::bit_set::BitSet;
use rustc_middle::mir::coverage::{ use rustc_middle::mir::coverage::{
CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op, CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op,
SourceRegion,
}; };
use rustc_middle::ty::Instance; use rustc_middle::ty::Instance;
use rustc_span::Span;
use tracing::{debug, instrument}; use tracing::{debug, instrument};
use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind}; use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind};
@ -220,16 +220,16 @@ impl<'tcx> FunctionCoverage<'tcx> {
}) })
} }
/// Yields all this function's coverage mappings, after simplifying away /// Converts this function's coverage mappings into an intermediate form
/// unused counters and counter expressions. /// that will be used by `mapgen` when preparing for FFI.
pub(crate) fn mapping_spans( pub(crate) fn counter_regions(
&self, &self,
) -> impl Iterator<Item = (MappingKind, Span)> + ExactSizeIterator + Captures<'_> { ) -> impl Iterator<Item = (MappingKind, &SourceRegion)> + ExactSizeIterator {
self.function_coverage_info.mappings.iter().map(move |mapping| { self.function_coverage_info.mappings.iter().map(move |mapping| {
let &Mapping { ref kind, span } = mapping; let Mapping { kind, source_region } = mapping;
let kind = let kind =
kind.map_terms(|term| if self.is_zero_term(term) { CovTerm::Zero } else { term }); kind.map_terms(|term| if self.is_zero_term(term) { CovTerm::Zero } else { term });
(kind, span) (kind, source_region)
}) })
} }

119
compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
View file

@ -1,14 +1,12 @@
mod spans;
use std::ffi::CString; use std::ffi::CString;
use std::sync::Arc; use std::iter;
use itertools::Itertools as _; use itertools::Itertools as _;
use rustc_abi::Align; use rustc_abi::Align;
use rustc_codegen_ssa::traits::{ use rustc_codegen_ssa::traits::{
BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods, BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods,
}; };
use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_index::IndexVec; use rustc_index::IndexVec;
use rustc_middle::mir::coverage::MappingKind; use rustc_middle::mir::coverage::MappingKind;
@ -17,7 +15,7 @@ use rustc_middle::{bug, mir};
use rustc_session::RemapFileNameExt; use rustc_session::RemapFileNameExt;
use rustc_session::config::RemapPathScopeComponents; use rustc_session::config::RemapPathScopeComponents;
use rustc_span::def_id::DefIdSet; use rustc_span::def_id::DefIdSet;
use rustc_span::{SourceFile, StableSourceFileId}; use rustc_span::{Span, Symbol};
use rustc_target::spec::HasTargetSpec; use rustc_target::spec::HasTargetSpec;
use tracing::debug; use tracing::debug;
@ -74,11 +72,11 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
.map(|(instance, function_coverage)| (instance, function_coverage.into_finished())) .map(|(instance, function_coverage)| (instance, function_coverage.into_finished()))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let all_files = function_coverage_entries let all_file_names = function_coverage_entries
.iter() .iter()
.map(|(_, fn_cov)| fn_cov.function_coverage_info.body_span) .map(|(_, fn_cov)| fn_cov.function_coverage_info.body_span)
.map(|span| tcx.sess.source_map().lookup_source_file(span.lo())); .map(|span| span_file_name(tcx, span));
let global_file_table = GlobalFileTable::new(all_files); let global_file_table = GlobalFileTable::new(all_file_names);
// Encode all filenames referenced by coverage mappings in this CGU. // Encode all filenames referenced by coverage mappings in this CGU.
let filenames_buffer = global_file_table.make_filenames_buffer(tcx); let filenames_buffer = global_file_table.make_filenames_buffer(tcx);
@ -105,9 +103,16 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
encode_mappings_for_function(tcx, &global_file_table, &function_coverage); encode_mappings_for_function(tcx, &global_file_table, &function_coverage);
if coverage_mapping_buffer.is_empty() { if coverage_mapping_buffer.is_empty() {
debug!("function has no mappings to embed; skipping"); 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; continue;
} }
}
if !is_used { if !is_used {
unused_function_names.push(mangled_function_name); unused_function_names.push(mangled_function_name);
@ -143,34 +148,29 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
} }
} }
/// Maps "global" (per-CGU) file ID numbers to their underlying source files. /// Maps "global" (per-CGU) file ID numbers to their underlying filenames.
struct GlobalFileTable { struct GlobalFileTable {
/// This "raw" table doesn't include the working dir, so a file's /// This "raw" table doesn't include the working dir, so a filename's
/// global ID is its index in this set **plus one**. /// global ID is its index in this set **plus one**.
raw_file_table: FxIndexMap<StableSourceFileId, Arc<SourceFile>>, raw_file_table: FxIndexSet<Symbol>,
} }
impl GlobalFileTable { impl GlobalFileTable {
fn new(all_files: impl IntoIterator<Item = Arc<SourceFile>>) -> Self { fn new(all_file_names: impl IntoIterator<Item = Symbol>) -> Self {
// Collect all of the files into a set. Files usually come in contiguous // Collect all of the filenames into a set. Filenames usually come in
// runs, so we can dedup adjacent ones to save work. // contiguous runs, so we can dedup adjacent ones to save work.
let mut raw_file_table = all_files let mut raw_file_table = all_file_names.into_iter().dedup().collect::<FxIndexSet<Symbol>>();
.into_iter()
.dedup_by(|a, b| a.stable_id == b.stable_id)
.map(|f| (f.stable_id, f))
.collect::<FxIndexMap<StableSourceFileId, Arc<SourceFile>>>();
// Sort the file table by its underlying filenames. // Sort the file table by its actual string values, not the arbitrary
raw_file_table.sort_unstable_by(|_, a, _, b| { // ordering of its symbols.
Ord::cmp(&a.name, &b.name).then_with(|| Ord::cmp(&a.stable_id, &b.stable_id)) raw_file_table.sort_unstable_by(|a, b| a.as_str().cmp(b.as_str()));
});
Self { raw_file_table } Self { raw_file_table }
} }
fn global_file_id_for_file(&self, file: &SourceFile) -> GlobalFileId { fn global_file_id_for_file_name(&self, file_name: Symbol) -> GlobalFileId {
let raw_id = self.raw_file_table.get_index_of(&file.stable_id).unwrap_or_else(|| { let raw_id = self.raw_file_table.get_index_of(&file_name).unwrap_or_else(|| {
bug!("file not found in prepared global file table: {:?}", file.name); bug!("file name not found in prepared global file table: {file_name}");
}); });
// The raw file table doesn't include an entry for the working dir // 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. // (which has ID 0), so add 1 to get the correct ID.
@ -178,27 +178,24 @@ impl GlobalFileTable {
} }
fn make_filenames_buffer(&self, tcx: TyCtxt<'_>) -> Vec<u8> { 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) // LLVM Coverage Mapping Format version 6 (zero-based encoded as 5)
// requires setting the first filename to the compilation directory. // requires setting the first filename to the compilation directory.
// Since rustc generates coverage maps with relative paths, the // Since rustc generates coverage maps with relative paths, the
// compilation directory can be combined with the relative paths // compilation directory can be combined with the relative paths
// to get absolute paths, if needed. // to get absolute paths, if needed.
table.push( use rustc_session::RemapFileNameExt;
tcx.sess use rustc_session::config::RemapPathScopeComponents;
let working_dir: &str = &tcx
.sess
.opts .opts
.working_dir .working_dir
.for_scope(tcx.sess, RemapPathScopeComponents::MACRO) .for_scope(tcx.sess, RemapPathScopeComponents::MACRO)
.to_string_lossy(), .to_string_lossy();
);
// Add the regular entries after the base directory. // Insert the working dir at index 0, before the other filenames.
table.extend(self.raw_file_table.values().map(|file| { let filenames =
file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy() iter::once(working_dir).chain(self.raw_file_table.iter().map(Symbol::as_str));
})); llvm_cov::write_filenames_to_buffer(filenames)
llvm_cov::write_filenames_to_buffer(table.iter().map(|f| f.as_ref()))
} }
} }
@ -211,7 +208,7 @@ rustc_index::newtype_index! {
/// An index into a function's list of global file IDs. That underlying list /// 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 /// of local-to-global mappings will be embedded in the function's record in
/// the `__llvm_covfun` linker section. /// the `__llvm_covfun` linker section.
struct LocalFileId {} pub(crate) struct LocalFileId {}
} }
/// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU) /// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU)
@ -237,6 +234,13 @@ 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, /// Using the expressions and counter regions collected for a single function,
/// generate the variable-sized payload of its corresponding `__llvm_covfun` /// generate the variable-sized payload of its corresponding `__llvm_covfun`
/// entry. The payload is returned as a vector of bytes. /// entry. The payload is returned as a vector of bytes.
@ -247,13 +251,11 @@ fn encode_mappings_for_function(
global_file_table: &GlobalFileTable, global_file_table: &GlobalFileTable,
function_coverage: &FunctionCoverage<'_>, function_coverage: &FunctionCoverage<'_>,
) -> Vec<u8> { ) -> Vec<u8> {
let mapping_spans = function_coverage.mapping_spans(); let counter_regions = function_coverage.counter_regions();
if mapping_spans.is_empty() { if counter_regions.is_empty() {
return Vec::new(); return Vec::new();
} }
let fn_cov_info = function_coverage.function_coverage_info;
let expressions = function_coverage.counter_expressions().collect::<Vec<_>>(); let expressions = function_coverage.counter_expressions().collect::<Vec<_>>();
let mut virtual_file_mapping = VirtualFileMapping::default(); let mut virtual_file_mapping = VirtualFileMapping::default();
@ -263,39 +265,34 @@ fn encode_mappings_for_function(
let mut mcdc_decision_regions = vec![]; let mut mcdc_decision_regions = vec![];
// Currently a function's mappings must all be in the same file as its body span. // Currently a function's mappings must all be in the same file as its body span.
let source_map = tcx.sess.source_map(); let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span);
let source_file = source_map.lookup_source_file(fn_cov_info.body_span.lo());
// Look up the global file ID for that file. // Look up the global file ID for that filename.
let global_file_id = global_file_table.global_file_id_for_file(&source_file); 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. // 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 = virtual_file_mapping.local_id_for_global(global_file_id);
debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'");
let make_cov_span = |span| { // For each counter/region pair in this function+file, convert it to a
spans::make_coverage_span(local_file_id, source_map, fn_cov_info, &source_file, span)
};
// For each coverage mapping span in this function+file, convert it to a
// form suitable for FFI. // form suitable for FFI.
for (mapping_kind, span) in mapping_spans { for (mapping_kind, region) in counter_regions {
debug!("Adding counter {mapping_kind:?} to map for {span:?}"); debug!("Adding counter {mapping_kind:?} to map for {region:?}");
let Some(cov_span) = make_cov_span(span) else { continue }; let span = ffi::CoverageSpan::from_source_region(local_file_id, region);
match mapping_kind { match mapping_kind {
MappingKind::Code(term) => { MappingKind::Code(term) => {
code_regions code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) });
.push(ffi::CodeRegion { cov_span, counter: ffi::Counter::from_term(term) });
} }
MappingKind::Branch { true_term, false_term } => { MappingKind::Branch { true_term, false_term } => {
branch_regions.push(ffi::BranchRegion { branch_regions.push(ffi::BranchRegion {
cov_span, span,
true_counter: ffi::Counter::from_term(true_term), true_counter: ffi::Counter::from_term(true_term),
false_counter: ffi::Counter::from_term(false_term), false_counter: ffi::Counter::from_term(false_term),
}); });
} }
MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => { MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
mcdc_branch_regions.push(ffi::MCDCBranchRegion { mcdc_branch_regions.push(ffi::MCDCBranchRegion {
cov_span, span,
true_counter: ffi::Counter::from_term(true_term), true_counter: ffi::Counter::from_term(true_term),
false_counter: ffi::Counter::from_term(false_term), false_counter: ffi::Counter::from_term(false_term),
mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params), mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params),
@ -303,7 +300,7 @@ fn encode_mappings_for_function(
} }
MappingKind::MCDCDecision(mcdc_decision_params) => { MappingKind::MCDCDecision(mcdc_decision_params) => {
mcdc_decision_regions.push(ffi::MCDCDecisionRegion { mcdc_decision_regions.push(ffi::MCDCDecisionRegion {
cov_span, span,
mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params), mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params),
}); });
} }

124
compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs
View file

@ -1,124 +0,0 @@
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 {
// 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()?
}
/// 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
}
}

1
compiler/rustc_codegen_llvm/src/lib.rs
View file

@ -17,7 +17,6 @@
#![feature(iter_intersperse)] #![feature(iter_intersperse)]
#![feature(let_chains)] #![feature(let_chains)]
#![feature(rustdoc_internals)] #![feature(rustdoc_internals)]
#![feature(try_blocks)]
#![warn(unreachable_pub)] #![warn(unreachable_pub)]
// tidy-alphabetical-end // tidy-alphabetical-end

18
compiler/rustc_middle/src/mir/coverage.rs
View file

@ -155,6 +155,22 @@ 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(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
#[derive(TyEncodable, TyDecodable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, TypeFoldable, TypeVisitable)]
pub enum Op { pub enum Op {
@ -216,7 +232,7 @@ impl MappingKind {
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
pub struct Mapping { pub struct Mapping {
pub kind: MappingKind, pub kind: MappingKind,
pub span: Span, pub source_region: SourceRegion,
} }
/// Stores per-function coverage information attached to a `mir::Body`, /// Stores per-function coverage information attached to a `mir::Body`,

4
compiler/rustc_middle/src/mir/pretty.rs
View file

@ -603,8 +603,8 @@ fn write_function_coverage_info(
for (id, expression) in expressions.iter_enumerated() { for (id, expression) in expressions.iter_enumerated() {
writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?; writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?;
} }
for coverage::Mapping { kind, span } in mappings { for coverage::Mapping { kind, source_region } in mappings {
writeln!(w, "{INDENT}coverage {kind:?} => {span:?};")?; writeln!(w, "{INDENT}coverage {kind:?} => {source_region:?};")?;
} }
writeln!(w)?; writeln!(w)?;

166
compiler/rustc_mir_transform/src/coverage/mod.rs
View file

@ -13,15 +13,16 @@ use rustc_hir::intravisit::{Visitor, walk_expr};
use rustc_middle::hir::map::Map; use rustc_middle::hir::map::Map;
use rustc_middle::hir::nested_filter; use rustc_middle::hir::nested_filter;
use rustc_middle::mir::coverage::{ use rustc_middle::mir::coverage::{
CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, SourceRegion,
}; };
use rustc_middle::mir::{ use rustc_middle::mir::{
self, BasicBlock, BasicBlockData, SourceInfo, Statement, StatementKind, Terminator, self, BasicBlock, BasicBlockData, SourceInfo, Statement, StatementKind, Terminator,
TerminatorKind, TerminatorKind,
}; };
use rustc_middle::ty::TyCtxt; use rustc_middle::ty::TyCtxt;
use rustc_span::Span;
use rustc_span::def_id::LocalDefId; 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 tracing::{debug, debug_span, trace};
use crate::coverage::counters::{CounterIncrementSite, CoverageCounters}; use crate::coverage::counters::{CounterIncrementSite, CoverageCounters};
@ -96,7 +97,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
let coverage_counters = let coverage_counters =
CoverageCounters::make_bcb_counters(&basic_coverage_blocks, &bcbs_with_counter_mappings); CoverageCounters::make_bcb_counters(&basic_coverage_blocks, &bcbs_with_counter_mappings);
let mappings = create_mappings(&extracted_mappings, &coverage_counters); let mappings = create_mappings(tcx, &hir_info, &extracted_mappings, &coverage_counters);
if mappings.is_empty() { if mappings.is_empty() {
// No spans could be converted into valid mappings, so skip this function. // No spans could be converted into valid mappings, so skip this function.
debug!("no spans could be converted into valid mappings; skipping"); debug!("no spans could be converted into valid mappings; skipping");
@ -135,12 +136,18 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
/// ///
/// Precondition: All BCBs corresponding to those spans have been given /// Precondition: All BCBs corresponding to those spans have been given
/// coverage counters. /// coverage counters.
fn create_mappings( fn create_mappings<'tcx>(
tcx: TyCtxt<'tcx>,
hir_info: &ExtractedHirInfo,
extracted_mappings: &ExtractedMappings, extracted_mappings: &ExtractedMappings,
coverage_counters: &CoverageCounters, coverage_counters: &CoverageCounters,
) -> Vec<Mapping> { ) -> 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 = let term_for_bcb =
|bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters"); |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. // Fully destructure the mappings struct to make sure we don't miss any kinds.
let ExtractedMappings { let ExtractedMappings {
@ -153,20 +160,22 @@ fn create_mappings(
} = extracted_mappings; } = extracted_mappings;
let mut mappings = Vec::new(); let mut mappings = Vec::new();
mappings.extend(code_mappings.iter().map( mappings.extend(code_mappings.iter().filter_map(
// Ordinary code mappings are the simplest kind. // Ordinary code mappings are the simplest kind.
|&mappings::CodeMapping { span, bcb }| { |&mappings::CodeMapping { span, bcb }| {
let source_region = region_for_span(span)?;
let kind = MappingKind::Code(term_for_bcb(bcb)); let kind = MappingKind::Code(term_for_bcb(bcb));
Mapping { kind, span } Some(Mapping { kind, source_region })
}, },
)); ));
mappings.extend(branch_pairs.iter().map( mappings.extend(branch_pairs.iter().filter_map(
|&mappings::BranchPair { span, true_bcb, false_bcb }| { |&mappings::BranchPair { span, true_bcb, false_bcb }| {
let true_term = term_for_bcb(true_bcb); let true_term = term_for_bcb(true_bcb);
let false_term = term_for_bcb(false_bcb); let false_term = term_for_bcb(false_bcb);
let kind = MappingKind::Branch { true_term, false_term }; let kind = MappingKind::Branch { true_term, false_term };
Mapping { kind, span } let source_region = region_for_span(span)?;
Some(Mapping { kind, source_region })
}, },
)); ));
@ -174,7 +183,7 @@ fn create_mappings(
|bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters"); |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. // MCDC branch mappings are appended with their decisions in case decisions were ignored.
mappings.extend(mcdc_degraded_branches.iter().map( mappings.extend(mcdc_degraded_branches.iter().filter_map(
|&mappings::MCDCBranch { |&mappings::MCDCBranch {
span, span,
true_bcb, true_bcb,
@ -183,9 +192,10 @@ fn create_mappings(
true_index: _, true_index: _,
false_index: _, false_index: _,
}| { }| {
let source_region = region_for_span(span)?;
let true_term = term_for_bcb(true_bcb); let true_term = term_for_bcb(true_bcb);
let false_term = term_for_bcb(false_bcb); let false_term = term_for_bcb(false_bcb);
Mapping { kind: MappingKind::Branch { true_term, false_term }, span } Some(Mapping { kind: MappingKind::Branch { true_term, false_term }, source_region })
}, },
)); ));
@ -193,7 +203,7 @@ fn create_mappings(
let num_conditions = branches.len() as u16; let num_conditions = branches.len() as u16;
let conditions = branches let conditions = branches
.into_iter() .into_iter()
.map( .filter_map(
|&mappings::MCDCBranch { |&mappings::MCDCBranch {
span, span,
true_bcb, true_bcb,
@ -202,29 +212,31 @@ fn create_mappings(
true_index: _, true_index: _,
false_index: _, false_index: _,
}| { }| {
let source_region = region_for_span(span)?;
let true_term = term_for_bcb(true_bcb); let true_term = term_for_bcb(true_bcb);
let false_term = term_for_bcb(false_bcb); let false_term = term_for_bcb(false_bcb);
Mapping { Some(Mapping {
kind: MappingKind::MCDCBranch { kind: MappingKind::MCDCBranch {
true_term, true_term,
false_term, false_term,
mcdc_params: condition_info, mcdc_params: condition_info,
}, },
span, source_region,
} })
}, },
) )
.collect::<Vec<_>>(); .collect::<Vec<_>>();
if conditions.len() == num_conditions as usize { if conditions.len() == num_conditions as usize
&& let Some(source_region) = region_for_span(decision.span)
{
// LLVM requires end index for counter mapping regions. // LLVM requires end index for counter mapping regions.
let kind = MappingKind::MCDCDecision(DecisionInfo { let kind = MappingKind::MCDCDecision(DecisionInfo {
bitmap_idx: (decision.bitmap_idx + decision.num_test_vectors) as u32, bitmap_idx: (decision.bitmap_idx + decision.num_test_vectors) as u32,
num_conditions, num_conditions,
}); });
mappings.extend( mappings.extend(
std::iter::once(Mapping { kind, span: decision.span }) std::iter::once(Mapping { kind, source_region }).chain(conditions.into_iter()),
.chain(conditions.into_iter()),
); );
} else { } else {
mappings.extend(conditions.into_iter().map(|mapping| { mappings.extend(conditions.into_iter().map(|mapping| {
@ -233,7 +245,10 @@ fn create_mappings(
else { else {
unreachable!("all mappings here are MCDCBranch as shown above"); unreachable!("all mappings here are MCDCBranch as shown above");
}; };
Mapping { kind: MappingKind::Branch { true_term, false_term }, span: mapping.span } Mapping {
kind: MappingKind::Branch { true_term, false_term },
source_region: mapping.source_region,
}
})) }))
} }
} }
@ -376,6 +391,121 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
data.statements.insert(0, statement); 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. /// Function information extracted from HIR by the coverage instrumentor.
#[derive(Debug)] #[derive(Debug)]
struct ExtractedHirInfo { struct ExtractedHirInfo {

12
tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff
View file

@ -33,12 +33,12 @@
+ coverage ExpressionId(3) => Expression { lhs: Counter(3), op: Add, rhs: Counter(2) }; + 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(4) => Expression { lhs: Expression(3), op: Add, rhs: Counter(1) };
+ coverage ExpressionId(5) => Expression { lhs: Expression(4), op: Add, rhs: Expression(2) }; + 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 (#0); + coverage Code(Counter(0)) => 14:1 - 15:21;
+ coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:16:17: 16:33 (#0); + coverage Code(Counter(3)) => 16:17 - 16:33;
+ coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:17:17: 17:33 (#0); + coverage Code(Counter(2)) => 17:17 - 17:33;
+ coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:18:17: 18:33 (#0); + coverage Code(Counter(1)) => 18:17 - 18:33;
+ coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:19:17: 19:33 (#0); + coverage Code(Expression(2)) => 19:17 - 19:33;
+ coverage Code(Expression(5)) => $DIR/branch_match_arms.rs:21:2: 21:2 (#0); + coverage Code(Expression(5)) => 21:1 - 21:2;
+ +
bb0: { bb0: {
+ Coverage::CounterIncrement(0); + Coverage::CounterIncrement(0);

2
tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff
View file

@ -5,7 +5,7 @@
let mut _0: bool; let mut _0: bool;
+ coverage body span: $DIR/instrument_coverage.rs:19:18: 21:2 (#0) + coverage body span: $DIR/instrument_coverage.rs:19:18: 21:2 (#0)
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:19:1: 21:2 (#0); + coverage Code(Counter(0)) => 19:1 - 21:2;
+ +
bb0: { bb0: {
+ Coverage::CounterIncrement(0); + Coverage::CounterIncrement(0);

10
tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff
View file

@ -9,11 +9,11 @@
+ coverage body span: $DIR/instrument_coverage.rs:10:11: 16:2 (#0) + 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 ExpressionId(0) => Expression { lhs: Counter(0), op: Add, rhs: Counter(1) };
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:10:1: 10:11 (#0); + coverage Code(Counter(0)) => 10:1 - 10:11;
+ coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:12:12: 12:17 (#0); + coverage Code(Expression(0)) => 12:12 - 12:17;
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:13:13: 13:18 (#0); + coverage Code(Counter(0)) => 13:13 - 13:18;
+ coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:14:10: 14:10 (#0); + coverage Code(Counter(1)) => 14:9 - 14:10;
+ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:16:2: 16:2 (#0); + coverage Code(Counter(0)) => 16:1 - 16:2;
+ +
bb0: { bb0: {
+ Coverage::CounterIncrement(0); + Coverage::CounterIncrement(0);

10
tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff
View file

@ -9,11 +9,11 @@
coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#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 ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) };
coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0); coverage Code(Counter(0)) => 13:1 - 14:36;
coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); coverage Code(Expression(0)) => 14:37 - 14:39;
coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); coverage Code(Counter(1)) => 14:38 - 14:39;
coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); coverage Code(Counter(0)) => 15:1 - 15:2;
coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36;
bb0: { bb0: {
Coverage::CounterIncrement(0); Coverage::CounterIncrement(0);

10
tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff
View file

@ -9,11 +9,11 @@
+ coverage body span: $DIR/instrument_coverage_cleanup.rs:13:11: 15:2 (#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 ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) };
+ coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1: 14:36 (#0); + coverage Code(Counter(0)) => 13:1 - 14:36;
+ coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); + coverage Code(Expression(0)) => 14:37 - 14:39;
+ coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); + coverage Code(Counter(1)) => 14:38 - 14:39;
+ coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); + coverage Code(Counter(0)) => 15:1 - 15:2;
+ coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); + coverage Branch { true_term: Expression(0), false_term: Counter(1) } => 14:8 - 14:36;
+ +
bb0: { bb0: {
+ Coverage::CounterIncrement(0); + Coverage::CounterIncrement(0);