about summary refs log tree commit diff
diff options
context:
space:
mode:
authorZalathar <Zalathar@users.noreply.github.com>2024-05-01 17:33:33 +1000
committerZalathar <Zalathar@users.noreply.github.com>2024-05-30 15:38:46 +1000
commit35a8746832a4d9103f317c5cac56cfa1e0316d8c (patch)
tree42409627b5bbc32a3ba1f675d7b147759d0bd6ee
parent20174e6638eb0d5ac0195706ce6cc7185949a407 (diff)
downloadrust-35a8746832a4d9103f317c5cac56cfa1e0316d8c.tar.gz
rust-35a8746832a4d9103f317c5cac56cfa1e0316d8c.zip
coverage: Instrument the RHS value of lazy logical operators
When a lazy logical operator (`&&` or `||`) occurs outside of an `if`
condition, it normally doesn't have any associated control-flow branch, so we
don't have an existing way to track whether it was true or false.

This patch adds special code to handle this case, by inserting extra MIR blocks
in a diamond shape after evaluating the RHS. This gives us a place to insert
the appropriate marker statements, which can then be given their own counters.
-rw-r--r--compiler/rustc_mir_build/src/build/coverageinfo.rs57
-rw-r--r--compiler/rustc_mir_build/src/build/expr/into.rs8
-rw-r--r--tests/coverage/condition/conditions.cov-map125
-rw-r--r--tests/coverage/condition/conditions.coverage5
4 files changed, 148 insertions, 47 deletions
diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs
index ee9eeb62990..855dcbbcb34 100644
--- a/compiler/rustc_mir_build/src/build/coverageinfo.rs
+++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs
@@ -157,6 +157,63 @@ impl BranchInfoBuilder {
 }
 
 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),
+        );
+
+        branch_info.add_two_way_branch(&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(
diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs
index 9349133d2db..c08a3b6691a 100644
--- a/compiler/rustc_mir_build/src/build/expr/into.rs
+++ b/compiler/rustc_mir_build/src/build/expr/into.rs
@@ -183,9 +183,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()
             }
diff --git a/tests/coverage/condition/conditions.cov-map b/tests/coverage/condition/conditions.cov-map
index 6c81f7bda98..74726e269fc 100644
--- a/tests/coverage/condition/conditions.cov-map
+++ b/tests/coverage/condition/conditions.cov-map
@@ -1,89 +1,118 @@
 Function name: conditions::assign_3_and_or
-Raw bytes (60): 0x[01, 01, 06, 0d, 13, 09, 16, 01, 05, 01, 05, 09, 16, 01, 05, 08, 01, 1c, 01, 00, 2f, 03, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 16, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 20, 0d, 09, 00, 12, 00, 13, 13, 00, 17, 00, 18, 03, 01, 05, 01, 02]
+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, 2f, 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: 6
-- expression 0 operands: lhs = Counter(3), rhs = Expression(4, Add)
-- expression 1 operands: lhs = Counter(2), rhs = Expression(5, Sub)
+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(0), rhs = Counter(1)
-- expression 4 operands: lhs = Counter(2), rhs = Expression(5, Sub)
-- expression 5 operands: lhs = Counter(0), rhs = Counter(1)
-Number of file 0 mappings: 8
+- 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, 47)
 - Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10)
-    = (c3 + (c2 + (c0 - c1)))
+    = ((c2 + c3) + c4)
 - Code(Counter(0)) at (prev + 0, 13) to (start + 0, 14)
-- Branch { true: Counter(1), false: Expression(5, Sub) } 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(3), false: Counter(2) } at (prev + 0, 18) to (start + 0, 19)
-    true  = c3
-    false = c2
+- 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)
-    = (c2 + (c0 - c1))
+    = ((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)
-    = (c3 + (c2 + (c0 - c1)))
+    = ((c2 + c3) + c4)
 
 Function name: conditions::assign_3_or_and
-Raw bytes (56): 0x[01, 01, 04, 05, 07, 09, 0d, 01, 05, 01, 05, 08, 01, 17, 01, 00, 2f, 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, 09, 00, 17, 00, 18, 03, 01, 05, 01, 02]
+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, 2f, 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: 4
+Number of expressions: 9
 - expression 0 operands: lhs = Counter(1), rhs = Expression(1, Add)
-- expression 1 operands: lhs = Counter(2), rhs = Counter(3)
-- expression 2 operands: lhs = Counter(0), rhs = Counter(1)
+- 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)
-Number of file 0 mappings: 8
+- 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, 47)
 - Code(Expression(0, Add)) at (prev + 1, 9) to (start + 0, 10)
