MCDC Coverage: instrument last boolean RHS operands from condition coverage

compiler/rustc_mir_build/src/build/coverageinfo.rs

@@ -117,17 +117,37 @@ impl BranchInfoBuilder {
         }
     }
 
-    fn add_two_way_branch<'tcx>(
+    fn register_two_way_branch<'tcx>(
         &mut self,
+        tcx: TyCtxt<'tcx>,
         cfg: &mut CFG<'tcx>,
         source_info: SourceInfo,
         true_block: BasicBlock,
         false_block: BasicBlock,
     ) {
-        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);
+        let this = self;
 
-        self.branch_spans.push(BranchSpan { span: source_info.span, true_marker, false_marker });
+        // Separate path for handling branches when MC/DC is enabled.
+        if let Some(mcdc_info) = this.mcdc_info.as_mut() {
+            let inject_block_marker =
+                |source_info, block| this.markers.inject_block_marker(cfg, source_info, block);
+            mcdc_info.visit_evaluated_condition(
+                tcx,
+                source_info,
+                true_block,
+                false_block,
+                inject_block_marker,
+            );
+        } else {
+            let true_marker = this.markers.inject_block_marker(cfg, source_info, true_block);
+            let false_marker = this.markers.inject_block_marker(cfg, source_info, false_block);
+
+            this.branch_spans.push(BranchSpan {
+                span: source_info.span,
+                true_marker,
+                false_marker,
+            });
+        }
     }
 
     pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
@@ -155,7 +175,71 @@ impl BranchInfoBuilder {
     }
 }
 
-impl Builder<'_, '_> {
+impl<'tcx> Builder<'_, 'tcx> {
+    /// If condition coverage is enabled, inject extra blocks and marker statements
+    /// that will let us track the value of the condition in `place`.
+    pub(crate) fn visit_coverage_standalone_condition(
+        &mut self,
+        mut expr_id: ExprId,     // Expression giving the span of the condition
+        place: mir::Place<'tcx>, // Already holds the boolean condition value
+        block: &mut BasicBlock,
+    ) {
+        // Bail out if condition coverage is not enabled for this function.
+        let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
+        if !self.tcx.sess.instrument_coverage_condition() {
+            return;
+        };
+
+        // Remove any wrappers, so that we can inspect the real underlying expression.
+        while let ExprKind::Use { source: inner } | ExprKind::Scope { value: inner, .. } =
+            self.thir[expr_id].kind
+        {
+            expr_id = inner;
+        }
+        // If the expression is a lazy logical op, it will naturally get branch
+        // coverage as part of its normal lowering, so we can disregard it here.
+        if let ExprKind::LogicalOp { .. } = self.thir[expr_id].kind {
+            return;
+        }
+
+        let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope };
+
+        // Using the boolean value that has already been stored in `place`, set up
+        // control flow in the shape of a diamond, so that we can place separate
+        // marker statements in the true and false blocks. The coverage MIR pass
+        // will use those markers to inject coverage counters as appropriate.
+        //
+        //          block
+        //         /     \
+        // true_block   false_block
+        //  (marker)     (marker)
+        //         \     /
+        //        join_block
+
+        let true_block = self.cfg.start_new_block();
+        let false_block = self.cfg.start_new_block();
+        self.cfg.terminate(
+            *block,
+            source_info,
+            mir::TerminatorKind::if_(mir::Operand::Copy(place), true_block, false_block),
+        );
+
+        // Separate path for handling branches when MC/DC is enabled.
+        branch_info.register_two_way_branch(
+            self.tcx,
+            &mut self.cfg,
+            source_info,
+            true_block,
+            false_block,
+        );
+
+        let join_block = self.cfg.start_new_block();
+        self.cfg.goto(true_block, source_info, join_block);
+        self.cfg.goto(false_block, source_info, join_block);
+        // Any subsequent codegen in the caller should use the new join block.
+        *block = join_block;
+    }
+
     /// If branch coverage is enabled, inject marker statements into `then_block`
     /// and `else_block`, and record their IDs in the table of branch spans.
     pub(crate) fn visit_coverage_branch_condition(
@@ -178,21 +262,12 @@ 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 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;
-        }
-
-        branch_info.add_two_way_branch(&mut self.cfg, source_info, then_block, else_block);
+        branch_info.register_two_way_branch(
+            self.tcx,
+            &mut self.cfg,
+            source_info,
+            then_block,
+            else_block,
+        );
     }
 }

compiler/rustc_mir_build/src/build/expr/into.rs

@@ -147,7 +147,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             }
             ExprKind::LogicalOp { op, lhs, rhs } => {
                 let condition_scope = this.local_scope();
-                let source_info = this.source_info(expr.span);
+                let expr_span = expr.span;
+                let source_info = this.source_info(expr_span);
+
+                this.visit_coverage_branch_operation(op, expr_span);
+
                 // We first evaluate the left-hand side of the predicate ...
                 let (then_block, else_block) =
                     this.in_if_then_scope(condition_scope, expr.span, |this| {
@@ -181,9 +185,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         const_: Const::from_bool(this.tcx, constant),
                     },
                 );
-                let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs));
+                let mut rhs_block = unpack!(this.expr_into_dest(destination, continuation, rhs));
+                // Instrument the lowered RHS's value for condition coverage.
+                // (Does nothing if condition coverage is not enabled.)
+                this.visit_coverage_standalone_condition(rhs, destination, &mut rhs_block);
+
                 let target = this.cfg.start_new_block();
