mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #124603 - Zalathar:mcdc-mappings, r=Nadrieril

Browse Source 
coverage: Split out MC/DC mappings from `BcbMappingKind`

These variants were added to `BcbMappingKind` as part of the [MC/DC coverage](https://en.wikipedia.org/wiki/Modified_Condition/Decision_Coverage) implementation in #123409, because that was the path-of-least-resistance for integrating them into the existing code.

However, they ultimately represent complex concepts that the enum was not intended to handle, leading to more complexity in the code that processes them. This PR therefore follows in the footsteps of #124545, and splits the MC/DC mappings out into their own dedicated vectors of structs.

After that, `BcbMappingKind` itself ends up having only one variant (`Code`), so this PR also flattens that enum into its enclosing struct, renamed to `mapping::CodeMapping`.

---

No functional changes.

This will conflict slightly with #124571, but hopefully that should be easy to resolve either way.

`@rustbot` label +A-code-coverage
This commit is contained in:
bors 2024-05-05 19:23:20 +00:00
commit 9c9b568792
3 changed files with 158 additions and 157 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -2,9 +2,8 @@
use rustc_data_structures::graph::DirectedGraph;
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::coverage::{
BlockMarkerId, BranchSpan, ConditionInfo, CoverageKind, MCDCBranchSpan, MCDCDecisionSpan,
};
use rustc_index::IndexVec;
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, ConditionInfo, CoverageKind};
use rustc_middle::mir::{self, BasicBlock, StatementKind};
use rustc_span::Span;
@ -13,55 +12,53 @@
extract_refined_covspans, unexpand_into_body_span_with_visible_macro,
};
use crate::coverage::ExtractedHirInfo;
use rustc_index::IndexVec;
#[derive(Clone, Debug)]
pub(super) enum BcbMappingKind {
/// Associates an ordinary executable code span with its corresponding BCB.
Code(BasicCoverageBlock),
// Ordinary branch mappings are stored separately, so they don't have a
// variant in this enum.
//
/// Associates a mcdc branch span with condition info besides fields for normal branch.
MCDCBranch {
true_bcb: BasicCoverageBlock,
false_bcb: BasicCoverageBlock,
/// If `None`, this actually represents a normal branch mapping inserted
/// for code that was too complex for MC/DC.
condition_info: Option<ConditionInfo>,
decision_depth: u16,
},
/// Associates a mcdc decision with its join BCB.
MCDCDecision {
end_bcbs: BTreeSet<BasicCoverageBlock>,
bitmap_idx: u32,
conditions_num: u16,
decision_depth: u16,
},
}
/// Associates an ordinary executable code span with its corresponding BCB.
#[derive(Debug)]
pub(super) struct BcbMapping {
pub(super) kind: BcbMappingKind,
pub(super) struct CodeMapping {
pub(super) span: Span,
pub(super) bcb: BasicCoverageBlock,
}
/// This is separate from [`BcbMappingKind`] to help prepare for larger changes
/// This is separate from [`MCDCBranch`] to help prepare for larger changes
/// that will be needed for improved branch coverage in the future.
/// (See <https://github.com/rust-lang/rust/pull/124217>.)
#[derive(Debug)]
pub(super) struct BcbBranchPair {
pub(super) struct BranchPair {
pub(super) span: Span,
pub(super) true_bcb: BasicCoverageBlock,
pub(super) false_bcb: BasicCoverageBlock,
}
/// Associates an MC/DC branch span with condition info besides fields for normal branch.
#[derive(Debug)]
pub(super) struct MCDCBranch {
pub(super) span: Span,
pub(super) true_bcb: BasicCoverageBlock,
pub(super) false_bcb: BasicCoverageBlock,
/// If `None`, this actually represents a normal branch mapping inserted
/// for code that was too complex for MC/DC.
pub(super) condition_info: Option<ConditionInfo>,
pub(super) decision_depth: u16,
}
/// Associates an MC/DC decision with its join BCBs.
#[derive(Debug)]
pub(super) struct MCDCDecision {
pub(super) span: Span,
pub(super) end_bcbs: BTreeSet<BasicCoverageBlock>,
pub(super) bitmap_idx: u32,
pub(super) conditions_num: u16,
pub(super) decision_depth: u16,
}
pub(super) struct CoverageSpans {
bcb_has_mappings: BitSet<BasicCoverageBlock>,
pub(super) mappings: Vec<BcbMapping>,
pub(super) branch_pairs: Vec<BcbBranchPair>,
pub(super) code_mappings: Vec<CodeMapping>,
pub(super) branch_pairs: Vec<BranchPair>,
test_vector_bitmap_bytes: u32,
pub(super) mcdc_branches: Vec<MCDCBranch>,
pub(super) mcdc_decisions: Vec<MCDCDecision>,
}
impl CoverageSpans {
@ -83,8 +80,10 @@ pub(super) fn generate_coverage_spans(
hir_info: &ExtractedHirInfo,
basic_coverage_blocks: &CoverageGraph,
) -> Option<CoverageSpans> {
let mut mappings = vec![];
let mut code_mappings = vec![];
let mut branch_pairs = vec![];
let mut mcdc_branches = vec![];
let mut mcdc_decisions = vec![];
if hir_info.is_async_fn {
// An async function desugars into a function that returns a future,
@ -92,17 +91,27 @@ pub(super) fn generate_coverage_spans(
// outer function will be unhelpful, so just keep the signature span
// and ignore all of the spans in the MIR body.
if let Some(span) = hir_info.fn_sig_span_extended {
mappings.push(BcbMapping { kind: BcbMappingKind::Code(START_BCB), span });
code_mappings.push(CodeMapping { span, bcb: START_BCB });
}
} else {
extract_refined_covspans(mir_body, hir_info, basic_coverage_blocks, &mut mappings);
extract_refined_covspans(mir_body, hir_info, basic_coverage_blocks, &mut code_mappings);
branch_pairs.extend(extract_branch_pairs(mir_body, hir_info, basic_coverage_blocks));
mappings.extend(extract_mcdc_mappings(mir_body, hir_info.body_span, basic_coverage_blocks));
extract_mcdc_mappings(
mir_body,
hir_info.body_span,
basic_coverage_blocks,
&mut mcdc_branches,
&mut mcdc_decisions,
);
}
if mappings.is_empty() && branch_pairs.is_empty() {
if code_mappings.is_empty()
&& branch_pairs.is_empty()
&& mcdc_branches.is_empty()
&& mcdc_decisions.is_empty()
{
return None;
}
@ -111,29 +120,36 @@ pub(super) fn generate_coverage_spans(
let mut insert = |bcb| {
bcb_has_mappings.insert(bcb);
};
let mut test_vector_bitmap_bytes = 0;
for BcbMapping { kind, span: _ } in &mappings {
match *kind {
BcbMappingKind::Code(bcb) => insert(bcb),
BcbMappingKind::MCDCBranch { true_bcb, false_bcb, .. } => {
insert(true_bcb);
insert(false_bcb);
}
BcbMappingKind::MCDCDecision { bitmap_idx, conditions_num, .. } => {
// `bcb_has_mappings` is used for inject coverage counters
// but they are not needed for decision BCBs.
// While the length of test vector bitmap should be calculated here.
test_vector_bitmap_bytes = test_vector_bitmap_bytes
.max(bitmap_idx + (1_u32 << conditions_num as u32).div_ceil(8));
}
}
for &CodeMapping { span: _, bcb } in &code_mappings {
insert(bcb);
}
for &BcbBranchPair { true_bcb, false_bcb, .. } in &branch_pairs {
for &BranchPair { true_bcb, false_bcb, .. } in &branch_pairs {
insert(true_bcb);
insert(false_bcb);
}
for &MCDCBranch { true_bcb, false_bcb, .. } in &mcdc_branches {
insert(true_bcb);
insert(false_bcb);
}
Some(CoverageSpans { bcb_has_mappings, mappings, branch_pairs, test_vector_bitmap_bytes })
// Determine the length of the test vector bitmap.
let test_vector_bitmap_bytes = mcdc_decisions
.iter()
.map(|&MCDCDecision { bitmap_idx, conditions_num, .. }| {
bitmap_idx + (1_u32 << u32::from(conditions_num)).div_ceil(8)
})
.max()
.unwrap_or(0);
Some(CoverageSpans {
bcb_has_mappings,
code_mappings,
branch_pairs,
test_vector_bitmap_bytes,
mcdc_branches,
mcdc_decisions,
})
}
fn resolve_block_markers(
@ -167,7 +183,7 @@ pub(super) fn extract_branch_pairs(
mir_body: &mir::Body<'_>,
hir_info: &ExtractedHirInfo,
basic_coverage_blocks: &CoverageGraph,
) -> Vec<BcbBranchPair> {
) -> Vec<BranchPair> {
let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return vec![] };
let block_markers = resolve_block_markers(branch_info, mir_body);
@ -190,7 +206,7 @@ pub(super) fn extract_branch_pairs(
let true_bcb = bcb_from_marker(true_marker)?;
let false_bcb = bcb_from_marker(false_marker)?;
Some(BcbBranchPair { span, true_bcb, false_bcb })
Some(BranchPair { span, true_bcb, false_bcb })
})
.collect::<Vec<_>>()
}
@ -199,10 +215,10 @@ pub(super) fn extract_mcdc_mappings(
mir_body: &mir::Body<'_>,
body_span: Span,
basic_coverage_blocks: &CoverageGraph,
) -> Vec<BcbMapping> {
let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else {
return vec![];
};
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 block_markers = resolve_block_markers(branch_info, mir_body);
@ -223,53 +239,42 @@ pub(super) fn extract_mcdc_mappings(
Some((span, true_bcb, false_bcb))
};
let mcdc_branch_filter_map = |&MCDCBranchSpan {
span: raw_span,
true_marker,
false_marker,
condition_info,
decision_depth,
}| {
check_branch_bcb(raw_span, true_marker, false_marker).map(|(span, true_bcb, false_bcb)| {
BcbMapping {
kind: BcbMappingKind::MCDCBranch {
true_bcb,
false_bcb,
condition_info,
decision_depth,
},
span,
}
})
};
mcdc_branches.extend(branch_info.mcdc_branch_spans.iter().filter_map(
|&mir::coverage::MCDCBranchSpan {
span: raw_span,
condition_info,
true_marker,
false_marker,
decision_depth,
}| {
let (span, true_bcb, false_bcb) =
check_branch_bcb(raw_span, true_marker, false_marker)?;
Some(MCDCBranch { span, true_bcb, false_bcb, condition_info, decision_depth })
},
));
let mut next_bitmap_idx = 0;
let decision_filter_map = |decision: &MCDCDecisionSpan| {
let (span, _) = unexpand_into_body_span_with_visible_macro(decision.span, body_span)?;
mcdc_decisions.extend(branch_info.mcdc_decision_spans.iter().filter_map(
|decision: &mir::coverage::MCDCDecisionSpan| {
let (span, _) = unexpand_into_body_span_with_visible_macro(decision.span, body_span)?;
let end_bcbs = decision
.end_markers
.iter()
.map(|&marker| bcb_from_marker(marker))
.collect::<Option<_>>()?;
let end_bcbs = decision
.end_markers
.iter()
.map(|&marker| bcb_from_marker(marker))
.collect::<Option<_>>()?;
let bitmap_idx = next_bitmap_idx;
next_bitmap_idx += (1_u32 << decision.conditions_num).div_ceil(8);
let bitmap_idx = next_bitmap_idx;
next_bitmap_idx += (1_u32 << decision.conditions_num).div_ceil(8);
Some(BcbMapping {
kind: BcbMappingKind::MCDCDecision {
Some(MCDCDecision {
span,
end_bcbs,
bitmap_idx,
conditions_num: decision.conditions_num as u16,
decision_depth: decision.decision_depth,
},
span,
})
};
std::iter::empty()
.chain(branch_info.mcdc_branch_spans.iter().filter_map(mcdc_branch_filter_map))
.chain(branch_info.mcdc_decision_spans.iter().filter_map(decision_filter_map))
.collect::<Vec<_>>()
})
},
));
}

94
compiler/rustc_mir_transform/src/coverage/mod.rs
View File

@ -9,7 +9,7 @@
use self::counters::{CounterIncrementSite, CoverageCounters};
use self::graph::{BasicCoverageBlock, CoverageGraph};
use self::mappings::{BcbBranchPair, BcbMapping, BcbMappingKind, CoverageSpans};
use self::mappings::CoverageSpans;
use crate::MirPass;
@ -103,12 +103,9 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
inject_mcdc_statements(mir_body, &basic_coverage_blocks, &coverage_spans);
let mcdc_num_condition_bitmaps = coverage_spans
.mappings
.mcdc_decisions
.iter()
.filter_map(|bcb_mapping| match bcb_mapping.kind {
BcbMappingKind::MCDCDecision { decision_depth, .. } => Some(decision_depth),
_ => None,
})
.map(|&mappings::MCDCDecision { decision_depth, .. }| decision_depth)
.max()
.map_or(0, |max| usize::from(max) + 1);
@ -149,44 +146,45 @@ fn create_mappings<'tcx>(
.expect("all BCBs with spans were given counters")
.as_term()
};
let region_for_span = |span: Span| make_code_region(source_map, file_name, span, body_span);
let mut mappings = Vec::new();
mappings.extend(coverage_spans.mappings.iter().filter_map(
|BcbMapping { kind: bcb_mapping_kind, span }| {
let kind = match *bcb_mapping_kind {
BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)),
BcbMappingKind::MCDCBranch {
true_bcb, false_bcb, condition_info: None, ..
} => MappingKind::Branch {
true_term: term_for_bcb(true_bcb),
false_term: term_for_bcb(false_bcb),
},
BcbMappingKind::MCDCBranch {
true_bcb,
false_bcb,
condition_info: Some(mcdc_params),
..
} => MappingKind::MCDCBranch {
true_term: term_for_bcb(true_bcb),
false_term: term_for_bcb(false_bcb),
mcdc_params,
},
BcbMappingKind::MCDCDecision { bitmap_idx, conditions_num, .. } => {
MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num })
}
};
let code_region = make_code_region(source_map, file_name, *span, body_span)?;
mappings.extend(coverage_spans.code_mappings.iter().filter_map(
|&mappings::CodeMapping { span, bcb }| {
let code_region = region_for_span(span)?;
let kind = MappingKind::Code(term_for_bcb(bcb));
Some(Mapping { kind, code_region })
},
));
mappings.extend(coverage_spans.branch_pairs.iter().filter_map(
|&BcbBranchPair { span, true_bcb, false_bcb }| {
|&mappings::BranchPair { span, true_bcb, false_bcb }| {
let true_term = term_for_bcb(true_bcb);
let false_term = term_for_bcb(false_bcb);
let kind = MappingKind::Branch { true_term, false_term };
let code_region = make_code_region(source_map, file_name, span, body_span)?;
let code_region = region_for_span(span)?;
Some(Mapping { kind, code_region })
},
));
mappings.extend(coverage_spans.mcdc_branches.iter().filter_map(
|&mappings::MCDCBranch { span, true_bcb, false_bcb, condition_info, decision_depth: _ }| {
let code_region = region_for_span(span)?;
let true_term = term_for_bcb(true_bcb);
let false_term = term_for_bcb(false_bcb);
let kind = match condition_info {
Some(mcdc_params) => MappingKind::MCDCBranch { true_term, false_term, mcdc_params },
None => MappingKind::Branch { true_term, false_term },
};
Some(Mapping { kind, code_region })
},
));
mappings.extend(coverage_spans.mcdc_decisions.iter().filter_map(
|&mappings::MCDCDecision { span, bitmap_idx, conditions_num, .. }| {
let code_region = region_for_span(span)?;
let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num });
Some(Mapping { kind, code_region })
},
));
@ -258,13 +256,13 @@ fn inject_mcdc_statements<'tcx>(
}
// Inject test vector update first because `inject_statement` always insert new statement at head.
for (end_bcbs, bitmap_idx, decision_depth) in
coverage_spans.mappings.iter().filter_map(|mapping| match &mapping.kind {
BcbMappingKind::MCDCDecision { end_bcbs, bitmap_idx, decision_depth, .. } => {
Some((end_bcbs, *bitmap_idx, *decision_depth))
}
_ => None,
})
for &mappings::MCDCDecision {
span: _,
ref end_bcbs,
bitmap_idx,
conditions_num: _,
decision_depth,
} in &coverage_spans.mcdc_decisions
{
for end in end_bcbs {
let end_bb = basic_coverage_blocks[*end].leader_bb();
@ -276,24 +274,22 @@ fn inject_mcdc_statements<'tcx>(
}
}
for (true_bcb, false_bcb, condition_id, decision_depth) in
coverage_spans.mappings.iter().filter_map(|mapping| match mapping.kind {
BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info, decision_depth } => {
Some((true_bcb, false_bcb, condition_info?.condition_id, decision_depth))
}
_ => None,
})
for &mappings::MCDCBranch { span: _, true_bcb, false_bcb, condition_info, decision_depth } in
&coverage_spans.mcdc_branches
{
let Some(condition_info) = condition_info else { continue };
let id = condition_info.condition_id;
let true_bb = basic_coverage_blocks[true_bcb].leader_bb();
inject_statement(
mir_body,
CoverageKind::CondBitmapUpdate { id: condition_id, value: true, decision_depth },
CoverageKind::CondBitmapUpdate { id, value: true, decision_depth },
true_bb,
);
let false_bb = basic_coverage_blocks[false_bcb].leader_bb();
inject_statement(
mir_body,
CoverageKind::CondBitmapUpdate { id: condition_id, value: false, decision_depth },
CoverageKind::CondBitmapUpdate { id, value: false, decision_depth },
false_bb,
);
}

8
compiler/rustc_mir_transform/src/coverage/spans.rs
View File

@ -2,7 +2,7 @@
use rustc_span::{BytePos, Span};
use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
use crate::coverage::mappings::{BcbMapping, BcbMappingKind};
use crate::coverage::mappings;
use crate::coverage::spans::from_mir::SpanFromMir;
use crate::coverage::ExtractedHirInfo;
@ -17,14 +17,14 @@ pub(super) fn extract_refined_covspans(
mir_body: &mir::Body<'_>,
hir_info: &ExtractedHirInfo,
basic_coverage_blocks: &CoverageGraph,
mappings: &mut impl Extend<BcbMapping>,
code_mappings: &mut impl Extend<mappings::CodeMapping>,
) {
let sorted_spans =
from_mir::mir_to_initial_sorted_coverage_spans(mir_body, hir_info, basic_coverage_blocks);
let coverage_spans = SpansRefiner::refine_sorted_spans(sorted_spans);
mappings.extend(coverage_spans.into_iter().map(|RefinedCovspan { bcb, span, .. }| {
code_mappings.extend(coverage_spans.into_iter().map(|RefinedCovspan { bcb, span, .. }| {
// Each span produced by the generator represents an ordinary code region.
BcbMapping { kind: BcbMappingKind::Code(bcb), span }
mappings::CodeMapping { span, bcb }
}));
}