Skip to content

Commit 34ed51c

Browse files
committed
coverage: Store coverage source regions as Span until codegen
1 parent c3780e1 commit 34ed51c

13 files changed

+189
-217
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

+6-22
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId, SourceRegion};
2-
3-
use crate::coverageinfo::mapgen::LocalFileId;
1+
use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId};
42

53
/// Must match the layout of `LLVMRustCounterKind`.
64
#[derive(Copy, Clone, Debug)]
@@ -126,30 +124,16 @@ pub(crate) struct CoverageSpan {
126124
/// Local index into the function's local-to-global file ID table.
127125
/// The value at that index is itself an index into the coverage filename
128126
/// table in the CGU's `__llvm_covmap` section.
129-
file_id: u32,
127+
pub(crate) file_id: u32,
130128

131129
/// 1-based starting line of the source code span.
132-
start_line: u32,
130+
pub(crate) start_line: u32,
133131
/// 1-based starting column of the source code span.
134-
start_col: u32,
132+
pub(crate) start_col: u32,
135133
/// 1-based ending line of the source code span.
136-
end_line: u32,
134+
pub(crate) end_line: u32,
137135
/// 1-based ending column of the source code span. High bit must be unset.
138-
end_col: u32,
139-
}
140-
141-
impl CoverageSpan {
142-
pub(crate) fn from_source_region(
143-
local_file_id: LocalFileId,
144-
code_region: &SourceRegion,
145-
) -> Self {
146-
let file_id = local_file_id.as_u32();
147-
let &SourceRegion { start_line, start_col, end_line, end_col } = code_region;
148-
// Internally, LLVM uses the high bit of `end_col` to distinguish between
149-
// code regions and gap regions, so it can't be used by the column number.
150-
assert!(end_col & (1u32 << 31) == 0, "high bit of `end_col` must be unset: {end_col:#X}");
151-
Self { file_id, start_line, start_col, end_line, end_col }
152-
}
136+
pub(crate) end_col: u32,
153137
}
154138

155139
/// Holds tables of the various region types in one struct.

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use crate::coverageinfo::mapgen::covfun::prepare_covfun_record;
2222
use crate::llvm;
2323

2424
mod covfun;
25+
mod spans;
2526

2627
/// Generates and exports the coverage map, which is embedded in special
2728
/// linker sections in the final binary.
@@ -182,7 +183,7 @@ rustc_index::newtype_index! {
182183
/// An index into a function's list of global file IDs. That underlying list
183184
/// of local-to-global mappings will be embedded in the function's record in
184185
/// the `__llvm_covfun` linker section.
185-
pub(crate) struct LocalFileId {}
186+
struct LocalFileId {}
186187
}
187188

188189
/// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU)

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs

+11-3
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,12 @@ use rustc_middle::mir::coverage::{
1414
CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping, MappingKind, Op,
1515
};
1616
use rustc_middle::ty::{Instance, TyCtxt};
17+
use rustc_span::Span;
1718
use rustc_target::spec::HasTargetSpec;
1819
use tracing::debug;
1920

2021
use crate::common::CodegenCx;
21-
use crate::coverageinfo::mapgen::{GlobalFileTable, VirtualFileMapping, span_file_name};
22+
use crate::coverageinfo::mapgen::{GlobalFileTable, VirtualFileMapping, span_file_name, spans};
2223
use crate::coverageinfo::{ffi, llvm_cov};
2324
use crate::llvm;
2425

@@ -117,6 +118,8 @@ fn fill_region_tables<'tcx>(
117118
) {
118119
// Currently a function's mappings must all be in the same file as its body span.
119120
let file_name = span_file_name(tcx, fn_cov_info.body_span);
121+
let source_map = tcx.sess.source_map();
122+
let source_file = source_map.lookup_source_file(fn_cov_info.body_span.lo());
120123

121124
// Look up the global file ID for that filename.
122125
let global_file_id = global_file_table.global_file_id_for_file_name(file_name);
@@ -128,15 +131,20 @@ fn fill_region_tables<'tcx>(
128131
let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } =
129132
&mut covfun.regions;
130133

