From 5f40a4f7a0aa6552e615fe89a6018e36c93f672e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Mon, 25 Jul 2022 00:00:00 +0000 Subject: [PATCH] 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 #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; } ... } ``` --- compiler/rustc_mir_transform/src/simplify.rs | 12 +++++- .../expected_show_coverage.inline-dead.txt | 39 +++++++++++-------- .../run-make-fulldeps/coverage/inline-dead.rs | 9 ++++- 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_mir_transform/src/simplify.rs b/compiler/rustc_mir_transform/src/simplify.rs index d305960b485..180f4c7dcd6 100644 --- a/compiler/rustc_mir_transform/src/simplify.rs +++ b/compiler/rustc_mir_transform/src/simplify.rs @@ -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()`). @@ -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; } diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline-dead.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline-dead.txt index d102d9ecf7d..effdef80e8e 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline-dead.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline-dead.txt @@ -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::()); - 6| 1|} - 7| | - 8| |#[inline] - 9| 1|fn live() -> 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() -> 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|} diff --git a/src/test/run-make-fulldeps/coverage/inline-dead.rs b/src/test/run-make-fulldeps/coverage/inline-dead.rs index cd1ae911a5f..854fa062967 100644 --- a/src/test/run-make-fulldeps/coverage/inline-dead.rs +++ b/src/test/run-make-fulldeps/coverage/inline-dead.rs @@ -1,8 +1,15 @@ // Regression test for issue #98833. -// compile-flags: -Zinline-mir +// compile-flags: -Zinline-mir -Cdebug-assertions=off fn main() { println!("{}", live::()); + + let f = |x: bool| { + debug_assert!( + x + ); + }; + f(false); } #[inline]