mcdc-coverage: Add MCDCBlockMarker and MCDCDecisionMarker to the MIR

Author: RenjiSann

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -99,7 +99,10 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
             .or_insert_with(|| FunctionCoverageCollector::new(instance, function_coverage_info));
 
         match *kind {
-            CoverageKind::SpanMarker | CoverageKind::BlockMarker { .. } => unreachable!(
+            CoverageKind::SpanMarker
+            | CoverageKind::BlockMarker { .. }
+            | CoverageKind::MCDCBlockMarker { .. }
+            | CoverageKind::MCDCDecisionMarker { .. } => unreachable!(
                 "marker statement {kind:?} should have been removed by CleanupPostBorrowck"
             ),
             CoverageKind::CounterIncrement { id } => {

compiler/rustc_middle/src/mir/coverage.rs

@@ -15,6 +15,13 @@ rustc_index::newtype_index! {
     pub struct BlockMarkerId {}
 }
 
+rustc_index::newtype_index! {
+    #[derive(HashStable)]
+    #[encodable]
+    #[debug_format = "DecisionMarkerId({})"]
+    pub struct DecisionMarkerId {}
+}
+
 rustc_index::newtype_index! {
     /// ID of a coverage counter. Values ascend from 0.
     ///
@@ -97,6 +104,18 @@ pub enum CoverageKind {
     /// Should be erased before codegen (at some point after `InstrumentCoverage`).
     BlockMarker { id: BlockMarkerId },
 
+    /// Same as BlockMarker, but carries a reference to its parent decision for
+    /// MCDC coverage.
+    ///
+    /// Has no effect during codegen.
+    MCDCBlockMarker { id: BlockMarkerId, decision_id: DecisionMarkerId },
+
+    /// Marks the first condition of a decision (boolean expression). All
+    /// conditions in the same decision will reference this id.
+    ///
+    /// Has no effect during codegen.
+    MCDCDecisionMarker { id: DecisionMarkerId },
+
     /// Marks the point in MIR control flow represented by a coverage counter.
     ///
     /// This is eventually lowered to `llvm.instrprof.increment` in LLVM IR.
@@ -122,6 +141,10 @@ impl Debug for CoverageKind {
         match self {
             SpanMarker => write!(fmt, "SpanMarker"),
             BlockMarker { id } => write!(fmt, "BlockMarker({:?})", id.index()),
+            MCDCBlockMarker { id, decision_id } => {
+                write!(fmt, "MCDCBlockMarker({:?}, {:?})", id.index(), decision_id.index())
+            }
+            MCDCDecisionMarker { id } => write!(fmt, "MCDCDecisionMarker({:?})", id.index()),
             CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()),
             ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()),
         }
@@ -234,12 +257,16 @@ pub struct BranchInfo {
     /// data structures without having to scan the entire body first.
     pub num_block_markers: usize,
     pub branch_spans: Vec<BranchSpan>,
+    pub decisions: IndexVec<DecisionMarkerId, Span>,
 }
 
 #[derive(Clone, Debug)]
 #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub struct BranchSpan {
+    /// Source code region associated to the branch.
     pub span: Span,
+    /// Decision structure the branch is part of. Only used in the MCDC coverage.
+    pub decision_id: DecisionMarkerId,
     pub true_marker: BlockMarkerId,
     pub false_marker: BlockMarkerId,
 }

compiler/rustc_middle/src/mir/pretty.rs

@@ -477,7 +477,7 @@ fn write_coverage_branch_info(
 ) -> io::Result<()> {
     let coverage::BranchInfo { branch_spans, .. } = branch_info;
 
-    for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans {
+    for coverage::BranchSpan { span, true_marker, false_marker, .. } in branch_spans {
         writeln!(
             w,
             "{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",

compiler/rustc_middle/src/ty/structural_impls.rs

@@ -407,6 +407,7 @@ TrivialTypeTraversalImpls! {
     ::rustc_hir::MatchSource,
     ::rustc_target::asm::InlineAsmRegOrRegClass,
     crate::mir::coverage::BlockMarkerId,
+    crate::mir::coverage::DecisionMarkerId,
     crate::mir::coverage::CounterId,
     crate::mir::coverage::ExpressionId,
     crate::mir::Local,

compiler/rustc_mir_build/src/build/coverageinfo.rs

@@ -2,11 +2,13 @@ use std::assert_matches::assert_matches;
 use std::collections::hash_map::Entry;
 
 use rustc_data_structures::fx::FxHashMap;
-use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind};
+use rustc_index::IndexVec;
+use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind, DecisionMarkerId};
 use rustc_middle::mir::{self, BasicBlock, UnOp};
 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::Builder;
 
@@ -16,6 +18,7 @@ pub(crate) struct BranchInfoBuilder {
 
     num_block_markers: usize,
     branch_spans: Vec<BranchSpan>,
+    decisions: IndexVec<DecisionMarkerId, Span>,
 }
 
 #[derive(Clone, Copy)]
@@ -32,8 +35,15 @@ 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.
     pub(crate) fn new_if_enabled(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<Self> {
-        if tcx.sess.instrument_coverage_branch() && tcx.is_eligible_for_coverage(def_id) {
-            Some(Self { nots: FxHashMap::default(), num_block_markers: 0, branch_spans: vec![] })
+        if (tcx.sess.instrument_coverage_branch() || tcx.sess.instrument_coverage_mcdc())
+            && tcx.is_eligible_for_coverage(def_id)
+        {
+            Some(Self {
+                nots: FxHashMap::default(),
+                num_block_markers: 0,
+                branch_spans: vec![],
+                decisions: IndexVec::new(),
+            })
         } else {
             None
         }
@@ -86,14 +96,14 @@ impl BranchInfoBuilder {
     }
 
     pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
-        let Self { nots: _, num_block_markers, branch_spans } = self;
+        let Self { nots: _, num_block_markers, branch_spans, decisions } = self;
 
         if num_block_markers == 0 {
             assert!(branch_spans.is_empty());
             return None;
         }
 
-        Some(Box::new(mir::coverage::BranchInfo { num_block_markers, branch_spans }))
+        Some(Box::new(mir::coverage::BranchInfo { num_block_markers, branch_spans, decisions }))
     }
 }
 
@@ -105,6 +115,7 @@ impl Builder<'_, '_> {
         mut expr_id: ExprId,
         mut then_block: BasicBlock,
         mut else_block: BasicBlock,
+        decision_id: Option<DecisionMarkerId>, // If MCDC is enabled
     ) {
         // Bail out if branch coverage is not enabled for this function.
         let Some(branch_info) = self.coverage_branch_info.as_ref() else { return };
@@ -125,9 +136,15 @@ impl Builder<'_, '_> {
         let mut inject_branch_marker = |block: BasicBlock| {
             let id = branch_info.next_block_marker_id();
 
+            let cov_kind = if let Some(decision_id) = decision_id {
+                CoverageKind::MCDCBlockMarker { id, decision_id }
+            } else {
+                CoverageKind::BlockMarker { id }
+            };
+
             let marker_statement = mir::Statement {
                 source_info,
-                kind: mir::StatementKind::Coverage(CoverageKind::BlockMarker { id }),
+                kind: mir::StatementKind::Coverage(cov_kind),
             };
             self.cfg.push(block, marker_statement);
 
@@ -139,8 +156,41 @@ impl Builder<'_, '_> {
 
         branch_info.branch_spans.push(BranchSpan {
             span: source_info.span,
+            // FIXME: Handle case when MCDC is disabled better than just putting 0.
+            decision_id: decision_id.unwrap_or(DecisionMarkerId::from_u32(0)),
             true_marker,
             false_marker,
         });
     }
+
+    /// If MCDC coverage is enabled, inject a marker in all the decisions
+    /// (boolean expressions)
+    pub(crate) fn visit_coverage_decision(
+        &mut self,
+        expr_id: ExprId,
+        block: BasicBlock,
+    ) -> Option<DecisionMarkerId> {
+        // Early return if MCDC coverage is not enabled.
+        if !self.tcx.sess.instrument_coverage_mcdc() {
+            return None;
+        }
+        let Some(branch_info) = self.coverage_branch_info.as_mut() else {
+            return None;
+        };
+
+        let span = self.thir[expr_id].span;
+        let decision_id = branch_info.decisions.push(span);
+
+        // Inject a decision marker
+        let source_info = self.source_info(span);
+        let marker_statement = mir::Statement {
+            source_info,
+            kind: mir::StatementKind::Coverage(
+                CoverageKind::MCDCDecisionMarker { id: decision_id },
+            ),
+        };
+        self.cfg.push(block, marker_statement);
+
+        Some(decision_id)
+    }
 }

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -16,6 +16,7 @@ use rustc_data_structures::{
 };
 use rustc_hir::{BindingAnnotation, ByRef};
 use rustc_middle::middle::region;
+use rustc_middle::mir::coverage::DecisionMarkerId;
 use rustc_middle::mir::{self, *};
 use rustc_middle::thir::{self, *};
 use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty};
@@ -41,6 +42,9 @@ struct ThenElseArgs {
     /// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`].
     /// When false (for match guards), `let` bindings won't be declared.
     declare_let_bindings: bool,
+
+    /// Id of the parent decision id MCDC is enabled
+    decision_id: Option<DecisionMarkerId>,
 }
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
@@ -58,10 +62,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         variable_source_info: SourceInfo,
         declare_let_bindings: bool,
     ) -> BlockAnd<()> {
+        // Get an ID for the decision if MCDC is enabled.
+        let decision_id = self.visit_coverage_decision(expr_id, block);
+
         self.then_else_break_inner(
             block,
             expr_id,
-            ThenElseArgs { temp_scope_override, variable_source_info, declare_let_bindings },
+            ThenElseArgs {
+                temp_scope_override,
+                variable_source_info,
+                declare_let_bindings,
+                decision_id,
+            },
         )
     }
 
@@ -159,7 +171,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                 // Record branch coverage info for this condition.
                 // (Does nothing if branch coverage is not enabled.)
-                this.visit_coverage_branch_condition(expr_id, then_block, else_block);
+                this.visit_coverage_branch_condition(
+                    expr_id,
+                    then_block,
+                    else_block,
+                    args.decision_id,
+                );
 
                 let source_info = this.source_info(expr_span);
                 this.cfg.terminate(block, source_info, term);

compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs

@@ -5,7 +5,7 @@
 //!   - [`AscribeUserType`]
 //!   - [`FakeRead`]
 //!   - [`Assign`] statements with a [`Fake`] borrow
-//!   - [`Coverage`] statements of kind [`BlockMarker`] or [`SpanMarker`]
+//!   - [`Coverage`] statements of kind [`BlockMarker`], [`SpanMarker`], [`MCDCBlockMarker`], or [`MCDCDecisionMarker`]
 //!
 //! [`AscribeUserType`]: rustc_middle::mir::StatementKind::AscribeUserType
 //! [`Assign`]: rustc_middle::mir::StatementKind::Assign
@@ -15,6 +15,8 @@
 //! [`Coverage`]: rustc_middle::mir::StatementKind::Coverage
 //! [`BlockMarker`]: rustc_middle::mir::coverage::CoverageKind::BlockMarker
 //! [`SpanMarker`]: rustc_middle::mir::coverage::CoverageKind::SpanMarker
+//! [`MCDCBlockMarker`]: rustc_middle::mir::coverage::CoverageKind::MCDCBlockMarker
+//! [`MCDCDecisionMarker`]: rustc_middle::mir::coverage::CoverageKind::MCDCDecisionMarker
 
 use crate::MirPass;
 use rustc_middle::mir::coverage::CoverageKind;
@@ -33,7 +35,10 @@ impl<'tcx> MirPass<'tcx> for CleanupPostBorrowck {
                     | StatementKind::Coverage(
                         // These kinds of coverage statements are markers inserted during
                         // MIR building, and are not needed after InstrumentCoverage.
-                        CoverageKind::BlockMarker { .. } | CoverageKind::SpanMarker { .. },
+                        CoverageKind::BlockMarker { .. }
+                        | CoverageKind::SpanMarker { .. }
+                        | CoverageKind::MCDCBlockMarker { .. }
+                        | CoverageKind::MCDCDecisionMarker { .. },
                     )
                     | StatementKind::FakeRead(..) => statement.make_nop(),
                     _ => (),

compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs

@@ -222,8 +222,13 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option<Span> {
         | StatementKind::PlaceMention(..)
         | StatementKind::AscribeUserType(_, _) => Some(statement.source_info.span),
 
-        // Block markers are used for branch coverage, so ignore them here.
-        StatementKind::Coverage(CoverageKind::BlockMarker { .. }) => None,
+        StatementKind::Coverage(
+            // Block markers are used for branch coverage, so ignore them here.
+            CoverageKind::BlockMarker {..}
+            // Ignore MCDC markers as well
+            | CoverageKind::MCDCBlockMarker{ .. }
+            | CoverageKind::MCDCDecisionMarker{ .. }
+        ) => None,
 
         // These coverage statements should not exist prior to coverage instrumentation.
         StatementKind::Coverage(
@@ -378,16 +383,22 @@ pub(super) fn extract_branch_mappings(
     // Fill out the mapping from block marker IDs to their enclosing blocks.
     for (bb, data) in mir_body.basic_blocks.iter_enumerated() {
         for statement in &data.statements {
-            if let StatementKind::Coverage(CoverageKind::BlockMarker { id }) = statement.kind {
-                block_markers[id] = Some(bb);
+            if let StatementKind::Coverage(kind) = &statement.kind {
+                match kind {
+                    CoverageKind::BlockMarker { id }
+                    | CoverageKind::MCDCBlockMarker { id, decision_id: _ } => {
+                        block_markers[*id] = Some(bb);
+                    }
+                    _ => (),
+                }
             }
         }
     }
 
     branch_info
         .branch_spans
         .iter()
-        .filter_map(|&BranchSpan { span: raw_span, true_marker, false_marker }| {
+        .filter_map(|&BranchSpan { span: raw_span, decision_id: _, true_marker, false_marker }| {
             // For now, ignore any branch span that was introduced by
             // expansion. This makes things like assert macros less noisy.
             if !raw_span.ctxt().outer_expn_data().is_root() {