From dfa6441354181ad829baca363b619d0aeb295798 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 12 Dec 2023 13:18:15 +1100 Subject: [PATCH 1/3] coverage: Skip instrumenting a function if no spans were extracted --- compiler/rustc_mir_transform/src/coverage/mod.rs | 7 +++++-- compiler/rustc_mir_transform/src/coverage/spans.rs | 12 ++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 65a0924f1c9fb..709d1fdc21a08 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -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 diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 05ad14f1525cd..462e54c386c7a 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -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 { 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 { From c57f28bbf75572e10c6165c33a2aef922315fdb4 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 12 Dec 2023 13:49:19 +1100 Subject: [PATCH 2/3] coverage: Avoid creating `func_coverage` for marker statements Coverage marker statements should have no effect on codegen, but in some cases they could have the side-effect of creating a `func_coverage` entry for their enclosing function. That can lead to an ICE for functions that don't actually have any coverage spans. --- .../rustc_codegen_llvm/src/coverageinfo/mod.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 8386f067bafba..0befbb5a39be3 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -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`, From e0de1439708d72a319ca334ef1d3237a7a4a6800 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 12 Dec 2023 12:55:40 +1100 Subject: [PATCH 3/3] coverage: Regression test for markers in a function with no spans --- tests/coverage/no_spans_if_not.cov-map | 8 +++++++ tests/coverage/no_spans_if_not.coverage | 30 +++++++++++++++++++++++++ tests/coverage/no_spans_if_not.rs | 29 ++++++++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 tests/coverage/no_spans_if_not.cov-map create mode 100644 tests/coverage/no_spans_if_not.coverage create mode 100644 tests/coverage/no_spans_if_not.rs diff --git a/tests/coverage/no_spans_if_not.cov-map b/tests/coverage/no_spans_if_not.cov-map new file mode 100644 index 0000000000000..5277267ec1b01 --- /dev/null +++ b/tests/coverage/no_spans_if_not.cov-map @@ -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) + diff --git a/tests/coverage/no_spans_if_not.coverage b/tests/coverage/no_spans_if_not.coverage new file mode 100644 index 0000000000000..1b6bbc75a04e4 --- /dev/null +++ b/tests/coverage/no_spans_if_not.coverage @@ -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 , + 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| |} + diff --git a/tests/coverage/no_spans_if_not.rs b/tests/coverage/no_spans_if_not.rs new file mode 100644 index 0000000000000..2bbdc11cd5e59 --- /dev/null +++ b/tests/coverage/no_spans_if_not.rs @@ -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 , +// "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 { + () + } + } +}