Skip to content

Commit 5db359c

Browse files
committed
coverage: Allow make_code_region to fail gracefully
Currently there's no way for this function to return `None`, but the next patch will make it potentially able do so in some unknown corner-cases. If it does fail, then skipping a span or function is better than causing an ICE that the user might have no way to avoid without disabling coverage entirely.
1 parent 7f845cc commit 5db359c

File tree

1 file changed

+27
-12
lines changed
  • compiler/rustc_mir_transform/src/coverage

1 file changed

+27
-12
lines changed

compiler/rustc_mir_transform/src/coverage/mod.rs

+27-12
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,9 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
116116
.make_bcb_counters(&self.basic_coverage_blocks, bcb_has_coverage_spans);
117117

118118
let mappings = self.create_mappings_and_inject_coverage_statements(&coverage_spans);
119+
if mappings.is_empty() {
120+
return;
121+
}
119122

120123
self.mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
121124
function_source_hash: self.function_source_hash,
@@ -144,18 +147,24 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
144147

145148
// Process the counters and spans associated with BCB nodes.
146149
for (bcb, counter_kind) in self.coverage_counters.bcb_node_counters() {
147-
let spans = coverage_spans.spans_for_bcb(bcb);
148-
let has_mappings = !spans.is_empty();
149-
150150
// If this BCB has any coverage spans, add corresponding mappings to
151151
// the mappings table.
152-
if has_mappings {
152+
let has_mappings = {
153153
let term = counter_kind.as_term();
154-
mappings.extend(spans.iter().map(|&span| {
155-
let code_region = make_code_region(source_map, file_name, span, body_span);
156-
Mapping { code_region, term }
154+
let old_mappings_len = mappings.len();
155+
156+
mappings.extend(coverage_spans.spans_for_bcb(bcb).iter().filter_map(|&span| {
157+
let Some(code_region) =
158+
make_code_region(source_map, file_name, span, body_span)
159+
else {
160+
debug!(?span, "Couldn't convert span to code region");
161+
return None;
162+
};
163+
Some(Mapping { code_region, term })
157164
}));
158-
}
165+
166+
mappings.len() > old_mappings_len
167+
};
159168

160169
let do_inject = match counter_kind {
161170
// Counter-increment statements always need to be injected.
@@ -247,13 +256,19 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
247256
data.statements.insert(0, statement);
248257
}
249258

250-
/// Convert the Span into its file name, start line and column, and end line and column
259+
/// Convert the Span into its file name, start line and column, and end line and column.
260+
///
261+
/// Returns `None` if the conversion failed for some reason. There is no known example
262+
/// of code that would cause this to happen, but it's hard to rule out entirely
263+
/// (especially in the presence of complex macros or other expansions), and if it does
264+
/// happen then skipping a span or function is better than an ICE that the user might
265+
/// have no way to avoid.
251266
fn make_code_region(
252267
source_map: &SourceMap,
253268
file_name: Symbol,
254269
span: Span,
255270
body_span: Span,
256-
) -> CodeRegion {
271+
) -> Option<CodeRegion> {
257272
debug!(
258273
"Called make_code_region(file_name={}, span={}, body_span={})",
259274
file_name,
@@ -275,13 +290,13 @@ fn make_code_region(
275290
start_line = source_map.doctest_offset_line(&file.name, start_line);
276291
end_line = source_map.doctest_offset_line(&file.name, end_line);
277292
}
278-
CodeRegion {
293+
Some(CodeRegion {
279294
file_name,
280295
start_line: start_line as u32,
281296
start_col: start_col as u32,
282297
end_line: end_line as u32,
283298
end_col: end_col as u32,
284-
}
299+
})
285300
}
286301

287302
fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {

0 commit comments

Comments
 (0)