Rollup merge of #118852 - Zalathar:no-spans, r=cjgillot

coverage: Skip instrumenting a function if no spans were extracted from MIR

The immediate symptoms of #118643 were fixed by #118666, but some users reported that their builds now encounter another coverage-related ICE:

```
error: internal compiler error: compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs:98:17: A used function should have had coverage mapping data but did not: (...)
```

I was able to reproduce at least one cause of this error: if no relevant spans could be extracted from a function, but the function contains `CoverageKind::SpanMarker` statements, then codegen still thinks the function is instrumented and complains about the fact that it has no coverage spans.

This PR prevents that from happening in two ways:
- If we didn't extract any relevant spans from MIR, skip instrumenting the entire function and don't create a `FunctionCoverateInfo` for it.
- If coverage codegen sees a `CoverageKind::SpanMarker` statement, skip it early and avoid creating `func_coverage`.

---

Fixes #118850.
This commit is contained in:
Matthias Krüger 2023-12-18 08:08:22 +01:00 committed by GitHub
commit 418ae3e9a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 93 additions and 7 deletions

View File

@ -85,6 +85,14 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
let bx = self;
match coverage.kind {
// Marker statements have no effect during codegen,
// so return early and don't create `func_coverage`.
CoverageKind::SpanMarker => return,
// Match exhaustively to ensure that newly-added kinds are classified correctly.
CoverageKind::CounterIncrement { .. } | CoverageKind::ExpressionUsed { .. } => {}
}
let Some(function_coverage_info) =
bx.tcx.instance_mir(instance.def).function_coverage_info.as_deref()
else {
@ -100,9 +108,9 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
let Coverage { kind } = coverage;
match *kind {
// Span markers are only meaningful during MIR instrumentation,
// and have no effect during codegen.
CoverageKind::SpanMarker => {}
CoverageKind::SpanMarker => unreachable!(
"unexpected marker statement {kind:?} should have caused an early return"
),
CoverageKind::CounterIncrement { id } => {
func_coverage.mark_counter_id_seen(id);
// We need to explicitly drop the `RefMut` before calling into `instrprof_increment`,

View File

@ -99,12 +99,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
fn inject_counters(&'a mut self) {
////////////////////////////////////////////////////
// Compute coverage spans from the `CoverageGraph`.
let coverage_spans = CoverageSpans::generate_coverage_spans(
let Some(coverage_spans) = CoverageSpans::generate_coverage_spans(
self.mir_body,
self.fn_sig_span,
self.body_span,
&self.basic_coverage_blocks,
);
) else {
// No relevant spans were found in MIR, so skip instrumenting this function.
return;
};
////////////////////////////////////////////////////
// Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure

View File

@ -15,12 +15,16 @@ pub(super) struct CoverageSpans {
}
impl CoverageSpans {
/// Extracts coverage-relevant spans from MIR, and associates them with
/// their corresponding BCBs.
///
/// Returns `None` if no coverage-relevant spans could be extracted.
pub(super) fn generate_coverage_spans(
mir_body: &mir::Body<'_>,
fn_sig_span: Span,
body_span: Span,
basic_coverage_blocks: &CoverageGraph,
) -> Self {
) -> Option<Self> {
let coverage_spans = CoverageSpansGenerator::generate_coverage_spans(
mir_body,
fn_sig_span,
@ -28,13 +32,17 @@ impl CoverageSpans {
basic_coverage_blocks,
);
if coverage_spans.is_empty() {
return None;
}
// Group the coverage spans by BCB, with the BCBs in sorted order.
let mut bcb_to_spans = IndexVec::from_elem_n(Vec::new(), basic_coverage_blocks.num_nodes());
for CoverageSpan { bcb, span, .. } in coverage_spans {
bcb_to_spans[bcb].push(span);
}
Self { bcb_to_spans }
Some(Self { bcb_to_spans })
}
pub(super) fn bcb_has_coverage_spans(&self, bcb: BasicCoverageBlock) -> bool {

View File

@ -0,0 +1,8 @@
Function name: no_spans_if_not::main
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 01, 02, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 11, 1) to (start + 2, 2)

View File

@ -0,0 +1,30 @@
LL| |// edition: 2021
LL| |
LL| |// If the span extractor can't find any relevant spans for a function,
LL| |// but the function contains coverage span-marker statements (e.g. inserted
LL| |// for `if !`), coverage codegen may think that it is instrumented and
LL| |// consequently complain that it has no spans.
LL| |//
LL| |// Regression test for <https://github.com/rust-lang/rust/issues/118850>,
LL| |// "A used function should have had coverage mapping data but did not".
LL| |
LL| 1|fn main() {
LL| 1| affected_function();
LL| 1|}
LL| |
LL| |macro_rules! macro_that_defines_a_function {
LL| | (fn $name:ident () $body:tt) => {
LL| | fn $name () $body
LL| | }
LL| |}
LL| |
LL| |macro_that_defines_a_function! {
LL| | fn affected_function() {
LL| | if !false {
LL| | ()
LL| | } else {
LL| | ()
LL| | }
LL| | }
LL| |}

View File

@ -0,0 +1,29 @@
// edition: 2021
// If the span extractor can't find any relevant spans for a function,
// but the function contains coverage span-marker statements (e.g. inserted
// for `if !`), coverage codegen may think that it is instrumented and
// consequently complain that it has no spans.
//
// Regression test for <https://github.com/rust-lang/rust/issues/118850>,
// "A used function should have had coverage mapping data but did not".
fn main() {
affected_function();
}
macro_rules! macro_that_defines_a_function {
(fn $name:ident () $body:tt) => {
fn $name () $body
}
}
macro_that_defines_a_function! {
fn affected_function() {
if !false {
()
} else {
()
}
}
}