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

coverage: Detach MC/DC branch spans from regular branch spans

Browse Source 
MC/DC's reliance on the existing branch coverage types is making it much harder
to improve branch coverage.
This commit is contained in:
Zalathar 2024-04-21 12:06:03 +10:00
parent a892c2387e
commit 97bf553682
6 changed files with 101 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -314,7 +314,9 @@ fn default() -> Self {
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
pub struct MCDCBranchSpan {
pub span: Span,
pub condition_info: ConditionInfo,
/// If `None`, this actually represents a normal branch span inserted for
/// code that was too complex for MC/DC.
pub condition_info: Option<ConditionInfo>,
pub true_marker: BlockMarkerId,
pub false_marker: BlockMarkerId,
}

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

@ -491,7 +491,7 @@ fn write_coverage_branch_info(
writeln!(
w,
"{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",
condition_info.condition_id
condition_info.map(|info| info.condition_id)
)?;
}

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

@ -22,6 +22,7 @@ pub(crate) struct BranchInfoBuilder {
num_block_markers: usize,
branch_spans: Vec<BranchSpan>,
mcdc_branch_spans: Vec<MCDCBranchSpan>,
mcdc_decision_spans: Vec<MCDCDecisionSpan>,
mcdc_state: Option<MCDCState>,
@ -95,13 +96,7 @@ fn visit_with_not_info(&mut self, thir: &Thir<'_>, expr_id: ExprId, not_info: No
}
}
fn record_conditions_operation(&mut self, logical_op: LogicalOp, span: Span) {
if let Some(mcdc_state) = self.mcdc_state.as_mut() {
mcdc_state.record_conditions(logical_op, span);
}
}
fn fetch_condition_info(
fn fetch_mcdc_condition_info(
&mut self,
tcx: TyCtxt<'_>,
true_marker: BlockMarkerId,
@ -121,14 +116,9 @@ fn fetch_condition_info(
_ => {
// Do not generate mcdc mappings and statements for decisions with too many conditions.
let rebase_idx = self.mcdc_branch_spans.len() - decision.conditions_num + 1;
let to_normal_branches = self.mcdc_branch_spans.split_off(rebase_idx);
self.branch_spans.extend(to_normal_branches.into_iter().map(
|MCDCBranchSpan { span, true_marker, false_marker, .. }| BranchSpan {
span,
true_marker,
false_marker,
},
));
for branch in &mut self.mcdc_branch_spans[rebase_idx..] {
branch.condition_info = None;
}
// ConditionInfo of this branch shall also be reset.
condition_info = None;
@ -157,7 +147,7 @@ pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
branch_spans,
mcdc_branch_spans,
mcdc_decision_spans,
..
mcdc_state: _,
} = self;
if num_block_markers == 0 {
@ -355,27 +345,30 @@ pub(crate) fn visit_coverage_branch_condition(
let true_marker = inject_branch_marker(then_block);
let false_marker = inject_branch_marker(else_block);
if let Some(condition_info) =
branch_info.fetch_condition_info(self.tcx, true_marker, false_marker)
{
if branch_info.mcdc_state.is_some() {
let condition_info =
branch_info.fetch_mcdc_condition_info(self.tcx, true_marker, false_marker);
branch_info.mcdc_branch_spans.push(MCDCBranchSpan {
span: source_info.span,
condition_info,
true_marker,
false_marker,
});
} else {
return;
}
branch_info.branch_spans.push(BranchSpan {
span: source_info.span,
true_marker,
false_marker,
});
}
}
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() {
branch_info.record_conditions_operation(logical_op, span);
if let Some(branch_info) = self.coverage_branch_info.as_mut()
&& let Some(mcdc_state) = branch_info.mcdc_state.as_mut()
{
mcdc_state.record_conditions(logical_op, span);
}
}
}

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

@ -149,13 +149,21 @@ fn create_mappings<'tcx>(
true_term: term_for_bcb(true_bcb),
false_term: term_for_bcb(false_bcb),
},
BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info } => {
MappingKind::MCDCBranch {
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),
mcdc_params: condition_info,
}
}
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 })
}
@ -249,7 +257,7 @@ fn inject_mcdc_statements<'tcx>(
for (true_bcb, false_bcb, condition_id) in
coverage_spans.all_bcb_mappings().filter_map(|mapping| match mapping.kind {
BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info } => {
Some((true_bcb, false_bcb, condition_info.condition_id))
Some((true_bcb, false_bcb, condition_info?.condition_id))
}
_ => None,
})

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

