From 9918693037dce8aa4bb6f08741b6812923486c18 Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Wed, 19 Jun 2024 11:26:03 +0200 Subject: Merging upstream version 1.76.0+dfsg1. Signed-off-by: Daniel Baumann --- .../rustc_mir_transform/src/coverage/counters.rs | 315 ++++++++++----------- compiler/rustc_mir_transform/src/coverage/graph.rs | 69 ++--- compiler/rustc_mir_transform/src/coverage/mod.rs | 176 ++++++------ compiler/rustc_mir_transform/src/coverage/query.rs | 4 +- compiler/rustc_mir_transform/src/coverage/spans.rs | 203 ++++++------- .../src/coverage/spans/from_mir.rs | 40 ++- .../src/coverage/test_macros/Cargo.toml | 7 - .../src/coverage/test_macros/src/lib.rs | 6 - compiler/rustc_mir_transform/src/coverage/tests.rs | 92 +++--- 9 files changed, 422 insertions(+), 490 deletions(-) delete mode 100644 compiler/rustc_mir_transform/src/coverage/test_macros/Cargo.toml delete mode 100644 compiler/rustc_mir_transform/src/coverage/test_macros/src/lib.rs (limited to 'compiler/rustc_mir_transform/src/coverage') diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index b34ec95b4..604589e5b 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -1,18 +1,16 @@ -use super::graph; - -use graph::{BasicCoverageBlock, BcbBranch, CoverageGraph, TraverseCoverageGraphWithLoops}; - use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::graph::WithNumNodes; use rustc_index::bit_set::BitSet; use rustc_index::IndexVec; use rustc_middle::mir::coverage::*; +use super::graph::{BasicCoverageBlock, CoverageGraph, TraverseCoverageGraphWithLoops}; + use std::fmt::{self, Debug}; /// The coverage counter or counter expression associated with a particular /// BCB node or BCB edge. -#[derive(Clone)] +#[derive(Clone, Copy)] pub(super) enum BcbCounter { Counter { id: CounterId }, Expression { id: ExpressionId }, @@ -88,11 +86,20 @@ impl CoverageCounters { BcbCounter::Counter { id } } - fn make_expression(&mut self, lhs: CovTerm, op: Op, rhs: CovTerm) -> BcbCounter { - let id = self.expressions.push(Expression { lhs, op, rhs }); + fn make_expression(&mut self, lhs: BcbCounter, op: Op, rhs: BcbCounter) -> BcbCounter { + let expression = Expression { lhs: lhs.as_term(), op, rhs: rhs.as_term() }; + let id = self.expressions.push(expression); BcbCounter::Expression { id } } + /// Variant of `make_expression` that makes `lhs` optional and assumes [`Op::Add`]. + /// + /// This is useful when using [`Iterator::fold`] to build an arbitrary-length sum. + fn make_sum_expression(&mut self, lhs: Option, rhs: BcbCounter) -> BcbCounter { + let Some(lhs) = lhs else { return rhs }; + self.make_expression(lhs, Op::Add, rhs) + } + /// Counter IDs start from one and go up. fn next_counter(&mut self) -> CounterId { let next = self.next_counter_id; @@ -109,7 +116,7 @@ impl CoverageCounters { self.expressions.len() } - fn set_bcb_counter(&mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter) -> CovTerm { + fn set_bcb_counter(&mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter) -> BcbCounter { assert!( // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also // have an expression (to be injected into an existing `BasicBlock` represented by this @@ -118,14 +125,13 @@ impl CoverageCounters { "attempt to add a `Counter` to a BCB target with existing incoming edge counters" ); - let term = counter_kind.as_term(); if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) { bug!( "attempt to set a BasicCoverageBlock coverage counter more than once; \ {bcb:?} already had counter {replaced:?}", ); } else { - term + counter_kind } } @@ -134,11 +140,13 @@ impl CoverageCounters { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, counter_kind: BcbCounter, - ) -> CovTerm { + ) -> BcbCounter { // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also // have an expression (to be injected into an existing `BasicBlock` represented by this // `BasicCoverageBlock`). - if let Some(node_counter) = self.bcb_counter(to_bcb) && !node_counter.is_expression() { + if let Some(node_counter) = self.bcb_counter(to_bcb) + && !node_counter.is_expression() + { bug!( "attempt to add an incoming edge counter from {from_bcb:?} \ when the target BCB already has {node_counter:?}" @@ -146,19 +154,18 @@ impl CoverageCounters { } self.bcb_has_incoming_edge_counters.insert(to_bcb); - let term = counter_kind.as_term(); if let Some(replaced) = self.bcb_edge_counters.insert((from_bcb, to_bcb), counter_kind) { bug!( "attempt to set an edge counter more than once; from_bcb: \ {from_bcb:?} already had counter {replaced:?}", ); } else { - term + counter_kind } } - pub(super) fn bcb_counter(&self, bcb: BasicCoverageBlock) -> Option<&BcbCounter> { - self.bcb_counters[bcb].as_ref() + pub(super) fn bcb_counter(&self, bcb: BasicCoverageBlock) -> Option { + self.bcb_counters[bcb] } pub(super) fn bcb_node_counters( @@ -220,15 +227,11 @@ impl<'a> MakeBcbCounters<'a> { // all `BasicCoverageBlock` nodes in the loop are visited before visiting any node outside // the loop. The `traversal` state includes a `context_stack`, providing a way to know if // the current BCB is in one or more nested loops or not. - let mut traversal = TraverseCoverageGraphWithLoops::new(&self.basic_coverage_blocks); + let mut traversal = TraverseCoverageGraphWithLoops::new(self.basic_coverage_blocks); while let Some(bcb) = traversal.next() { if bcb_has_coverage_spans(bcb) { debug!("{:?} has at least one coverage span. Get or make its counter", bcb); - let branching_counter_operand = self.get_or_make_counter_operand(bcb); - - if self.bcb_needs_branch_counters(bcb) { - self.make_branch_counters(&traversal, bcb, branching_counter_operand); - } + self.make_node_and_branch_counters(&traversal, bcb); } else { debug!( "{:?} does not have any coverage spans. A counter will only be added if \ @@ -245,100 +248,93 @@ impl<'a> MakeBcbCounters<'a> { ); } - fn make_branch_counters( + fn make_node_and_branch_counters( &mut self, traversal: &TraverseCoverageGraphWithLoops<'_>, - branching_bcb: BasicCoverageBlock, - branching_counter_operand: CovTerm, + from_bcb: BasicCoverageBlock, ) { - let branches = self.bcb_branches(branching_bcb); + // First, ensure that this node has a counter of some kind. + // We might also use its term later to compute one of the branch counters. + let from_bcb_operand = self.get_or_make_counter_operand(from_bcb); + + let branch_target_bcbs = self.basic_coverage_blocks.successors[from_bcb].as_slice(); + + // If this node doesn't have multiple out-edges, or all of its out-edges + // already have counters, then we don't need to create edge counters. + let needs_branch_counters = branch_target_bcbs.len() > 1 + && branch_target_bcbs + .iter() + .any(|&to_bcb| self.branch_has_no_counter(from_bcb, to_bcb)); + if !needs_branch_counters { + return; + } + debug!( - "{:?} has some branch(es) without counters:\n {}", - branching_bcb, - branches + "{from_bcb:?} has some branch(es) without counters:\n {}", + branch_target_bcbs .iter() - .map(|branch| { format!("{:?}: {:?}", branch, self.branch_counter(branch)) }) + .map(|&to_bcb| { + format!("{from_bcb:?}->{to_bcb:?}: {:?}", self.branch_counter(from_bcb, to_bcb)) + }) .collect::>() .join("\n "), ); - // Use the `traversal` state to decide if a subset of the branches exit a loop, making it - // likely that branch is executed less than branches that do not exit the same loop. In this - // case, any branch that does not exit the loop (and has not already been assigned a - // counter) should be counted by expression, if possible. (If a preferred expression branch - // is not selected based on the loop context, select any branch without an existing - // counter.) - let expression_branch = self.choose_preferred_expression_branch(traversal, &branches); - - // Assign a Counter or Expression to each branch, plus additional `Expression`s, as needed, - // to sum up intermediate results. - let mut some_sumup_counter_operand = None; - for branch in branches { - // Skip the selected `expression_branch`, if any. It's expression will be assigned after - // all others. - if branch != expression_branch { - let branch_counter_operand = if branch.is_only_path_to_target() { - debug!( - " {:?} has only one incoming edge (from {:?}), so adding a \ - counter", - branch, branching_bcb - ); - self.get_or_make_counter_operand(branch.target_bcb) - } else { - debug!(" {:?} has multiple incoming edges, so adding an edge counter", branch); - self.get_or_make_edge_counter_operand(branching_bcb, branch.target_bcb) - }; - if let Some(sumup_counter_operand) = - some_sumup_counter_operand.replace(branch_counter_operand) - { - let intermediate_expression = self.coverage_counters.make_expression( - branch_counter_operand, - Op::Add, - sumup_counter_operand, - ); - debug!(" [new intermediate expression: {:?}]", intermediate_expression); - let intermediate_expression_operand = intermediate_expression.as_term(); - some_sumup_counter_operand.replace(intermediate_expression_operand); - } - } - } + // Of the branch edges that don't have counters yet, one can be given an expression + // (computed from the other edges) instead of a dedicated counter. + let expression_to_bcb = self.choose_preferred_expression_branch(traversal, from_bcb); - // Assign the final expression to the `expression_branch` by subtracting the total of all - // other branches from the counter of the branching BCB. - let sumup_counter_operand = - some_sumup_counter_operand.expect("sumup_counter_operand should have a value"); + // For each branch arm other than the one that was chosen to get an expression, + // ensure that it has a counter (existing counter/expression or a new counter), + // and accumulate the corresponding terms into a single sum term. + let sum_of_all_other_branches: BcbCounter = { + let _span = debug_span!("sum_of_all_other_branches", ?expression_to_bcb).entered(); + branch_target_bcbs + .iter() + .copied() + // Skip the chosen branch, since we'll calculate it from the other branches. + .filter(|&to_bcb| to_bcb != expression_to_bcb) + .fold(None, |accum, to_bcb| { + let _span = debug_span!("to_bcb", ?accum, ?to_bcb).entered(); + let branch_counter = self.get_or_make_edge_counter_operand(from_bcb, to_bcb); + Some(self.coverage_counters.make_sum_expression(accum, branch_counter)) + }) + .expect("there must be at least one other branch") + }; + + // For the branch that was chosen to get an expression, create that expression + // by taking the count of the node we're branching from, and subtracting the + // sum of all the other branches. debug!( - "Making an expression for the selected expression_branch: {:?} \ - (expression_branch predecessors: {:?})", - expression_branch, - self.bcb_predecessors(expression_branch.target_bcb), + "Making an expression for the selected expression_branch: \ + {expression_to_bcb:?} (expression_branch predecessors: {:?})", + self.bcb_predecessors(expression_to_bcb), ); let expression = self.coverage_counters.make_expression( - branching_counter_operand, + from_bcb_operand, Op::Subtract, - sumup_counter_operand, + sum_of_all_other_branches, ); - debug!("{:?} gets an expression: {:?}", expression_branch, expression); - let bcb = expression_branch.target_bcb; - if expression_branch.is_only_path_to_target() { - self.coverage_counters.set_bcb_counter(bcb, expression); + debug!("{expression_to_bcb:?} gets an expression: {expression:?}"); + if self.basic_coverage_blocks.bcb_has_multiple_in_edges(expression_to_bcb) { + self.coverage_counters.set_bcb_edge_counter(from_bcb, expression_to_bcb, expression); } else { - self.coverage_counters.set_bcb_edge_counter(branching_bcb, bcb, expression); + self.coverage_counters.set_bcb_counter(expression_to_bcb, expression); } } #[instrument(level = "debug", skip(self))] - fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> CovTerm { + fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> BcbCounter { // If the BCB already has a counter, return it. - if let Some(counter_kind) = &self.coverage_counters.bcb_counters[bcb] { + if let Some(counter_kind) = self.coverage_counters.bcb_counters[bcb] { debug!("{bcb:?} already has a counter: {counter_kind:?}"); - return counter_kind.as_term(); + return counter_kind; } // A BCB with only one incoming edge gets a simple `Counter` (via `make_counter()`). // Also, a BCB that loops back to itself gets a simple `Counter`. This may indicate the // program results in a tight infinite loop, but it should still compile. - let one_path_to_target = self.bcb_has_one_path_to_target(bcb); + let one_path_to_target = !self.basic_coverage_blocks.bcb_has_multiple_in_edges(bcb); if one_path_to_target || self.bcb_predecessors(bcb).contains(&bcb) { let counter_kind = self.coverage_counters.make_counter(); if one_path_to_target { @@ -353,40 +349,25 @@ impl<'a> MakeBcbCounters<'a> { return self.coverage_counters.set_bcb_counter(bcb, counter_kind); } - // A BCB with multiple incoming edges can compute its count by `Expression`, summing up the - // counters and/or expressions of its incoming edges. This will recursively get or create - // counters for those incoming edges first, then call `make_expression()` to sum them up, - // with additional intermediate expressions as needed. - let _sumup_debug_span = debug_span!("(preparing sum-up expression)").entered(); - - let mut predecessors = self.bcb_predecessors(bcb).to_owned().into_iter(); - let first_edge_counter_operand = - self.get_or_make_edge_counter_operand(predecessors.next().unwrap(), bcb); - let mut some_sumup_edge_counter_operand = None; - for predecessor in predecessors { - let edge_counter_operand = self.get_or_make_edge_counter_operand(predecessor, bcb); - if let Some(sumup_edge_counter_operand) = - some_sumup_edge_counter_operand.replace(edge_counter_operand) - { - let intermediate_expression = self.coverage_counters.make_expression( - sumup_edge_counter_operand, - Op::Add, - edge_counter_operand, - ); - debug!("new intermediate expression: {intermediate_expression:?}"); - let intermediate_expression_operand = intermediate_expression.as_term(); - some_sumup_edge_counter_operand.replace(intermediate_expression_operand); - } - } - let counter_kind = self.coverage_counters.make_expression( - first_edge_counter_operand, - Op::Add, - some_sumup_edge_counter_operand.unwrap(), - ); - drop(_sumup_debug_span); - - debug!("{bcb:?} gets a new counter (sum of predecessor counters): {counter_kind:?}"); - self.coverage_counters.set_bcb_counter(bcb, counter_kind) + // A BCB with multiple incoming edges can compute its count by ensuring that counters + // exist for each of those edges, and then adding them up to get a total count. + let sum_of_in_edges: BcbCounter = { + let _span = debug_span!("sum_of_in_edges", ?bcb).entered(); + // We avoid calling `self.bcb_predecessors` here so that we can + // call methods on `&mut self` inside the fold. + self.basic_coverage_blocks.predecessors[bcb] + .iter() + .copied() + .fold(None, |accum, from_bcb| { + let _span = debug_span!("from_bcb", ?accum, ?from_bcb).entered(); + let edge_counter = self.get_or_make_edge_counter_operand(from_bcb, bcb); + Some(self.coverage_counters.make_sum_expression(accum, edge_counter)) + }) + .expect("there must be at least one in-edge") + }; + + debug!("{bcb:?} gets a new counter (sum of predecessor counters): {sum_of_in_edges:?}"); + self.coverage_counters.set_bcb_counter(bcb, sum_of_in_edges) } #[instrument(level = "debug", skip(self))] @@ -394,20 +375,26 @@ impl<'a> MakeBcbCounters<'a> { &mut self, from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, - ) -> CovTerm { + ) -> BcbCounter { + // If the target BCB has only one in-edge (i.e. this one), then create + // a node counter instead, since it will have the same value. + if !self.basic_coverage_blocks.bcb_has_multiple_in_edges(to_bcb) { + assert_eq!([from_bcb].as_slice(), self.basic_coverage_blocks.predecessors[to_bcb]); + return self.get_or_make_counter_operand(to_bcb); + } + // If the source BCB has only one successor (assumed to be the given target), an edge // counter is unnecessary. Just get or make a counter for the source BCB. - let successors = self.bcb_successors(from_bcb).iter(); - if successors.len() == 1 { + if self.bcb_successors(from_bcb).len() == 1 { return self.get_or_make_counter_operand(from_bcb); } // If the edge already has a counter, return it. - if let Some(counter_kind) = + if let Some(&counter_kind) = self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb)) { debug!("Edge {from_bcb:?}->{to_bcb:?} already has a counter: {counter_kind:?}"); - return counter_kind.as_term(); + return counter_kind; } // Make a new counter to count this edge. @@ -421,16 +408,19 @@ impl<'a> MakeBcbCounters<'a> { fn choose_preferred_expression_branch( &self, traversal: &TraverseCoverageGraphWithLoops<'_>, - branches: &[BcbBranch], - ) -> BcbBranch { - let good_reloop_branch = self.find_good_reloop_branch(traversal, &branches); - if let Some(reloop_branch) = good_reloop_branch { - assert!(self.branch_has_no_counter(&reloop_branch)); - debug!("Selecting reloop branch {reloop_branch:?} to get an expression"); - reloop_branch + from_bcb: BasicCoverageBlock, + ) -> BasicCoverageBlock { + let good_reloop_branch = self.find_good_reloop_branch(traversal, from_bcb); + if let Some(reloop_target) = good_reloop_branch { + assert!(self.branch_has_no_counter(from_bcb, reloop_target)); + debug!("Selecting reloop target {reloop_target:?} to get an expression"); + reloop_target } else { - let &branch_without_counter = - branches.iter().find(|&branch| self.branch_has_no_counter(branch)).expect( + let &branch_without_counter = self + .bcb_successors(from_bcb) + .iter() + .find(|&&to_bcb| self.branch_has_no_counter(from_bcb, to_bcb)) + .expect( "needs_branch_counters was `true` so there should be at least one \ branch", ); @@ -451,26 +441,28 @@ impl<'a> MakeBcbCounters<'a> { fn find_good_reloop_branch( &self, traversal: &TraverseCoverageGraphWithLoops<'_>, - branches: &[BcbBranch], - ) -> Option { + from_bcb: BasicCoverageBlock, + ) -> Option { + let branch_target_bcbs = self.bcb_successors(from_bcb); + // Consider each loop on the current traversal context stack, top-down. for reloop_bcbs in traversal.reloop_bcbs_per_loop() { let mut all_branches_exit_this_loop = true; // Try to find a branch that doesn't exit this loop and doesn't // already have a counter. - for &branch in branches { + for &branch_target_bcb in branch_target_bcbs { // A branch is a reloop branch if it dominates any BCB that has // an edge back to the loop header. (Other branches are exits.) let is_reloop_branch = reloop_bcbs.iter().any(|&reloop_bcb| { - self.basic_coverage_blocks.dominates(branch.target_bcb, reloop_bcb) + self.basic_coverage_blocks.dominates(branch_target_bcb, reloop_bcb) }); if is_reloop_branch { all_branches_exit_this_loop = false; - if self.branch_has_no_counter(&branch) { + if self.branch_has_no_counter(from_bcb, branch_target_bcb) { // We found a good branch to be given an expression. - return Some(branch); + return Some(branch_target_bcb); } // Keep looking for another reloop branch without a counter. } else { @@ -503,36 +495,23 @@ impl<'a> MakeBcbCounters<'a> { } #[inline] - fn bcb_branches(&self, from_bcb: BasicCoverageBlock) -> Vec { - self.bcb_successors(from_bcb) - .iter() - .map(|&to_bcb| BcbBranch::from_to(from_bcb, to_bcb, &self.basic_coverage_blocks)) - .collect::>() - } - - fn bcb_needs_branch_counters(&self, bcb: BasicCoverageBlock) -> bool { - let branch_needs_a_counter = |branch: &BcbBranch| self.branch_has_no_counter(branch); - let branches = self.bcb_branches(bcb); - branches.len() > 1 && branches.iter().any(branch_needs_a_counter) - } - - fn branch_has_no_counter(&self, branch: &BcbBranch) -> bool { - self.branch_counter(branch).is_none() + fn branch_has_no_counter( + &self, + from_bcb: BasicCoverageBlock, + to_bcb: BasicCoverageBlock, + ) -> bool { + self.branch_counter(from_bcb, to_bcb).is_none() } - fn branch_counter(&self, branch: &BcbBranch) -> Option<&BcbCounter> { - let to_bcb = branch.target_bcb; - if let Some(from_bcb) = branch.edge_from_bcb { + fn branch_counter( + &self, + from_bcb: BasicCoverageBlock, + to_bcb: BasicCoverageBlock, + ) -> Option<&BcbCounter> { + if self.basic_coverage_blocks.bcb_has_multiple_in_edges(to_bcb) { self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb)) } else { self.coverage_counters.bcb_counters[to_bcb].as_ref() } } - - /// Returns true if the BasicCoverageBlock has zero or one incoming edge. (If zero, it should be - /// the entry point for the function.) - #[inline] - fn bcb_has_one_path_to_target(&self, bcb: BasicCoverageBlock) -> bool { - self.bcb_predecessors(bcb).len() <= 1 - } } diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index 6bab62aa8..263bfdaaa 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -38,7 +38,7 @@ impl CoverageGraph { } let bcb_data = &bcbs[bcb]; let mut bcb_successors = Vec::new(); - for successor in bcb_filtered_successors(&mir_body, bcb_data.last_bb()) + for successor in bcb_filtered_successors(mir_body, bcb_data.last_bb()) .filter_map(|successor_bb| bb_to_bcb[successor_bb]) { if !seen[successor] { @@ -62,6 +62,14 @@ impl CoverageGraph { Self { bcbs, bb_to_bcb, successors, predecessors, dominators: None }; let dominators = dominators::dominators(&basic_coverage_blocks); basic_coverage_blocks.dominators = Some(dominators); + + // The coverage graph's entry-point node (bcb0) always starts with bb0, + // which never has predecessors. Any other blocks merged into bcb0 can't + // have multiple (coverage-relevant) predecessors, so bcb0 always has + // zero in-edges. + assert!(basic_coverage_blocks[START_BCB].leader_bb() == mir::START_BLOCK); + assert!(basic_coverage_blocks.predecessors[START_BCB].is_empty()); + basic_coverage_blocks } @@ -199,6 +207,25 @@ impl CoverageGraph { pub fn cmp_in_dominator_order(&self, a: BasicCoverageBlock, b: BasicCoverageBlock) -> Ordering { self.dominators.as_ref().unwrap().cmp_in_dominator_order(a, b) } + + /// Returns true if the given node has 2 or more in-edges, i.e. 2 or more + /// predecessors. + /// + /// This property is interesting to code that assigns counters to nodes and + /// edges, because if a node _doesn't_ have multiple in-edges, then there's + /// no benefit in having a separate counter for its in-edge, because it + /// would have the same value as the node's own counter. + /// + /// FIXME: That assumption might not be true for [`TerminatorKind::Yield`]? + #[inline(always)] + pub(super) fn bcb_has_multiple_in_edges(&self, bcb: BasicCoverageBlock) -> bool { + // Even though bcb0 conceptually has an extra virtual in-edge due to + // being the entry point, we've already asserted that it has no _other_ + // in-edges, so there's no possibility of it having _multiple_ in-edges. + // (And since its virtual in-edge doesn't exist in the graph, that edge + // can't have a separate counter anyway.) + self.predecessors[bcb].len() > 1 + } } impl Index for CoverageGraph { @@ -264,6 +291,7 @@ impl graph::WithPredecessors for CoverageGraph { rustc_index::newtype_index! { /// A node in the control-flow graph of CoverageGraph. + #[orderable] #[debug_format = "bcb{}"] pub(super) struct BasicCoverageBlock { const START_BCB = 0; @@ -318,45 +346,6 @@ impl BasicCoverageBlockData { } } -/// Represents a successor from a branching BasicCoverageBlock (such as the arms of a `SwitchInt`) -/// as either the successor BCB itself, if it has only one incoming edge, or the successor _plus_ -/// the specific branching BCB, representing the edge between the two. The latter case -/// distinguishes this incoming edge from other incoming edges to the same `target_bcb`. -#[derive(Clone, Copy, PartialEq, Eq)] -pub(super) struct BcbBranch { - pub edge_from_bcb: Option, - pub target_bcb: BasicCoverageBlock, -} - -impl BcbBranch { - pub fn from_to( - from_bcb: BasicCoverageBlock, - to_bcb: BasicCoverageBlock, - basic_coverage_blocks: &CoverageGraph, - ) -> Self { - let edge_from_bcb = if basic_coverage_blocks.predecessors[to_bcb].len() > 1 { - Some(from_bcb) - } else { - None - }; - Self { edge_from_bcb, target_bcb: to_bcb } - } - - pub fn is_only_path_to_target(&self) -> bool { - self.edge_from_bcb.is_none() - } -} - -impl std::fmt::Debug for BcbBranch { - fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if let Some(from_bcb) = self.edge_from_bcb { - write!(fmt, "{:?}->{:?}", from_bcb, self.target_bcb) - } else { - write!(fmt, "{:?}", self.target_bcb) - } - } -} - // Returns the subset of a block's successors that are relevant to the coverage // graph, i.e. those that do not represent unwinds or unreachable branches. // FIXME(#78544): MIR InstrumentCoverage: Improve coverage of `#[should_panic]` tests and diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 97e4468a0..c5a339128 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -13,7 +13,6 @@ use self::spans::CoverageSpans; use crate::MirPass; -use rustc_data_structures::sync::Lrc; use rustc_middle::hir; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::coverage::*; @@ -22,9 +21,9 @@ use rustc_middle::mir::{ TerminatorKind, }; use rustc_middle::ty::TyCtxt; -use rustc_span::def_id::DefId; +use rustc_span::def_id::LocalDefId; use rustc_span::source_map::SourceMap; -use rustc_span::{ExpnKind, SourceFile, Span, Symbol}; +use rustc_span::{ExpnKind, Span, Symbol}; /// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected /// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen @@ -39,31 +38,19 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage { fn run_pass(&self, tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) { let mir_source = mir_body.source; - // If the InstrumentCoverage pass is called on promoted MIRs, skip them. - // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 - if mir_source.promoted.is_some() { - trace!( - "InstrumentCoverage skipped for {:?} (already promoted for Miri evaluation)", - mir_source.def_id() - ); - return; - } + // This pass runs after MIR promotion, but before promoted MIR starts to + // be transformed, so it should never see promoted MIR. + assert!(mir_source.promoted.is_none()); + + let def_id = mir_source.def_id().expect_local(); - let is_fn_like = - tcx.hir().get_by_def_id(mir_source.def_id().expect_local()).fn_kind().is_some(); - - // Only instrument functions, methods, and closures (not constants since they are evaluated - // at compile time by Miri). - // FIXME(#73156): Handle source code coverage in const eval, but note, if and when const - // expressions get coverage spans, we will probably have to "carve out" space for const - // expressions from coverage spans in enclosing MIR's, like we do for closures. (That might - // be tricky if const expressions have no corresponding statements in the enclosing MIR. - // Closures are carved out by their initial `Assign` statement.) - if !is_fn_like { - trace!("InstrumentCoverage skipped for {:?} (not an fn-like)", mir_source.def_id()); + if !is_eligible_for_coverage(tcx, def_id) { + trace!("InstrumentCoverage skipped for {def_id:?} (not eligible)"); return; } + // An otherwise-eligible function is still skipped if its start block + // is known to be unreachable. match mir_body.basic_blocks[mir::START_BLOCK].terminator().kind { TerminatorKind::Unreachable => { trace!("InstrumentCoverage skipped for unreachable `START_BLOCK`"); @@ -72,81 +59,43 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage { _ => {} } - let codegen_fn_attrs = tcx.codegen_fn_attrs(mir_source.def_id()); - if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_COVERAGE) { - return; - } - - trace!("InstrumentCoverage starting for {:?}", mir_source.def_id()); + trace!("InstrumentCoverage starting for {def_id:?}"); Instrumentor::new(tcx, mir_body).inject_counters(); - trace!("InstrumentCoverage done for {:?}", mir_source.def_id()); + trace!("InstrumentCoverage done for {def_id:?}"); } } struct Instrumentor<'a, 'tcx> { tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>, - source_file: Lrc, - fn_sig_span: Span, - body_span: Span, - function_source_hash: u64, + hir_info: ExtractedHirInfo, basic_coverage_blocks: CoverageGraph, coverage_counters: CoverageCounters, } impl<'a, 'tcx> Instrumentor<'a, 'tcx> { fn new(tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self { - let source_map = tcx.sess.source_map(); - let def_id = mir_body.source.def_id(); - let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id); - - let body_span = get_body_span(tcx, hir_body, mir_body); - - let source_file = source_map.lookup_source_file(body_span.lo()); - let fn_sig_span = match some_fn_sig.filter(|fn_sig| { - fn_sig.span.eq_ctxt(body_span) - && Lrc::ptr_eq(&source_file, &source_map.lookup_source_file(fn_sig.span.lo())) - }) { - Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()), - None => body_span.shrink_to_lo(), - }; + let hir_info = extract_hir_info(tcx, mir_body.source.def_id().expect_local()); - debug!( - "instrumenting {}: {:?}, fn sig span: {:?}, body span: {:?}", - if tcx.is_closure(def_id) { "closure" } else { "function" }, - def_id, - fn_sig_span, - body_span - ); + debug!(?hir_info, "instrumenting {:?}", mir_body.source.def_id()); - let function_source_hash = hash_mir_source(tcx, hir_body); let basic_coverage_blocks = CoverageGraph::from_mir(mir_body); let coverage_counters = CoverageCounters::new(&basic_coverage_blocks); - Self { - tcx, - mir_body, - source_file, - fn_sig_span, - body_span, - function_source_hash, - basic_coverage_blocks, - coverage_counters, - } + Self { tcx, mir_body, hir_info, basic_coverage_blocks, coverage_counters } } fn inject_counters(&'a mut self) { - let fn_sig_span = self.fn_sig_span; - let body_span = self.body_span; - //////////////////////////////////////////////////// // Compute coverage spans from the `CoverageGraph`. - let coverage_spans = CoverageSpans::generate_coverage_spans( - &self.mir_body, - fn_sig_span, - body_span, + let Some(coverage_spans) = CoverageSpans::generate_coverage_spans( + self.mir_body, + &self.hir_info, &self.basic_coverage_blocks, - ); + ) else { + // No relevant spans were found in MIR, so skip instrumenting this function. + return; + }; //////////////////////////////////////////////////// // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure @@ -160,7 +109,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { let mappings = self.create_mappings_and_inject_coverage_statements(&coverage_spans); self.mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { - function_source_hash: self.function_source_hash, + function_source_hash: self.hir_info.function_source_hash, num_counters: self.coverage_counters.num_counters(), expressions: self.coverage_counters.take_expressions(), mappings, @@ -175,11 +124,12 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { coverage_spans: &CoverageSpans, ) -> Vec { let source_map = self.tcx.sess.source_map(); - let body_span = self.body_span; + let body_span = self.hir_info.body_span; + let source_file = source_map.lookup_source_file(body_span.lo()); use rustc_session::RemapFileNameExt; let file_name = - Symbol::intern(&self.source_file.name.for_codegen(self.tcx.sess).to_string_lossy()); + Symbol::intern(&source_file.name.for_codegen(self.tcx.sess).to_string_lossy()); let mut mappings = Vec::new(); @@ -240,7 +190,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { ); // Inject a counter into the newly-created BB. - inject_statement(self.mir_body, self.make_mir_coverage_kind(&counter_kind), new_bb); + inject_statement(self.mir_body, self.make_mir_coverage_kind(counter_kind), new_bb); } mappings @@ -325,27 +275,77 @@ fn make_code_region( } } -fn fn_sig_and_body( - tcx: TyCtxt<'_>, - def_id: DefId, -) -> (Option<&rustc_hir::FnSig<'_>>, &rustc_hir::Body<'_>) { +fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { + // Only instrument functions, methods, and closures (not constants since they are evaluated + // at compile time by Miri). + // FIXME(#73156): Handle source code coverage in const eval, but note, if and when const + // expressions get coverage spans, we will probably have to "carve out" space for const + // expressions from coverage spans in enclosing MIR's, like we do for closures. (That might + // be tricky if const expressions have no corresponding statements in the enclosing MIR. + // Closures are carved out by their initial `Assign` statement.) + if !tcx.def_kind(def_id).is_fn_like() { + trace!("InstrumentCoverage skipped for {def_id:?} (not an fn-like)"); + return false; + } + + if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NO_COVERAGE) { + return false; + } + + true +} + +/// Function information extracted from HIR by the coverage instrumentor. +#[derive(Debug)] +struct ExtractedHirInfo { + function_source_hash: u64, + is_async_fn: bool, + fn_sig_span: Span, + body_span: Span, +} + +fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHirInfo { // FIXME(#79625): Consider improving MIR to provide the information needed, to avoid going back // to HIR for it. - let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); + + let hir_node = tcx.hir_node_by_def_id(def_id); let (_, fn_body_id) = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); - (hir_node.fn_sig(), tcx.hir().body(fn_body_id)) + let hir_body = tcx.hir().body(fn_body_id); + + let is_async_fn = hir_node.fn_sig().is_some_and(|fn_sig| fn_sig.header.is_async()); + let body_span = get_body_span(tcx, hir_body, def_id); + + // The actual signature span is only used if it has the same context and + // filename as the body, and precedes the body. + let maybe_fn_sig_span = hir_node.fn_sig().map(|fn_sig| fn_sig.span); + let fn_sig_span = maybe_fn_sig_span + .filter(|&fn_sig_span| { + let source_map = tcx.sess.source_map(); + let file_idx = |span: Span| source_map.lookup_source_file_idx(span.lo()); + + fn_sig_span.eq_ctxt(body_span) + && fn_sig_span.hi() <= body_span.lo() + && file_idx(fn_sig_span) == file_idx(body_span) + }) + // If so, extend it to the start of the body span. + .map(|fn_sig_span| fn_sig_span.with_hi(body_span.lo())) + // Otherwise, create a dummy signature span at the start of the body. + .unwrap_or_else(|| body_span.shrink_to_lo()); + + let function_source_hash = hash_mir_source(tcx, hir_body); + + ExtractedHirInfo { function_source_hash, is_async_fn, fn_sig_span, body_span } } fn get_body_span<'tcx>( tcx: TyCtxt<'tcx>, hir_body: &rustc_hir::Body<'tcx>, - mir_body: &mut mir::Body<'tcx>, + def_id: LocalDefId, ) -> Span { let mut body_span = hir_body.value.span; - let def_id = mir_body.source.def_id(); - if tcx.is_closure(def_id) { + if tcx.is_closure(def_id.to_def_id()) { // If the MIR function is a closure, and if the closure body span // starts from a macro, but it's content is not in that macro, try // to find a non-macro callsite, and instrument the spans there diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 809407f89..dfc7c3a71 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -2,9 +2,9 @@ use super::*; use rustc_data_structures::captures::Captures; use rustc_middle::mir::coverage::*; -use rustc_middle::mir::{Body, Coverage, CoverageIdsInfo}; +use rustc_middle::mir::{Body, CoverageIdsInfo}; use rustc_middle::query::Providers; -use rustc_middle::ty::{self, TyCtxt}; +use rustc_middle::ty::{self}; /// A `query` provider for retrieving coverage information injected into MIR. pub(crate) fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index b318134ae..ae43a18ad 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -6,6 +6,7 @@ use rustc_middle::mir; use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol, DUMMY_SP}; use super::graph::{BasicCoverageBlock, CoverageGraph, START_BCB}; +use crate::coverage::ExtractedHirInfo; mod from_mir; @@ -15,26 +16,32 @@ pub(super) struct CoverageSpans { } impl CoverageSpans { + /// Extracts coverage-relevant spans from MIR, and associates them with + /// their corresponding BCBs. + /// + /// Returns `None` if no coverage-relevant spans could be extracted. pub(super) fn generate_coverage_spans( mir_body: &mir::Body<'_>, - fn_sig_span: Span, - body_span: Span, + hir_info: &ExtractedHirInfo, basic_coverage_blocks: &CoverageGraph, - ) -> Self { + ) -> Option { let coverage_spans = CoverageSpansGenerator::generate_coverage_spans( mir_body, - fn_sig_span, - body_span, + hir_info, basic_coverage_blocks, ); + if coverage_spans.is_empty() { + return None; + } + // Group the coverage spans by BCB, with the BCBs in sorted order. let mut bcb_to_spans = IndexVec::from_elem_n(Vec::new(), basic_coverage_blocks.num_nodes()); for CoverageSpan { bcb, span, .. } in coverage_spans { bcb_to_spans[bcb].push(span); } - Self { bcb_to_spans } + Some(Self { bcb_to_spans }) } pub(super) fn bcb_has_coverage_spans(&self, bcb: BasicCoverageBlock) -> bool { @@ -89,10 +96,10 @@ impl CoverageSpan { } } - pub fn merge_from(&mut self, mut other: CoverageSpan) { - debug_assert!(self.is_mergeable(&other)); + pub fn merge_from(&mut self, other: &Self) { + debug_assert!(self.is_mergeable(other)); self.span = self.span.to(other.span); - self.merged_spans.append(&mut other.merged_spans); + self.merged_spans.extend_from_slice(&other.merged_spans); } pub fn cutoff_statements_at(&mut self, cutoff_pos: BytePos) { @@ -129,16 +136,14 @@ impl CoverageSpan { /// If the span is part of a macro, and the macro is visible (expands directly to the given /// body_span), returns the macro name symbol. pub fn visible_macro(&self, body_span: Span) -> Option { - if let Some(current_macro) = self.current_macro() - && self - .expn_span - .parent_callsite() - .unwrap_or_else(|| bug!("macro must have a parent")) - .eq_ctxt(body_span) - { - return Some(current_macro); - } - None + let current_macro = self.current_macro()?; + let parent_callsite = self.expn_span.parent_callsite()?; + + // In addition to matching the context of the body span, the parent callsite + // must also be the source callsite, i.e. the parent must have no parent. + let is_visible_macro = + parent_callsite.parent_callsite().is_none() && parent_callsite.eq_ctxt(body_span); + is_visible_macro.then_some(current_macro) } pub fn is_macro_expansion(&self) -> bool { @@ -224,19 +229,17 @@ impl<'a> CoverageSpansGenerator<'a> { /// to be). pub(super) fn generate_coverage_spans( mir_body: &mir::Body<'_>, - fn_sig_span: Span, // Ensured to be same SourceFile and SyntaxContext as `body_span` - body_span: Span, + hir_info: &ExtractedHirInfo, basic_coverage_blocks: &'a CoverageGraph, ) -> Vec { let sorted_spans = from_mir::mir_to_initial_sorted_coverage_spans( mir_body, - fn_sig_span, - body_span, + hir_info, basic_coverage_blocks, ); let coverage_spans = Self { - body_span, + body_span: hir_info.body_span, basic_coverage_blocks, sorted_spans_iter: sorted_spans.into_iter(), some_curr: None, @@ -269,7 +272,7 @@ impl<'a> CoverageSpansGenerator<'a> { if curr.is_mergeable(prev) { debug!(" same bcb (and neither is a closure), merge with prev={prev:?}"); let prev = self.take_prev(); - self.curr_mut().merge_from(prev); + self.curr_mut().merge_from(&prev); self.maybe_push_macro_name_span(); // Note that curr.span may now differ from curr_original_span } else if prev.span.hi() <= curr.span.lo() { @@ -277,7 +280,7 @@ impl<'a> CoverageSpansGenerator<'a> { " different bcbs and disjoint spans, so keep curr for next iter, and add prev={prev:?}", ); let prev = self.take_prev(); - self.push_refined_span(prev); + self.refined_spans.push(prev); self.maybe_push_macro_name_span(); } else if prev.is_closure { // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the @@ -321,33 +324,30 @@ impl<'a> CoverageSpansGenerator<'a> { } } - let prev = self.take_prev(); - debug!(" AT END, adding last prev={prev:?}"); - - // Take `pending_dups` so that we can drain it while calling self methods. - // It is never used as a field after this point. - for dup in std::mem::take(&mut self.pending_dups) { + // Drain any remaining dups into the output. + for dup in self.pending_dups.drain(..) { debug!(" ...adding at least one pending dup={:?}", dup); - self.push_refined_span(dup); + self.refined_spans.push(dup); } - // Async functions wrap a closure that implements the body to be executed. The enclosing - // function is called and returns an `impl Future` without initially executing any of the - // body. To avoid showing the return from the enclosing function as a "covered" return from - // the closure, the enclosing function's `TerminatorKind::Return`s `CoverageSpan` is - // excluded. The closure's `Return` is the only one that will be counted. This provides - // adequate coverage, and more intuitive counts. (Avoids double-counting the closing brace - // of the function body.) - let body_ends_with_closure = if let Some(last_covspan) = self.refined_spans.last() { - last_covspan.is_closure && last_covspan.span.hi() == self.body_span.hi() - } else { - false - }; - - if !body_ends_with_closure { - self.push_refined_span(prev); + // There is usually a final span remaining in `prev` after the loop ends, + // so add it to the output as well. + if let Some(prev) = self.some_prev.take() { + debug!(" AT END, adding last prev={prev:?}"); + self.refined_spans.push(prev); } + // Do one last merge pass, to simplify the output. + self.refined_spans.dedup_by(|b, a| { + if a.is_mergeable(b) { + debug!(?a, ?b, "merging list-adjacent refined spans"); + a.merge_from(b); + true + } else { + false + } + }); + // Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage // regions for the current function leave room for the closure's own coverage regions // (injected separately, from the closure's own MIR). @@ -355,18 +355,6 @@ impl<'a> CoverageSpansGenerator<'a> { self.refined_spans } - fn push_refined_span(&mut self, covspan: CoverageSpan) { - if let Some(last) = self.refined_spans.last_mut() - && last.is_mergeable(&covspan) - { - // Instead of pushing the new span, merge it with the last refined span. - debug!(?last, ?covspan, "merging new refined span with last refined span"); - last.merge_from(covspan); - } else { - self.refined_spans.push(covspan); - } - } - /// If `curr` is part of a new macro expansion, carve out and push a separate /// span that ends just after the macro name and its subsequent `!`. fn maybe_push_macro_name_span(&mut self) { @@ -379,57 +367,59 @@ impl<'a> CoverageSpansGenerator<'a> { return; } - let merged_prefix_len = self.curr_original_span.lo() - curr.span.lo(); - let after_macro_bang = merged_prefix_len + BytePos(visible_macro.as_str().len() as u32 + 1); - if self.curr().span.lo() + after_macro_bang > self.curr().span.hi() { + // The split point is relative to `curr_original_span`, + // because `curr.span` may have been merged with preceding spans. + let split_point_after_macro_bang = self.curr_original_span.lo() + + BytePos(visible_macro.as_str().len() as u32) + + BytePos(1); // add 1 for the `!` + debug_assert!(split_point_after_macro_bang <= curr.span.hi()); + if split_point_after_macro_bang > curr.span.hi() { // Something is wrong with the macro name span; - // return now to avoid emitting malformed mappings. - // FIXME(#117788): Track down why this happens. + // return now to avoid emitting malformed mappings (e.g. #117788). return; } + let mut macro_name_cov = curr.clone(); - self.curr_mut().span = curr.span.with_lo(curr.span.lo() + after_macro_bang); - macro_name_cov.span = - macro_name_cov.span.with_hi(macro_name_cov.span.lo() + after_macro_bang); + macro_name_cov.span = macro_name_cov.span.with_hi(split_point_after_macro_bang); + self.curr_mut().span = curr.span.with_lo(split_point_after_macro_bang); + debug!( " and curr starts a new macro expansion, so add a new span just for \ the macro `{visible_macro}!`, new span={macro_name_cov:?}", ); - self.push_refined_span(macro_name_cov); + self.refined_spans.push(macro_name_cov); } + #[track_caller] fn curr(&self) -> &CoverageSpan { - self.some_curr - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + self.some_curr.as_ref().unwrap_or_else(|| bug!("some_curr is None (curr)")) } + #[track_caller] fn curr_mut(&mut self) -> &mut CoverageSpan { - self.some_curr - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + self.some_curr.as_mut().unwrap_or_else(|| bug!("some_curr is None (curr_mut)")) } /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the /// `curr` coverage span. + #[track_caller] fn take_curr(&mut self) -> CoverageSpan { - self.some_curr.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + self.some_curr.take().unwrap_or_else(|| bug!("some_curr is None (take_curr)")) } + #[track_caller] fn prev(&self) -> &CoverageSpan { - self.some_prev - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + self.some_prev.as_ref().unwrap_or_else(|| bug!("some_prev is None (prev)")) } + #[track_caller] fn prev_mut(&mut self) -> &mut CoverageSpan { - self.some_prev - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + self.some_prev.as_mut().unwrap_or_else(|| bug!("some_prev is None (prev_mut)")) } + #[track_caller] fn take_prev(&mut self) -> CoverageSpan { - self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + self.some_prev.take().unwrap_or_else(|| bug!("some_prev is None (take_prev)")) } /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the @@ -452,19 +442,14 @@ impl<'a> CoverageSpansGenerator<'a> { previous iteration, or prev started a new disjoint span" ); if last_dup.span.hi() <= self.curr().span.lo() { - // Temporarily steal `pending_dups` into a local, so that we can - // drain it while calling other self methods. - let mut pending_dups = std::mem::take(&mut self.pending_dups); - for dup in pending_dups.drain(..) { + for dup in self.pending_dups.drain(..) { debug!(" ...adding at least one pending={:?}", dup); - self.push_refined_span(dup); + self.refined_spans.push(dup); } - // The list of dups is now empty, but we can recycle its capacity. - assert!(pending_dups.is_empty() && self.pending_dups.is_empty()); - self.pending_dups = pending_dups; } else { self.pending_dups.clear(); } + assert!(self.pending_dups.is_empty()); } /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. @@ -475,7 +460,9 @@ impl<'a> CoverageSpansGenerator<'a> { } while let Some(curr) = self.sorted_spans_iter.next() { debug!("FOR curr={:?}", curr); - if let Some(prev) = &self.some_prev && prev.span.lo() > curr.span.lo() { + if let Some(prev) = &self.some_prev + && prev.span.lo() > curr.span.lo() + { // Skip curr because prev has already advanced beyond the end of curr. // This can only happen if a prior iteration updated `prev` to skip past // a region of code, such as skipping past a closure. @@ -509,22 +496,18 @@ impl<'a> CoverageSpansGenerator<'a> { let has_pre_closure_span = prev.span.lo() < right_cutoff; let has_post_closure_span = prev.span.hi() > right_cutoff; - // Temporarily steal `pending_dups` into a local, so that we can - // mutate and/or drain it while calling other self methods. - let mut pending_dups = std::mem::take(&mut self.pending_dups); - if has_pre_closure_span { let mut pre_closure = self.prev().clone(); pre_closure.span = pre_closure.span.with_hi(left_cutoff); debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); - if !pending_dups.is_empty() { - for mut dup in pending_dups.iter().cloned() { - dup.span = dup.span.with_hi(left_cutoff); - debug!(" ...and at least one pre_closure dup={:?}", dup); - self.push_refined_span(dup); - } + + for mut dup in self.pending_dups.iter().cloned() { + dup.span = dup.span.with_hi(left_cutoff); + debug!(" ...and at least one pre_closure dup={:?}", dup); + self.refined_spans.push(dup); } - self.push_refined_span(pre_closure); + + self.refined_spans.push(pre_closure); } if has_post_closure_span { @@ -533,19 +516,17 @@ impl<'a> CoverageSpansGenerator<'a> { // about how the `CoverageSpan`s are ordered.) self.prev_mut().span = self.prev().span.with_lo(right_cutoff); debug!(" Mutated prev.span to start after the closure. prev={:?}", self.prev()); - for dup in pending_dups.iter_mut() { + + for dup in &mut self.pending_dups { debug!(" ...and at least one overlapping dup={:?}", dup); dup.span = dup.span.with_lo(right_cutoff); } + let closure_covspan = self.take_curr(); // Prevent this curr from becoming prev. - self.push_refined_span(closure_covspan); // since self.prev() was already updated + self.refined_spans.push(closure_covspan); // since self.prev() was already updated } else { - pending_dups.clear(); + self.pending_dups.clear(); } - - // Restore the modified post-closure spans, or the empty vector's capacity. - assert!(self.pending_dups.is_empty()); - self.pending_dups = pending_dups; } /// Called if `curr.span` equals `prev_original_span` (and potentially equal to all @@ -641,7 +622,7 @@ impl<'a> CoverageSpansGenerator<'a> { } else { debug!(" ... adding modified prev={:?}", self.prev()); let prev = self.take_prev(); - self.push_refined_span(prev); + self.refined_spans.push(prev); } } else { // with `pending_dups`, `prev` cannot have any statements that don't overlap diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 6189e5379..a9c4ea33d 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -7,13 +7,22 @@ use rustc_span::Span; use crate::coverage::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; use crate::coverage::spans::CoverageSpan; +use crate::coverage::ExtractedHirInfo; pub(super) fn mir_to_initial_sorted_coverage_spans( mir_body: &mir::Body<'_>, - fn_sig_span: Span, - body_span: Span, + hir_info: &ExtractedHirInfo, basic_coverage_blocks: &CoverageGraph, ) -> Vec { + let &ExtractedHirInfo { is_async_fn, fn_sig_span, body_span, .. } = hir_info; + if is_async_fn { + // An async function desugars into a function that returns a future, + // with the user code wrapped in a closure. Any spans in the desugared + // outer function will be unhelpful, so just produce a single span + // associating the function signature with its entry BCB. + return vec![CoverageSpan::for_fn_sig(fn_sig_span)]; + } + let mut initial_spans = Vec::with_capacity(mir_body.basic_blocks.len() * 2); for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() { initial_spans.extend(bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data)); @@ -63,14 +72,14 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( let statement_spans = data.statements.iter().filter_map(move |statement| { let expn_span = filtered_statement_span(statement)?; - let span = function_source_span(expn_span, body_span); + let span = unexpand_into_body_span(expn_span, body_span)?; Some(CoverageSpan::new(span, expn_span, bcb, is_closure(statement))) }); let terminator_span = Some(data.terminator()).into_iter().filter_map(move |terminator| { let expn_span = filtered_terminator_span(terminator)?; - let span = function_source_span(expn_span, body_span); + let span = unexpand_into_body_span(expn_span, body_span)?; Some(CoverageSpan::new(span, expn_span, bcb, false)) }); @@ -92,13 +101,13 @@ fn is_closure(statement: &Statement<'_>) -> bool { /// If the MIR `Statement` has a span contributive to computing coverage spans, /// return it; otherwise return `None`. fn filtered_statement_span(statement: &Statement<'_>) -> Option { + use mir::coverage::CoverageKind; + match statement.kind { // These statements have spans that are often outside the scope of the executed source code // for their parent `BasicBlock`. StatementKind::StorageLive(_) | StatementKind::StorageDead(_) - // Coverage should not be encountered, but don't inject coverage coverage - | StatementKind::Coverage(_) // Ignore `ConstEvalCounter`s | StatementKind::ConstEvalCounter // Ignore `Nop`s @@ -122,9 +131,13 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option { // If and when the Issue is resolved, remove this special case match pattern: StatementKind::FakeRead(box (FakeReadCause::ForGuardBinding, _)) => None, - // Retain spans from all other statements + // Retain spans from most other statements. StatementKind::FakeRead(box (_, _)) // Not including `ForGuardBinding` | StatementKind::Intrinsic(..) + | StatementKind::Coverage(box mir::Coverage { + // The purpose of `SpanMarker` is to be matched and accepted here. + kind: CoverageKind::SpanMarker + }) | StatementKind::Assign(_) | StatementKind::SetDiscriminant { .. } | StatementKind::Deinit(..) @@ -133,6 +146,11 @@ fn filtered_statement_span(statement: &Statement<'_>) -> Option { | StatementKind::AscribeUserType(_, _) => { Some(statement.source_info.span) } + + StatementKind::Coverage(box mir::Coverage { + // These coverage statements should not exist prior to coverage instrumentation. + kind: CoverageKind::CounterIncrement { .. } | CoverageKind::ExpressionUsed { .. } + }) => bug!("Unexpected coverage statement found during coverage instrumentation: {statement:?}"), } } @@ -180,14 +198,16 @@ fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option { /// Returns an extrapolated span (pre-expansion[^1]) corresponding to a range /// within the function's body source. This span is guaranteed to be contained /// within, or equal to, the `body_span`. If the extrapolated span is not -/// contained within the `body_span`, the `body_span` is returned. +/// contained within the `body_span`, `None` is returned. /// /// [^1]Expansions result from Rust syntax including macros, syntactic sugar, /// etc.). #[inline] -fn function_source_span(span: Span, body_span: Span) -> Span { +fn unexpand_into_body_span(span: Span, body_span: Span) -> Option { use rustc_span::source_map::original_sp; + // FIXME(#118525): Consider switching from `original_sp` to `Span::find_ancestor_inside`, + // which is similar but gives slightly different results in some edge cases. let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt()); - if body_span.contains(original_span) { original_span } else { body_span } + body_span.contains(original_span).then_some(original_span) } diff --git a/compiler/rustc_mir_transform/src/coverage/test_macros/Cargo.toml b/compiler/rustc_mir_transform/src/coverage/test_macros/Cargo.toml deleted file mode 100644 index f753caa91..000000000 --- a/compiler/rustc_mir_transform/src/coverage/test_macros/Cargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "coverage_test_macros" -version = "0.0.0" -edition = "2021" - -[lib] -proc-macro = true diff --git a/compiler/rustc_mir_transform/src/coverage/test_macros/src/lib.rs b/compiler/rustc_mir_transform/src/coverage/test_macros/src/lib.rs deleted file mode 100644 index f41adf667..000000000 --- a/compiler/rustc_mir_transform/src/coverage/test_macros/src/lib.rs +++ /dev/null @@ -1,6 +0,0 @@ -use proc_macro::TokenStream; - -#[proc_macro] -pub fn let_bcb(item: TokenStream) -> TokenStream { - format!("let bcb{item} = graph::BasicCoverageBlock::from_usize({item});").parse().unwrap() -} diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index 702fe5f56..931bc8e58 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -27,15 +27,17 @@ use super::counters; use super::graph::{self, BasicCoverageBlock}; -use coverage_test_macros::let_bcb; - use itertools::Itertools; use rustc_data_structures::graph::WithNumNodes; use rustc_data_structures::graph::WithSuccessors; use rustc_index::{Idx, IndexVec}; use rustc_middle::mir::*; use rustc_middle::ty; -use rustc_span::{self, BytePos, Pos, Span, DUMMY_SP}; +use rustc_span::{BytePos, Pos, Span, DUMMY_SP}; + +fn bcb(index: u32) -> BasicCoverageBlock { + BasicCoverageBlock::from_u32(index) +} // All `TEMP_BLOCK` targets should be replaced before calling `to_body() -> mir::Body`. const TEMP_BLOCK: BasicBlock = BasicBlock::MAX; @@ -300,12 +302,15 @@ fn goto_switchint<'a>() -> Body<'a> { mir_body } -macro_rules! assert_successors { - ($basic_coverage_blocks:ident, $i:ident, [$($successor:ident),*]) => { - let mut successors = $basic_coverage_blocks.successors[$i].clone(); - successors.sort_unstable(); - assert_eq!(successors, vec![$($successor),*]); - } +#[track_caller] +fn assert_successors( + basic_coverage_blocks: &graph::CoverageGraph, + bcb: BasicCoverageBlock, + expected_successors: &[BasicCoverageBlock], +) { + let mut successors = basic_coverage_blocks.successors[bcb].clone(); + successors.sort_unstable(); + assert_eq!(successors, expected_successors); } #[test] @@ -334,13 +339,9 @@ fn test_covgraph_goto_switchint() { basic_coverage_blocks.iter_enumerated().collect::>() ); - let_bcb!(0); - let_bcb!(1); - let_bcb!(2); - - assert_successors!(basic_coverage_blocks, bcb0, [bcb1, bcb2]); - assert_successors!(basic_coverage_blocks, bcb1, []); - assert_successors!(basic_coverage_blocks, bcb2, []); + assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1), bcb(2)]); + assert_successors(&basic_coverage_blocks, bcb(1), &[]); + assert_successors(&basic_coverage_blocks, bcb(2), &[]); } /// Create a mock `Body` with a loop. @@ -418,15 +419,10 @@ fn test_covgraph_switchint_then_loop_else_return() { basic_coverage_blocks.iter_enumerated().collect::>() ); - let_bcb!(0); - let_bcb!(1); - let_bcb!(2); - let_bcb!(3); - - assert_successors!(basic_coverage_blocks, bcb0, [bcb1]); - assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]); - assert_successors!(basic_coverage_blocks, bcb2, []); - assert_successors!(basic_coverage_blocks, bcb3, [bcb1]); + assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1)]); + assert_successors(&basic_coverage_blocks, bcb(1), &[bcb(2), bcb(3)]); + assert_successors(&basic_coverage_blocks, bcb(2), &[]); + assert_successors(&basic_coverage_blocks, bcb(3), &[bcb(1)]); } /// Create a mock `Body` with nested loops. @@ -546,21 +542,13 @@ fn test_covgraph_switchint_loop_then_inner_loop_else_break() { basic_coverage_blocks.iter_enumerated().collect::>() ); - let_bcb!(0); - let_bcb!(1); - let_bcb!(2); - let_bcb!(3); - let_bcb!(4); - let_bcb!(5); - let_bcb!(6); - - assert_successors!(basic_coverage_blocks, bcb0, [bcb1]); - assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]); - assert_successors!(basic_coverage_blocks, bcb2, []); - assert_successors!(basic_coverage_blocks, bcb3, [bcb4]); - assert_successors!(basic_coverage_blocks, bcb4, [bcb5, bcb6]); - assert_successors!(basic_coverage_blocks, bcb5, [bcb1]); - assert_successors!(basic_coverage_blocks, bcb6, [bcb4]); + assert_successors(&basic_coverage_blocks, bcb(0), &[bcb(1)]); + assert_successors(&basic_coverage_blocks, bcb(1), &[bcb(2), bcb(3)]); + assert_successors(&basic_coverage_blocks, bcb(2), &[]); + assert_successors(&basic_coverage_blocks, bcb(3), &[bcb(4)]); + assert_successors(&basic_coverage_blocks, bcb(4), &[bcb(5), bcb(6)]); + assert_successors(&basic_coverage_blocks, bcb(5), &[bcb(1)]); + assert_successors(&basic_coverage_blocks, bcb(6), &[bcb(4)]); } #[test] @@ -595,10 +583,7 @@ fn test_find_loop_backedges_one() { backedges ); - let_bcb!(1); - let_bcb!(3); - - assert_eq!(backedges[bcb1], vec![bcb3]); + assert_eq!(backedges[bcb(1)], &[bcb(3)]); } #[test] @@ -613,13 +598,8 @@ fn test_find_loop_backedges_two() { backedges ); - let_bcb!(1); - let_bcb!(4); - let_bcb!(5); - let_bcb!(6); - - assert_eq!(backedges[bcb1], vec![bcb5]); - assert_eq!(backedges[bcb4], vec![bcb6]); + assert_eq!(backedges[bcb(1)], &[bcb(5)]); + assert_eq!(backedges[bcb(4)], &[bcb(6)]); } #[test] @@ -632,13 +612,11 @@ fn test_traverse_coverage_with_loops() { traversed_in_order.push(bcb); } - let_bcb!(6); - // bcb0 is visited first. Then bcb1 starts the first loop, and all remaining nodes, *except* // bcb6 are inside the first loop. assert_eq!( *traversed_in_order.last().expect("should have elements"), - bcb6, + bcb(6), "bcb6 should not be visited until all nodes inside the first loop have been visited" ); } @@ -656,20 +634,18 @@ fn test_make_bcb_counters() { coverage_counters.make_bcb_counters(&basic_coverage_blocks, bcb_has_coverage_spans); assert_eq!(coverage_counters.num_expressions(), 0); - let_bcb!(1); assert_eq!( 0, // bcb1 has a `Counter` with id = 0 - match coverage_counters.bcb_counter(bcb1).expect("should have a counter") { + match coverage_counters.bcb_counter(bcb(1)).expect("should have a counter") { counters::BcbCounter::Counter { id, .. } => id, _ => panic!("expected a Counter"), } .as_u32() ); - let_bcb!(2); assert_eq!( 1, // bcb2 has a `Counter` with id = 1 - match coverage_counters.bcb_counter(bcb2).expect("should have a counter") { + match coverage_counters.bcb_counter(bcb(2)).expect("should have a counter") { counters::BcbCounter::Counter { id, .. } => id, _ => panic!("expected a Counter"), } -- cgit v1.2.3