Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coverage: Optionally instrument the RHS of lazy logical operators #125756

Merged
merged 3 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
18 changes: 17 additions & 1 deletion compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,23 @@ pub enum CoverageLevel {
Block,
/// Also instrument branch points (includes block coverage).
Branch,
/// Instrument for MC/DC. Mostly a superset of branch coverage, but might
/// Same as branch coverage, but also adds branch instrumentation for
/// certain boolean expressions that are not directly used for branching.
///
/// For example, in the following code, `b` does not directly participate
/// in a branch, but condition coverage will instrument it as its own
/// artificial branch:
/// ```
/// # let (a, b) = (false, true);
/// let x = a && b;
/// // ^ last operand
/// ```
///
/// This level is mainly intended to be a stepping-stone towards full MC/DC
/// instrumentation, so it might be removed in the future when MC/DC is
/// sufficiently complete, or if it is making MC/DC changes difficult.
Condition,
/// Instrument for MC/DC. Mostly a superset of condition coverage, but might
/// differ in some corner cases.
Mcdc,
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ mod desc {
pub const parse_optimization_fuel: &str = "crate=integer";
pub const parse_dump_mono_stats: &str = "`markdown` (default) or `json`";
pub const parse_instrument_coverage: &str = parse_bool;
pub const parse_coverage_options: &str = "`block` | `branch` | `mcdc`";
pub const parse_coverage_options: &str = "`block` | `branch` | `condition` | `mcdc`";
pub const parse_instrument_xray: &str = "either a boolean (`yes`, `no`, `on`, `off`, etc), or a comma separated list of settings: `always` or `never` (mutually exclusive), `ignore-loops`, `instruction-threshold=N`, `skip-entry`, `skip-exit`";
pub const parse_unpretty: &str = "`string` or `string=string`";
pub const parse_treat_err_as_bug: &str = "either no value or a non-negative number";
Expand Down Expand Up @@ -961,6 +961,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,
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions src/doc/unstable-book/src/compiler-flags/coverage-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ This option controls details of the coverage instrumentation performed by

Multiple options can be passed, separated by commas. Valid options are:

- `block`, `branch`, `mcdc`:
- `block`, `branch`, `condition`, `mcdc`:
Sets the level of coverage instrumentation.
Setting the level will override any previously-specified level.
- `block` (default):
Blocks in the control-flow graph will be instrumented for coverage.
- `branch`:
In addition to block coverage, also enables branch coverage instrumentation.
- `condition`:
In addition to branch coverage, also instruments some boolean expressions
as branches, even if they are not directly used as branch conditions.
- `mcdc`:
In addition to block and branch coverage, also enables MC/DC instrumentation.
In addition to condition coverage, also enables MC/DC instrumentation.
(Branch coverage instrumentation may differ in some cases.)
152 changes: 152 additions & 0 deletions tests/coverage/condition/conditions.cov-map
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
Function name: conditions::assign_3_and_or
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: 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, 47)
- 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_3_or_and
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: 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, 47)
- 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_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)

Loading
Loading