Skip to content

Commit

Permalink
Unrolled build for rust-lang#118852
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#118852 - Zalathar:no-spans, r=cjgillot

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

The immediate symptoms of rust-lang#118643 were fixed by rust-lang#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 rust-lang#118850.
  • Loading branch information
rust-timer authored Dec 18, 2023
2 parents cda4736 + e0de143 commit 470180f
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 7 deletions.
14 changes: 11 additions & 3 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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`,
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,34 @@ 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,
body_span,
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 {
Expand Down
8 changes: 8 additions & 0 deletions tests/coverage/no_spans_if_not.cov-map
Original file line number Diff line number Diff line change
@@ -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)

30 changes: 30 additions & 0 deletions tests/coverage/no_spans_if_not.coverage
Original file line number Diff line number Diff line change
@@ -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| |}

29 changes: 29 additions & 0 deletions tests/coverage/no_spans_if_not.rs
Original file line number Diff line number Diff line change
@@ -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 {
()
}
}
}

0 comments on commit 470180f

Please sign in to comment.