From a76e250abdc5a3e1929334a33655f0393f31183d Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 30 Apr 2024 10:05:50 +0800 Subject: [PATCH 1/3] coverage. Add BlockMarkerGen to avoid ownership gymnastics --- .../rustc_mir_build/src/build/coverageinfo.rs | 64 ++++++++++--------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index a1c20fcfd8365..e3929f1053423 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -20,7 +20,7 @@ pub(crate) struct BranchInfoBuilder { /// Maps condition expressions to their enclosing `!`, for better instrumentation. nots: FxHashMap, - num_block_markers: usize, + markers: BlockMarkerGen, branch_spans: Vec, mcdc_branch_spans: Vec, @@ -38,6 +38,35 @@ struct NotInfo { is_flipped: bool, } +#[derive(Default)] +struct BlockMarkerGen { + num_block_markers: usize, +} + +impl BlockMarkerGen { + fn next_block_marker_id(&mut self) -> BlockMarkerId { + let id = BlockMarkerId::from_usize(self.num_block_markers); + self.num_block_markers += 1; + id + } + + fn inject_block_marker( + &mut self, + cfg: &mut CFG<'_>, + source_info: SourceInfo, + block: BasicBlock, + ) -> BlockMarkerId { + let id = self.next_block_marker_id(); + let marker_statement = mir::Statement { + source_info, + kind: mir::StatementKind::Coverage(CoverageKind::BlockMarker { id }), + }; + cfg.push(block, marker_statement); + + id + } +} + impl BranchInfoBuilder { /// Creates a new branch info builder, but only if branch coverage instrumentation /// is enabled and `def_id` represents a function that is eligible for coverage. @@ -45,7 +74,7 @@ impl BranchInfoBuilder { if tcx.sess.instrument_coverage_branch() && tcx.is_eligible_for_coverage(def_id) { Some(Self { nots: FxHashMap::default(), - num_block_markers: 0, + markers: BlockMarkerGen::default(), branch_spans: vec![], mcdc_branch_spans: vec![], mcdc_decision_spans: vec![], @@ -145,39 +174,16 @@ impl BranchInfoBuilder { true_block: BasicBlock, false_block: BasicBlock, ) { - let true_marker = self.inject_block_marker(cfg, source_info, true_block); - let false_marker = self.inject_block_marker(cfg, source_info, false_block); + let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block); + let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block); self.branch_spans.push(BranchSpan { span: source_info.span, true_marker, false_marker }); } - fn next_block_marker_id(&mut self) -> BlockMarkerId { - let id = BlockMarkerId::from_usize(self.num_block_markers); - self.num_block_markers += 1; - id - } - - fn inject_block_marker( - &mut self, - cfg: &mut CFG<'_>, - source_info: SourceInfo, - block: BasicBlock, - ) -> BlockMarkerId { - let id = self.next_block_marker_id(); - - let marker_statement = mir::Statement { - source_info, - kind: mir::StatementKind::Coverage(CoverageKind::BlockMarker { id }), - }; - cfg.push(block, marker_statement); - - id - } - pub(crate) fn into_done(self) -> Option> { let Self { nots: _, - num_block_markers, + markers: BlockMarkerGen { num_block_markers }, branch_spans, mcdc_branch_spans, mcdc_decision_spans, @@ -386,7 +392,7 @@ impl Builder<'_, '_> { // Separate path for handling branches when MC/DC is enabled. if branch_info.mcdc_state.is_some() { let mut inject_block_marker = - |block| branch_info.inject_block_marker(&mut self.cfg, source_info, block); + |block| branch_info.markers.inject_block_marker(&mut self.cfg, source_info, block); let true_marker = inject_block_marker(then_block); let false_marker = inject_block_marker(else_block); let (decision_depth, condition_info) = branch_info From e198c51f16e5ed8480bee455c3a85071cd64d947 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 30 Apr 2024 10:21:46 +0800 Subject: [PATCH 2/3] coverage. Add MCDCInfoBuilder to isolate all mcdc stuff from BranchInfoBuilder --- .../rustc_mir_build/src/build/coverageinfo.rs | 196 ++++++++++-------- 1 file changed, 110 insertions(+), 86 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index e3929f1053423..f0c0abfccec63 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -23,9 +23,7 @@ pub(crate) struct BranchInfoBuilder { markers: BlockMarkerGen, branch_spans: Vec, - mcdc_branch_spans: Vec, - mcdc_decision_spans: Vec, - mcdc_state: Option, + mcdc_info: Option, } #[derive(Clone, Copy)] @@ -76,9 +74,7 @@ impl BranchInfoBuilder { nots: FxHashMap::default(), markers: BlockMarkerGen::default(), branch_spans: vec![], - mcdc_branch_spans: vec![], - mcdc_decision_spans: vec![], - mcdc_state: MCDCState::new_if_enabled(tcx), + mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new), }) } else { None @@ -125,48 +121,6 @@ impl BranchInfoBuilder { } } - fn fetch_mcdc_condition_info( - &mut self, - tcx: TyCtxt<'_>, - true_marker: BlockMarkerId, - false_marker: BlockMarkerId, - ) -> Option<(u16, ConditionInfo)> { - let mcdc_state = self.mcdc_state.as_mut()?; - let decision_depth = mcdc_state.decision_depth(); - let (mut condition_info, decision_result) = - mcdc_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.conditions_num { - 0 => { - unreachable!("Decision with no condition is not expected"); - } - 1..=MAX_CONDITIONS_NUM_IN_DECISION => { - self.mcdc_decision_spans.push(decision); - } - _ => { - // 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; - 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; - - tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { - span: decision.span, - conditions_num: decision.conditions_num, - max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, - }); - } - } - } - condition_info.map(|cond_info| (decision_depth, cond_info)) - } - fn add_two_way_branch<'tcx>( &mut self, cfg: &mut CFG<'tcx>, @@ -185,9 +139,7 @@ impl BranchInfoBuilder { nots: _, markers: BlockMarkerGen { num_block_markers }, branch_spans, - mcdc_branch_spans, - mcdc_decision_spans, - mcdc_state: _, + mcdc_info, } = self; if num_block_markers == 0 { @@ -195,6 +147,9 @@ impl BranchInfoBuilder { return None; } + let (mcdc_decision_spans, mcdc_branch_spans) = + mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default(); + Some(Box::new(mir::coverage::BranchInfo { num_block_markers, branch_spans, @@ -221,20 +176,23 @@ struct MCDCState { } impl MCDCState { - fn new_if_enabled(tcx: TyCtxt<'_>) -> Option { - tcx.sess - .instrument_coverage_mcdc() - .then(|| Self { decision_ctx_stack: vec![MCDCDecisionCtx::default()] }) + 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 { - u16::try_from( - self.decision_ctx_stack.len().checked_sub(1).expect("Unexpected empty decision stack"), - ) - .expect("decision depth did not fit in u16, this is likely to be an instrumentation error") + 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"), + } } // At first we assign ConditionIds for each sub expression. @@ -279,8 +237,9 @@ impl MCDCState { // - 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 decision_ctx = - self.decision_ctx_stack.last_mut().expect("Unexpected empty decision_ctx_stack"); + 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); @@ -343,8 +302,9 @@ impl MCDCState { true_marker: BlockMarkerId, false_marker: BlockMarkerId, ) -> (Option, Option) { - let decision_ctx = - self.decision_ctx_stack.last_mut().expect("Unexpected empty decision_ctx_stack"); + 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); }; @@ -366,6 +326,74 @@ impl MCDCState { } } +struct MCDCInfoBuilder { + branch_spans: Vec, + decision_spans: Vec, + state: MCDCState, +} + +impl MCDCInfoBuilder { + fn new() -> Self { + Self { branch_spans: vec![], decision_spans: vec![], state: MCDCState::new() } + } + + fn visit_evaluated_condition( + &mut self, + tcx: TyCtxt<'_>, + source_info: SourceInfo, + true_block: BasicBlock, + false_block: BasicBlock, + mut inject_block_marker: impl FnMut(SourceInfo, 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.conditions_num { + 0 => { + unreachable!("Decision with no condition is not expected"); + } + 1..=MAX_CONDITIONS_NUM_IN_DECISION => { + self.decision_spans.push(decision); + } + _ => { + // Do not generate mcdc mappings and statements for decisions with too many conditions. + let rebase_idx = self.branch_spans.len() - decision.conditions_num + 1; + for branch in &mut self.branch_spans[rebase_idx..] { + branch.condition_info = None; + } + + // ConditionInfo of this branch shall also be reset. + condition_info = None; + + tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { + span: decision.span, + conditions_num: decision.conditions_num, + max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, + }); + } + } + } + self.branch_spans.push(MCDCBranchSpan { + span: source_info.span, + condition_info, + true_marker, + false_marker, + decision_depth, + }); + } + + fn into_done(self) -> (Vec, Vec) { + (self.decision_spans, self.branch_spans) + } +} + impl Builder<'_, '_> { /// If branch coverage is enabled, inject marker statements into `then_block` /// and `else_block`, and record their IDs in the table of branch spans. @@ -390,23 +418,17 @@ impl Builder<'_, '_> { let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope }; // Separate path for handling branches when MC/DC is enabled. - if branch_info.mcdc_state.is_some() { - let mut inject_block_marker = - |block| branch_info.markers.inject_block_marker(&mut self.cfg, source_info, block); - let true_marker = inject_block_marker(then_block); - let false_marker = inject_block_marker(else_block); - let (decision_depth, condition_info) = branch_info - .fetch_mcdc_condition_info(self.tcx, true_marker, false_marker) - .map_or((0, None), |(decision_depth, condition_info)| { - (decision_depth, Some(condition_info)) - }); - branch_info.mcdc_branch_spans.push(MCDCBranchSpan { - span: source_info.span, - condition_info, - true_marker, - false_marker, - decision_depth, - }); + if let Some(mcdc_info) = branch_info.mcdc_info.as_mut() { + let inject_block_marker = |source_info, block| { + branch_info.markers.inject_block_marker(&mut self.cfg, source_info, block) + }; + mcdc_info.visit_evaluated_condition( + self.tcx, + source_info, + then_block, + else_block, + inject_block_marker, + ); return; } @@ -415,25 +437,27 @@ impl Builder<'_, '_> { pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) { if let Some(branch_info) = self.coverage_branch_info.as_mut() - && let Some(mcdc_state) = branch_info.mcdc_state.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() { - mcdc_state.record_conditions(logical_op, span); + mcdc_info.state.record_conditions(logical_op, span); } } pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) { if let Some(branch_info) = self.coverage_branch_info.as_mut() - && let Some(mcdc_state) = branch_info.mcdc_state.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() { - mcdc_state.decision_ctx_stack.push(MCDCDecisionCtx::default()); + mcdc_info.state.decision_ctx_stack.push(MCDCDecisionCtx::default()); }; } pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) { if let Some(branch_info) = self.coverage_branch_info.as_mut() - && let Some(mcdc_state) = branch_info.mcdc_state.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() { - mcdc_state.decision_ctx_stack.pop().expect("Unexpected empty decision stack"); + if mcdc_info.state.decision_ctx_stack.pop().is_none() { + bug!("Unexpected empty decision stack"); + } }; } } From 6c8b492f02733120a05b257033db849a2a82c5f4 Mon Sep 17 00:00:00 2001 From: zhuyunxing Date: Tue, 30 Apr 2024 10:25:54 +0800 Subject: [PATCH 3/3] coverage. Split mcdc builder to a sub module of coverageinfo --- .../rustc_mir_build/src/build/coverageinfo.rs | 273 +---------------- .../src/build/coverageinfo/mcdc.rs | 275 ++++++++++++++++++ 2 files changed, 279 insertions(+), 269 deletions(-) create mode 100644 compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index f0c0abfccec63..e2a5f97a84744 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -1,20 +1,16 @@ +mod mcdc; use std::assert_matches::assert_matches; use std::collections::hash_map::Entry; -use std::collections::VecDeque; use rustc_data_structures::fx::FxHashMap; -use rustc_middle::mir::coverage::{ - BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, MCDCBranchSpan, - MCDCDecisionSpan, -}; +use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind}; use rustc_middle::mir::{self, BasicBlock, SourceInfo, UnOp}; -use rustc_middle::thir::{ExprId, ExprKind, LogicalOp, Thir}; +use rustc_middle::thir::{ExprId, ExprKind, Thir}; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::LocalDefId; -use rustc_span::Span; +use crate::build::coverageinfo::mcdc::MCDCInfoBuilder; use crate::build::{Builder, CFG}; -use crate::errors::MCDCExceedsConditionNumLimit; pub(crate) struct BranchInfoBuilder { /// Maps condition expressions to their enclosing `!`, for better instrumentation. @@ -159,241 +155,6 @@ impl BranchInfoBuilder { } } -/// 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_NUM_IN_DECISION: usize = 6; - -#[derive(Default)] -struct MCDCDecisionCtx { - /// To construct condition evaluation tree. - decision_stack: VecDeque, - processing_decision: Option, -} - -struct MCDCState { - decision_ctx_stack: 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"), - } - } - - // 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. - // - // Example: "x = (A && B) || (C && D) || (D && F)" - // - // Visit Depth1: - // (A && B) || (C && D) || (D && F) - // ^-------LHS--------^ ^-RHS--^ - // ID=1 ID=2 - // - // Visit LHS-Depth2: - // (A && B) || (C && D) - // ^-LHS--^ ^-RHS--^ - // ID=1 ID=3 - // - // Visit LHS-Depth3: - // (A && B) - // LHS RHS - // ID=1 ID=4 - // - // Visit RHS-Depth3: - // (C && D) - // LHS RHS - // ID=3 ID=5 - // - // Visit RHS-Depth2: (D && F) - // LHS RHS - // ID=2 ID=6 - // - // Visit Depth1: - // (A && B) || (C && D) || (D && F) - // ID=1 ID=4 ID=3 ID=5 ID=2 ID=6 - // - // A node ID of '0' always means MC/DC isn't being tracked. - // - // If a "next" node ID is '0', it means it's the end of the test vector. - // - // 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, - conditions_num: 0, - end_markers: vec![], - decision_depth, - }), - }; - - let parent_condition = decision_ctx.decision_stack.pop_back().unwrap_or_default(); - let lhs_id = if parent_condition.condition_id == ConditionId::NONE { - decision.conditions_num += 1; - ConditionId::from(decision.conditions_num) - } else { - parent_condition.condition_id - }; - - decision.conditions_num += 1; - let rhs_condition_id = ConditionId::from(decision.conditions_num); - - let (lhs, rhs) = match op { - LogicalOp::And => { - let lhs = ConditionInfo { - condition_id: lhs_id, - true_next_id: rhs_condition_id, - false_next_id: parent_condition.false_next_id, - }; - let rhs = ConditionInfo { - condition_id: rhs_condition_id, - true_next_id: parent_condition.true_next_id, - false_next_id: parent_condition.false_next_id, - }; - (lhs, rhs) - } - LogicalOp::Or => { - let lhs = ConditionInfo { - condition_id: lhs_id, - true_next_id: parent_condition.true_next_id, - false_next_id: rhs_condition_id, - }; - let rhs = ConditionInfo { - condition_id: rhs_condition_id, - true_next_id: parent_condition.true_next_id, - false_next_id: parent_condition.false_next_id, - }; - (lhs, rhs) - } - }; - // 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); - } - - fn take_condition( - &mut self, - 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"); - }; - if condition_info.true_next_id == ConditionId::NONE { - decision.end_markers.push(true_marker); - } - if condition_info.false_next_id == ConditionId::NONE { - decision.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) - } - } -} - -struct MCDCInfoBuilder { - branch_spans: Vec, - decision_spans: Vec, - state: MCDCState, -} - -impl MCDCInfoBuilder { - fn new() -> Self { - Self { branch_spans: vec![], decision_spans: vec![], state: MCDCState::new() } - } - - fn visit_evaluated_condition( - &mut self, - tcx: TyCtxt<'_>, - source_info: SourceInfo, - true_block: BasicBlock, - false_block: BasicBlock, - mut inject_block_marker: impl FnMut(SourceInfo, 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.conditions_num { - 0 => { - unreachable!("Decision with no condition is not expected"); - } - 1..=MAX_CONDITIONS_NUM_IN_DECISION => { - self.decision_spans.push(decision); - } - _ => { - // Do not generate mcdc mappings and statements for decisions with too many conditions. - let rebase_idx = self.branch_spans.len() - decision.conditions_num + 1; - for branch in &mut self.branch_spans[rebase_idx..] { - branch.condition_info = None; - } - - // ConditionInfo of this branch shall also be reset. - condition_info = None; - - tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { - span: decision.span, - conditions_num: decision.conditions_num, - max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, - }); - } - } - } - self.branch_spans.push(MCDCBranchSpan { - span: source_info.span, - condition_info, - true_marker, - false_marker, - decision_depth, - }); - } - - fn into_done(self) -> (Vec, Vec) { - (self.decision_spans, self.branch_spans) - } -} - impl Builder<'_, '_> { /// If branch coverage is enabled, inject marker statements into `then_block` /// and `else_block`, and record their IDs in the table of branch spans. @@ -434,30 +195,4 @@ impl Builder<'_, '_> { branch_info.add_two_way_branch(&mut self.cfg, source_info, then_block, else_block); } - - pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) { - if let Some(branch_info) = self.coverage_branch_info.as_mut() - && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() - { - mcdc_info.state.record_conditions(logical_op, span); - } - } - - pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) { - if let Some(branch_info) = self.coverage_branch_info.as_mut() - && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() - { - mcdc_info.state.decision_ctx_stack.push(MCDCDecisionCtx::default()); - }; - } - - pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) { - if let Some(branch_info) = self.coverage_branch_info.as_mut() - && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() - { - if mcdc_info.state.decision_ctx_stack.pop().is_none() { - bug!("Unexpected empty decision stack"); - } - }; - } } diff --git a/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs new file mode 100644 index 0000000000000..566dba460d43b --- /dev/null +++ b/compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs @@ -0,0 +1,275 @@ +use std::collections::VecDeque; + +use rustc_middle::mir::coverage::{ + BlockMarkerId, ConditionId, ConditionInfo, MCDCBranchSpan, MCDCDecisionSpan, +}; +use rustc_middle::mir::{BasicBlock, SourceInfo}; +use rustc_middle::thir::LogicalOp; +use rustc_middle::ty::TyCtxt; +use rustc_span::Span; + +use crate::build::Builder; +use crate::errors::MCDCExceedsConditionNumLimit; + +/// 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_NUM_IN_DECISION: usize = 6; + +#[derive(Default)] +struct MCDCDecisionCtx { + /// To construct condition evaluation tree. + decision_stack: VecDeque, + processing_decision: Option, +} + +struct MCDCState { + decision_ctx_stack: 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"), + } + } + + // 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. + // + // Example: "x = (A && B) || (C && D) || (D && F)" + // + // Visit Depth1: + // (A && B) || (C && D) || (D && F) + // ^-------LHS--------^ ^-RHS--^ + // ID=1 ID=2 + // + // Visit LHS-Depth2: + // (A && B) || (C && D) + // ^-LHS--^ ^-RHS--^ + // ID=1 ID=3 + // + // Visit LHS-Depth3: + // (A && B) + // LHS RHS + // ID=1 ID=4 + // + // Visit RHS-Depth3: + // (C && D) + // LHS RHS + // ID=3 ID=5 + // + // Visit RHS-Depth2: (D && F) + // LHS RHS + // ID=2 ID=6 + // + // Visit Depth1: + // (A && B) || (C && D) || (D && F) + // ID=1 ID=4 ID=3 ID=5 ID=2 ID=6 + // + // A node ID of '0' always means MC/DC isn't being tracked. + // + // If a "next" node ID is '0', it means it's the end of the test vector. + // + // 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, + conditions_num: 0, + end_markers: vec![], + decision_depth, + }), + }; + + let parent_condition = decision_ctx.decision_stack.pop_back().unwrap_or_default(); + let lhs_id = if parent_condition.condition_id == ConditionId::NONE { + decision.conditions_num += 1; + ConditionId::from(decision.conditions_num) + } else { + parent_condition.condition_id + }; + + decision.conditions_num += 1; + let rhs_condition_id = ConditionId::from(decision.conditions_num); + + let (lhs, rhs) = match op { + LogicalOp::And => { + let lhs = ConditionInfo { + condition_id: lhs_id, + true_next_id: rhs_condition_id, + false_next_id: parent_condition.false_next_id, + }; + let rhs = ConditionInfo { + condition_id: rhs_condition_id, + true_next_id: parent_condition.true_next_id, + false_next_id: parent_condition.false_next_id, + }; + (lhs, rhs) + } + LogicalOp::Or => { + let lhs = ConditionInfo { + condition_id: lhs_id, + true_next_id: parent_condition.true_next_id, + false_next_id: rhs_condition_id, + }; + let rhs = ConditionInfo { + condition_id: rhs_condition_id, + true_next_id: parent_condition.true_next_id, + false_next_id: parent_condition.false_next_id, + }; + (lhs, rhs) + } + }; + // 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); + } + + fn take_condition( + &mut self, + 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"); + }; + if condition_info.true_next_id == ConditionId::NONE { + decision.end_markers.push(true_marker); + } + if condition_info.false_next_id == ConditionId::NONE { + decision.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) + } + } +} + +pub struct MCDCInfoBuilder { + branch_spans: Vec, + decision_spans: Vec, + state: MCDCState, +} + +impl MCDCInfoBuilder { + pub fn new() -> Self { + Self { branch_spans: vec![], decision_spans: vec![], state: MCDCState::new() } + } + + pub fn visit_evaluated_condition( + &mut self, + tcx: TyCtxt<'_>, + source_info: SourceInfo, + true_block: BasicBlock, + false_block: BasicBlock, + mut inject_block_marker: impl FnMut(SourceInfo, 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.conditions_num { + 0 => { + unreachable!("Decision with no condition is not expected"); + } + 1..=MAX_CONDITIONS_NUM_IN_DECISION => { + self.decision_spans.push(decision); + } + _ => { + // Do not generate mcdc mappings and statements for decisions with too many conditions. + let rebase_idx = self.branch_spans.len() - decision.conditions_num + 1; + for branch in &mut self.branch_spans[rebase_idx..] { + branch.condition_info = None; + } + + // ConditionInfo of this branch shall also be reset. + condition_info = None; + + tcx.dcx().emit_warn(MCDCExceedsConditionNumLimit { + span: decision.span, + conditions_num: decision.conditions_num, + max_conditions_num: MAX_CONDITIONS_NUM_IN_DECISION, + }); + } + } + } + self.branch_spans.push(MCDCBranchSpan { + span: source_info.span, + condition_info, + true_marker, + false_marker, + decision_depth, + }); + } + + pub fn into_done(self) -> (Vec, Vec) { + (self.decision_spans, self.branch_spans) + } +} + +impl Builder<'_, '_> { + pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) { + if let Some(branch_info) = self.coverage_branch_info.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() + { + mcdc_info.state.record_conditions(logical_op, span); + } + } + + pub(crate) fn mcdc_increment_depth_if_enabled(&mut self) { + if let Some(branch_info) = self.coverage_branch_info.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() + { + mcdc_info.state.decision_ctx_stack.push(MCDCDecisionCtx::default()); + }; + } + + pub(crate) fn mcdc_decrement_depth_if_enabled(&mut self) { + if let Some(branch_info) = self.coverage_branch_info.as_mut() + && let Some(mcdc_info) = branch_info.mcdc_info.as_mut() + { + if mcdc_info.state.decision_ctx_stack.pop().is_none() { + bug!("Unexpected empty decision stack"); + } + }; + } +}