Skip to content

Commit b9fb1a6

Browse files
committed
coverage: Store coverage source regions as Span until codegen
1 parent 87fe7de commit b9fb1a6

13 files changed

+199
-226
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
/// Must match the layout of `LLVMRustCoverageCodeRegion`.

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ use rustc_data_structures::fx::FxIndexSet;
33
use rustc_index::bit_set::BitSet;
44
use rustc_middle::mir::coverage::{
55
CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op,
6-
SourceRegion,
76
};
87
use rustc_middle::ty::Instance;
8+
use rustc_span::Span;
99
use tracing::{debug, instrument};
1010

1111
use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind};
@@ -220,16 +220,16 @@ impl<'tcx> FunctionCoverage<'tcx> {
220220
})
221221
}
222222

223-
/// Converts this function's coverage mappings into an intermediate form
224-
/// that will be used by `mapgen` when preparing for FFI.
225-
pub(crate) fn counter_regions(
223+
/// Yields all this function's coverage mappings, after simplifying away
224+
/// unused counters and counter expressions.
225+
pub(crate) fn mapping_spans(
226226
&self,
227-
) -> impl Iterator<Item = (MappingKind, &SourceRegion)> + ExactSizeIterator {
227+
) -> impl Iterator<Item = (MappingKind, Span)> + ExactSizeIterator + Captures<'_> {
228228
self.function_coverage_info.mappings.iter().map(move |mapping| {
229-
let Mapping { kind, source_region } = mapping;
229+
let &Mapping { ref kind, span } = mapping;
230230
let kind =
231231
kind.map_terms(|term| if self.is_zero_term(term) { CovTerm::Zero } else { term });
232-
(kind, source_region)
232+
(kind, span)
233233
})
234234
}
235235

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

+18-8
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
mod spans;
2+
13
use std::ffi::CString;
24
use std::iter;
35

@@ -201,7 +203,7 @@ rustc_index::newtype_index! {
201203
/// An index into a function's list of global file IDs. That underlying list
202204
/// of local-to-global mappings will be embedded in the function's record in
203205
/// the `__llvm_covfun` linker section.
204-
pub(crate) struct LocalFileId {}
206+
struct LocalFileId {}
205207
}
206208

207209
/// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU)
@@ -244,11 +246,13 @@ fn encode_mappings_for_function(
244246
global_file_table: &GlobalFileTable,
245247
function_coverage: &FunctionCoverage<'_>,
246248
) -> Vec<u8> {
247-
let counter_regions = function_coverage.counter_regions();
248-
if counter_regions.is_empty() {
249+
let mapping_spans = function_coverage.mapping_spans();
250+
if mapping_spans.is_empty() {
249251
return Vec::new();
250252
}
251253

254+
let fn_cov_info = function_coverage.function_coverage_info;
255+
252256
let expressions = function_coverage.counter_expressions().collect::<Vec<_>>();
253257

254258
let mut virtual_file_mapping = VirtualFileMapping::default();
@@ -258,7 +262,9 @@ fn encode_mappings_for_function(
258262
let mut mcdc_decision_regions = vec![];
259263

260264
// Currently a function's mappings must all be in the same file as its body span.
261-
let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span);
265+
let file_name = span_file_name(tcx, fn_cov_info.body_span);
266+
let source_map = tcx.sess.source_map();
267+
let source_file = source_map.lookup_source_file(fn_cov_info.body_span.lo());
262268

263269
// Look up the global file ID for that filename.
264270
let global_file_id = global_file_table.global_file_id_for_file_name(file_name);
@@ -267,11 +273,15 @@ fn encode_mappings_for_function(
267273
let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id);
268274
debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'");
269275

270-
// For each counter/region pair in this function+file, convert it to a
276+
let make_cov_span = |span| {
277+
spans::make_coverage_span(local_file_id, source_map, fn_cov_info, &source_file, span)
278+
};
279+
280+
// For each coverage mapping span in this function+file, convert it to a
271281
// form suitable for FFI.
272-
for (mapping_kind, region) in counter_regions {
273-
debug!("Adding counter {mapping_kind:?} to map for {region:?}");
274-
let cov_span = ffi::CoverageSpan::from_source_region(local_file_id, region);
282+
for (mapping_kind, span) in mapping_spans {
283+
debug!("Adding counter {mapping_kind:?} to map for {span:?}");
284+
let Some(cov_span) = make_cov_span(span) else { continue };
275285
match mapping_kind {
276286
MappingKind::Code(term) => {
277287
code_regions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
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+
// We're only checking for specific ASCII characters, so we don't
81+
// have to worry about multi-byte code points.
82+
if try_next && src.as_bytes()[end] == b'{' {
83+
Some(span.with_hi(hi + BytePos(1)))
84+
} else if try_prev && src.as_bytes()[start - 1] == b'}' {
85+
Some(span.with_lo(lo - BytePos(1)))
86+
} else {
87+
None
88+
}
89+
})
90+
.ok()?
91+
}
92+
93+
/// If `llvm-cov` sees a source region that is improperly ordered (end < start),
94+
/// it will immediately exit with a fatal error. To prevent that from happening,
95+
/// discard regions that are improperly ordered, or might be interpreted in a
96+
/// way that makes them improperly ordered.
97+
fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option<ffi::CoverageSpan> {
98+
let ffi::CoverageSpan { file_id: _, start_line, start_col, end_line, end_col } = cov_span;
99+
100+
// Line/column coordinates are supposed to be 1-based. If we ever emit
101+
// coordinates of 0, `llvm-cov` might misinterpret them.
102+
let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0);
103+
// Coverage mappings use the high bit of `end_col` to indicate that a
104+
// region is actually a "gap" region, so make sure it's unset.
105+
let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0;
106+
// If a region is improperly ordered (end < start), `llvm-cov` will exit
107+
// with a fatal error, which is inconvenient for users and hard to debug.
108+
let is_ordered = (start_line, start_col) <= (end_line, end_col);
109+
110+
if all_nonzero && end_col_has_high_bit_unset && is_ordered {
111+
Some(cov_span)
112+
} else {
113+
debug!(
114+
?cov_span,
115+
?all_nonzero,
116+
?end_col_has_high_bit_unset,
117+
?is_ordered,
118+
"Skipping source region that would be misinterpreted or rejected by LLVM"
119+
);
120+
// If this happens in a debug build, ICE to make it easier to notice.
121+
debug_assert!(false, "Improper source region: {cov_span:?}");
122+
None
123+
}
124+
}

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
@@ -155,22 +155,6 @@ impl Debug for CoverageKind {
155155
}
156156
}
157157

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

238222
/// 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)