From 881b0b40151b7771db703a526b4c7f41945808df Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 21 May 2024 15:51:54 +0800 Subject: [PATCH 1/3] coverage. Warn about too much decision depth --- compiler/rustc_mir_build/messages.ftl | 2 ++ .../src/build/coverageinfo/mcdc.rs | 33 ++++++++++++------- compiler/rustc_mir_build/src/errors.rs | 8 +++++ 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 0c277811fdacf..3de84e7f635f7 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -105,6 +105,8 @@ mir_build_deref_raw_pointer_requires_unsafe_unsafe_op_in_unsafe_fn_allowed = mir_build_exceeds_mcdc_condition_limit = number of conditions in decision ({$num_conditions}) exceeds limit ({$max_conditions}), so MC/DC analysis will not count this expression +mir_build_exceeds_mcdc_decision_depth = number of decisions evaluated simultaneously exceeds limit ({$max_decision_depth}). MCDC analysis will not count this expression + mir_build_extern_static_requires_unsafe = use of extern static is unsafe and requires unsafe block .note = extern statics are not controlled by the Rust type system: invalid data, aliasing violations or data races will cause undefined behavior diff --git a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs index 3aa6e708476d5..b09c0b2764184 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs @@ -10,13 +10,17 @@ use rustc_middle::ty::TyCtxt; use rustc_span::Span; use crate::build::Builder; -use crate::errors::MCDCExceedsConditionLimit; +use crate::errors::{MCDCExceedsConditionLimit, MCDCExceedsDecisionDepth}; /// The MCDC bitmap scales exponentially (2^n) based on the number of conditions seen, /// So llvm sets a maximum value prevents the bitmap footprint from growing too large without the user's knowledge. /// This limit may be relaxed if the [upstream change](https://github.com/llvm/llvm-project/pull/82448) is merged. const MAX_CONDITIONS_IN_DECISION: usize = 6; +/// MCDC allocates an i32 variable on stack for each depth. Ignore decisions nested too much to prevent it +/// consuming excessive memory. +const MAX_DECISION_DEPTH: u16 = 0x3FFF; + #[derive(Default)] struct MCDCDecisionCtx { /// To construct condition evaluation tree. @@ -208,14 +212,14 @@ impl MCDCInfoBuilder { // is empty, i.e. when all the conditions of the decision were instrumented, // and the decision is "complete". if let Some(decision) = decision_result { - match decision.num_conditions { - 0 => { + match (decision.num_conditions, decision_depth) { + (0, _) => { unreachable!("Decision with no condition is not expected"); } - 1..=MAX_CONDITIONS_IN_DECISION => { + (1..=MAX_CONDITIONS_IN_DECISION, 0..=MAX_DECISION_DEPTH) => { self.decision_spans.push(decision); } - _ => { + (_, _) => { // Do not generate mcdc mappings and statements for decisions with too many conditions. // Therefore, first erase the condition info of the (N-1) previous branch spans. let rebase_idx = self.branch_spans.len() - (decision.num_conditions - 1); @@ -225,12 +229,19 @@ impl MCDCInfoBuilder { // Then, erase this last branch span's info too, for a total of N. condition_info = None; - - tcx.dcx().emit_warn(MCDCExceedsConditionLimit { - span: decision.span, - num_conditions: decision.num_conditions, - max_conditions: MAX_CONDITIONS_IN_DECISION, - }); + if decision.num_conditions > MAX_CONDITIONS_IN_DECISION { + tcx.dcx().emit_warn(MCDCExceedsConditionLimit { + span: decision.span, + num_conditions: decision.num_conditions, + max_conditions: MAX_CONDITIONS_IN_DECISION, + }); + } + if decision_depth > MAX_DECISION_DEPTH { + tcx.dcx().emit_warn(MCDCExceedsDecisionDepth { + span: decision.span, + max_decision_depth: MAX_DECISION_DEPTH.into(), + }); + } } } } diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 7c73d8a6d47d4..52b654804e179 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -844,6 +844,14 @@ pub(crate) struct MCDCExceedsConditionLimit { pub(crate) max_conditions: usize, } +#[derive(Diagnostic)] +#[diag(mir_build_exceeds_mcdc_decision_depth)] +pub(crate) struct MCDCExceedsDecisionDepth { + #[primary_span] + pub span: Span, + pub max_decision_depth: usize, +} + #[derive(Diagnostic)] #[diag(mir_build_pattern_not_covered, code = E0005)] pub(crate) struct PatternNotCovered<'s, 'tcx> { From e16030817b8bb5d0039239b8c9f5f9fb256a41e9 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 9 Jul 2024 15:07:09 +0800 Subject: [PATCH 2/3] coverage. Refactor MCDCInfoBuilder for pattern matching implementation and change the way to calculate decision depth --- compiler/rustc_middle/src/mir/coverage.rs | 7 +- .../rustc_mir_build/src/build/coverageinfo.rs | 11 +- .../src/build/coverageinfo/mcdc.rs | 492 +++++++++++++----- .../rustc_mir_build/src/build/expr/into.rs | 4 + .../rustc_mir_build/src/build/matches/mod.rs | 2 - .../mcdc/nested_in_boolean_exprs.cov-map | 161 ++++++ .../mcdc/nested_in_boolean_exprs.coverage | 195 +++++++ .../coverage/mcdc/nested_in_boolean_exprs.rs | 49 ++ 8 files changed, 777 insertions(+), 144 deletions(-) create mode 100644 tests/coverage/mcdc/nested_in_boolean_exprs.cov-map create mode 100644 tests/coverage/mcdc/nested_in_boolean_exprs.coverage create mode 100644 tests/coverage/mcdc/nested_in_boolean_exprs.rs diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 2a593340849ef..88e5384eef936 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -50,7 +50,6 @@ rustc_index::newtype_index! { #[debug_format = "ExpressionId({})"] pub struct ExpressionId {} } - rustc_index::newtype_index! { /// ID of a mcdc condition. Used by llvm to check mcdc coverage. /// @@ -332,3 +331,9 @@ pub struct MCDCDecisionSpan { pub end_markers: Vec, pub decision_depth: u16, } + +impl MCDCDecisionSpan { + pub fn new(span: Span) -> Self { + Self { span, num_conditions: 0, end_markers: Vec::new(), decision_depth: 0 } + } +} diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 204ee45bfa2d7..6f70a952bf291 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -144,10 +144,10 @@ impl CoverageInfoBuilder { // Separate path for handling branches when MC/DC is enabled. if let Some(mcdc_info) = self.mcdc_info.as_mut() { let inject_block_marker = - |source_info, block| self.markers.inject_block_marker(cfg, source_info, block); + |block| self.markers.inject_block_marker(cfg, source_info, block); mcdc_info.visit_evaluated_condition( tcx, - source_info, + source_info.span, true_block, false_block, inject_block_marker, @@ -175,8 +175,13 @@ impl CoverageInfoBuilder { let branch_spans = branch_info.map(|branch_info| branch_info.branch_spans).unwrap_or_default(); - let (mcdc_decision_spans, mcdc_branch_spans) = + let (mut mcdc_branch_spans, mcdc_spans) = mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default(); + let mut mcdc_decision_spans = Vec::with_capacity(mcdc_spans.len()); + for (decision, conditions) in mcdc_spans { + mcdc_branch_spans.extend(conditions); + mcdc_decision_spans.push(decision); + } // For simplicity, always return an info struct (without Option), even // if there's nothing interesting in it. diff --git a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs index b09c0b2764184..a9b761d4c6904 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs @@ -1,11 +1,14 @@ +use std::cell::Cell; use std::collections::VecDeque; +use std::rc::Rc; +use rustc_data_structures::fx::FxIndexMap; use rustc_middle::bug; use rustc_middle::mir::coverage::{ BlockMarkerId, ConditionId, ConditionInfo, MCDCBranchSpan, MCDCDecisionSpan, }; -use rustc_middle::mir::{BasicBlock, SourceInfo}; -use rustc_middle::thir::LogicalOp; +use rustc_middle::mir::BasicBlock; +use rustc_middle::thir::{ExprKind, LogicalOp}; use rustc_middle::ty::TyCtxt; use rustc_span::Span; @@ -21,34 +24,25 @@ const MAX_CONDITIONS_IN_DECISION: usize = 6; /// consuming excessive memory. const MAX_DECISION_DEPTH: u16 = 0x3FFF; -#[derive(Default)] -struct MCDCDecisionCtx { +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +struct DecisionId(usize); + +#[derive(Debug)] +struct BooleanDecisionCtx { + id: DecisionId, + decision_info: MCDCDecisionSpan, /// To construct condition evaluation tree. decision_stack: VecDeque, - processing_decision: Option, -} - -struct MCDCState { - decision_ctx_stack: Vec, + conditions: Vec, } -impl MCDCState { - fn new() -> Self { - Self { decision_ctx_stack: vec![MCDCDecisionCtx::default()] } - } - - /// Decision depth is given as a u16 to reduce the size of the `CoverageKind`, - /// as it is very unlikely that the depth ever reaches 2^16. - #[inline] - fn decision_depth(&self) -> u16 { - match u16::try_from(self.decision_ctx_stack.len()) - .expect( - "decision depth did not fit in u16, this is likely to be an instrumentation error", - ) - .checked_sub(1) - { - Some(d) => d, - None => bug!("Unexpected empty decision stack"), +impl BooleanDecisionCtx { + fn new(id: DecisionId) -> Self { + Self { + id, + decision_info: MCDCDecisionSpan::new(Span::default()), + decision_stack: VecDeque::new(), + conditions: vec![], } } @@ -92,34 +86,17 @@ impl MCDCState { // As the compiler tracks expression in pre-order, we can ensure that condition info of parents are always properly assigned when their children are visited. // - If the op is AND, the "false_next" of LHS and RHS should be the parent's "false_next". While "true_next" of the LHS is the RHS, the "true next" of RHS is the parent's "true_next". // - If the op is OR, the "true_next" of LHS and RHS should be the parent's "true_next". While "false_next" of the LHS is the RHS, the "false next" of RHS is the parent's "false_next". - fn record_conditions(&mut self, op: LogicalOp, span: Span) { - let decision_depth = self.decision_depth(); - let Some(decision_ctx) = self.decision_ctx_stack.last_mut() else { - bug!("Unexpected empty decision_ctx_stack") - }; - let decision = match decision_ctx.processing_decision.as_mut() { - Some(decision) => { - decision.span = decision.span.to(span); - decision - } - None => decision_ctx.processing_decision.insert(MCDCDecisionSpan { - span, - num_conditions: 0, - end_markers: vec![], - decision_depth, - }), - }; - - let parent_condition = decision_ctx.decision_stack.pop_back().unwrap_or_default(); + fn record_conditions(&mut self, op: LogicalOp) { + let parent_condition = self.decision_stack.pop_back().unwrap_or_default(); let lhs_id = if parent_condition.condition_id == ConditionId::NONE { - decision.num_conditions += 1; - ConditionId::from(decision.num_conditions) + self.decision_info.num_conditions += 1; + ConditionId::from(self.decision_info.num_conditions) } else { parent_condition.condition_id }; - decision.num_conditions += 1; - let rhs_condition_id = ConditionId::from(decision.num_conditions); + self.decision_info.num_conditions += 1; + let rhs_condition_id = ConditionId::from(self.decision_info.num_conditions); let (lhs, rhs) = match op { LogicalOp::And => { @@ -150,112 +127,342 @@ impl MCDCState { } }; // We visit expressions tree in pre-order, so place the left-hand side on the top. - decision_ctx.decision_stack.push_back(rhs); - decision_ctx.decision_stack.push_back(lhs); + self.decision_stack.push_back(rhs); + self.decision_stack.push_back(lhs); } - fn take_condition( + fn finish_two_way_branch( &mut self, + span: Span, true_marker: BlockMarkerId, false_marker: BlockMarkerId, - ) -> (Option, Option) { - let Some(decision_ctx) = self.decision_ctx_stack.last_mut() else { - bug!("Unexpected empty decision_ctx_stack") - }; - let Some(condition_info) = decision_ctx.decision_stack.pop_back() else { - return (None, None); - }; - let Some(decision) = decision_ctx.processing_decision.as_mut() else { - bug!("Processing decision should have been created before any conditions are taken"); - }; + ) { + let condition_info = self.decision_stack.pop_back().unwrap_or_default(); if condition_info.true_next_id == ConditionId::NONE { - decision.end_markers.push(true_marker); + self.decision_info.end_markers.push(true_marker); } if condition_info.false_next_id == ConditionId::NONE { - decision.end_markers.push(false_marker); + self.decision_info.end_markers.push(false_marker); } - if decision_ctx.decision_stack.is_empty() { - (Some(condition_info), decision_ctx.processing_decision.take()) - } else { - (Some(condition_info), None) + self.conditions.push(MCDCBranchSpan { + span, + condition_info: Some(condition_info), + true_marker, + false_marker, + decision_depth: 0, + }); + // In case this decision had only one condition + self.decision_info.num_conditions = self.decision_info.num_conditions.max(1); + } + + fn is_finished(&self) -> bool { + self.decision_stack.is_empty() + } + + fn into_done(self) -> (DecisionId, MCDCDecisionSpan, Vec) { + (self.id, self.decision_info, self.conditions) + } +} + +#[derive(Debug)] +enum DecisionCtx { + Boolean(BooleanDecisionCtx), + #[allow(unused)] + Matching, +} + +impl DecisionCtx { + fn new_boolean(id: DecisionId) -> Self { + Self::Boolean(BooleanDecisionCtx::new(id)) + } +} + +pub(crate) struct MCDCStateGuard { + state_stashed_ref: Option>>, +} + +impl Drop for MCDCStateGuard { + fn drop(&mut self) { + if let Some(stashed) = self.state_stashed_ref.take() { + stashed.set(false); + } + } +} + +/// `MCDCState` represents a layer to hold decisions. Decisions produced +/// by same state are nested in same decision. +#[derive(Debug)] +struct MCDCState { + current_ctx: Option, + nested_decision_records: Vec, + // `Stashed` means we are processing a decision nested in decision of this state. + stashed: Rc>, +} + +impl MCDCState { + fn new() -> Self { + Self { + current_ctx: None, + nested_decision_records: vec![], + stashed: Rc::new(Cell::new(false)), } } + + fn is_stashed(&self) -> bool { + self.stashed.get() + } + + fn take_ctx(&mut self) -> Option<(DecisionCtx, Vec)> { + let ctx = self.current_ctx.take()?; + let nested_decisions_id = std::mem::take(&mut self.nested_decision_records); + Some((ctx, nested_decisions_id)) + } + + // Return `true` if there is no decision being processed currently. + fn is_empty(&self) -> bool { + self.current_ctx.is_none() + } + + fn record_nested_decision(&mut self, id: DecisionId) { + self.nested_decision_records.push(id); + } + + fn inherit_nested_decisions(&mut self, nested_decisions_id: Vec) { + self.nested_decision_records.extend(nested_decisions_id); + } + + fn take_current_nested_decisions(&mut self) -> Vec { + std::mem::take(&mut self.nested_decision_records) + } +} + +#[derive(Debug)] +struct MCDCTargetInfo { + decision: MCDCDecisionSpan, + conditions: Vec, + nested_decisions_id: Vec, +} + +impl MCDCTargetInfo { + fn set_depth(&mut self, depth: u16) { + self.decision.decision_depth = depth; + self.conditions.iter_mut().for_each(|branch| branch.decision_depth = depth); + } +} + +#[derive(Default)] +struct DecisionIdGen(usize); +impl DecisionIdGen { + fn next_decision_id(&mut self) -> DecisionId { + let id = DecisionId(self.0); + self.0 += 1; + id + } } pub(crate) struct MCDCInfoBuilder { - branch_spans: Vec, - decision_spans: Vec, - state: MCDCState, + normal_branch_spans: Vec, + mcdc_targets: FxIndexMap, + state_stack: Vec, + decision_id_gen: DecisionIdGen, } impl MCDCInfoBuilder { pub(crate) fn new() -> Self { - Self { branch_spans: vec![], decision_spans: vec![], state: MCDCState::new() } + Self { + normal_branch_spans: vec![], + mcdc_targets: FxIndexMap::default(), + state_stack: vec![], + decision_id_gen: DecisionIdGen::default(), + } + } + + fn has_processing_decision(&self) -> bool { + // Check from top to get working states a bit quicker. + !self.state_stack.iter().rev().all(|state| state.is_empty() && !state.is_stashed()) + } + + fn current_state_mut(&mut self) -> &mut MCDCState { + let current_idx = self.state_stack.len() - 1; + &mut self.state_stack[current_idx] + } + + fn ensure_active_state(&mut self) { + let mut active_state_idx = None; + // Down to the first non-stashed state or non-empty state, which can be ensured to be + // processed currently. + for (idx, state) in self.state_stack.iter().enumerate().rev() { + if state.is_stashed() { + active_state_idx = Some(idx + 1); + break; + } else if !state.is_empty() { + active_state_idx = Some(idx); + break; + } + } + match active_state_idx { + // There are some states were created for nested decisions but now + // since the lower state has been unstashed they should be removed. + Some(idx) if idx + 1 < self.state_stack.len() => { + let expected_len = idx + 1; + let nested_decisions_id = self + .state_stack + .iter_mut() + .skip(expected_len) + .map(|state| state.take_current_nested_decisions().into_iter()) + .flatten() + .collect(); + self.state_stack.truncate(expected_len); + self.state_stack[idx].inherit_nested_decisions(nested_decisions_id); + } + // The top state is just wanted. + Some(idx) if idx + 1 == self.state_stack.len() => {} + // Otherwise no available state yet, create a new one. + _ => self.state_stack.push(MCDCState::new()), + } + } + + fn ensure_boolean_decision(&mut self, condition_span: Span) -> &mut BooleanDecisionCtx { + self.ensure_active_state(); + let state = self.state_stack.last_mut().expect("ensured just now"); + let DecisionCtx::Boolean(ctx) = state.current_ctx.get_or_insert_with(|| { + DecisionCtx::new_boolean(self.decision_id_gen.next_decision_id()) + }) else { + unreachable!("ensured above"); + }; + + if ctx.decision_info.span == Span::default() { + ctx.decision_info.span = condition_span; + } else { + ctx.decision_info.span = ctx.decision_info.span.to(condition_span); + } + ctx + } + + fn append_normal_branches(&mut self, mut branches: Vec) { + branches.iter_mut().for_each(|branch| branch.condition_info = None); + self.normal_branch_spans.extend(branches); + } + + fn append_mcdc_info(&mut self, tcx: TyCtxt<'_>, id: DecisionId, info: MCDCTargetInfo) -> bool { + let num_conditions = info.conditions.len(); + match num_conditions { + 0 => { + unreachable!("Decision with no condition is not expected"); + } + // Ignore decisions with only one condition given that mcdc for them is completely equivalent to branch coverage. + 2..=MAX_CONDITIONS_IN_DECISION => { + self.mcdc_targets.insert(id, info); + true + } + _ => { + self.append_normal_branches(info.conditions); + self.current_state_mut().inherit_nested_decisions(info.nested_decisions_id); + if num_conditions > MAX_CONDITIONS_IN_DECISION { + tcx.dcx().emit_warn(MCDCExceedsConditionLimit { + span: info.decision.span, + num_conditions, + max_conditions: MAX_CONDITIONS_IN_DECISION, + }); + } + false + } + } + } + + fn normalize_depth_from(&mut self, tcx: TyCtxt<'_>, id: DecisionId) { + let Some(entry_decision) = self.mcdc_targets.get_mut(&id) else { + bug!("unknown mcdc decision"); + }; + let mut next_nested_records = entry_decision.nested_decisions_id.clone(); + let mut depth = 0; + while !next_nested_records.is_empty() { + depth += 1; + for id in std::mem::take(&mut next_nested_records) { + let Some(nested_target) = self.mcdc_targets.get_mut(&id) else { + continue; + }; + nested_target.set_depth(depth); + next_nested_records.extend(nested_target.nested_decisions_id.iter().copied()); + if depth > MAX_DECISION_DEPTH { + tcx.dcx().emit_warn(MCDCExceedsDecisionDepth { + span: nested_target.decision.span, + max_decision_depth: MAX_DECISION_DEPTH.into(), + }); + let branches = std::mem::take(&mut nested_target.conditions); + self.append_normal_branches(branches); + self.mcdc_targets.swap_remove(&id); + } + } + } + } + + // If `entry_decision_id` is some, there must be at least one mcdc decision being produced. + fn on_ctx_finished(&mut self, tcx: TyCtxt<'_>, entry_decision_id: Option) { + match (self.has_processing_decision(), entry_decision_id) { + // Can not be nested in other decisions, depth is accumulated starting from this decision. + (false, Some(id)) => self.normalize_depth_from(tcx, id), + // May be nested in other decisions, record it. + (true, Some(id)) => self.current_state_mut().record_nested_decision(id), + // No decision is produced this time and no other parent decision to be processing. + // All "nested decisions" now get zero depth and then calculate depth of their children. + (false, None) => { + for root_decision in self.current_state_mut().take_current_nested_decisions() { + self.normalize_depth_from(tcx, root_decision); + } + } + (true, None) => {} + } } pub(crate) fn visit_evaluated_condition( &mut self, tcx: TyCtxt<'_>, - source_info: SourceInfo, + span: Span, true_block: BasicBlock, false_block: BasicBlock, - mut inject_block_marker: impl FnMut(SourceInfo, BasicBlock) -> BlockMarkerId, + mut inject_block_marker: impl FnMut(BasicBlock) -> BlockMarkerId, ) { - let true_marker = inject_block_marker(source_info, true_block); - let false_marker = inject_block_marker(source_info, false_block); - - let decision_depth = self.state.decision_depth(); - let (mut condition_info, decision_result) = - self.state.take_condition(true_marker, false_marker); - // take_condition() returns Some for decision_result when the decision stack - // is empty, i.e. when all the conditions of the decision were instrumented, - // and the decision is "complete". - if let Some(decision) = decision_result { - match (decision.num_conditions, decision_depth) { - (0, _) => { - unreachable!("Decision with no condition is not expected"); - } - (1..=MAX_CONDITIONS_IN_DECISION, 0..=MAX_DECISION_DEPTH) => { - self.decision_spans.push(decision); - } - (_, _) => { - // Do not generate mcdc mappings and statements for decisions with too many conditions. - // Therefore, first erase the condition info of the (N-1) previous branch spans. - let rebase_idx = self.branch_spans.len() - (decision.num_conditions - 1); - for branch in &mut self.branch_spans[rebase_idx..] { - branch.condition_info = None; - } + let true_marker = inject_block_marker(true_block); + let false_marker = inject_block_marker(false_block); + let decision = self.ensure_boolean_decision(span); + decision.finish_two_way_branch(span, true_marker, false_marker); - // Then, erase this last branch span's info too, for a total of N. - condition_info = None; - if decision.num_conditions > MAX_CONDITIONS_IN_DECISION { - tcx.dcx().emit_warn(MCDCExceedsConditionLimit { - span: decision.span, - num_conditions: decision.num_conditions, - max_conditions: MAX_CONDITIONS_IN_DECISION, - }); - } - if decision_depth > MAX_DECISION_DEPTH { - tcx.dcx().emit_warn(MCDCExceedsDecisionDepth { - span: decision.span, - max_decision_depth: MAX_DECISION_DEPTH.into(), - }); - } - } - } + if !decision.is_finished() { + return; } - self.branch_spans.push(MCDCBranchSpan { - span: source_info.span, - condition_info, - true_marker, - false_marker, - decision_depth, - }); + + let Some((DecisionCtx::Boolean(ctx), nested_decisions_id)) = + self.current_state_mut().take_ctx() + else { + unreachable!("ensured boolean ctx above"); + }; + + let (id, decision, conditions) = ctx.into_done(); + let info = MCDCTargetInfo { decision, conditions, nested_decisions_id }; + let entry_decision_id = self.append_mcdc_info(tcx, id, info).then_some(id); + self.on_ctx_finished(tcx, entry_decision_id); } - pub(crate) fn into_done(self) -> (Vec, Vec) { - (self.decision_spans, self.branch_spans) + pub(crate) fn into_done( + self, + ) -> (Vec, Vec<(MCDCDecisionSpan, Vec)>) { + let MCDCInfoBuilder { + normal_branch_spans, + mcdc_targets, + state_stack: _, + decision_id_gen: _, + } = self; + + let mcdc_spans = mcdc_targets + .into_values() + .map(|MCDCTargetInfo { decision, conditions, nested_decisions_id: _ }| { + (decision, conditions) + }) + .collect(); + + (normal_branch_spans, mcdc_spans) } } @@ -264,25 +471,34 @@ impl Builder<'_, '_> { 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); + let decision = mcdc_info.ensure_boolean_decision(span); + decision.record_conditions(logical_op); } } - pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) { - 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(coverage_info) = self.coverage_info.as_mut() - && let Some(mcdc_info) = coverage_info.mcdc_info.as_mut() + pub(crate) fn mcdc_prepare_ctx_for(&mut self, expr_kind: &ExprKind<'_>) -> MCDCStateGuard { + if let Some(branch_info) = self.coverage_info.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() { - if mcdc_info.state.decision_ctx_stack.pop().is_none() { - bug!("Unexpected empty decision stack"); + match expr_kind { + ExprKind::Unary { .. } | ExprKind::Scope { .. } => {} + ExprKind::LogicalOp { .. } => { + // By here a decision is going to be produced + mcdc_info.ensure_active_state(); + } + _ => { + // Non-logical expressions leads to nested decisions only when a decision is being processed. + // In such cases just mark the state `stashed`. If a nested decision following, a new active state will be + // created at the previous arm. The current top state will be unstashed when the guard is dropped. + if mcdc_info.has_processing_decision() { + let stashed = &mcdc_info.current_state_mut().stashed; + if !stashed.replace(true) { + return MCDCStateGuard { state_stashed_ref: Some(stashed.clone()) }; + } + } + } } }; + MCDCStateGuard { state_stashed_ref: None } } } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 942c69b5c0a75..186e0ab7c3b2d 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -33,6 +33,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let expr_span = expr.span; let source_info = this.source_info(expr_span); + // Prepare mcdc context so that the nested decision get proper depth if any. + // The guard automatically marks the context processing parent decisions restored on dropping. + let _mcdc_guard = this.mcdc_prepare_ctx_for(&expr.kind); + let expr_is_block_or_scope = matches!(expr.kind, ExprKind::Block { .. } | ExprKind::Scope { .. }); diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 841ef2719c99d..143f947b53e1f 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -207,10 +207,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Increment the decision depth, in case we encounter boolean expressions // further down. - this.mcdc_increment_depth_if_enabled(); let place = unpack!(block = this.as_temp(block, Some(temp_scope), expr_id, mutability)); - this.mcdc_decrement_depth_if_enabled(); let operand = Operand::Move(Place::from(place)); diff --git a/tests/coverage/mcdc/nested_in_boolean_exprs.cov-map b/tests/coverage/mcdc/nested_in_boolean_exprs.cov-map new file mode 100644 index 0000000000000..e168a12417a24 --- /dev/null +++ b/tests/coverage/mcdc/nested_in_boolean_exprs.cov-map @@ -0,0 +1,161 @@ +Function name: nested_in_boolean_exprs::assign_nested_func_call +Raw bytes (102): 0x[01, 01, 07, 07, 0d, 05, 09, 01, 05, 01, 05, 16, 11, 01, 05, 11, 15, 0c, 01, 12, 01, 00, 37, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 28, 01, 02, 00, 0d, 00, 1d, 30, 05, 16, 01, 00, 02, 00, 0d, 00, 0e, 30, 09, 0d, 02, 00, 00, 00, 12, 00, 1d, 16, 00, 16, 00, 17, 28, 00, 02, 00, 16, 00, 1c, 30, 11, 12, 01, 02, 00, 00, 16, 00, 17, 11, 00, 1b, 00, 1c, 30, 15, 1a, 02, 00, 00, 00, 1b, 00, 1c, 03, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 7 +- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(3) +- expression 1 operands: lhs = Counter(1), rhs = Counter(2) +- expression 2 operands: lhs = Counter(0), rhs = Counter(1) +- expression 3 operands: lhs = Counter(0), rhs = Counter(1) +- expression 4 operands: lhs = Expression(5, Sub), rhs = Counter(4) +- expression 5 operands: lhs = Counter(0), rhs = Counter(1) +- expression 6 operands: lhs = Counter(4), rhs = Counter(5) +Number of file 0 mappings: 12 +- Code(Counter(0)) at (prev + 18, 1) to (start + 0, 55) +- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) + = ((c1 + c2) + c3) +- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) +- MCDCDecision { bitmap_idx: 1, conditions_num: 2 } at (prev + 0, 13) to (start + 0, 29) +- MCDCBranch { true: Counter(1), false: Expression(5, Sub), condition_id: 1, true_next_id: 0, false_next_id: 2 } at (prev + 0, 13) to (start + 0, 14) + true = c1 + false = (c0 - c1) +- MCDCBranch { true: Counter(2), false: Counter(3), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 18) to (start + 0, 29) + true = c2 + false = c3 +- Code(Expression(5, Sub)) at (prev + 0, 22) to (start + 0, 23) + = (c0 - c1) +- MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 0, 22) to (start + 0, 28) +- MCDCBranch { true: Counter(4), false: Expression(4, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 22) to (start + 0, 23) + true = c4 + false = ((c0 - c1) - c4) +- Code(Counter(4)) at (prev + 0, 27) to (start + 0, 28) +- MCDCBranch { true: Counter(5), false: Expression(6, Sub), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 27) to (start + 0, 28) + true = c5 + false = (c4 - c5) +- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) + = ((c1 + c2) + c3) + +Function name: nested_in_boolean_exprs::assign_nested_if +Raw bytes (120): 0x[01, 01, 0b, 07, 0d, 05, 09, 01, 05, 01, 05, 2a, 11, 01, 05, 11, 15, 11, 15, 15, 26, 2a, 11, 01, 05, 0e, 01, 09, 01, 00, 30, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 28, 01, 02, 00, 0d, 00, 33, 30, 05, 2a, 01, 00, 02, 00, 0d, 00, 0e, 30, 09, 0d, 02, 00, 00, 00, 12, 00, 33, 2a, 00, 15, 00, 16, 28, 00, 02, 00, 15, 00, 1b, 30, 11, 26, 01, 02, 00, 00, 15, 00, 16, 11, 00, 1a, 00, 1b, 30, 1e, 15, 02, 00, 00, 00, 1a, 00, 1b, 1e, 00, 1e, 00, 23, 23, 00, 2d, 00, 31, 03, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 11 +- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(3) +- expression 1 operands: lhs = Counter(1), rhs = Counter(2) +- expression 2 operands: lhs = Counter(0), rhs = Counter(1) +- expression 3 operands: lhs = Counter(0), rhs = Counter(1) +- expression 4 operands: lhs = Expression(10, Sub), rhs = Counter(4) +- expression 5 operands: lhs = Counter(0), rhs = Counter(1) +- expression 6 operands: lhs = Counter(4), rhs = Counter(5) +- expression 7 operands: lhs = Counter(4), rhs = Counter(5) +- expression 8 operands: lhs = Counter(5), rhs = Expression(9, Sub) +- expression 9 operands: lhs = Expression(10, Sub), rhs = Counter(4) +- expression 10 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 14 +- Code(Counter(0)) at (prev + 9, 1) to (start + 0, 48) +- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) + = ((c1 + c2) + c3) +- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) +- MCDCDecision { bitmap_idx: 1, conditions_num: 2 } at (prev + 0, 13) to (start + 0, 51) +- MCDCBranch { true: Counter(1), false: Expression(10, Sub), condition_id: 1, true_next_id: 0, false_next_id: 2 } at (prev + 0, 13) to (start + 0, 14) + true = c1 + false = (c0 - c1) +- MCDCBranch { true: Counter(2), false: Counter(3), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 18) to (start + 0, 51) + true = c2 + false = c3 +- Code(Expression(10, Sub)) at (prev + 0, 21) to (start + 0, 22) + = (c0 - c1) +- MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 0, 21) to (start + 0, 27) +- MCDCBranch { true: Counter(4), false: Expression(9, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 21) to (start + 0, 22) + true = c4 + false = ((c0 - c1) - c4) +- Code(Counter(4)) at (prev + 0, 26) to (start + 0, 27) +- MCDCBranch { true: Expression(7, Sub), false: Counter(5), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 26) to (start + 0, 27) + true = (c4 - c5) + false = c5 +- Code(Expression(7, Sub)) at (prev + 0, 30) to (start + 0, 35) + = (c4 - c5) +- Code(Expression(8, Add)) at (prev + 0, 45) to (start + 0, 49) + = (c5 + ((c0 - c1) - c4)) +- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) + = ((c1 + c2) + c3) + +Function name: nested_in_boolean_exprs::foo +Raw bytes (9): 0x[01, 01, 00, 01, 01, 0e, 01, 02, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 14, 1) to (start + 2, 2) + +Function name: nested_in_boolean_exprs::func_call_nested_if +Raw bytes (120): 0x[01, 01, 0b, 07, 0d, 05, 09, 01, 05, 01, 05, 2a, 11, 01, 05, 11, 15, 11, 15, 15, 26, 2a, 11, 01, 05, 0e, 01, 17, 01, 00, 33, 03, 01, 09, 00, 0a, 01, 00, 11, 00, 12, 28, 01, 02, 00, 11, 00, 37, 30, 05, 2a, 01, 00, 02, 00, 11, 00, 12, 30, 09, 0d, 02, 00, 00, 00, 16, 00, 37, 2a, 00, 19, 00, 1a, 28, 00, 02, 00, 19, 00, 1f, 30, 11, 26, 01, 02, 00, 00, 19, 00, 1a, 11, 00, 1e, 00, 1f, 30, 1e, 15, 02, 00, 00, 00, 1e, 00, 1f, 1e, 00, 22, 00, 27, 23, 00, 31, 00, 35, 03, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 11 +- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(3) +- expression 1 operands: lhs = Counter(1), rhs = Counter(2) +- expression 2 operands: lhs = Counter(0), rhs = Counter(1) +- expression 3 operands: lhs = Counter(0), rhs = Counter(1) +- expression 4 operands: lhs = Expression(10, Sub), rhs = Counter(4) +- expression 5 operands: lhs = Counter(0), rhs = Counter(1) +- expression 6 operands: lhs = Counter(4), rhs = Counter(5) +- expression 7 operands: lhs = Counter(4), rhs = Counter(5) +- expression 8 operands: lhs = Counter(5), rhs = Expression(9, Sub) +- expression 9 operands: lhs = Expression(10, Sub), rhs = Counter(4) +- expression 10 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 14 +- Code(Counter(0)) at (prev + 23, 1) to (start + 0, 51) +- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) + = ((c1 + c2) + c3) +- Code(Counter(0)) at (prev + 0, 17) to (start + 0, 18) +- MCDCDecision { bitmap_idx: 1, conditions_num: 2 } at (prev + 0, 17) to (start + 0, 55) +- MCDCBranch { true: Counter(1), false: Expression(10, Sub), condition_id: 1, true_next_id: 0, false_next_id: 2 } at (prev + 0, 17) to (start + 0, 18) + true = c1 + false = (c0 - c1) +- MCDCBranch { true: Counter(2), false: Counter(3), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 22) to (start + 0, 55) + true = c2 + false = c3 +- Code(Expression(10, Sub)) at (prev + 0, 25) to (start + 0, 26) + = (c0 - c1) +- MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 0, 25) to (start + 0, 31) +- MCDCBranch { true: Counter(4), false: Expression(9, Sub), condition_id: 1, true_next_id: 2, false_next_id: 0 } at (prev + 0, 25) to (start + 0, 26) + true = c4 + false = ((c0 - c1) - c4) +- Code(Counter(4)) at (prev + 0, 30) to (start + 0, 31) +- MCDCBranch { true: Expression(7, Sub), false: Counter(5), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 30) to (start + 0, 31) + true = (c4 - c5) + false = c5 +- Code(Expression(7, Sub)) at (prev + 0, 34) to (start + 0, 39) + = (c4 - c5) +- Code(Expression(8, Add)) at (prev + 0, 49) to (start + 0, 53) + = (c5 + ((c0 - c1) - c4)) +- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) + = ((c1 + c2) + c3) + +Function name: nested_in_boolean_exprs::func_call_with_unary_not +Raw bytes (64): 0x[01, 01, 04, 07, 0d, 05, 09, 01, 05, 01, 05, 08, 01, 1c, 01, 00, 2f, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 28, 00, 02, 00, 0d, 00, 19, 30, 05, 0e, 01, 00, 02, 00, 0d, 00, 0e, 0e, 00, 12, 00, 19, 30, 09, 0d, 02, 00, 00, 00, 12, 00, 19, 03, 01, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 4 +- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(3) +- expression 1 operands: lhs = Counter(1), rhs = Counter(2) +- expression 2 operands: lhs = Counter(0), rhs = Counter(1) +- expression 3 operands: lhs = Counter(0), rhs = Counter(1) +Number of file 0 mappings: 8 +- Code(Counter(0)) at (prev + 28, 1) to (start + 0, 47) +- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10) + = ((c1 + c2) + c3) +- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14) +- MCDCDecision { bitmap_idx: 0, conditions_num: 2 } at (prev + 0, 13) to (start + 0, 25) +- MCDCBranch { true: Counter(1), false: Expression(3, Sub), condition_id: 1, true_next_id: 0, false_next_id: 2 } at (prev + 0, 13) to (start + 0, 14) + true = c1 + false = (c0 - c1) +- Code(Expression(3, Sub)) at (prev + 0, 18) to (start + 0, 25) + = (c0 - c1) +- MCDCBranch { true: Counter(2), false: Counter(3), condition_id: 2, true_next_id: 0, false_next_id: 0 } at (prev + 0, 18) to (start + 0, 25) + true = c2 + false = c3 +- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2) + = ((c1 + c2) + c3) + diff --git a/tests/coverage/mcdc/nested_in_boolean_exprs.coverage b/tests/coverage/mcdc/nested_in_boolean_exprs.coverage new file mode 100644 index 0000000000000..dda4790eb8a3b --- /dev/null +++ b/tests/coverage/mcdc/nested_in_boolean_exprs.coverage @@ -0,0 +1,195 @@ + LL| |#![feature(coverage_attribute)] + LL| |//@ edition: 2021 + LL| |//@ min-llvm-version: 18 + LL| |//@ compile-flags: -Zcoverage-options=mcdc + LL| |//@ llvm-cov-flags: --show-branches=count --show-mcdc + LL| | + LL| |use core::hint::black_box; + LL| | + LL| 3|fn assign_nested_if(a: bool, b: bool, c: bool) { + LL| 3| let x = a || if b && c { false } else { true }; + ^2 ^1 ^1 ^1 + ------------------ + | Branch (LL:13): [True: 1, False: 2] + | Branch (LL:18): [True: 1, False: 1] + | Branch (LL:21): [True: 1, False: 1] + | Branch (LL:26): [True: 1, False: 0] + ------------------ + |---> MC/DC Decision Region (LL:13) to (LL:51) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:13) + | Condition C2 --> (LL:18) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, F = F } + | 2 { T, - = T } + | 3 { F, T = T } + | + | C1-Pair: covered: (1,2) + | C2-Pair: covered: (1,3) + | MC/DC Coverage for Decision: 100.00% + | + |---> MC/DC Decision Region (LL:21) to (LL:27) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:21) + | Condition C2 --> (LL:26) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, - = F } + | 2 { T, T = T } + | + | C1-Pair: covered: (1,2) + | C2-Pair: not covered + | MC/DC Coverage for Decision: 50.00% + | + ------------------ + LL| 3| black_box(x); + LL| 3|} + LL| | + LL| 6|fn foo(a: bool) -> bool { + LL| 6| black_box(a) + LL| 6|} + LL| | + LL| 3|fn assign_nested_func_call(a: bool, b: bool, c: bool) { + LL| 3| let x = a || foo(b && c); + ^2 ^1 + ------------------ + | Branch (LL:13): [True: 1, False: 2] + | Branch (LL:18): [True: 1, False: 1] + | Branch (LL:22): [True: 1, False: 1] + | Branch (LL:27): [True: 1, False: 0] + ------------------ + |---> MC/DC Decision Region (LL:13) to (LL:29) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:13) + | Condition C2 --> (LL:18) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, F = F } + | 2 { T, - = T } + | 3 { F, T = T } + | + | C1-Pair: covered: (1,2) + | C2-Pair: covered: (1,3) + | MC/DC Coverage for Decision: 100.00% + | + |---> MC/DC Decision Region (LL:22) to (LL:28) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:22) + | Condition C2 --> (LL:27) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, - = F } + | 2 { T, T = T } + | + | C1-Pair: covered: (1,2) + | C2-Pair: not covered + | MC/DC Coverage for Decision: 50.00% + | + ------------------ + LL| 3| black_box(x); + LL| 3|} + LL| | + LL| 3|fn func_call_nested_if(a: bool, b: bool, c: bool) { + LL| 3| let x = foo(a || if b && c { false } else { true }); + ^2 ^1 ^1 ^1 + ------------------ + | Branch (LL:17): [True: 1, False: 2] + | Branch (LL:22): [True: 1, False: 1] + | Branch (LL:25): [True: 1, False: 1] + | Branch (LL:30): [True: 1, False: 0] + ------------------ + |---> MC/DC Decision Region (LL:17) to (LL:55) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:17) + | Condition C2 --> (LL:22) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, F = F } + | 2 { T, - = T } + | 3 { F, T = T } + | + | C1-Pair: covered: (1,2) + | C2-Pair: covered: (1,3) + | MC/DC Coverage for Decision: 100.00% + | + |---> MC/DC Decision Region (LL:25) to (LL:31) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:25) + | Condition C2 --> (LL:30) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, - = F } + | 2 { T, T = T } + | + | C1-Pair: covered: (1,2) + | C2-Pair: not covered + | MC/DC Coverage for Decision: 50.00% + | + ------------------ + LL| 3| black_box(x); + LL| 3|} + LL| | + LL| 2|fn func_call_with_unary_not(a: bool, b: bool) { + LL| 2| let x = a || foo(!b); + ^1 + ------------------ + | Branch (LL:13): [True: 1, False: 1] + | Branch (LL:18): [True: 0, False: 1] + ------------------ + |---> MC/DC Decision Region (LL:13) to (LL:25) + | + | Number of Conditions: 2 + | Condition C1 --> (LL:13) + | Condition C2 --> (LL:18) + | + | Executed MC/DC Test Vectors: + | + | C1, C2 Result + | 1 { F, F = F } + | 2 { T, - = T } + | + | C1-Pair: covered: (1,2) + | C2-Pair: not covered + | MC/DC Coverage for Decision: 50.00% + | + ------------------ + LL| 2| black_box(x); + LL| 2|} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | assign_nested_if(true, false, true); + LL| | assign_nested_if(false, false, true); + LL| | assign_nested_if(false, true, true); + LL| | + LL| | assign_nested_func_call(true, false, true); + LL| | assign_nested_func_call(false, false, true); + LL| | assign_nested_func_call(false, true, true); + LL| | + LL| | func_call_nested_if(true, false, true); + LL| | func_call_nested_if(false, false, true); + LL| | func_call_nested_if(false, true, true); + LL| | + LL| | func_call_with_unary_not(true, false); + LL| | func_call_with_unary_not(false, true); + LL| |} + diff --git a/tests/coverage/mcdc/nested_in_boolean_exprs.rs b/tests/coverage/mcdc/nested_in_boolean_exprs.rs new file mode 100644 index 0000000000000..8420677f99964 --- /dev/null +++ b/tests/coverage/mcdc/nested_in_boolean_exprs.rs @@ -0,0 +1,49 @@ +#![feature(coverage_attribute)] +//@ edition: 2021 +//@ min-llvm-version: 18 +//@ compile-flags: -Zcoverage-options=mcdc +//@ llvm-cov-flags: --show-branches=count --show-mcdc + +use core::hint::black_box; + +fn assign_nested_if(a: bool, b: bool, c: bool) { + let x = a || if b && c { false } else { true }; + black_box(x); +} + +fn foo(a: bool) -> bool { + black_box(a) +} + +fn assign_nested_func_call(a: bool, b: bool, c: bool) { + let x = a || foo(b && c); + black_box(x); +} + +fn func_call_nested_if(a: bool, b: bool, c: bool) { + let x = foo(a || if b && c { false } else { true }); + black_box(x); +} + +fn func_call_with_unary_not(a: bool, b: bool) { + let x = a || foo(!b); + black_box(x); +} + +#[coverage(off)] +fn main() { + assign_nested_if(true, false, true); + assign_nested_if(false, false, true); + assign_nested_if(false, true, true); + + assign_nested_func_call(true, false, true); + assign_nested_func_call(false, false, true); + assign_nested_func_call(false, true, true); + + func_call_nested_if(true, false, true); + func_call_nested_if(false, false, true); + func_call_nested_if(false, true, true); + + func_call_with_unary_not(true, false); + func_call_with_unary_not(false, true); +} From bc11434a0943acc43788216934bb14c724c75358 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 9 Jul 2024 15:35:41 +0800 Subject: [PATCH 3/3] coverage. Group MCDC decisions and conditions until instrument mappings and support multiple branch markers --- compiler/rustc_middle/src/mir/coverage.rs | 21 +-- compiler/rustc_middle/src/mir/pretty.rs | 29 ++-- .../rustc_mir_build/src/build/coverageinfo.rs | 11 +- .../src/build/coverageinfo/mcdc.rs | 27 ++-- .../src/coverage/counters.rs | 13 +- .../src/coverage/mappings.rs | 153 ++++++++++-------- .../rustc_mir_transform/src/coverage/mod.rs | 153 +++++++++++------- 7 files changed, 235 insertions(+), 172 deletions(-) diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 88e5384eef936..c9575d37eac25 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -274,8 +274,10 @@ pub struct CoverageInfoHi { /// data structures without having to scan the entire body first. pub num_block_markers: usize, pub branch_spans: Vec, - pub mcdc_branch_spans: Vec, - pub mcdc_decision_spans: Vec, + /// Branch spans generated by mcdc. Because of some limits mcdc builder give up generating + /// decisions including them so that they are handled as normal branch spans. + pub mcdc_degraded_spans: Vec, + pub mcdc_spans: Vec<(MCDCDecisionSpan, Vec)>, } #[derive(Clone, Debug)] @@ -308,12 +310,12 @@ impl Default for ConditionInfo { #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct MCDCBranchSpan { pub span: Span, - /// If `None`, this actually represents a normal branch span inserted for - /// code that was too complex for MC/DC. - pub condition_info: Option, - pub true_marker: BlockMarkerId, - pub false_marker: BlockMarkerId, - pub decision_depth: u16, + pub condition_info: ConditionInfo, + // For boolean decisions and most pattern matching decisions there is only + // one true marker and one false marker in each branch. But for matching decisions + // with `|` there can be several. + pub true_markers: Vec, + pub false_markers: Vec, } #[derive(Copy, Clone, Debug)] @@ -327,13 +329,12 @@ pub struct DecisionInfo { #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct MCDCDecisionSpan { pub span: Span, - pub num_conditions: usize, pub end_markers: Vec, pub decision_depth: u16, } impl MCDCDecisionSpan { pub fn new(span: Span) -> Self { - Self { span, num_conditions: 0, end_markers: Vec::new(), decision_depth: 0 } + Self { span, end_markers: Vec::new(), decision_depth: 0 } } } diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 223249952dc74..86effaf25d59b 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -490,8 +490,8 @@ fn write_coverage_info_hi( let coverage::CoverageInfoHi { num_block_markers: _, branch_spans, - mcdc_branch_spans, - mcdc_decision_spans, + mcdc_degraded_spans, + mcdc_spans, } = coverage_info_hi; // Only add an extra trailing newline if we printed at least one thing. @@ -505,29 +505,30 @@ fn write_coverage_info_hi( did_print = true; } - for coverage::MCDCBranchSpan { - span, - condition_info, - true_marker, - false_marker, - decision_depth, - } in mcdc_branch_spans - { + for coverage::MCDCBranchSpan { span, true_markers, false_markers, .. } in mcdc_degraded_spans { writeln!( w, - "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?}, depth: {decision_depth:?} }} => {span:?}", - condition_info.map(|info| info.condition_id) + "{INDENT}coverage branch {{ true: {true_markers:?}, false: {false_markers:?} }} => {span:?}", )?; did_print = true; } - for coverage::MCDCDecisionSpan { span, num_conditions, end_markers, decision_depth } in - mcdc_decision_spans + for (coverage::MCDCDecisionSpan { span, end_markers, decision_depth }, conditions) in mcdc_spans { + let num_conditions = conditions.len(); writeln!( w, "{INDENT}coverage mcdc decision {{ num_conditions: {num_conditions:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}" )?; + for coverage::MCDCBranchSpan { span, condition_info, true_markers, false_markers } in + conditions + { + writeln!( + w, + "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_markers:?}, false: {false_markers:?} }} => {span:?}", + condition_info.condition_id + )?; + } did_print = true; } diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 6f70a952bf291..89a4a714bc35f 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -175,21 +175,16 @@ impl CoverageInfoBuilder { let branch_spans = branch_info.map(|branch_info| branch_info.branch_spans).unwrap_or_default(); - let (mut mcdc_branch_spans, mcdc_spans) = + let (mcdc_degraded_spans, mcdc_spans) = mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default(); - let mut mcdc_decision_spans = Vec::with_capacity(mcdc_spans.len()); - for (decision, conditions) in mcdc_spans { - mcdc_branch_spans.extend(conditions); - mcdc_decision_spans.push(decision); - } // 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, + mcdc_degraded_spans, + mcdc_spans, }) } } diff --git a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs index a9b761d4c6904..a87eb55217ab7 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs @@ -34,6 +34,7 @@ struct BooleanDecisionCtx { /// To construct condition evaluation tree. decision_stack: VecDeque, conditions: Vec, + condition_id_counter: usize, } impl BooleanDecisionCtx { @@ -43,9 +44,15 @@ impl BooleanDecisionCtx { decision_info: MCDCDecisionSpan::new(Span::default()), decision_stack: VecDeque::new(), conditions: vec![], + condition_id_counter: 0, } } + fn next_condition_id(&mut self) -> ConditionId { + self.condition_id_counter += 1; + ConditionId::from_usize(self.condition_id_counter) + } + // At first we assign ConditionIds for each sub expression. // If the sub expression is composite, re-assign its ConditionId to its LHS and generate a new ConditionId for its RHS. // @@ -89,14 +96,12 @@ impl BooleanDecisionCtx { fn record_conditions(&mut self, op: LogicalOp) { let parent_condition = self.decision_stack.pop_back().unwrap_or_default(); let lhs_id = if parent_condition.condition_id == ConditionId::NONE { - self.decision_info.num_conditions += 1; - ConditionId::from(self.decision_info.num_conditions) + ConditionId::from(self.next_condition_id()) } else { parent_condition.condition_id }; - self.decision_info.num_conditions += 1; - let rhs_condition_id = ConditionId::from(self.decision_info.num_conditions); + let rhs_condition_id = self.next_condition_id(); let (lhs, rhs) = match op { LogicalOp::And => { @@ -147,13 +152,10 @@ impl BooleanDecisionCtx { self.conditions.push(MCDCBranchSpan { span, - condition_info: Some(condition_info), - true_marker, - false_marker, - decision_depth: 0, + condition_info, + true_markers: vec![true_marker], + false_markers: vec![false_marker], }); - // In case this decision had only one condition - self.decision_info.num_conditions = self.decision_info.num_conditions.max(1); } fn is_finished(&self) -> bool { @@ -247,7 +249,6 @@ struct MCDCTargetInfo { impl MCDCTargetInfo { fn set_depth(&mut self, depth: u16) { self.decision.decision_depth = depth; - self.conditions.iter_mut().for_each(|branch| branch.decision_depth = depth); } } @@ -341,7 +342,9 @@ impl MCDCInfoBuilder { } fn append_normal_branches(&mut self, mut branches: Vec) { - branches.iter_mut().for_each(|branch| branch.condition_info = None); + branches + .iter_mut() + .for_each(|branch| branch.condition_info.condition_id = ConditionId::NONE); self.normal_branch_spans.extend(branches); } diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index a8b0f4a8d6df4..f068494e23405 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -101,7 +101,12 @@ impl CoverageCounters { BcbCounter::Counter { id } } - fn make_expression(&mut self, lhs: BcbCounter, op: Op, rhs: BcbCounter) -> BcbCounter { + pub(super) fn make_expression( + &mut self, + lhs: BcbCounter, + op: Op, + rhs: BcbCounter, + ) -> BcbCounter { let new_expr = BcbExpression { lhs, op, rhs }; *self .expressions_memo @@ -159,7 +164,11 @@ impl CoverageCounters { /// Variant of `make_expression` that makes `lhs` optional and assumes [`Op::Add`]. /// /// This is useful when using [`Iterator::fold`] to build an arbitrary-length sum. - fn make_sum_expression(&mut self, lhs: Option, rhs: BcbCounter) -> BcbCounter { + pub(super) fn make_sum_expression( + &mut self, + lhs: Option, + rhs: BcbCounter, + ) -> BcbCounter { let Some(lhs) = lhs else { return rhs }; self.make_expression(lhs, Op::Add, rhs) } diff --git a/compiler/rustc_mir_transform/src/coverage/mappings.rs b/compiler/rustc_mir_transform/src/coverage/mappings.rs index 2ac08ea85d253..43e99058df9fd 100644 --- a/compiler/rustc_mir_transform/src/coverage/mappings.rs +++ b/compiler/rustc_mir_transform/src/coverage/mappings.rs @@ -36,12 +36,9 @@ pub(super) struct BranchPair { #[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, - pub(super) decision_depth: u16, + pub(super) true_bcbs: Vec, + pub(super) false_bcbs: Vec, + pub(super) condition_info: ConditionInfo, } /// Associates an MC/DC decision with its join BCBs. @@ -50,7 +47,6 @@ pub(super) struct MCDCDecision { pub(super) span: Span, pub(super) end_bcbs: BTreeSet, pub(super) bitmap_idx: u32, - pub(super) num_conditions: u16, pub(super) decision_depth: u16, } @@ -63,8 +59,8 @@ pub(super) struct ExtractedMappings { pub(super) code_mappings: Vec, pub(super) branch_pairs: Vec, pub(super) mcdc_bitmap_bytes: u32, - pub(super) mcdc_branches: Vec, - pub(super) mcdc_decisions: Vec, + pub(super) mcdc_degraded_branches: Vec, + pub(super) mcdc_mappings: Vec<(MCDCDecision, Vec)>, } /// Extracts coverage-relevant spans from MIR, and associates them with @@ -78,8 +74,8 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( let mut code_mappings = vec![]; let mut branch_pairs = vec![]; let mut mcdc_bitmap_bytes = 0; - let mut mcdc_branches = vec![]; - let mut mcdc_decisions = vec![]; + let mut mcdc_degraded_branches = vec![]; + let mut mcdc_mappings = vec![]; if hir_info.is_async_fn || tcx.sess.coverage_no_mir_spans() { // An async function desugars into a function that returns a future, @@ -105,8 +101,8 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( hir_info.body_span, basic_coverage_blocks, &mut mcdc_bitmap_bytes, - &mut mcdc_branches, - &mut mcdc_decisions, + &mut mcdc_degraded_branches, + &mut mcdc_mappings, ); ExtractedMappings { @@ -114,8 +110,8 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>( code_mappings, branch_pairs, mcdc_bitmap_bytes, - mcdc_branches, - mcdc_decisions, + mcdc_degraded_branches, + mcdc_mappings, } } @@ -127,8 +123,8 @@ impl ExtractedMappings { code_mappings, branch_pairs, mcdc_bitmap_bytes: _, - mcdc_branches, - mcdc_decisions, + mcdc_degraded_branches, + mcdc_mappings, } = self; // Identify which BCBs have one or more mappings. @@ -144,16 +140,18 @@ impl ExtractedMappings { insert(true_bcb); insert(false_bcb); } - for &MCDCBranch { true_bcb, false_bcb, .. } in mcdc_branches { - insert(true_bcb); - insert(false_bcb); + for MCDCBranch { true_bcbs, false_bcbs, .. } in mcdc_degraded_branches + .iter() + .chain(mcdc_mappings.iter().map(|(_, branches)| branches.into_iter()).flatten()) + { + true_bcbs.into_iter().chain(false_bcbs.into_iter()).copied().for_each(&mut insert); } // MC/DC decisions refer to BCBs, but don't require those BCBs to have counters. if bcbs_with_counter_mappings.is_empty() { debug_assert!( - mcdc_decisions.is_empty(), - "A function with no counter mappings shouldn't have any decisions: {mcdc_decisions:?}", + mcdc_mappings.is_empty(), + "A function with no counter mappings shouldn't have any decisions: {mcdc_mappings:?}", ); } @@ -233,8 +231,8 @@ pub(super) fn extract_mcdc_mappings( body_span: Span, basic_coverage_blocks: &CoverageGraph, mcdc_bitmap_bytes: &mut u32, - mcdc_branches: &mut impl Extend, - mcdc_decisions: &mut impl Extend, + mcdc_degraded_branches: &mut impl Extend, + mcdc_mappings: &mut impl Extend<(MCDCDecision, Vec)>, ) { let Some(coverage_info_hi) = mir_body.coverage_info_hi.as_deref() else { return }; @@ -243,57 +241,70 @@ pub(super) fn extract_mcdc_mappings( let bcb_from_marker = |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); - let check_branch_bcb = - |raw_span: Span, true_marker: BlockMarkerId, false_marker: BlockMarkerId| { - // 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(raw_span, body_span)?; - - let true_bcb = bcb_from_marker(true_marker)?; - let false_bcb = bcb_from_marker(false_marker)?; - Some((span, true_bcb, false_bcb)) - }; + let check_branch_bcb = |raw_span: Span, + true_markers: &[BlockMarkerId], + false_markers: &[BlockMarkerId]| { + // 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(raw_span, body_span)?; + + let true_bcbs = + true_markers.into_iter().copied().map(&bcb_from_marker).collect::>>()?; + let false_bcbs = + false_markers.into_iter().copied().map(&bcb_from_marker).collect::>>()?; + + Some((span, true_bcbs, false_bcbs)) + }; + + let extract_condition_mapping = |&mir::coverage::MCDCBranchSpan { + span: raw_span, + condition_info, + ref true_markers, + ref false_markers, + }| { + let (span, true_bcbs, false_bcbs) = + check_branch_bcb(raw_span, true_markers, false_markers)?; + Some(MCDCBranch { span, true_bcbs, false_bcbs, condition_info }) + }; + + mcdc_degraded_branches + .extend(coverage_info_hi.mcdc_degraded_spans.iter().filter_map(extract_condition_mapping)); + + mcdc_mappings.extend(coverage_info_hi.mcdc_spans.iter().filter_map(|(decision, branches)| { + if branches.len() == 0 { + return None; + } + let span = unexpand_into_body_span(decision.span, body_span)?; - mcdc_branches.extend(coverage_info_hi.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 }) - }, - )); - - 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)?; - - let end_bcbs = decision - .end_markers - .iter() - .map(|&marker| bcb_from_marker(marker)) - .collect::>()?; + let end_bcbs = decision + .end_markers + .iter() + .map(|&marker| bcb_from_marker(marker)) + .collect::>()?; + let branch_mappings: Vec<_> = + branches.into_iter().filter_map(extract_condition_mapping).collect(); + if branch_mappings.len() == branches.len() { // Each decision containing N conditions needs 2^N bits of space in // the bitmap, rounded up to a whole number of bytes. // The decision's "bitmap index" points to its first byte in the bitmap. let bitmap_idx = *mcdc_bitmap_bytes; - *mcdc_bitmap_bytes += (1_u32 << decision.num_conditions).div_ceil(8); - - Some(MCDCDecision { - span, - end_bcbs, - bitmap_idx, - num_conditions: decision.num_conditions as u16, - decision_depth: decision.decision_depth, - }) - }, - )); + *mcdc_bitmap_bytes += (1_u32 << branch_mappings.len()).div_ceil(8); + + Some(( + MCDCDecision { + span, + end_bcbs, + bitmap_idx, // Assigned in `coverage::create_mappings` + decision_depth: decision.decision_depth, + }, + branch_mappings, + )) + } else { + None + } + })); } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 3772a8f511814..de15e90f38d6b 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -25,7 +25,7 @@ use rustc_span::source_map::SourceMap; use rustc_span::{BytePos, Pos, RelativeBytePos, Span, Symbol}; use crate::coverage::counters::{CounterIncrementSite, CoverageCounters}; -use crate::coverage::graph::CoverageGraph; +use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph}; use crate::coverage::mappings::ExtractedMappings; use crate::MirPass; @@ -95,10 +95,10 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: } let bcb_has_counter_mappings = |bcb| bcbs_with_counter_mappings.contains(bcb); - let coverage_counters = + let mut coverage_counters = CoverageCounters::make_bcb_counters(&basic_coverage_blocks, bcb_has_counter_mappings); - let mappings = create_mappings(tcx, &hir_info, &extracted_mappings, &coverage_counters); + let mappings = create_mappings(tcx, &hir_info, &extracted_mappings, &mut coverage_counters); if mappings.is_empty() { // No spans could be converted into valid mappings, so skip this function. debug!("no spans could be converted into valid mappings; skipping"); @@ -115,9 +115,9 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: inject_mcdc_statements(mir_body, &basic_coverage_blocks, &extracted_mappings); let mcdc_num_condition_bitmaps = extracted_mappings - .mcdc_decisions + .mcdc_mappings .iter() - .map(|&mappings::MCDCDecision { decision_depth, .. }| decision_depth) + .map(|&(mappings::MCDCDecision { decision_depth, .. }, _)| decision_depth) .max() .map_or(0, |max| usize::from(max) + 1); @@ -140,7 +140,7 @@ fn create_mappings<'tcx>( tcx: TyCtxt<'tcx>, hir_info: &ExtractedHirInfo, extracted_mappings: &ExtractedMappings, - coverage_counters: &CoverageCounters, + coverage_counters: &mut CoverageCounters, ) -> Vec { let source_map = tcx.sess.source_map(); let body_span = hir_info.body_span; @@ -166,8 +166,8 @@ fn create_mappings<'tcx>( code_mappings, branch_pairs, mcdc_bitmap_bytes: _, - mcdc_branches, - mcdc_decisions, + mcdc_degraded_branches, + mcdc_mappings, } = extracted_mappings; let mut mappings = Vec::new(); @@ -189,27 +189,76 @@ fn create_mappings<'tcx>( Some(Mapping { kind, code_region }) }, )); + let mut term_for_sum_of_bcbs = |bcbs: &[BasicCoverageBlock]| { + let counters = bcbs + .into_iter() + .copied() + .map(|bcb| { + coverage_counters.bcb_counter(bcb).expect("all BCBs with spans were given counters") + }) + .collect::>(); + let counter = counters + .into_iter() + .fold(None, |acc, val| Some(coverage_counters.make_sum_expression(acc, val))) + .expect("counters must be non-empty"); + counter.as_term() + }; - mappings.extend(mcdc_branches.iter().filter_map( - |&mappings::MCDCBranch { span, true_bcb, false_bcb, condition_info, decision_depth: _ }| { + // MCDC branch mappings are appended with their decisions in case decisions were ignored. + mappings.extend(mcdc_degraded_branches.iter().filter_map( + |&mappings::MCDCBranch { span, ref true_bcbs, ref false_bcbs, condition_info: _ }| { 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 }) + let true_term = term_for_sum_of_bcbs(true_bcbs); + let false_term = term_for_sum_of_bcbs(false_bcbs); + Some(Mapping { kind: MappingKind::Branch { true_term, false_term }, code_region }) }, )); - mappings.extend(mcdc_decisions.iter().filter_map( - |&mappings::MCDCDecision { span, bitmap_idx, num_conditions, .. }| { - let code_region = region_for_span(span)?; - let kind = MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, num_conditions }); - Some(Mapping { kind, code_region }) - }, - )); + for (decision, branches) in mcdc_mappings { + let num_conditions = branches.len() as u16; + let conditions = branches + .into_iter() + .filter_map( + |&mappings::MCDCBranch { span, ref true_bcbs, ref false_bcbs, condition_info }| { + let code_region = region_for_span(span)?; + let true_term = term_for_sum_of_bcbs(true_bcbs); + let false_term = term_for_sum_of_bcbs(false_bcbs); + Some(Mapping { + kind: MappingKind::MCDCBranch { + true_term, + false_term, + mcdc_params: condition_info, + }, + code_region, + }) + }, + ) + .collect::>(); + + if conditions.len() == num_conditions as usize + && let Some(code_region) = region_for_span(decision.span) + { + let kind = MappingKind::MCDCDecision(DecisionInfo { + bitmap_idx: decision.bitmap_idx, + num_conditions, + }); + mappings.extend( + std::iter::once(Mapping { kind, code_region }).chain(conditions.into_iter()), + ); + } else { + mappings.extend(conditions.into_iter().map(|mapping| { + let MappingKind::MCDCBranch { true_term, false_term, mcdc_params: _ } = + mapping.kind + else { + unreachable!("all mappings here are MCDCBranch as shown above"); + }; + Mapping { + kind: MappingKind::Branch { true_term, false_term }, + code_region: mapping.code_region, + } + })) + } + } mappings } @@ -278,43 +327,37 @@ fn inject_mcdc_statements<'tcx>( basic_coverage_blocks: &CoverageGraph, extracted_mappings: &ExtractedMappings, ) { - // Inject test vector update first because `inject_statement` always insert new statement at head. - for &mappings::MCDCDecision { - span: _, - ref end_bcbs, - bitmap_idx, - num_conditions: _, - decision_depth, - } in &extracted_mappings.mcdc_decisions - { - for end in end_bcbs { - let end_bb = basic_coverage_blocks[*end].leader_bb(); + for (decision, conditions) in &extracted_mappings.mcdc_mappings { + // Inject test vector update first because `inject_statement` always insert new statement at head. + for &end in &decision.end_bcbs { + let end_bb = basic_coverage_blocks[end].leader_bb(); inject_statement( mir_body, - CoverageKind::TestVectorBitmapUpdate { bitmap_idx, decision_depth }, + CoverageKind::TestVectorBitmapUpdate { + bitmap_idx: decision.bitmap_idx, + decision_depth: decision.decision_depth, + }, end_bb, ); } - } - - for &mappings::MCDCBranch { span: _, true_bcb, false_bcb, condition_info, decision_depth } in - &extracted_mappings.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, value: true, decision_depth }, - true_bb, - ); - let false_bb = basic_coverage_blocks[false_bcb].leader_bb(); - inject_statement( - mir_body, - CoverageKind::CondBitmapUpdate { id, value: false, decision_depth }, - false_bb, - ); + for &mappings::MCDCBranch { span: _, ref true_bcbs, false_bcbs: _, condition_info } in + conditions + { + let id = condition_info.condition_id; + for &bcb in true_bcbs { + let bb = basic_coverage_blocks[bcb].leader_bb(); + inject_statement( + mir_body, + CoverageKind::CondBitmapUpdate { + id, + value: true, + decision_depth: decision.decision_depth, + }, + bb, + ); + } + } } }