-                this.cfg.goto(rhs, source_info, target);
+                this.cfg.goto(rhs_block, source_info, target);
                 this.cfg.goto(short_circuit, source_info, target);
                 target.unit()
             }

compiler/rustc_session/src/config.rs

@@ -159,8 +159,24 @@ pub enum CoverageLevel {
     Block,
     /// Also instrument branch points (includes block coverage).
     Branch,
-    /// Instrument for MC/DC. Mostly a superset of branch coverage, but might
-    /// differ in some corner cases.
+    /// Same as branch condition, with a single different case:
+    /// In boolean expressions that are not inside a control-flow decision,
+    /// it will intentionally insert an instrumented branch for the last operand.
+    ///
+    /// Example:
+    /// ```
+    /// # let (a, b) = (false, true);
+    /// let x = a && b;
+    /// //           ^ last operand
+    /// ```
+    /// Condition coverage does track true/false coverage for `b`,
+    /// but branch coverage doesn't.
+    ///
+    /// The main purpose of this coverage level is to be reused by MCDC.
+    Condition,
+    /// Instrument for MC/DC. Enables condition coverage under the hood.
+    /// Mostly a superset of branch coverage, but might differ in some
+    /// corner cases.
     Mcdc,
 }
 

compiler/rustc_session/src/options.rs

