rust-lang/rust
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Combination of commits

Browse Source 
Fixes multiple issue with counters, with simplification

  Includes a change to the implicit else span in ast_lowering, so coverage
  of the implicit else no longer spans the `then` block.

  Adds coverage for unused closures and async function bodies.

  Fixes: #78542

Adding unreachable regions for known MIR missing from coverage map

Cleaned up PR commits, and removed link-dead-code requirement and tests

  Coverage no longer depends on Issue #76038 (`-C link-dead-code` is
  no longer needed or enforced, so MSVC can use the same tests as
  Linux and MacOS now)

Restrict adding unreachable regions to covered files

  Improved the code that adds coverage for uncalled functions (with MIR
  but not-codegenned) to avoid generating coverage in files not already
  included in the files with covered functions.

Resolved last known issue requiring --emit llvm-ir workaround

  Fixed bugs in how unreachable code spans were added.
This commit is contained in:
Rich Kadel
2020-11-30 23:58:08 -08:00
parent f6c9c1a576
commit def932ca86
354 changed files with 12634 additions and 20486 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
compiler/rustc_mir/src/transform/coverage/graph.rs
View File

@@ -118,18 +118,8 @@ impl CoverageGraph {
match term.kind {
TerminatorKind::Return { .. }
// FIXME(richkadel): Add test(s) for `Abort` coverage.
| TerminatorKind::Abort
// FIXME(richkadel): Add test(s) for `Assert` coverage.
// Should `Assert` be handled like `FalseUnwind` instead? Since we filter out unwind
// branches when creating the BCB CFG, aren't `Assert`s (without unwinds) just like
// `FalseUnwinds` (which are kind of like `Goto`s)?
| TerminatorKind::Assert { .. }
// FIXME(richkadel): Add test(s) for `Yield` coverage, and confirm coverage is
// sensible for code using the `yield` keyword.
| TerminatorKind::Yield { .. }
// FIXME(richkadel): Also add coverage tests using async/await, and threading.
| TerminatorKind::SwitchInt { .. } => {
// The `bb` has more than one _outgoing_ edge, or exits the function. Save the
// current sequence of `basic_blocks` gathered to this point, as a new
@@ -147,6 +137,14 @@ impl CoverageGraph {
// `Terminator`s `successors()` list) checking the number of successors won't
// work.
}
// The following `TerminatorKind`s are either not expected outside an unwind branch,
// or they should not (under normal circumstances) branch. Coverage graphs are
// simplified by assuring coverage results are accurate for well-behaved programs.
// Programs that panic and unwind may record slightly inaccurate coverage results
// for a coverage region containing the `Terminator` that began the panic. This
// is as intended. (See Issue #78544 for a possible future option to support
// coverage in test programs that panic.)
TerminatorKind::Goto { .. }
| TerminatorKind::Resume
| TerminatorKind::Unreachable
@@ -154,6 +152,7 @@ impl CoverageGraph {
| TerminatorKind::DropAndReplace { .. }
| TerminatorKind::Call { .. }
| TerminatorKind::GeneratorDrop
| TerminatorKind::Assert { .. }
| TerminatorKind::FalseEdge { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::InlineAsm { .. } => {}
@@ -278,6 +277,7 @@ rustc_index::newtype_index! {
/// A node in the [control-flow graph][CFG] of CoverageGraph.
pub(super) struct BasicCoverageBlock {
DEBUG_FORMAT = "bcb{}",
const START_BCB = 0,
}
}

75
compiler/rustc_mir/src/transform/coverage/mod.rs
View File

@@ -88,6 +88,7 @@ struct Instrumentor<'a, 'tcx> {
pass_name: &'a str,
tcx: TyCtxt<'tcx>,
mir_body: &'a mut mir::Body<'tcx>,
fn_sig_span: Span,
body_span: Span,
basic_coverage_blocks: CoverageGraph,
coverage_counters: CoverageCounters,
@@ -95,14 +96,19 @@ struct Instrumentor<'a, 'tcx> {
impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self {
let hir_body = hir_body(tcx, mir_body.source.def_id());
let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, mir_body.source.def_id());
let body_span = hir_body.value.span;
let fn_sig_span = match some_fn_sig {
Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()),
None => body_span.shrink_to_lo(),
};
let function_source_hash = hash_mir_source(tcx, hir_body);
let basic_coverage_blocks = CoverageGraph::from_mir(mir_body);
Self {
pass_name,
tcx,
mir_body,
fn_sig_span,
body_span,
basic_coverage_blocks,
coverage_counters: CoverageCounters::new(function_source_hash),
@@ -114,9 +120,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
let source_map = tcx.sess.source_map();
let mir_source = self.mir_body.source;
let def_id = mir_source.def_id();
let fn_sig_span = self.fn_sig_span;
let body_span = self.body_span;
debug!("instrumenting {:?}, span: {}", def_id, source_map.span_to_string(body_span));
debug!(
"instrumenting {:?}, fn sig span: {}, body span: {}",
def_id,
source_map.span_to_string(fn_sig_span),
source_map.span_to_string(body_span)
);
let mut graphviz_data = debug::GraphvizData::new();
let mut debug_used_expressions = debug::UsedExpressions::new();
@@ -138,6 +150,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
// Compute `CoverageSpan`s from the `CoverageGraph`.
let coverage_spans = CoverageSpans::generate_coverage_spans(
&self.mir_body,
fn_sig_span,
body_span,
&self.basic_coverage_blocks,
);
@@ -272,49 +285,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
bug!("Every BasicCoverageBlock should have a Counter or Expression");
};
graphviz_data.add_bcb_coverage_span_with_counter(bcb, &covspan, &counter_kind);
// FIXME(#78542): Can spans for `TerminatorKind::Goto` be improved to avoid special
// cases?
let some_code_region = if self.is_code_region_redundant(bcb, span, body_span) {
None
} else {
Some(make_code_region(file_name, &source_file, span, body_span))
};
inject_statement(self.mir_body, counter_kind, self.bcb_last_bb(bcb), some_code_region);
inject_statement(
self.mir_body,
counter_kind,
self.bcb_last_bb(bcb),
Some(make_code_region(file_name, &source_file, span, body_span)),
);
}
}
/// Returns true if the type of `BasicCoverageBlock` (specifically, it's `BasicBlock`s
/// `TerminatorKind`) with the given `Span` (relative to the `body_span`) is known to produce
/// a redundant coverage count.
///
/// There is at least one case for this, and if it's not handled, the last line in a function
/// will be double-counted.
///
/// If this method returns `true`, the counter (which other `Expressions` may depend on) is
/// still injected, but without an associated code region.
// FIXME(#78542): Can spans for `TerminatorKind::Goto` be improved to avoid special cases?
fn is_code_region_redundant(
&self,
bcb: BasicCoverageBlock,
span: Span,
body_span: Span,
) -> bool {
if span.hi() == body_span.hi() {
// All functions execute a `Return`-terminated `BasicBlock`, regardless of how the
// function returns; but only some functions also _can_ return after a `Goto` block
// that ends on the closing brace of the function (with the `Return`). When this
// happens, the last character is counted 2 (or possibly more) times, when we know
// the function returned only once (of course). By giving all `Goto` terminators at
// the end of a function a `non-reportable` code region, they are still counted
// if appropriate, but they don't increment the line counter, as long as their is
// also a `Return` on that last line.
if let TerminatorKind::Goto { .. } = self.bcb_terminator(bcb).kind {
return true;
}
}
false
}
/// `inject_coverage_span_counters()` looped through the `CoverageSpan`s and injected the
/// counter from the `CoverageSpan`s `BasicCoverageBlock`, removing it from the BCB in the
/// process (via `take_counter()`).
@@ -411,11 +390,6 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
self.bcb_data(bcb).last_bb()
}
#[inline]
fn bcb_terminator(&self, bcb: BasicCoverageBlock) -> &Terminator<'tcx> {
self.bcb_data(bcb).terminator(self.mir_body)
}
#[inline]
fn bcb_data(&self, bcb: BasicCoverageBlock) -> &BasicCoverageBlockData {
&self.basic_coverage_blocks[bcb]
@@ -521,10 +495,13 @@ fn make_code_region(
}
}
fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> {
fn fn_sig_and_body<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> (Option<&'tcx rustc_hir::FnSig<'tcx>>, &'tcx rustc_hir::Body<'tcx>) {
let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local");
let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body");
tcx.hir().body(fn_body_id)
(hir::map::fn_sig(hir_node), tcx.hir().body(fn_body_id))
}
fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 {

37
compiler/rustc_mir/src/transform/coverage/query.rs
View File

@@ -1,6 +1,8 @@
use super::*;
use rustc_middle::mir::coverage::*;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{Coverage, CoverageInfo, Location};
use rustc_middle::mir::{self, Coverage, CoverageInfo, Location};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::TyCtxt;
use rustc_span::def_id::DefId;
@@ -9,6 +11,8 @@ use rustc_span::def_id::DefId;
/// counter) and `FunctionCoverage::new()` (to extract the coverage map metadata from the MIR).
pub(crate) fn provide(providers: &mut Providers) {
providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id);
providers.covered_file_name = |tcx, def_id| covered_file_name(tcx, def_id);
providers.covered_code_regions = |tcx, def_id| covered_code_regions(tcx, def_id);
}
/// The `num_counters` argument to `llvm.instrprof.increment` is the max counter_id + 1, or in
@@ -123,3 +127,34 @@ fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo
coverage_visitor.info
}
fn covered_file_name<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option<Symbol> {
let mir_body = tcx.optimized_mir(def_id);
for bb_data in mir_body.basic_blocks().iter() {
for statement in bb_data.statements.iter() {
if let StatementKind::Coverage(box ref coverage) = statement.kind {
if let Some(code_region) = coverage.code_region.as_ref() {
return Some(code_region.file_name);
}
}
}
}
None
}
fn covered_code_regions<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Vec<&'tcx CodeRegion> {
let mir_body: &'tcx mir::Body<'tcx> = tcx.optimized_mir(def_id);
mir_body
.basic_blocks()
.iter()
.map(|data| {
data.statements.iter().filter_map(|statement| match statement.kind {
StatementKind::Coverage(box ref coverage) => {
coverage.code_region.as_ref() // may be None
}
_ => None,
})
})
.flatten()
.collect()
}

110
compiler/rustc_mir/src/transform/coverage/spans.rs
View File

@@ -1,10 +1,9 @@
use super::debug::term_type;
use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph};
use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB};
use crate::util::spanview::source_range_no_file;
use rustc_data_structures::graph::WithNumNodes;
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::{
self, AggregateKind, BasicBlock, FakeReadCause, Rvalue, Statement, StatementKind, Terminator,
TerminatorKind,
@@ -74,6 +73,10 @@ pub(super) struct CoverageSpan {
}
impl CoverageSpan {
pub fn for_fn_sig(fn_sig_span: Span) -> Self {
Self { span: fn_sig_span, bcb: START_BCB, coverage_statements: vec![], is_closure: false }
}
pub fn for_statement(
statement: &Statement<'tcx>,
span: Span,
@@ -82,10 +85,10 @@ impl CoverageSpan {
stmt_index: usize,
) -> Self {
let is_closure = match statement.kind {
StatementKind::Assign(box (
_,
Rvalue::Aggregate(box AggregateKind::Closure(_, _), _),
)) => true,
StatementKind::Assign(box (_, Rvalue::Aggregate(box ref kind, _))) => match kind {
AggregateKind::Closure(_, _) | AggregateKind::Generator(_, _, _) => true,
_ => false,
},
_ => false,
};
@@ -109,9 +112,6 @@ impl CoverageSpan {
pub fn merge_from(&mut self, mut other: CoverageSpan) {
debug_assert!(self.is_mergeable(&other));
self.span = self.span.to(other.span);
if other.is_closure {
self.is_closure = true;
}
self.coverage_statements.append(&mut other.coverage_statements);
}
@@ -171,6 +171,9 @@ pub struct CoverageSpans<'a, 'tcx> {
/// The MIR, used to look up `BasicBlockData`.
mir_body: &'a mir::Body<'tcx>,
/// A `Span` covering the signature of function for the MIR.
fn_sig_span: Span,
/// A `Span` covering the function body of the MIR (typically from left curly brace to right
/// curly brace).
body_span: Span,
@@ -216,11 +219,13 @@ pub struct CoverageSpans<'a, 'tcx> {
impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
pub(super) fn generate_coverage_spans(
mir_body: &'a mir::Body<'tcx>,
fn_sig_span: Span,
body_span: Span,
basic_coverage_blocks: &'a CoverageGraph,
) -> Vec<CoverageSpan> {
let mut coverage_spans = CoverageSpans {
mir_body,
fn_sig_span,
body_span,
basic_coverage_blocks,
sorted_spans_iter: None,
@@ -277,6 +282,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
return initial_spans;
}
initial_spans.push(CoverageSpan::for_fn_sig(self.fn_sig_span));
initial_spans.sort_unstable_by(|a, b| {
if a.span.lo() == b.span.lo() {
if a.span.hi() == b.span.hi() {
@@ -331,7 +338,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
prev={:?}",
self.prev()
);
self.discard_curr();
self.take_curr();
} else if self.curr().is_closure {
self.carve_out_span_for_closure();
} else if self.prev_original_span == self.curr().span {
@@ -345,28 +352,28 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
debug!(" AT END, adding last prev={:?}", self.prev());
let prev = self.take_prev();
let CoverageSpans {
mir_body, basic_coverage_blocks, pending_dups, mut refined_spans, ..
} = self;
let CoverageSpans { pending_dups, mut refined_spans, .. } = self;
for dup in pending_dups {
debug!(" ...adding at least one pending dup={:?}", dup);
refined_spans.push(dup);
}
refined_spans.push(prev);
// Remove `CoverageSpan`s with empty spans ONLY if the empty `CoverageSpan`s BCB also has at
// least one other non-empty `CoverageSpan`.
let mut has_coverage = BitSet::new_empty(basic_coverage_blocks.num_nodes());
for covspan in &refined_spans {
if !covspan.span.is_empty() {
has_coverage.insert(covspan.bcb);
}
// Async functions wrap a closure that implements the body to be executed. The enclosing
// function is initially called, posts the closure to the executor, and returns. To avoid
// showing the return from the enclosing function as a "covered" return from the closure,
// the enclosing function's `TerminatorKind::Return`s `CoverageSpan` is excluded. The
// closure's `Return` is the only one that will be counted. This provides adequate
// coverage, and more intuitive counts. (Avoids double-counting the closing brace of the
// function body.)
let body_ends_with_closure = if let Some(last_covspan) = refined_spans.last() {
last_covspan.is_closure && last_covspan.span.hi() == self.body_span.hi()
} else {
false
};
if !body_ends_with_closure {
refined_spans.push(prev);
}
refined_spans.retain(|covspan| {
!(covspan.span.is_empty()
&& is_goto(&basic_coverage_blocks[covspan.bcb].terminator(mir_body).kind)
&& has_coverage.contains(covspan.bcb))
});
// Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage
// regions for the current function leave room for the closure's own coverage regions
@@ -491,8 +498,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
/// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the
/// `curr` coverage span.
fn discard_curr(&mut self) {
self.some_curr = None;
fn take_curr(&mut self) -> CoverageSpan {
self.some_curr.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr"))
}
/// Returns true if the curr span should be skipped because prev has already advanced beyond the
@@ -508,11 +515,11 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
self.prev().span.hi() <= self.curr().span.lo()
}
/// If `prev`s span extends left of the closure (`curr`), carve out the closure's
/// span from `prev`'s span. (The closure's coverage counters will be injected when
/// processing the closure's own MIR.) Add the portion of the span to the left of the
/// closure; and if the span extends to the right of the closure, update `prev` to
/// that portion of the span. For any `pending_dups`, repeat the same process.
/// If `prev`s span extends left of the closure (`curr`), carve out the closure's span from
/// `prev`'s span. (The closure's coverage counters will be injected when processing the
/// closure's own MIR.) Add the portion of the span to the left of the closure; and if the span
/// extends to the right of the closure, update `prev` to that portion of the span. For any
/// `pending_dups`, repeat the same process.
fn carve_out_span_for_closure(&mut self) {
let curr_span = self.curr().span;
let left_cutoff = curr_span.lo();
@@ -541,7 +548,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
dup.span = dup.span.with_lo(right_cutoff);
}
self.pending_dups.append(&mut pending_dups);
self.discard_curr(); // since self.prev() was already updated
let closure_covspan = self.take_curr();
self.refined_spans.push(closure_covspan); // since self.prev() was already updated
} else {
pending_dups.clear();
}
@@ -705,30 +713,8 @@ pub(super) fn filtered_terminator_span(
| TerminatorKind::DropAndReplace { .. }
| TerminatorKind::SwitchInt { .. }
// For `FalseEdge`, only the `real` branch is taken, so it is similar to a `Goto`.
// FIXME(richkadel): Note that `Goto` was moved to it's own match arm, for the reasons
// described below. Add tests to confirm whether or not similar cases also apply to
// `FalseEdge`.
| TerminatorKind::FalseEdge { .. } => None,
// FIXME(#78542): Can spans for `TerminatorKind::Goto` be improved to avoid special cases?
//
// `Goto`s are often the targets of `SwitchInt` branches, and certain important
// optimizations to replace some `Counter`s with `Expression`s require a separate
// `BasicCoverageBlock` for each branch, to support the `Counter`, when needed.
//
// Also, some test cases showed that `Goto` terminators, and to some degree their `Span`s,
// provided useful context for coverage, such as to count and show when `if` blocks
// _without_ `else` blocks execute the `false` case (counting when the body of the `if`
// was _not_ taken). In these cases, the `Goto` span is ultimately given a `CoverageSpan`
// of 1 character, at the end of it's original `Span`.
//
// However, in other cases, a visible `CoverageSpan` is not wanted, but the `Goto`
// block must still be counted (for example, to contribute its count to an `Expression`
// that reports the execution count for some other block). In these cases, the code region
// is set to `None`. (See `Instrumentor::is_code_region_redundant()`.)
TerminatorKind::Goto { .. } => {
Some(function_source_span(terminator.source_info.span.shrink_to_hi(), body_span))
}
| TerminatorKind::FalseEdge { .. }
| TerminatorKind::Goto { .. } => None,
// Retain spans from all other terminators
TerminatorKind::Resume
@@ -749,11 +735,3 @@ fn function_source_span(span: Span, body_span: Span) -> Span {
let span = original_sp(span, body_span).with_ctxt(SyntaxContext::root());
if body_span.contains(span) { span } else { body_span }
}
#[inline(always)]
fn is_goto(term_kind: &TerminatorKind<'tcx>) -> bool {
match term_kind {
TerminatorKind::Goto { .. } => true,
_ => false,
}
}