Skip to content

coverage: Enlarge empty spans during MIR instrumentation, not codegen #144298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2025
+57 −46
Merged

coverage: Enlarge empty spans during MIR instrumentation, not codegen #144298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 4 additions & 24 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ impl Coords {
/// or other expansions), and if it does happen then skipping a span or function is
/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid.
pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) -> Option<Coords> {
let span = ensure_non_empty_span(source_map, span)?;
if span.is_empty() {
debug_assert!(false, "can't make coords from empty span: {span:?}");
return None;
}

let lo = span.lo();
let hi = span.hi();
Expand Down Expand Up @@ -70,29 +73,6 @@ pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span)
})
}

fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> {
if !span.is_empty() {
return Some(span);
}

// The span is empty, so try to enlarge it to cover an adjacent '{' or '}'.
source_map
.span_to_source(span, |src, start, end| try {
// Adjusting span endpoints by `BytePos(1)` is normally a bug,
// but in this case we have specifically checked that the character
// we're skipping over is one of two specific ASCII characters, so
// adjusting by exactly 1 byte is correct.
if src.as_bytes().get(end).copied() == Some(b'{') {
Some(span.with_hi(span.hi() + BytePos(1)))
} else if start > 0 && src.as_bytes()[start - 1] == b'}' {
Some(span.with_lo(span.lo() - BytePos(1)))
} else {
None
}
})
.ok()?
}

/// If `llvm-cov` sees a source region that is improperly ordered (end < start),
/// it will immediately exit with a fatal error. To prevent that from happening,
/// discard regions that are improperly ordered, or might be interpreted in a
Expand Down
38 changes: 36 additions & 2 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use rustc_data_structures::fx::FxHashSet;
use rustc_middle::mir;
use rustc_middle::ty::TyCtxt;
use rustc_span::{DesugaringKind, ExpnKind, MacroKind, Span};
use rustc_span::source_map::SourceMap;
use rustc_span::{BytePos, DesugaringKind, ExpnKind, MacroKind, Span};
use tracing::instrument;

use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
Expand Down Expand Up @@ -83,8 +84,18 @@ pub(super) fn extract_refined_covspans<'tcx>(
// Discard any span that overlaps with a hole.
discard_spans_overlapping_holes(&mut covspans, &holes);

// Perform more refinement steps after holes have been dealt with.
// Discard spans that overlap in unwanted ways.
let mut covspans = remove_unwanted_overlapping_spans(covspans);

// For all empty spans, either enlarge them to be non-empty, or discard them.
let source_map = tcx.sess.source_map();
covspans.retain_mut(|covspan| {
let Some(span) = ensure_non_empty_span(source_map, covspan.span) else { return false };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some local testing, it is the return false here that causes the issues of

malformed instrumentation profile data: function name is empty

So what spans are kept/discarded is changed by this PR. Perhaps due to some of the modifications to the spans that happen right above here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spans that are modified or discarded would have met the same fate later anyway, during codegen. This PR just makes that happen earlier.

So there's something mysterious going on, where discarding these spans early somehow causes us to emit malformed or incomplete instrprof metadata.

covspan.span = span;
true
});

// Merge covspans that can be merged.
covspans.dedup_by(|b, a| a.merge_if_eligible(b));

code_mappings.extend(covspans.into_iter().map(|Covspan { span, bcb }| {
Expand Down Expand Up @@ -230,3 +241,26 @@ fn compare_spans(a: Span, b: Span) -> std::cmp::Ordering {
// - Both have the same start and span A extends further right
.then_with(|| Ord::cmp(&a.hi(), &b.hi()).reverse())
}

fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> {
if !span.is_empty() {
return Some(span);
}

// The span is empty, so try to enlarge it to cover an adjacent '{' or '}'.
source_map
.span_to_source(span, |src, start, end| try {
// Adjusting span endpoints by `BytePos(1)` is normally a bug,
// but in this case we have specifically checked that the character
// we're skipping over is one of two specific ASCII characters, so
// adjusting by exactly 1 byte is correct.
if src.as_bytes().get(end).copied() == Some(b'{') {
Some(span.with_hi(span.hi() + BytePos(1)))
} else if start > 0 && src.as_bytes()[start - 1] == b'}' {
Some(span.with_lo(span.lo() - BytePos(1)))
} else {
None
}
})
.ok()?
}
21 changes: 9 additions & 12 deletions tests/coverage/async_closure.cov-map
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,32 +37,29 @@ Number of file 0 mappings: 8
Highest counter ID seen: c0

Function name: async_closure::main::{closure#0}
Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24]
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
Number of files: 1
- file 0 => $DIR/async_closure.rs
Number of expressions: 0
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35)
- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36)
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
Highest counter ID seen: c0

Function name: async_closure::main::{closure#0}
Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24]
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
Number of files: 1
- file 0 => $DIR/async_closure.rs
Number of expressions: 0
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35)
- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36)
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
Highest counter ID seen: c0

Function name: async_closure::main::{closure#0}::{closure#0}::<i16>
Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24]
Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
Number of files: 1
- file 0 => $DIR/async_closure.rs
Number of expressions: 0
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35)
- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36)
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
Highest counter ID seen: c0

2 changes: 1 addition & 1 deletion tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
+ coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:17: 19:18 (#0);
+ coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:23: 19:30 (#0);
+ coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:31: 19:32 (#0);
+ coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:2: 21:2 (#0);
+ coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:1: 21:2 (#0);
+
bb0: {
+ Coverage::VirtualCounter(bcb0);
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:27:1: 27:17 (#0);
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:28:5: 28:9 (#0);
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:2: 29:2 (#0);
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:1: 29:2 (#0);
+
bb0: {
+ Coverage::VirtualCounter(bcb0);
Expand Down
4 changes: 2 additions & 2 deletions tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:13:1: 13:10 (#0);
+ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage.rs:15:12: 15:15 (#0);
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:16:13: 16:18 (#0);
+ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage.rs:17:10: 17:10 (#0);
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:19:2: 19:2 (#0);
+ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage.rs:17:9: 17:10 (#0);
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:19:1: 19:2 (#0);
+
bb0: {
+ Coverage::VirtualCounter(bcb0);
Expand Down
4 changes: 2 additions & 2 deletions tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:13:1: 13:10 (#0);
coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
coverage Code { bcb: bcb3 } => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0);
coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0);
coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0);
coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:38: 14:39 (#0);
coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:1: 15:2 (#0);
coverage Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);

bb0: {
Expand Down
4 changes: 2 additions & 2 deletions tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:13:1: 13:10 (#0);
+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
+ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0);
+ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0);
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0);
+ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:38: 14:39 (#0);
+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:1: 15:2 (#0);
+ coverage Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
+
bb0: {
Expand Down
Loading