@ -21,7 +21,9 @@ pub(super) enum BcbMappingKind {
MCDCBranch {
true_bcb: BasicCoverageBlock,
false_bcb: BasicCoverageBlock,
condition_info: ConditionInfo,
/// If `None`, this actually represents a normal branch mapping inserted
/// for code that was too complex for MC/DC.
condition_info: Option<ConditionInfo>,
},
/// Associates a mcdc decision with its join BCB.
MCDCDecision { end_bcbs: BTreeSet<BasicCoverageBlock>, bitmap_idx: u32, conditions_num: u16 },
@ -85,6 +87,12 @@ pub(super) fn generate_coverage_spans(
}));
mappings.extend(from_mir::extract_branch_mappings(
mir_body,
hir_info,
basic_coverage_blocks,
));
mappings.extend(from_mir::extract_mcdc_mappings(
mir_body,
hir_info.body_span,
basic_coverage_blocks,

74
compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
View File

@ -366,15 +366,10 @@ fn new(
}
}
pub(super) fn extract_branch_mappings(
fn resolve_block_markers(
branch_info: &mir::coverage::BranchInfo,
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![];
};
) -> IndexVec<BlockMarkerId, Option<BasicBlock>> {
let mut block_markers = IndexVec::<BlockMarkerId, Option<BasicBlock>>::from_elem_n(
None,
branch_info.num_block_markers,
@ -389,6 +384,58 @@ pub(super) fn extract_branch_mappings(
}
}
block_markers
}
// FIXME: There is currently a lot of redundancy between
// `extract_branch_mappings` and `extract_mcdc_mappings`. This is needed so
// that they can each be modified without interfering with the other, but in
// the long term we should try to bring them together again when branch coverage
// and MC/DC coverage support are more mature.
pub(super) fn extract_branch_mappings(
mir_body: &mir::Body<'_>,
hir_info: &ExtractedHirInfo,
basic_coverage_blocks: &CoverageGraph,
) -> Vec<BcbMapping> {
let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return vec![] };
let block_markers = resolve_block_markers(branch_info, mir_body);
branch_info
.branch_spans
.iter()
.filter_map(|&BranchSpan { span: raw_span, true_marker, false_marker }| {
// For now, ignore any branch span that was introduced by
// expansion. This makes things like assert macros less noisy.
if !raw_span.ctxt().outer_expn_data().is_root() {
return None;
}
let (span, _) =
unexpand_into_body_span_with_visible_macro(raw_span, hir_info.body_span)?;
let bcb_from_marker =
|marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?);
let true_bcb = bcb_from_marker(true_marker)?;
let false_bcb = bcb_from_marker(false_marker)?;
Some(BcbMapping { kind: BcbMappingKind::Branch { true_bcb, false_bcb }, span })
})
.collect::<Vec<_>>()
}
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![];
};
let block_markers = resolve_block_markers(branch_info, mir_body);
let bcb_from_marker =
|marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?);
@ -406,12 +453,6 @@ pub(super) fn extract_branch_mappings(
Some((span, true_bcb, false_bcb))
};
let branch_filter_map = |&BranchSpan { span: raw_span, true_marker, false_marker }| {
check_branch_bcb(raw_span, true_marker, false_marker).map(|(span, true_bcb, false_bcb)| {
BcbMapping { kind: BcbMappingKind::Branch { true_bcb, false_bcb }, span }
})
};
let mcdc_branch_filter_map =
|&MCDCBranchSpan { span: raw_span, true_marker, false_marker, condition_info }| {
check_branch_bcb(raw_span, true_marker, false_marker).map(
@ -446,10 +487,7 @@ pub(super) fn extract_branch_mappings(
})
};
branch_info
.branch_spans
.iter()
.filter_map(branch_filter_map)
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<_>>()