134+
let make_cov_span = |span: Span| {
135+
spans::make_coverage_span(local_file_id, source_map, fn_cov_info, &source_file, span)
136+
};
137+
131138
// For each counter/region pair in this function+file, convert it to a
132139
// form suitable for FFI.
133140
let is_zero_term = |term| !covfun.is_used || ids_info.is_zero_term(term);
134-
for Mapping { kind, ref source_region } in &fn_cov_info.mappings {
141+
for &Mapping { ref kind, span } in &fn_cov_info.mappings {
135142
// If the mapping refers to counters/expressions that were removed by
136143
// MIR opts, replace those occurrences with zero.
137144
let kind = kind.map_terms(|term| if is_zero_term(term) { CovTerm::Zero } else { term });
138145

139-
let cov_span = ffi::CoverageSpan::from_source_region(local_file_id, source_region);
146+
let Some(cov_span) = make_cov_span(span) else { continue };
147+
140148
match kind {
141149
MappingKind::Code(term) => {
142150
code_regions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
use rustc_middle::mir::coverage::FunctionCoverageInfo;
2+
use rustc_span::source_map::SourceMap;
3+
use rustc_span::{BytePos, Pos, SourceFile, Span};
4+
use tracing::debug;
5+
6+
use crate::coverageinfo::ffi;
7+
use crate::coverageinfo::mapgen::LocalFileId;
8+
9+
/// Converts the span into its start line and column, and end line and column.
10+
///
11+
/// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by
12+
/// the compiler, these column numbers are denoted in **bytes**, because that's what
13+
/// LLVM's `llvm-cov` tool expects to see in coverage maps.
14+
///
15+
/// Returns `None` if the conversion failed for some reason. This shouldn't happen,
16+
/// but it's hard to rule out entirely (especially in the presence of complex macros
17+
/// or other expansions), and if it does happen then skipping a span or function is
18+
/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid.
19+
pub(crate) fn make_coverage_span(
20+
file_id: LocalFileId,
21+
source_map: &SourceMap,
22+
fn_cov_info: &FunctionCoverageInfo,
23+
file: &SourceFile,
24+
span: Span,
25+
) -> Option<ffi::CoverageSpan> {
26+
let span = ensure_non_empty_span(source_map, fn_cov_info, span)?;
27+
28+
let lo = span.lo();
29+
let hi = span.hi();
30+
31+
// Column numbers need to be in bytes, so we can't use the more convenient
32+
// `SourceMap` methods for looking up file coordinates.
33+
let line_and_byte_column = |pos: BytePos| -> Option<(usize, usize)> {
34+
let rpos = file.relative_position(pos);
35+
let line_index = file.lookup_line(rpos)?;
36+
let line_start = file.lines()[line_index];
37+
// Line numbers and column numbers are 1-based, so add 1 to each.
38+
Some((line_index + 1, (rpos - line_start).to_usize() + 1))
39+
};
40+
41+
let (mut start_line, start_col) = line_and_byte_column(lo)?;
42+
let (mut end_line, end_col) = line_and_byte_column(hi)?;
43+
44+
// Apply an offset so that code in doctests has correct line numbers.
45+
// FIXME(#79417): Currently we have no way to offset doctest _columns_.
46+
start_line = source_map.doctest_offset_line(&file.name, start_line);
47+
end_line = source_map.doctest_offset_line(&file.name, end_line);
48+
49+
check_coverage_span(ffi::CoverageSpan {
50+
file_id: file_id.as_u32(),
51+
start_line: start_line as u32,
52+
start_col: start_col as u32,
53+
end_line: end_line as u32,
54+
end_col: end_col as u32,
55+
})
56+
}
57+
58+
fn ensure_non_empty_span(
59+
source_map: &SourceMap,
60+
fn_cov_info: &FunctionCoverageInfo,
61+
span: Span,
62+
) -> Option<Span> {
63+
if !span.is_empty() {
64+
return Some(span);
65+
}
66+
67+
let lo = span.lo();
68+
let hi = span.hi();
69+
70+
// The span is empty, so try to expand it to cover an adjacent '{' or '}',
71+
// but only within the bounds of the body span.
72+
let try_next = hi < fn_cov_info.body_span.hi();
73+
let try_prev = fn_cov_info.body_span.lo() < lo;
74+
if !(try_next || try_prev) {
75+
return None;
76+
}
77+
78+
source_map
79+
.span_to_source(span, |src, start, end| try {
80+
// Adjusting span endpoints by `BytePos(1)` is normally a bug,
81+
// but in this case we have specifically checked that the character
82+
// we're skipping over is one of two specific ASCII characters, so
83+
// adjusting by exactly 1 byte is correct.
84+
if try_next && src.as_bytes()[end] == b'{' {
85+
Some(span.with_hi(hi + BytePos(1)))
86+
} else if try_prev && src.as_bytes()[start - 1] == b'}' {
87+
Some(span.with_lo(lo - BytePos(1)))
88+
} else {
89+
None
90+
}
91+
})
92+
.ok()?
93+
}
94+
95+
/// If `llvm-cov` sees a source region that is improperly ordered (end < start),
96+
/// it will immediately exit with a fatal error. To prevent that from happening,
97+
/// discard regions that are improperly ordered, or might be interpreted in a
98+
/// way that makes them improperly ordered.
99+
fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option<ffi::CoverageSpan> {
100+
let ffi::CoverageSpan { file_id: _, start_line, start_col, end_line, end_col } = cov_span;
101+
102+
// Line/column coordinates are supposed to be 1-based. If we ever emit
103+
// coordinates of 0, `llvm-cov` might misinterpret them.
104+
let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0);
105+
// Coverage mappings use the high bit of `end_col` to indicate that a
106+
// region is actually a "gap" region, so make sure it's unset.
107+
let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0;
108+
// If a region is improperly ordered (end < start), `llvm-cov` will exit
109+
// with a fatal error, which is inconvenient for users and hard to debug.
110+
let is_ordered = (start_line, start_col) <= (end_line, end_col);
111+
112+
if all_nonzero && end_col_has_high_bit_unset && is_ordered {
113+
Some(cov_span)
114+
} else {
115+
debug!(
116+
?cov_span,
117+
?all_nonzero,
118+
?end_col_has_high_bit_unset,
119+
?is_ordered,
120+
"Skipping source region that would be misinterpreted or rejected by LLVM"
121+
);
122+
// If this happens in a debug build, ICE to make it easier to notice.
123+
debug_assert!(false, "Improper source region: {cov_span:?}");
124+
None
125+
}
126+
}

compiler/rustc_codegen_llvm/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#![feature(iter_intersperse)]
1818
#![feature(let_chains)]
1919
#![feature(rustdoc_internals)]
20+
#![feature(try_blocks)]
2021
#![warn(unreachable_pub)]
2122
// tidy-alphabetical-end
2223

compiler/rustc_middle/src/mir/coverage.rs

+1-17
Original file line numberDiff line numberDiff line change
@@ -154,22 +154,6 @@ impl Debug for CoverageKind {
154154
}
155155
}
156156

157-
#[derive(Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq, Eq, PartialOrd, Ord)]
158-
#[derive(TypeFoldable, TypeVisitable)]
159-
pub struct SourceRegion {
160-
pub start_line: u32,
161-
pub start_col: u32,
162-
pub end_line: u32,
163-
pub end_col: u32,
164-
}
165-
166-
impl Debug for SourceRegion {
167-
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
168-
let &Self { start_line, start_col, end_line, end_col } = self;
169-
write!(fmt, "{start_line}:{start_col} - {end_line}:{end_col}")
170-
}
171-
}
172-
173157
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
174158
#[derive(TyEncodable, TyDecodable, TypeFoldable, TypeVisitable)]
175159
pub enum Op {
@@ -231,7 +215,7 @@ impl MappingKind {
231215
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
232216
pub struct Mapping {
233217
pub kind: MappingKind,
234-
pub source_region: SourceRegion,
218+
pub span: Span,
235219
}
236220

237221
/// Stores per-function coverage information attached to a `mir::Body`,

compiler/rustc_middle/src/mir/pretty.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -603,8 +603,8 @@ fn write_function_coverage_info(
603603
for (id, expression) in expressions.iter_enumerated() {
604604
writeln!(w, "{INDENT}coverage {id:?} => {expression:?};")?;
605605
}
606-
for coverage::Mapping { kind, source_region } in mappings {
607-
writeln!(w, "{INDENT}coverage {kind:?} => {source_region:?};")?;
606+
for coverage::Mapping { kind, span } in mappings {
607+
writeln!(w, "{INDENT}coverage {kind:?} => {span:?};")?;
608608
}
609609
writeln!(w)?;
610610

0 commit comments

Comments
 (0)