Skip to content
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.

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

coverage: Don't show coverage for code paths that must panic/diverge #120013

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
68 changes: 52 additions & 16 deletions compiler/rustc_mir_transform/src/coverage/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,30 @@ impl CoverageGraph {
}
}

let mut basic_coverage_blocks =
Self { bcbs, bb_to_bcb, successors, predecessors, dominators: None };
let dominators = dominators::dominators(&basic_coverage_blocks);
basic_coverage_blocks.dominators = Some(dominators);
let mut this = Self { bcbs, bb_to_bcb, successors, predecessors, dominators: None };

this.dominators = Some(dominators::dominators(&this));

// Initialize the `must_diverge` flag for each BCB. Post-order traversal
// ensures that each node's successors have been processed first.
for bcb in graph::iterate::post_order_from(&this, this.start_node()) {
let terminator = mir_body[this[bcb].last_bb()].terminator();

// A BCB is assumed to always diverge if its terminator is not a
// return, and all of its successors always diverge.
let must_diverge = !bcb_filtered_successors(terminator).has_return_arc()
&& this.successors[bcb].iter().all(|&s| this[s].must_diverge);
this[bcb].must_diverge = must_diverge;
}

// The coverage graph's entry-point node (bcb0) always starts with bb0,
// which never has predecessors. Any other blocks merged into bcb0 can't
// have multiple (coverage-relevant) predecessors, so bcb0 always has
// zero in-edges.
assert!(basic_coverage_blocks[START_BCB].leader_bb() == mir::START_BLOCK);
assert!(basic_coverage_blocks.predecessors[START_BCB].is_empty());
assert!(this[START_BCB].leader_bb() == mir::START_BLOCK);
assert!(this.predecessors[START_BCB].is_empty());

basic_coverage_blocks
this
}

