Skip to content

Commit

Permalink
coverage: Instrument the RHS value of lazy logical operators
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Zalathar committed May 29, 2024
1 parent eba62a2 commit 0374975
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 17 deletions.
57 changes: 57 additions & 0 deletions compiler/rustc_mir_build/src/build/coverageinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
30 changes: 15 additions & 15 deletions tests/coverage/branch/conditions.cov-map
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
Function name: conditions::assign_3
Raw bytes (73): 0x[01, 01, 09, 07, 0b, 05, 09, 0d, 11, 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]
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 = Expression(1, Add), rhs = Expression(2, Add)
- expression 1 operands: lhs = Counter(1), rhs = Counter(2)
- expression 2 operands: lhs = Counter(3), rhs = Counter(4)
- 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)
Expand All @@ -15,7 +15,7 @@ Number of expressions: 9
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))
= (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
Expand All @@ -31,7 +31,7 @@ Number of file 0 mappings: 9
true = c2
false = c3
- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2)
= ((c1 + c2) + (c3 + c4))
= (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]
Expand Down Expand Up @@ -66,18 +66,18 @@ Number of file 0 mappings: 9
= ((c2 + c3) + c4)

Function name: conditions::assign_and
Raw bytes (51): 0x[01, 01, 04, 09, 07, 0d, 0e, 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]
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 = Counter(2), rhs = Expression(1, Add)
- expression 1 operands: lhs = Counter(3), rhs = Expression(3, Sub)
- 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)))
= ((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
Expand All @@ -87,7 +87,7 @@ Number of file 0 mappings: 7
true = c2
false = c3
- Code(Expression(0, Add)) at (prev + 1, 5) to (start + 1, 2)
= (c2 + (c3 + (c0 - c1)))
= ((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]
Expand Down Expand Up @@ -123,13 +123,13 @@ 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, 09, 0b, 0d, 02, 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]
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 = Counter(2), rhs = Expression(2, Add)
- expression 2 operands: lhs = Counter(3), rhs = Expression(0, Sub)
- 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)
Expand All @@ -140,7 +140,7 @@ Number of file 0 mappings: 5
true = c2
false = c3
- Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2)
= (c2 + (c3 + (c0 - c1)))
= ((c2 + c3) + (c0 - c1))

Function name: conditions::simple_assign
Raw bytes (9): 0x[01, 01, 00, 01, 01, 08, 01, 03, 02]
Expand Down

0 comments on commit 0374975

Please sign in to comment.