bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #127758 - Zalathar:expression-used, r=oli-obk

Browse source 
coverage: Restrict `ExpressionUsed` simplification to `Code` mappings

In the future, branch and MC/DC mappings might have expressions that don't correspond to any single point in the control-flow graph. That makes it trickier to keep track of which expressions should expect an `ExpressionUsed` node.

We therefore sidestep that complexity by only performing `ExpressionUsed` simplification for expressions associated directly with ordinary `Code` mappings.

(This simplification step is inherited from the original coverage implementation, which only supported `Code` mappings anyway, so there's no particular reason to extend it to other kinds of mappings unless we specifically choose to.)

Relevant to:
- #124154
- #126677
- #124278

```@rustbot``` label +A-code-coverage
This commit is contained in:
Matthias Krüger 2024-07-15 21:11:50 +02:00 • committed by GitHub
commit e5d65e46ed
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 37 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

11
compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
View file

@ -66,8 +66,15 @@ impl<'tcx> FunctionCoverageCollector<'tcx> {
// For each expression ID that is directly used by one or more mappings,
// mark it as not-yet-seen. This indicates that we expect to see a
// corresponding `ExpressionUsed` statement during MIR traversal.
for term in function_coverage_info.mappings.iter().flat_map(|m| m.kind.terms()) {
if let CovTerm::Expression(id) = term {
for mapping in function_coverage_info.mappings.iter() {
// Currently we only worry about ordinary code mappings.
// For branch and MC/DC mappings, expressions might not correspond
// to any particular point in the control-flow graph.
// (Keep this in sync with the injection of `ExpressionUsed`
// statements in the `InstrumentCoverage` MIR pass.)
if let MappingKind::Code(term) = mapping.kind
&& let CovTerm::Expression(id) = term
{
expressions_seen.remove(id);
}
}

13
compiler/rustc_middle/src/mir/coverage.rs
View file

@ -220,19 +220,6 @@ pub enum MappingKind {
}
impl MappingKind {
/// Iterator over all coverage terms in this mapping kind.
pub fn terms(&self) -> impl Iterator<Item = CovTerm> {
let zero = || None.into_iter().chain(None);
let one = |a| Some(a).into_iter().chain(None);
let two = |a, b| Some(a).into_iter().chain(Some(b));
match *self {
Self::Code(term) => one(term),
Self::Branch { true_term, false_term } => two(true_term, false_term),
Self::MCDCBranch { true_term, false_term, .. } => two(true_term, false_term),
Self::MCDCDecision(_) => zero(),
}
}
/// Returns a copy of this mapping kind, in which all coverage terms have
/// been replaced with ones returned by the given function.
pub fn map_terms(&self, map_fn: impl Fn(CovTerm) -> CovTerm) -> Self {

22
compiler/rustc_mir_transform/src/coverage/mappings.rs
View file

@ -56,6 +56,10 @@ pub(super) struct MCDCDecision {
#[derive(Default)]
pub(super) struct ExtractedMappings {
/// Store our own copy of [`CoverageGraph::num_nodes`], so that we don't
/// need access to the whole graph when allocating per-BCB data. This is
/// only public so that other code can still use exhaustive destructuring.
pub(super) num_bcbs: usize,
pub(super) code_mappings: Vec<CodeMapping>,
pub(super) branch_pairs: Vec<BranchPair>,
pub(super) mcdc_bitmap_bytes: u32,
@ -106,6 +110,7 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>(
);
ExtractedMappings {
num_bcbs: basic_coverage_blocks.num_nodes(),
code_mappings,
branch_pairs,
mcdc_bitmap_bytes,
@ -115,12 +120,10 @@ pub(super) fn extract_all_mapping_info_from_mir<'tcx>(
}
impl ExtractedMappings {
pub(super) fn all_bcbs_with_counter_mappings(
&self,
basic_coverage_blocks: &CoverageGraph, // Only used for allocating a correctly-sized set
) -> BitSet<BasicCoverageBlock> {
pub(super) fn all_bcbs_with_counter_mappings(&self) -> BitSet<BasicCoverageBlock> {
// Fully destructure self to make sure we don't miss any fields that have mappings.
let Self {
num_bcbs,
code_mappings,
branch_pairs,
mcdc_bitmap_bytes: _,
@ -129,7 +132,7 @@ impl ExtractedMappings {
} = self;
// Identify which BCBs have one or more mappings.
let mut bcbs_with_counter_mappings = BitSet::new_empty(basic_coverage_blocks.num_nodes());
let mut bcbs_with_counter_mappings = BitSet::new_empty(*num_bcbs);
let mut insert = |bcb| {
bcbs_with_counter_mappings.insert(bcb);
};
@ -156,6 +159,15 @@ impl ExtractedMappings {
bcbs_with_counter_mappings
}
/// Returns the set of BCBs that have one or more `Code` mappings.
pub(super) fn bcbs_with_ordinary_code_mappings(&self) -> BitSet<BasicCoverageBlock> {
let mut bcbs = BitSet::new_empty(self.num_bcbs);
for &CodeMapping { span: _, bcb } in &self.code_mappings {
bcbs.insert(bcb);
}
bcbs
}
}
fn resolve_block_markers(

17
compiler/rustc_mir_transform/src/coverage/mod.rs
View file

@ -25,7 +25,7 @@ use rustc_span::source_map::SourceMap;
use rustc_span::{BytePos, Pos, RelativeBytePos, Span, Symbol};
use crate::coverage::counters::{CounterIncrementSite, CoverageCounters};
use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
use crate::coverage::graph::CoverageGraph;
use crate::coverage::mappings::ExtractedMappings;
use crate::MirPass;
@ -88,8 +88,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
// every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock`
// and all `Expression` dependencies (operands) are also generated, for any other
// `BasicCoverageBlock`s not already associated with a coverage span.
let bcbs_with_counter_mappings =
extracted_mappings.all_bcbs_with_counter_mappings(&basic_coverage_blocks);
let bcbs_with_counter_mappings = extracted_mappings.all_bcbs_with_counter_mappings();
if bcbs_with_counter_mappings.is_empty() {
// No relevant spans were found in MIR, so skip instrumenting this function.
return;
@ -109,7 +108,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
inject_coverage_statements(
mir_body,
&basic_coverage_blocks,
bcb_has_counter_mappings,
&extracted_mappings,
&coverage_counters,
);
@ -163,6 +162,7 @@ fn create_mappings<'tcx>(
// Fully destructure the mappings struct to make sure we don't miss any kinds.
let ExtractedMappings {
num_bcbs: _,
code_mappings,
branch_pairs,
mcdc_bitmap_bytes: _,
@ -219,7 +219,7 @@ fn create_mappings<'tcx>(
fn inject_coverage_statements<'tcx>(
mir_body: &mut mir::Body<'tcx>,
basic_coverage_blocks: &CoverageGraph,
bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
extracted_mappings: &ExtractedMappings,
coverage_counters: &CoverageCounters,
) {
// Inject counter-increment statements into MIR.
@ -252,11 +252,16 @@ fn inject_coverage_statements<'tcx>(
// can check whether the injected statement survived MIR optimization.
// (BCB edges can't have spans, so we only need to process BCB nodes here.)
//
// We only do this for ordinary `Code` mappings, because branch and MC/DC
// mappings might have expressions that don't correspond to any single
// point in the control-flow graph.
//
// See the code in `rustc_codegen_llvm::coverageinfo::map_data` that deals
// with "expressions seen" and "zero terms".
let eligible_bcbs = extracted_mappings.bcbs_with_ordinary_code_mappings();
for (bcb, expression_id) in coverage_counters
.bcb_nodes_with_coverage_expressions()
.filter(|&(bcb, _)| bcb_has_coverage_spans(bcb))
.filter(|&(bcb, _)| eligible_bcbs.contains(bcb))
{
inject_statement(
mir_body,