fn compute_basic_coverage_blocks(
Expand Down Expand Up @@ -274,13 +285,15 @@ rustc_index::newtype_index! {
/// significance.
#[derive(Debug, Clone)]
pub(super) struct BasicCoverageBlockData {
pub(super) must_diverge: bool,
pub basic_blocks: Vec<BasicBlock>,
}

impl BasicCoverageBlockData {
pub fn from(basic_blocks: Vec<BasicBlock>) -> Self {
assert!(basic_blocks.len() > 0);
Self { basic_blocks }
// `must_diverge` is set by a separate postprocessing step.
Self { must_diverge: false, basic_blocks }
}

#[inline(always)]
Expand All @@ -303,14 +316,23 @@ enum CoverageSuccessors<'a> {
/// potentially be combined into the same BCB as that successor.
Chainable(BasicBlock),
/// The block cannot be combined into the same BCB as its successor(s).
NotChainable(&'a [BasicBlock]),
NotChainable { has_return_arc: bool, successors: &'a [BasicBlock] },
}

impl CoverageSuccessors<'_> {
fn is_chainable(&self) -> bool {
match self {
Self::Chainable(_) => true,
Self::NotChainable(_) => false,
Self::NotChainable { .. } => false,
}
}

/// If true, the block's terminator can return, so it should not be
/// assumed to diverge even if all of its successors diverge.
fn has_return_arc(&self) -> bool {
match *self {
Self::Chainable(_) => false,
Self::NotChainable { has_return_arc, .. } => has_return_arc,
}
}
}
Expand All @@ -322,7 +344,9 @@ impl IntoIterator for CoverageSuccessors<'_> {
fn into_iter(self) -> Self::IntoIter {
match self {
Self::Chainable(bb) => Some(bb).into_iter().chain((&[]).iter().copied()),
Self::NotChainable(bbs) => None.into_iter().chain(bbs.iter().copied()),
Self::NotChainable { successors, .. } => {
None.into_iter().chain(successors.iter().copied())
}
}
}
}
Expand All @@ -336,11 +360,17 @@ fn bcb_filtered_successors<'a, 'tcx>(terminator: &'a Terminator<'tcx>) -> Covera
match terminator.kind {
// A switch terminator can have many coverage-relevant successors.
// (If there is exactly one successor, we still treat it as not chainable.)
SwitchInt { ref targets, .. } => CoverageSuccessors::NotChainable(targets.all_targets()),
SwitchInt { ref targets, .. } => CoverageSuccessors::NotChainable {
has_return_arc: false,
successors: targets.all_targets(),
},

// A yield terminator has exactly 1 successor, but should not be chained,
// because its resume edge has a different execution count.
Yield { ref resume, .. } => CoverageSuccessors::NotChainable(std::slice::from_ref(resume)),
Yield { ref resume, .. } => CoverageSuccessors::NotChainable {
has_return_arc: true,
successors: std::slice::from_ref(resume),
},

// These terminators have exactly one coverage-relevant successor,
// and can be chained into it.
Expand All @@ -355,13 +385,19 @@ fn bcb_filtered_successors<'a, 'tcx>(terminator: &'a Terminator<'tcx>) -> Covera
Call { target: maybe_target, .. } | InlineAsm { destination: maybe_target, .. } => {
match maybe_target {
Some(target) => CoverageSuccessors::Chainable(target),
None => CoverageSuccessors::NotChainable(&[]),
None => CoverageSuccessors::NotChainable { has_return_arc: false, successors: &[] },
}
}

// These terminators have no actual coverage-relevant successors,
// but they do return from their enclosing function.
CoroutineDrop | Return => {
CoverageSuccessors::NotChainable { has_return_arc: true, successors: &[] }
}

// These terminators have no coverage-relevant successors.
CoroutineDrop | Return | Unreachable | UnwindResume | UnwindTerminate(_) => {
CoverageSuccessors::NotChainable(&[])
Unreachable | UnwindResume | UnwindTerminate(_) => {
CoverageSuccessors::NotChainable { has_return_arc: false, successors: &[] }
}
}
}
Expand Down
15 changes: 11 additions & 4 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,17 @@ pub(super) fn generate_coverage_spans(
basic_coverage_blocks,
);
let coverage_spans = SpansRefiner::refine_sorted_spans(sorted_spans);
mappings.extend(coverage_spans.into_iter().map(|RefinedCovspan { bcb, span, .. }| {
// Each span produced by the generator represents an ordinary code region.
BcbMapping { kind: BcbMappingKind::Code(bcb), span }
}));
mappings.extend(coverage_spans.into_iter().filter_map(
|RefinedCovspan { bcb, span, .. }| {
if basic_coverage_blocks[bcb].must_diverge {
// After refinement, discard spans associated with BCBs that must panic/diverge.
None
} else {
// Each span produced by the generator represents an ordinary code region.
Some(BcbMapping { kind: BcbMappingKind::Code(bcb), span })
}
},
));
}

if mappings.is_empty() {
Expand Down
7 changes: 3 additions & 4 deletions tests/coverage/abort.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,13 @@ Number of file 0 mappings: 13
- Code(Counter(3)) at (prev + 2, 5) to (start + 1, 2)

Function name: abort::might_abort
Raw bytes (21): 0x[01, 01, 01, 01, 05, 03, 01, 04, 01, 01, 14, 05, 02, 09, 01, 24, 02, 02, 0c, 03, 02]
Raw bytes (16): 0x[01, 01, 01, 01, 05, 02, 01, 04, 01, 01, 14, 02, 04, 0c, 03, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 1
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
Number of file 0 mappings: 3
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 4, 1) to (start + 1, 20)
- Code(Counter(1)) at (prev + 2, 9) to (start + 1, 36)
- Code(Expression(0, Sub)) at (prev + 2, 12) to (start + 3, 2)
- Code(Expression(0, Sub)) at (prev + 4, 12) to (start + 3, 2)
= (c0 - c1)

4 changes: 2 additions & 2 deletions tests/coverage/abort.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
LL| |
LL| 12|extern "C" fn might_abort(should_abort: bool) {
LL| 12| if should_abort {
LL| 0| println!("aborting...");
LL| 0| panic!("panics and aborts");
LL| | println!("aborting...");
LL| | panic!("panics and aborts");
LL| 12| } else {
LL| 12| println!("Don't Panic");
LL| 12| }
Expand Down
11 changes: 4 additions & 7 deletions tests/coverage/assert.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,11 @@ Number of file 0 mappings: 9
- Code(Counter(4)) at (prev + 2, 5) to (start + 1, 2)

Function name: assert::might_fail_assert
Raw bytes (21): 0x[01, 01, 01, 01, 05, 03, 01, 04, 01, 02, 0f, 02, 02, 25, 00, 3d, 05, 01, 01, 00, 02]
Raw bytes (14): 0x[01, 01, 00, 02, 01, 04, 01, 02, 0f, 05, 03, 01, 00, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 1
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
Number of file 0 mappings: 3
Number of expressions: 0
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 4, 1) to (start + 2, 15)
- Code(Expression(0, Sub)) at (prev + 2, 37) to (start + 0, 61)
= (c0 - c1)
- Code(Counter(1)) at (prev + 1, 1) to (start + 0, 2)
- Code(Counter(1)) at (prev + 3, 1) to (start + 0, 2)

1 change: 0 additions & 1 deletion tests/coverage/assert.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
LL| 4|fn might_fail_assert(one_plus_one: u32) {
LL| 4| println!("does 1 + 1 = {}?", one_plus_one);
LL| 4| assert_eq!(1 + 1, one_plus_one, "the argument was wrong");
^1
LL| 3|}
LL| |
LL| 1|fn main() -> Result<(), u8> {
Expand Down
32 changes: 13 additions & 19 deletions tests/coverage/bad_counter_ids.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,13 @@ Number of file 0 mappings: 2
- Code(Zero) at (prev + 3, 1) to (start + 0, 2)

Function name: bad_counter_ids::eq_bad_message
Raw bytes (21): 0x[01, 01, 01, 01, 00, 03, 01, 29, 01, 02, 0f, 02, 02, 20, 00, 2b, 00, 01, 01, 00, 02]
Raw bytes (14): 0x[01, 01, 00, 02, 01, 29, 01, 02, 0f, 00, 03, 01, 00, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 1
- expression 0 operands: lhs = Counter(0), rhs = Zero
Number of file 0 mappings: 3
Number of expressions: 0
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 41, 1) to (start + 2, 15)
- Code(Expression(0, Sub)) at (prev + 2, 32) to (start + 0, 43)
= (c0 - Zero)
- Code(Zero) at (prev + 1, 1) to (start + 0, 2)
- Code(Zero) at (prev + 3, 1) to (start + 0, 2)

Function name: bad_counter_ids::eq_good
Raw bytes (14): 0x[01, 01, 00, 02, 01, 10, 01, 02, 1f, 05, 03, 01, 00, 02]
Expand All @@ -29,14 +26,13 @@ Number of file 0 mappings: 2
- Code(Counter(1)) at (prev + 3, 1) to (start + 0, 2)

Function name: bad_counter_ids::eq_good_message
Raw bytes (19): 0x[01, 01, 00, 03, 01, 15, 01, 02, 0f, 00, 02, 20, 00, 2b, 05, 01, 01, 00, 02]
Raw bytes (14): 0x[01, 01, 00, 02, 01, 15, 01, 02, 0f, 05, 03, 01, 00, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 3
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 21, 1) to (start + 2, 15)
- Code(Zero) at (prev + 2, 32) to (start + 0, 43)
- Code(Counter(1)) at (prev + 1, 1) to (start + 0, 2)
- Code(Counter(1)) at (prev + 3, 1) to (start + 0, 2)

Function name: bad_counter_ids::ne_bad
Raw bytes (14): 0x[01, 01, 00, 02, 01, 2e, 01, 02, 1f, 00, 03, 01, 00, 02]
Expand All @@ -48,14 +44,13 @@ Number of file 0 mappings: 2
- Code(Zero) at (prev + 3, 1) to (start + 0, 2)

Function name: bad_counter_ids::ne_bad_message
Raw bytes (19): 0x[01, 01, 00, 03, 01, 33, 01, 02, 0f, 05, 02, 20, 00, 2b, 00, 01, 01, 00, 02]
Raw bytes (14): 0x[01, 01, 00, 02, 01, 33, 01, 02, 0f, 00, 03, 01, 00, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 3
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 51, 1) to (start + 2, 15)
- Code(Counter(1)) at (prev + 2, 32) to (start + 0, 43)
- Code(Zero) at (prev + 1, 1) to (start + 0, 2)
- Code(Zero) at (prev + 3, 1) to (start + 0, 2)

Function name: bad_counter_ids::ne_good
Raw bytes (16): 0x[01, 01, 01, 01, 00, 02, 01, 1a, 01, 02, 1f, 02, 03, 01, 00, 02]
Expand All @@ -69,14 +64,13 @@ Number of file 0 mappings: 2
= (c0 - Zero)

Function name: bad_counter_ids::ne_good_message
Raw bytes (21): 0x[01, 01, 01, 01, 00, 03, 01, 1f, 01, 02, 0f, 00, 02, 20, 00, 2b, 02, 01, 01, 00, 02]
Raw bytes (16): 0x[01, 01, 01, 01, 00, 02, 01, 1f, 01, 02, 0f, 02, 03, 01, 00, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 1
- expression 0 operands: lhs = Counter(0), rhs = Zero
Number of file 0 mappings: 3
Number of file 0 mappings: 2
- Code(Counter(0)) at (prev + 31, 1) to (start + 2, 15)
- Code(Zero) at (prev + 2, 32) to (start + 0, 43)
- Code(Expression(0, Sub)) at (prev + 1, 1) to (start + 0, 2)
- Code(Expression(0, Sub)) at (prev + 3, 1) to (start + 0, 2)
= (c0 - Zero)

2 changes: 0 additions & 2 deletions tests/coverage/bad_counter_ids.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
LL| 1|fn eq_good_message() {
LL| 1| println!("b");
LL| 1| assert_eq!(Foo(1), Foo(1), "message b");
^0
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

LL| 1|}
LL| |
LL| 1|fn ne_good() {
Expand All @@ -32,7 +31,6 @@
LL| 1|fn ne_good_message() {
LL| 1| println!("d");
LL| 1| assert_ne!(Foo(1), Foo(3), "message d");
^0
LL| 1|}
LL| |
LL| 1|fn eq_bad() {
Expand Down
33 changes: 10 additions & 23 deletions tests/coverage/coroutine.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,20 @@ Number of file 0 mappings: 4
= (c1 + (c0 - c1))

Function name: coroutine::main
Raw bytes (65): 0x[01, 01, 08, 07, 0d, 05, 09, 11, 15, 1e, 19, 11, 15, 15, 19, 1e, 19, 11, 15, 09, 01, 13, 01, 02, 16, 01, 07, 0b, 00, 2e, 11, 01, 2b, 00, 2d, 03, 01, 0e, 00, 35, 11, 02, 0b, 00, 2e, 1e, 01, 22, 00, 27, 1a, 00, 2c, 00, 2e, 17, 01, 0e, 00, 35, 1a, 02, 01, 00, 02]
Raw bytes (41): 0x[01, 01, 01, 09, 0d, 07, 01, 13, 01, 02, 16, 01, 07, 0b, 00, 2e, 09, 01, 2b, 00, 2d, 09, 03, 0b, 00, 2e, 02, 01, 22, 00, 27, 11, 00, 2c, 00, 2e, 11, 03, 01, 00, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 8
- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(3)
- expression 1 operands: lhs = Counter(1), rhs = Counter(2)
- expression 2 operands: lhs = Counter(4), rhs = Counter(5)
- expression 3 operands: lhs = Expression(7, Sub), rhs = Counter(6)
- expression 4 operands: lhs = Counter(4), rhs = Counter(5)
- expression 5 operands: lhs = Counter(5), rhs = Counter(6)
- expression 6 operands: lhs = Expression(7, Sub), rhs = Counter(6)
- expression 7 operands: lhs = Counter(4), rhs = Counter(5)
Number of file 0 mappings: 9
Number of expressions: 1
- expression 0 operands: lhs = Counter(2), rhs = Counter(3)
Number of file 0 mappings: 7
- Code(Counter(0)) at (prev + 19, 1) to (start + 2, 22)
- Code(Counter(0)) at (prev + 7, 11) to (start + 0, 46)
- Code(Counter(4)) at (prev + 1, 43) to (start + 0, 45)
- Code(Expression(0, Add)) at (prev + 1, 14) to (start + 0, 53)
= ((c1 + c2) + c3)
- Code(Counter(4)) at (prev + 2, 11) to (start + 0, 46)
- Code(Expression(7, Sub)) at (prev + 1, 34) to (start + 0, 39)
= (c4 - c5)
- Code(Expression(6, Sub)) at (prev + 0, 44) to (start + 0, 46)
= ((c4 - c5) - c6)
- Code(Expression(5, Add)) at (prev + 1, 14) to (start + 0, 53)
= (c5 + c6)
- Code(Expression(6, Sub)) at (prev + 2, 1) to (start + 0, 2)
= ((c4 - c5) - c6)
- Code(Counter(2)) at (prev + 1, 43) to (start + 0, 45)
- Code(Counter(2)) at (prev + 3, 11) to (start + 0, 46)
- Code(Expression(0, Sub)) at (prev + 1, 34) to (start + 0, 39)
= (c2 - c3)
- Code(Counter(4)) at (prev + 0, 44) to (start + 0, 46)
- Code(Counter(4)) at (prev + 3, 1) to (start + 0, 2)

Function name: coroutine::main::{closure#0}
Raw bytes (14): 0x[01, 01, 00, 02, 01, 15, 1c, 01, 1f, 05, 02, 10, 01, 06]
Expand Down
4 changes: 2 additions & 2 deletions tests/coverage/coroutine.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@
LL| |
LL| 1| match Pin::new(&mut coroutine).resume(()) {
LL| 1| CoroutineState::Yielded(Ok(1)) => {}
LL| 0| _ => panic!("unexpected return from resume"),
LL| | _ => panic!("unexpected return from resume"),
LL| | }
LL| 1| match Pin::new(&mut coroutine).resume(()) {
LL| 1| CoroutineState::Complete("foo") => {}
LL| 0| _ => panic!("unexpected return from resume"),
LL| | _ => panic!("unexpected return from resume"),
LL| | }
LL| 1|}

8 changes: 0 additions & 8 deletions tests/coverage/inline.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,6 @@ Number of file 0 mappings: 5
- Code(Expression(1, Sub)) at (prev + 3, 5) to (start + 1, 2)
= ((c0 + c1) - c1)

Function name: inline::error
Raw bytes (9): 0x[01, 01, 00, 01, 01, 31, 01, 01, 14]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 49, 1) to (start + 1, 20)

Function name: inline::length::<char>
Raw bytes (9): 0x[01, 01, 00, 01, 01, 1e, 01, 02, 02]
Number of files: 1
Expand Down
4 changes: 2 additions & 2 deletions tests/coverage/inline.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
LL| 6|}
LL| |
LL| |#[inline(always)]
LL| 0|fn error() {
LL| 0| panic!("error");
LL| |fn error() {
LL| | panic!("error");
LL| |}

Loading
Loading