nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-26 00:34:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

Rollup merge of #127352 - Zalathar:coverage-info, r=oli-obk

Browse Source 
coverage: Rename `mir::coverage::BranchInfo` to `CoverageInfoHi`

This opens the door to collecting and storing coverage information that is unrelated to branch coverage or MC/DC, during MIR building.

There is no change to the output of coverage instrumentation, but one deliberate change is that functions now *always* have an attached `CoverageInfoHi` (if coverage is enabled and they are eligible), even if they didn't collect any interesting branch information.

---

`@rustbot` label +A-code-coverage
This commit is contained in:
Guillaume Gomez 2024-07-05 11:33:17 +02:00 committed by GitHub
commit e4d7f7c9e6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 117 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
compiler/rustc_middle/src/mir/coverage.rs
View File

@ -103,7 +103,7 @@ pub enum CoverageKind {
SpanMarker,
/// Marks its enclosing basic block with an ID that can be referred to by
/// side data in [`BranchInfo`].
/// side data in [`CoverageInfoHi`].
///
/// Should be erased before codegen (at some point after `InstrumentCoverage`).
BlockMarker { id: BlockMarkerId },
@ -274,10 +274,15 @@ pub struct FunctionCoverageInfo {
pub mcdc_num_condition_bitmaps: usize,
}
/// Branch information recorded during THIR-to-MIR lowering, and stored in MIR.
/// Coverage information for a function, recorded during MIR building and
/// attached to the corresponding `mir::Body`. Used by the `InstrumentCoverage`
/// MIR pass.
///
/// ("Hi" indicates that this is "high-level" information collected at the
/// THIR/MIR boundary, before the MIR-based coverage instrumentation pass.)
#[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
pub struct BranchInfo {
pub struct CoverageInfoHi {
/// 1 more than the highest-numbered [`CoverageKind::BlockMarker`] that was
/// injected into the MIR body. This makes it possible to allocate per-ID
/// data structures without having to scan the entire body first.

13
compiler/rustc_middle/src/mir/mod.rs
View File

@ -430,11 +430,12 @@ pub struct Body<'tcx> {
pub tainted_by_errors: Option<ErrorGuaranteed>,
/// Branch coverage information collected during MIR building, to be used by
/// the `InstrumentCoverage` pass.
/// Coverage information collected from THIR/MIR during MIR building,
/// to be used by the `InstrumentCoverage` pass.
///
/// Only present if branch coverage is enabled and this function is eligible.
pub coverage_branch_info: Option<Box<coverage::BranchInfo>>,
/// Only present if coverage is enabled and this function is eligible.
/// Boxed to limit space overhead in non-coverage builds.
pub coverage_info_hi: Option<Box<coverage::CoverageInfoHi>>,
/// Per-function coverage information added by the `InstrumentCoverage`
/// pass, to be used in conjunction with the coverage statements injected
@ -484,7 +485,7 @@ impl<'tcx> Body<'tcx> {
is_polymorphic: false,
injection_phase: None,
tainted_by_errors,
coverage_branch_info: None,
coverage_info_hi: None,
function_coverage_info: None,
};
body.is_polymorphic = body.has_non_region_param();
@ -515,7 +516,7 @@ impl<'tcx> Body<'tcx> {
is_polymorphic: false,
injection_phase: None,
tainted_by_errors: None,
coverage_branch_info: None,
coverage_info_hi: None,
function_coverage_info: None,
};
body.is_polymorphic = body.has_non_region_param();

25
compiler/rustc_middle/src/mir/pretty.rs
View File

@ -473,8 +473,8 @@ pub fn write_mir_intro<'tcx>(
// Add an empty line before the first block is printed.
writeln!(w)?;
if let Some(branch_info) = &body.coverage_branch_info {
write_coverage_branch_info(branch_info, w)?;
if let Some(coverage_info_hi) = &body.coverage_info_hi {
write_coverage_info_hi(coverage_info_hi, w)?;
}
if let Some(function_coverage_info) = &body.function_coverage_info {
write_function_coverage_info(function_coverage_info, w)?;
@ -483,18 +483,26 @@ pub fn write_mir_intro<'tcx>(
Ok(())
}
fn write_coverage_branch_info(
branch_info: &coverage::BranchInfo,
fn write_coverage_info_hi(
coverage_info_hi: &coverage::CoverageInfoHi,
w: &mut dyn io::Write,
) -> io::Result<()> {
let coverage::BranchInfo { branch_spans, mcdc_branch_spans, mcdc_decision_spans, .. } =
branch_info;
let coverage::CoverageInfoHi {
num_block_markers: _,
branch_spans,
mcdc_branch_spans,
mcdc_decision_spans,
} = coverage_info_hi;
// Only add an extra trailing newline if we printed at least one thing.
let mut did_print = false;
for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans {
writeln!(
w,
"{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",
)?;
did_print = true;
}
for coverage::MCDCBranchSpan {
@ -510,6 +518,7 @@ fn write_coverage_branch_info(
"{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?}, depth: {decision_depth:?} }} => {span:?}",
condition_info.map(|info| info.condition_id)
)?;
did_print = true;
}
for coverage::MCDCDecisionSpan { span, num_conditions, end_markers, decision_depth } in
@ -519,10 +528,10 @@ fn write_coverage_branch_info(
w,
"{INDENT}coverage mcdc decision {{ num_conditions: {num_conditions:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}"
)?;
did_print = true;
}
if !branch_spans.is_empty() || !mcdc_branch_spans.is_empty() || !mcdc_decision_spans.is_empty()
{
if did_print {
writeln!(w)?;
}

87
compiler/rustc_mir_build/src/build/coverageinfo.rs
View File

@ -2,7 +2,7 @@ use std::assert_matches::assert_matches;
use std::collections::hash_map::Entry;
use rustc_data_structures::fx::FxHashMap;
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind};
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageInfoHi, CoverageKind};
use rustc_middle::mir::{self, BasicBlock, SourceInfo, UnOp};
use rustc_middle::thir::{ExprId, ExprKind, Pat, Thir};
use rustc_middle::ty::TyCtxt;
@ -13,16 +13,25 @@ use crate::build::{Builder, CFG};
mod mcdc;
pub(crate) struct BranchInfoBuilder {
/// Collects coverage-related information during MIR building, to eventually be
/// turned into a function's [`CoverageInfoHi`] when MIR building is complete.
pub(crate) struct CoverageInfoBuilder {
/// Maps condition expressions to their enclosing `!`, for better instrumentation.
nots: FxHashMap<ExprId, NotInfo>,
markers: BlockMarkerGen,
branch_spans: Vec<BranchSpan>,
/// Present if branch coverage is enabled.
branch_info: Option<BranchInfo>,
/// Present if MC/DC coverage is enabled.
mcdc_info: Option<MCDCInfoBuilder>,
}
#[derive(Default)]
struct BranchInfo {
branch_spans: Vec<BranchSpan>,
}
#[derive(Clone, Copy)]
struct NotInfo {
/// When visiting the associated expression as a branch condition, treat this
@ -62,20 +71,20 @@ impl BlockMarkerGen {
}
}
impl BranchInfoBuilder {
/// Creates a new branch info builder, but only if branch coverage instrumentation
impl CoverageInfoBuilder {
/// Creates a new coverage info builder, but only if coverage instrumentation
/// is enabled and `def_id` represents a function that is eligible for coverage.
pub(crate) fn new_if_enabled(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<Self> {
if tcx.sess.instrument_coverage_branch() && tcx.is_eligible_for_coverage(def_id) {
if !tcx.sess.instrument_coverage() || !tcx.is_eligible_for_coverage(def_id) {
return None;
}
Some(Self {
nots: FxHashMap::default(),
markers: BlockMarkerGen::default(),
branch_spans: vec![],
branch_info: tcx.sess.instrument_coverage_branch().then(BranchInfo::default),
mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new),
})
} else {
None
}
}
/// Unary `!` expressions inside an `if` condition are lowered by lowering
@ -88,6 +97,12 @@ impl BranchInfoBuilder {
pub(crate) fn visit_unary_not(&mut self, thir: &Thir<'_>, unary_not: ExprId) {
assert_matches!(thir[unary_not].kind, ExprKind::Unary { op: UnOp::Not, .. });
// The information collected by this visitor is only needed when branch
// coverage or higher is enabled.
if self.branch_info.is_none() {
return;
}
self.visit_with_not_info(
thir,
unary_not,
@ -137,40 +152,40 @@ impl BranchInfoBuilder {
false_block,
inject_block_marker,
);
} else {
return;
}
// Bail out if branch coverage is not enabled.
let Some(branch_info) = self.branch_info.as_mut() else { return };
let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);
self.branch_spans.push(BranchSpan {
branch_info.branch_spans.push(BranchSpan {
span: source_info.span,
true_marker,
false_marker,
});
}
}
pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
let Self {
nots: _,
markers: BlockMarkerGen { num_block_markers },
branch_spans,
mcdc_info,
} = self;
pub(crate) fn into_done(self) -> Box<CoverageInfoHi> {
let Self { nots: _, markers: BlockMarkerGen { num_block_markers }, branch_info, mcdc_info } =
self;
if num_block_markers == 0 {
assert!(branch_spans.is_empty());
return None;
}
let branch_spans =
branch_info.map(|branch_info| branch_info.branch_spans).unwrap_or_default();
let (mcdc_decision_spans, mcdc_branch_spans) =
mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default();
Some(Box::new(mir::coverage::BranchInfo {
// For simplicity, always return an info struct (without Option), even
// if there's nothing interesting in it.
Box::new(CoverageInfoHi {
num_block_markers,
branch_spans,
mcdc_branch_spans,
mcdc_decision_spans,
}))
})
}
}
@ -184,7 +199,7 @@ impl<'tcx> Builder<'_, 'tcx> {
block: &mut BasicBlock,
) {
// Bail out if condition coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
if !self.tcx.sess.instrument_coverage_condition() {
return;
};
@ -224,7 +239,7 @@ impl<'tcx> Builder<'_, 'tcx> {
);
// Separate path for handling branches when MC/DC is enabled.
branch_info.register_two_way_branch(
coverage_info.register_two_way_branch(
self.tcx,
&mut self.cfg,
source_info,
@ -247,12 +262,12 @@ impl<'tcx> Builder<'_, 'tcx> {
mut then_block: BasicBlock,
mut else_block: BasicBlock,
) {
// Bail out if branch coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
// Bail out if coverage is not enabled for this function.
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
// If this condition expression is nested within one or more `!` expressions,
// replace it with the enclosing `!` collected by `visit_unary_not`.
if let Some(&NotInfo { enclosing_not, is_flipped }) = branch_info.nots.get(&expr_id) {
if let Some(&NotInfo { enclosing_not, is_flipped }) = coverage_info.nots.get(&expr_id) {
expr_id = enclosing_not;
if is_flipped {
std::mem::swap(&mut then_block, &mut else_block);
@ -261,7 +276,7 @@ impl<'tcx> Builder<'_, 'tcx> {
let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope };
branch_info.register_two_way_branch(
coverage_info.register_two_way_branch(
self.tcx,
&mut self.cfg,
source_info,
@ -280,13 +295,11 @@ impl<'tcx> Builder<'_, 'tcx> {
true_block: BasicBlock,
false_block: BasicBlock,
) {
// Bail out if branch coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
// FIXME(#124144) This may need special handling when MC/DC is enabled.
// Bail out if coverage is not enabled for this function.
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
let source_info = SourceInfo { span: pattern.span, scope: self.source_scope };
branch_info.register_two_way_branch(
coverage_info.register_two_way_branch(
self.tcx,
&mut self.cfg,
source_info,

12
compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs
View File

@ -250,24 +250,24 @@ impl MCDCInfoBuilder {
impl Builder<'_, '_> {
pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) {
if let Some(branch_info) = self.coverage_branch_info.as_mut()
&& let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
if let Some(coverage_info) = self.coverage_info.as_mut()
&& let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
{
mcdc_info.state.record_conditions(logical_op, span);
}
}
pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) {
if let Some(branch_info) = self.coverage_branch_info.as_mut()
&& let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
if let Some(coverage_info) = self.coverage_info.as_mut()
&& let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
{
mcdc_info.state.decision_ctx_stack.push(MCDCDecisionCtx::default());
};
}
pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) {
if let Some(branch_info) = self.coverage_branch_info.as_mut()
&& let Some(mcdc_info) = branch_info.mcdc_info.as_mut()
if let Some(coverage_info) = self.coverage_info.as_mut()
&& let Some(mcdc_info) = coverage_info.mcdc_info.as_mut()
{
if mcdc_info.state.decision_ctx_stack.pop().is_none() {
bug!("Unexpected empty decision stack");

2
compiler/rustc_mir_build/src/build/custom/mod.rs
View File

@ -62,7 +62,7 @@ pub(super) fn build_custom_mir<'tcx>(
tainted_by_errors: None,
injection_phase: None,
pass_count: 0,
coverage_branch_info: None,
coverage_info_hi: None,
function_coverage_info: None,
};

4
compiler/rustc_mir_build/src/build/matches/mod.rs
View File

@ -160,8 +160,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// Improve branch coverage instrumentation by noting conditions
// nested within one or more `!` expressions.
// (Skipped if branch coverage is not enabled.)
if let Some(branch_info) = this.coverage_branch_info.as_mut() {
branch_info.visit_unary_not(this.thir, expr_id);
if let Some(coverage_info) = this.coverage_info.as_mut() {
coverage_info.visit_unary_not(this.thir, expr_id);
}
let local_scope = this.local_scope();

8
compiler/rustc_mir_build/src/build/mod.rs
View File

@ -218,8 +218,8 @@ struct Builder<'a, 'tcx> {
lint_level_roots_cache: GrowableBitSet<hir::ItemLocalId>,
/// Collects additional coverage information during MIR building.
/// Only present if branch coverage is enabled and this function is eligible.
coverage_branch_info: Option<coverageinfo::BranchInfoBuilder>,
/// Only present if coverage is enabled and this function is eligible.
coverage_info: Option<coverageinfo::CoverageInfoBuilder>,
}
type CaptureMap<'tcx> = SortedIndexMultiMap<usize, HirId, Capture<'tcx>>;
@ -773,7 +773,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
unit_temp: None,
var_debug_info: vec![],
lint_level_roots_cache: GrowableBitSet::new_empty(),
coverage_branch_info: coverageinfo::BranchInfoBuilder::new_if_enabled(tcx, def),
coverage_info: coverageinfo::CoverageInfoBuilder::new_if_enabled(tcx, def),
};
assert_eq!(builder.cfg.start_new_block(), START_BLOCK);
@ -802,7 +802,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.coroutine,
None,
);
body.coverage_branch_info = self.coverage_branch_info.and_then(|b| b.into_done());
body.coverage_info_hi = self.coverage_info.map(|b| b.into_done());
body
}

22
compiler/rustc_mir_transform/src/coverage/mappings.rs
View File

@ -3,7 +3,9 @@ use std::collections::BTreeSet;
use rustc_data_structures::graph::DirectedGraph;
use rustc_index::bit_set::BitSet;
use rustc_index::IndexVec;
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, ConditionInfo, CoverageKind};
use rustc_middle::mir::coverage::{
BlockMarkerId, BranchSpan, ConditionInfo, CoverageInfoHi, CoverageKind,
};
use rustc_middle::mir::{self, BasicBlock, StatementKind};
use rustc_middle::ty::TyCtxt;
use rustc_span::Span;
@ -157,12 +159,12 @@ impl ExtractedMappings {
}
fn resolve_block_markers(
branch_info: &mir::coverage::BranchInfo,
coverage_info_hi: &CoverageInfoHi,
mir_body: &mir::Body<'_>,
) -> IndexVec<BlockMarkerId, Option<BasicBlock>> {
let mut block_markers = IndexVec::<BlockMarkerId, Option<BasicBlock>>::from_elem_n(
None,
branch_info.num_block_markers,
coverage_info_hi.num_block_markers,
);
// Fill out the mapping from block marker IDs to their enclosing blocks.
@ -188,11 +190,11 @@ pub(super) fn extract_branch_pairs(
hir_info: &ExtractedHirInfo,
basic_coverage_blocks: &CoverageGraph,
) -> Vec<BranchPair> {
let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return vec![] };
let Some(coverage_info_hi) = mir_body.coverage_info_hi.as_deref() else { return vec![] };
let block_markers = resolve_block_markers(branch_info, mir_body);
let block_markers = resolve_block_markers(coverage_info_hi, mir_body);
branch_info
coverage_info_hi
.branch_spans
.iter()
.filter_map(|&BranchSpan { span: raw_span, true_marker, false_marker }| {
@ -222,9 +224,9 @@ pub(super) fn extract_mcdc_mappings(
mcdc_branches: &mut impl Extend<MCDCBranch>,
mcdc_decisions: &mut impl Extend<MCDCDecision>,
) {
let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return };
let Some(coverage_info_hi) = mir_body.coverage_info_hi.as_deref() else { return };
let block_markers = resolve_block_markers(branch_info, mir_body);
let block_markers = resolve_block_markers(coverage_info_hi, mir_body);
let bcb_from_marker =
|marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?);
@ -243,7 +245,7 @@ pub(super) fn extract_mcdc_mappings(
Some((span, true_bcb, false_bcb))
};
mcdc_branches.extend(branch_info.mcdc_branch_spans.iter().filter_map(
mcdc_branches.extend(coverage_info_hi.mcdc_branch_spans.iter().filter_map(
|&mir::coverage::MCDCBranchSpan {
span: raw_span,
condition_info,
@ -257,7 +259,7 @@ pub(super) fn extract_mcdc_mappings(
},
));
mcdc_decisions.extend(branch_info.mcdc_decision_spans.iter().filter_map(
mcdc_decisions.extend(coverage_info_hi.mcdc_decision_spans.iter().filter_map(
|decision: &mir::coverage::MCDCDecisionSpan| {
let span = unexpand_into_body_span(decision.span, body_span)?;