@@ -948,6 +948,7 @@ mod parse {
             match option {
                 "block" => slot.level = CoverageLevel::Block,
                 "branch" => slot.level = CoverageLevel::Branch,
+                "condition" => slot.level = CoverageLevel::Condition,
                 "mcdc" => slot.level = CoverageLevel::Mcdc,
                 _ => return false,
             }

compiler/rustc_session/src/session.rs

@@ -353,6 +353,11 @@ impl Session {
             && self.opts.unstable_opts.coverage_options.level >= CoverageLevel::Branch
     }
 
+    pub fn instrument_coverage_condition(&self) -> bool {
+        self.instrument_coverage()
+            && self.opts.unstable_opts.coverage_options.level >= CoverageLevel::Condition
+    }
+
     pub fn instrument_coverage_mcdc(&self) -> bool {
         self.instrument_coverage()
             && self.opts.unstable_opts.coverage_options.level >= CoverageLevel::Mcdc

tests/coverage/branch/conditions.cov-map

@@ -0,0 +1,152 @@
+Function name: conditions::assign_3
+Raw bytes (73): 0x[01, 01, 09, 05, 07, 0b, 11, 09, 0d, 01, 05, 01, 05, 22, 11, 01, 05, 22, 11, 01, 05, 09, 01, 17, 01, 00, 28, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 22, 00, 0d, 00, 0e, 22, 00, 12, 00, 13, 20, 1e, 11, 00, 12, 00, 13, 1e, 00, 17, 00, 18, 20, 09, 0d, 00, 17, 00, 18, 03, 01, 05, 01, 02]
+Number of files: 1
+- file 0 => global file 1
+Number of expressions: 9
+- expression 0 operands: lhs = Counter(1), rhs = Expression(1, Add)
+- expression 1 operands: lhs = Expression(2, Add), rhs = Counter(4)
+- expression 2 operands: lhs = Counter(2), rhs = Counter(3)
+- expression 3 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 4 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 5 operands: lhs = Expression(8, Sub), rhs = Counter(4)
+- expression 6 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 7 operands: lhs = Expression(8, Sub), rhs = Counter(4)
+- expression 8 operands: lhs = Counter(0), rhs = Counter(1)
+Number of file 0 mappings: 9
+- Code(Counter(0)) at (prev + 23, 1) to (start + 0, 40)
+- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10)
+    = (c1 + ((c2 + c3) + c4))
+- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14)
+- Branch { true: Counter(1), false: Expression(8, Sub) } at (prev + 0, 13) to (start + 0, 14)
+    true  = c1
+    false = (c0 - c1)
+- Code(Expression(8, Sub)) at (prev + 0, 18) to (start + 0, 19)
+    = (c0 - c1)
+- Branch { true: Expression(7, Sub), false: Counter(4) } at (prev + 0, 18) to (start + 0, 19)
+    true  = ((c0 - c1) - c4)
+    false = c4
+- Code(Expression(7, Sub)) at (prev + 0, 23) to (start + 0, 24)
+    = ((c0 - c1) - c4)
+- Branch { true: Counter(2), false: Counter(3) } at (prev + 0, 23) to (start + 0, 24)
+    true  = c2
+    false = c3
+- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2)
+    = (c1 + ((c2 + c3) + c4))
+
+Function name: conditions::assign_3_bis
+Raw bytes (69): 0x[01, 01, 07, 07, 11, 09, 0d, 01, 05, 05, 09, 16, 1a, 05, 09, 01, 05, 09, 01, 1c, 01, 00, 2c, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 1a, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 20, 09, 16, 00, 12, 00, 13, 13, 00, 17, 00, 18, 20, 0d, 11, 00, 17, 00, 18, 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(4)
+- expression 1 operands: lhs = Counter(2), rhs = Counter(3)
+- expression 2 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 3 operands: lhs = Counter(1), rhs = Counter(2)
+- expression 4 operands: lhs = Expression(5, Sub), rhs = Expression(6, Sub)
+- expression 5 operands: lhs = Counter(1), rhs = Counter(2)
+- expression 6 operands: lhs = Counter(0), rhs = Counter(1)
+Number of file 0 mappings: 9
+- Code(Counter(0)) at (prev + 28, 1) to (start + 0, 44)
+- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10)
+    = ((c2 + c3) + c4)
+- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14)
+- Branch { true: Counter(1), false: Expression(6, Sub) } at (prev + 0, 13) to (start + 0, 14)
+    true  = c1
+    false = (c0 - c1)
+- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19)
+- Branch { true: Counter(2), false: Expression(5, Sub) } at (prev + 0, 18) to (start + 0, 19)
+    true  = c2
+    false = (c1 - c2)
+- Code(Expression(4, Add)) at (prev + 0, 23) to (start + 0, 24)
+    = ((c1 - c2) + (c0 - c1))
+- Branch { true: Counter(3), false: Counter(4) } at (prev + 0, 23) to (start + 0, 24)
+    true  = c3
+    false = c4
+- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2)
+    = ((c2 + c3) + c4)
+
+Function name: conditions::assign_and
+Raw bytes (51): 0x[01, 01, 04, 07, 0e, 09, 0d, 01, 05, 01, 05, 07, 01, 0d, 01, 00, 21, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 0e, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 20, 09, 0d, 00, 12, 00, 13, 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 = Expression(3, Sub)
+- expression 1 operands: lhs = Counter(2), rhs = Counter(3)
+- expression 2 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 3 operands: lhs = Counter(0), rhs = Counter(1)
+Number of file 0 mappings: 7
+- Code(Counter(0)) at (prev + 13, 1) to (start + 0, 33)
+- Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10)
+    = ((c2 + c3) + (c0 - c1))
+- Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14)
+- Branch { true: Counter(1), false: Expression(3, Sub) } at (prev + 0, 13) to (start + 0, 14)
+    true  = c1
+    false = (c0 - c1)
+- Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19)
+- Branch { true: Counter(2), false: Counter(3) } at (prev + 0, 18) to (start + 0, 19)
+    true  = c2
+    false = c3
+- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2)
+    = ((c2 + c3) + (c0 - c1))
+
+Function name: conditions::assign_or
+Raw bytes (51): 0x[01, 01, 04, 07, 0d, 05, 09, 01, 05, 01, 05, 07, 01, 12, 01, 00, 20, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 0e, 00, 0d, 00, 0e, 0e, 00, 12, 00, 13, 20, 09, 0d, 00, 12, 00, 13, 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: 7
+- Code(Counter(0)) at (prev + 18, 1) to (start + 0, 32)
+- 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)
+- Branch { true: Counter(1), false: Expression(3, Sub) } at (prev + 0, 13) to (start + 0, 14)
+    true  = c1
+    false = (c0 - c1)
+- Code(Expression(3, Sub)) at (prev + 0, 18) to (start + 0, 19)
+    = (c0 - c1)
+- Branch { true: Counter(2), false: Counter(3) } at (prev + 0, 18) to (start + 0, 19)
+    true  = c2
+    false = c3
+- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2)
+    = ((c1 + c2) + c3)
+
+Function name: conditions::foo
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 21, 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 + 33, 1) to (start + 2, 2)
+
+Function name: conditions::func_call
+Raw bytes (39): 0x[01, 01, 03, 01, 05, 0b, 02, 09, 0d, 05, 01, 25, 01, 01, 0a, 20, 05, 02, 01, 09, 00, 0a, 05, 00, 0e, 00, 0f, 20, 09, 0d, 00, 0e, 00, 0f, 07, 01, 01, 00, 02]
+Number of files: 1
+- file 0 => global file 1
+Number of expressions: 3
+- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
+- expression 1 operands: lhs = Expression(2, Add), rhs = Expression(0, Sub)
+- expression 2 operands: lhs = Counter(2), rhs = Counter(3)
+Number of file 0 mappings: 5
+- Code(Counter(0)) at (prev + 37, 1) to (start + 1, 10)
+- Branch { true: Counter(1), false: Expression(0, Sub) } at (prev + 1, 9) to (start + 0, 10)
+    true  = c1
+    false = (c0 - c1)
+- Code(Counter(1)) at (prev + 0, 14) to (start + 0, 15)
+- Branch { true: Counter(2), false: Counter(3) } at (prev + 0, 14) to (start + 0, 15)
+    true  = c2
+    false = c3
+- Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2)
+    = ((c2 + c3) + (c0 - c1))
+
+Function name: conditions::simple_assign
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 08, 01, 03, 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 + 8, 1) to (start + 3, 2)
+