-    = (c1 + (c2 + c3))
+    = (c1 + ((c2 + c3) + c4))
 - 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)
+- Branch { true: Counter(1), false: Expression(8, 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)
+- Code(Expression(8, 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)
+- 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(Counter(2)) at (prev + 0, 23) to (start + 0, 24)
 - Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2)
-    = (c1 + (c2 + c3))
+    = (c1 + ((c2 + c3) + c4))
 
 Function name: conditions::assign_and
-Raw bytes (38): 0x[01, 01, 01, 01, 05, 06, 01, 0d, 01, 00, 21, 01, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 02, 00, 0d, 00, 0e, 05, 00, 12, 00, 13, 01, 01, 05, 01, 02]
+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: 1
-- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
-Number of file 0 mappings: 6
+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(Counter(0)) at (prev + 1, 9) to (start + 0, 10)
+- 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(0, Sub) } 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)
-- Code(Counter(0)) at (prev + 1, 5) to (start + 1, 2)
+- 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 (38): 0x[01, 01, 01, 01, 05, 06, 01, 12, 01, 00, 20, 01, 01, 09, 00, 0a, 01, 00, 0d, 00, 0e, 20, 05, 02, 00, 0d, 00, 0e, 02, 00, 12, 00, 13, 01, 01, 05, 01, 02]
+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: 1
-- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
-Number of file 0 mappings: 6
+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(Counter(0)) at (prev + 1, 9) to (start + 0, 10)
+- 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(0, Sub) } 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(0, Sub)) at (prev + 0, 18) to (start + 0, 19)
+- Code(Expression(3, Sub)) at (prev + 0, 18) to (start + 0, 19)
     = (c0 - c1)
-- Code(Counter(0)) at (prev + 1, 5) to (start + 1, 2)
+- 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]
@@ -94,18 +123,24 @@ Number of file 0 mappings: 1
 - Code(Counter(0)) at (prev + 33, 1) to (start + 2, 2)
 
 Function name: conditions::func_call
-Raw bytes (28): 0x[01, 01, 01, 01, 05, 04, 01, 25, 01, 01, 0a, 20, 05, 02, 01, 09, 00, 0a, 05, 00, 0e, 00, 0f, 01, 01, 01, 00, 02]
+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: 1
+Number of expressions: 3
 - expression 0 operands: lhs = Counter(0), rhs = Counter(1)
-Number of file 0 mappings: 4
+- 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)
-- Code(Counter(0)) at (prev + 1, 1) to (start + 0, 2)
+- 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]
diff --git a/tests/coverage/condition/conditions.coverage b/tests/coverage/condition/conditions.coverage
index a71ab8e5d89..3215b391d62 100644
--- a/tests/coverage/condition/conditions.coverage
+++ b/tests/coverage/condition/conditions.coverage
@@ -15,6 +15,7 @@
                                ^2
   ------------------
   |  Branch (LL:13): [True: 2, False: 1]
+  |  Branch (LL:18): [True: 1, False: 1]
   ------------------
    LL|      3|    black_box(x);
    LL|      3|}
@@ -24,6 +25,7 @@
                                ^1
   ------------------
   |  Branch (LL:13): [True: 2, False: 1]
+  |  Branch (LL:18): [True: 0, False: 1]
   ------------------
    LL|      3|    black_box(x);
    LL|      3|}
@@ -34,6 +36,7 @@
   ------------------
   |  Branch (LL:13): [True: 2, False: 2]
   |  Branch (LL:18): [True: 1, False: 1]
+  |  Branch (LL:23): [True: 1, False: 0]
   ------------------
    LL|      4|    black_box(x);
    LL|      4|}
@@ -44,6 +47,7 @@
   ------------------
   |  Branch (LL:13): [True: 2, False: 2]
   |  Branch (LL:18): [True: 1, False: 1]
+  |  Branch (LL:23): [True: 2, False: 1]
   ------------------
    LL|      4|    black_box(x);
    LL|      4|}
@@ -57,6 +61,7 @@
                            ^2
   ------------------
   |  Branch (LL:9): [True: 2, False: 1]
+  |  Branch (LL:14): [True: 1, False: 1]
   ------------------
    LL|      3|}
    LL|       |