Skip to content

Commit

Permalink
Rollup merge of rust-lang#99711 - tmiasko:coverage, r=wesleywiser
Browse files Browse the repository at this point in the history
Remove reachable coverage without counters

Remove reachable coverage without counters to maintain invariant that
either there is no coverage at all or there is a live coverage counter
left that provides the function source hash.

The motivating example would be a following closure:

```rust
    let f = |x: bool| {
        debug_assert!(x);
    };
```

Which, with span changes from rust-lang#93967, with disabled debug assertions,
after the final CFG simplifications but before removal of dead blocks,
gives rise to MIR:

```rust
fn main::{closure#0}(_1: &[closure@a.rs:2:13: 2:22], _2: bool) -> () {
    debug x => _2;
    let mut _0: ();

    bb0: {
        Coverage::Expression(4294967295) = 1 - 2;
        return;
    }

    ...
}
```

Which also makes the initial instrumentation quite suspect, although
this pull request doesn't attempt to address that aspect directly.

Fixes rust-lang#98833.

r? `@wesleywiser` `@richkadel`
  • Loading branch information
Dylan-DPC authored Jul 25, 2022
2 parents f3bf936 + 5f40a4f commit 0d25480
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 18 deletions.
12 changes: 11 additions & 1 deletion compiler/rustc_mir_transform/src/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
/// with `0` executions.
///
/// If there are no live `Counter` `Coverage` statements remaining, we remove
/// dead `Coverage` statements along with the dead blocks. Since at least one
/// `Coverage` statements along with the dead blocks. Since at least one
/// counter per function is required by LLVM (and necessary, to add the
/// `function_hash` to the counter's call to the LLVM intrinsic
/// `instrprof.increment()`).
Expand All @@ -342,6 +342,16 @@ fn save_unreachable_coverage(
}
}

for block in &mut basic_blocks.raw[..first_dead_block] {
for statement in &mut block.statements {
let StatementKind::Coverage(_) = &statement.kind else { continue };
let instance = statement.source_info.scope.inlined_instance(source_scopes);
if !live.contains(&instance) {
statement.make_nop();
}
}
}

if live.is_empty() {
return;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
1| |// Regression test for issue #98833.
2| |// compile-flags: -Zinline-mir
2| |// compile-flags: -Zinline-mir -Cdebug-assertions=off
3| |
4| 1|fn main() {
5| 1| println!("{}", live::<false>());
6| 1|}
7| |
8| |#[inline]
9| 1|fn live<const B: bool>() -> u32 {
10| 1| if B {
11| 0| dead()
12| | } else {
13| 1| 0
14| | }
15| 1|}
16| |
17| |#[inline]
18| 0|fn dead() -> u32 {
19| 0| 42
20| 0|}
6| 1|
7| 1| let f = |x: bool| {
8| | debug_assert!(
9| | x
10| | );
11| 1| };
12| 1| f(false);
13| 1|}
14| |
15| |#[inline]
16| 1|fn live<const B: bool>() -> u32 {
17| 1| if B {
18| 0| dead()
19| | } else {
20| 1| 0
21| | }
22| 1|}
23| |
24| |#[inline]
25| 0|fn dead() -> u32 {
26| 0| 42
27| 0|}

9 changes: 8 additions & 1 deletion src/test/run-make-fulldeps/coverage/inline-dead.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
// Regression test for issue #98833.
// compile-flags: -Zinline-mir
// compile-flags: -Zinline-mir -Cdebug-assertions=off

fn main() {
println!("{}", live::<false>());

let f = |x: bool| {
debug_assert!(
x
);
};
f(false);
}

#[inline]
Expand Down

0 comments on commit 0d25480

Please sign in to comment.