diff options
| author | Rich Kadel <richkadel@google.com> | 2021-04-02 00:08:48 -0700 |
|---|---|---|
| committer | Rich Kadel <richkadel@google.com> | 2021-04-02 17:16:36 -0700 |
| commit | 7ceff6835abc3ce991e1a8cdcbe2be2730335a65 (patch) | |
| tree | 30405b09690336982eb681147e486ea3f95c3b98 /src/test | |
| parent | 138fd56cf9598b4bf016634c768dca128a83a5d7 (diff) | |
| download | rust-7ceff6835abc3ce991e1a8cdcbe2be2730335a65.tar.gz rust-7ceff6835abc3ce991e1a8cdcbe2be2730335a65.zip | |
Translate counters from Rust 1-based to LLVM 0-based counter ids
A colleague contacted me and asked why Rust's counters start at 1, when Clangs appear to start at 0. There is a reason why Rust's internal counters start at 1 (see the docs), and I tried to keep them consistent when codegenned to LLVM's coverage mapping format. LLVM should be tolerant of missing counters, but as my colleague pointed out, `llvm-cov` will silently fail to generate a coverage report for a function based on LLVM's assumption that the counters are 0-based. See: https://github.com/llvm/llvm-project/blob/main/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp#L170 Apparently, if, for example, a function has no branches, it would have exactly 1 counter. `CounterValues.size()` would be 1, and (with the 1-based index), the counter ID would be 1. This would fail the check and abort reporting coverage for the function. It turns out that by correcting for this during coverage map generation, by subtracting 1 from the Rust Counter ID (both when generating the counter increment intrinsic call, and when adding counters to the map), some uncovered functions (including in tests) now appear covered! This corrects the coverage for a few tests!
Diffstat (limited to 'src/test')
6 files changed, 107 insertions, 72 deletions
diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt index d9097bb50e5..e0a5937c246 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt @@ -10,7 +10,7 @@ 10| | } 11| 1|} 12| | - 13| |async fn d() -> u8 { 1 } // should have a coverage count `0` (see below) + 13| 0|async fn d() -> u8 { 1 } 14| | 15| 0|async fn e() -> u8 { 1 } // unused function; executor does not block on `g()` 16| | @@ -66,7 +66,8 @@ 63| 1| 0 64| 1| } 65| 1| } - 66| 1| fn d() -> u8 { 1 } + 66| 1| fn d() -> u8 { 1 } // inner function is defined in-line, but the function is not executed + ^0 67| 1| fn f() -> u8 { 1 } 68| 1| match x { 69| 1| y if c(x) == y + 1 => { d(); } @@ -115,11 +116,14 @@ 109| | 110| 1| pub fn block_on<F: Future>(mut future: F) -> F::Output { 111| 1| let mut future = unsafe { Pin::new_unchecked(&mut future) }; - 112| 1| + 112| 1| use std::hint::unreachable_unchecked; 113| 1| static VTABLE: RawWakerVTable = RawWakerVTable::new( - 114| 1| |_| unimplemented!("clone"), - 115| 1| |_| unimplemented!("wake"), - 116| 1| |_| unimplemented!("wake_by_ref"), + 114| 1| |_| unsafe { unreachable_unchecked() }, // clone + ^0 + 115| 1| |_| unsafe { unreachable_unchecked() }, // wake + ^0 + 116| 1| |_| unsafe { unreachable_unchecked() }, // wake_by_ref + ^0 117| 1| |_| (), 118| 1| ); 119| 1| let waker = unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &VTABLE)) }; @@ -132,15 +136,4 @@ 126| | } 127| 1| } 128| |} - 129| | - 130| |// `llvm-cov show` shows no coverage results for the `d()`, even though the - 131| |// crate's LLVM IR shows the function exists and has an InstrProf PGO counter, - 132| |// and appears to be registered like all other counted functions. - 133| |// - 134| |// `llvm-cov show --debug` output shows there is at least one `Counter` for this - 135| |// line, but counters do not appear in the `Combined regions` section (unlike - 136| |// `f()`, which is similar, but does appear in `Combined regions`, and does show - 137| |// coverage). The only difference is, `f()` is awaited, but the call to await - 138| |// `d()` is not reached. (Note: `d()` will appear in coverage if the test is - 139| |// modified to cause it to be awaited.) diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt index 7d2abe05973..1b6bb9ff889 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt @@ -56,7 +56,7 @@ 46| 1|//! println!("called some_func()"); 47| 1|//! } 48| |//! - 49| |//! #[derive(Debug)] + 49| 0|//! #[derive(Debug)] 50| |//! struct SomeError; 51| |//! 52| |//! extern crate doctest_crate; diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline.txt index 6148d89ed75..31d3ddea8d9 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline.txt @@ -47,7 +47,7 @@ 46| 6|} 47| | 48| |#[inline(always)] - 49| |fn error() { - 50| | panic!("error"); - 51| |} + 49| 0|fn error() { + 50| 0| panic!("error"); + 51| 0|} diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.loops_branches.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.loops_branches.txt index 474f02b7007..81d5c7d9034 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.loops_branches.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.loops_branches.txt @@ -1,7 +1,7 @@ 1| |#![allow(unused_assignments, unused_variables, while_true)] 2| | - 3| |// This test confirms an earlier problem was resolved, supporting the MIR graph generated by the - 4| |// structure of this `fmt` function. + 3| |// This test confirms that (1) unexecuted infinite loops are handled correctly by the + 4| |// InstrumentCoverage MIR pass; and (2) Counter Expressions that subtract from zero can be dropped. 5| | 6| |struct DebugTest; 7| | @@ -16,23 +16,51 @@ ^0 16| | } else { 17| | } - 18| 1| Ok(()) - 19| 1| } - 20| |} - 21| | - 22| 1|fn main() { - 23| 1| let debug_test = DebugTest; - 24| 1| println!("{:?}", debug_test); - 25| 1|} - 26| | - 27| |/* - 28| | - 29| |This is the error message generated, before the issue was fixed: - 30| | - 31| |error: internal compiler error: compiler/rustc_mir/src/transform/coverage/mod.rs:374:42: - 32| |Error processing: DefId(0:6 ~ bug_incomplete_cov_graph_traversal_simplified[317d]::{impl#0}::fmt): - 33| |Error { message: "`TraverseCoverageGraphWithLoops` missed some `BasicCoverageBlock`s: - 34| |[bcb6, bcb7, bcb9]" } - 35| | - 36| |*/ + 18| | + 19| 10| for i in 0..10 { + 20| 10| if true { + 21| 10| if false { + 22| | while true {} + 23| 10| } + 24| 10| write!(f, "error")?; + ^0 + 25| | } else { + 26| | } + 27| | } + 28| 1| Ok(()) + 29| 1| } + 30| |} + 31| | + 32| |struct DisplayTest; + 33| | + 34| |impl std::fmt::Display for DisplayTest { + 35| 1| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + 36| 1| if false { + 37| | } else { + 38| 1| if false { + 39| | while true {} + 40| 1| } + 41| 1| write!(f, "error")?; + ^0 + 42| | } + 43| 10| for i in 0..10 { + 44| 10| if false { + 45| | } else { + 46| 10| if false { + 47| | while true {} + 48| 10| } + 49| 10| write!(f, "error")?; + ^0 + 50| | } + 51| | } + 52| 1| Ok(()) + 53| 1| } + 54| |} + 55| | + 56| 1|fn main() { + 57| 1| let debug_test = DebugTest; + 58| 1| println!("{:?}", debug_test); + 59| 1| let display_test = DisplayTest; + 60| 1| println!("{}", display_test); + 61| 1|} diff --git a/src/test/run-make-fulldeps/coverage/async.rs b/src/test/run-make-fulldeps/coverage/async.rs index f08a7b44fbd..67bf696d072 100644 --- a/src/test/run-make-fulldeps/coverage/async.rs +++ b/src/test/run-make-fulldeps/coverage/async.rs @@ -10,7 +10,7 @@ async fn c(x: u8) -> u8 { } } -async fn d() -> u8 { 1 } // should have a coverage count `0` (see below) +async fn d() -> u8 { 1 } async fn e() -> u8 { 1 } // unused function; executor does not block on `g()` @@ -63,7 +63,7 @@ fn j(x: u8) { 0 } } - fn d() -> u8 { 1 } + fn d() -> u8 { 1 } // inner function is defined in-line, but the function is not executed fn f() -> u8 { 1 } match x { y if c(x) == y + 1 => { d(); } @@ -109,11 +109,11 @@ mod executor { pub fn block_on<F: Future>(mut future: F) -> F::Output { let mut future = unsafe { Pin::new_unchecked(&mut future) }; - + use std::hint::unreachable_unchecked; static VTABLE: RawWakerVTable = RawWakerVTable::new( - |_| unimplemented!("clone"), - |_| unimplemented!("wake"), - |_| unimplemented!("wake_by_ref"), + |_| unsafe { unreachable_unchecked() }, // clone + |_| unsafe { unreachable_unchecked() }, // wake + |_| unsafe { unreachable_unchecked() }, // wake_by_ref |_| (), ); let waker = unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &VTABLE)) }; @@ -126,14 +126,3 @@ mod executor { } } } - -// `llvm-cov show` shows no coverage results for the `d()`, even though the -// crate's LLVM IR shows the function exists and has an InstrProf PGO counter, -// and appears to be registered like all other counted functions. -// -// `llvm-cov show --debug` output shows there is at least one `Counter` for this -// line, but counters do not appear in the `Combined regions` section (unlike -// `f()`, which is similar, but does appear in `Combined regions`, and does show -// coverage). The only difference is, `f()` is awaited, but the call to await -// `d()` is not reached. (Note: `d()` will appear in coverage if the test is -// modified to cause it to be awaited.) diff --git a/src/test/run-make-fulldeps/coverage/loops_branches.rs b/src/test/run-make-fulldeps/coverage/loops_branches.rs index 938421d32e7..4d9bbad3367 100644 --- a/src/test/run-make-fulldeps/coverage/loops_branches.rs +++ b/src/test/run-make-fulldeps/coverage/loops_branches.rs @@ -1,7 +1,7 @@ #![allow(unused_assignments, unused_variables, while_true)] -// This test confirms an earlier problem was resolved, supporting the MIR graph generated by the -// structure of this `fmt` function. +// This test confirms that (1) unexecuted infinite loops are handled correctly by the +// InstrumentCoverage MIR pass; and (2) Counter Expressions that subtract from zero can be dropped. struct DebugTest; @@ -15,6 +15,40 @@ impl std::fmt::Debug for DebugTest { write!(f, "error")?; } else { } + + for i in 0..10 { + if true { + if false { + while true {} + } + write!(f, "error")?; + } else { + } + } + Ok(()) + } +} + +struct DisplayTest; + +impl std::fmt::Display for DisplayTest { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + if false { + } else { + if false { + while true {} + } + write!(f, "error")?; + } + for i in 0..10 { + if false { + } else { + if false { + while true {} + } + write!(f, "error")?; + } + } Ok(()) } } @@ -22,15 +56,6 @@ impl std::fmt::Debug for DebugTest { fn main() { let debug_test = DebugTest; println!("{:?}", debug_test); + let display_test = DisplayTest; + println!("{}", display_test); } - -/* - -This is the error message generated, before the issue was fixed: - -error: internal compiler error: compiler/rustc_mir/src/transform/coverage/mod.rs:374:42: -Error processing: DefId(0:6 ~ bug_incomplete_cov_graph_traversal_simplified[317d]::{impl#0}::fmt): -Error { message: "`TraverseCoverageGraphWithLoops` missed some `BasicCoverageBlock`s: -[bcb6, bcb7, bcb9]" } - -*/ |
