Skip to content

Commit

Permalink
Rollup merge of #119401 - Zalathar:query-stability, r=Nilstrieb
Browse files Browse the repository at this point in the history
coverage: Avoid a possible query stability hazard in `CoverageCounters`

#119252 revealed a possible query stability hazard in `CoverageCounters`: we iterate over the entries of an `FxHashMap` in a way that allows the iteration order to potentially affect the relative creation order of MIR blocks.

I'm not sure whether there's an actual stability problem or not in practice, but it's certainly a hazard, and I don't see any reason not to switch over to `FxIndexMap` to avoid potential issues.

---

This can either be merged on its own, or incorporated into #119252.

cc `@Enselic`
r? `@cjgillot`
  • Loading branch information
matthiaskrgr authored Dec 29, 2023
2 parents 12ad777 + 8529b63 commit 4b9a76c
Showing 1 changed file with 6 additions and 3 deletions.
9 changes: 6 additions & 3 deletions compiler/rustc_mir_transform/src/coverage/counters.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::FxIndexMap;
use rustc_data_structures::graph::WithNumNodes;
use rustc_index::bit_set::BitSet;
use rustc_index::IndexVec;
Expand Down Expand Up @@ -47,7 +47,10 @@ pub(super) struct CoverageCounters {
bcb_counters: IndexVec<BasicCoverageBlock, Option<BcbCounter>>,
/// Coverage counters/expressions that are associated with the control-flow
/// edge between two BCBs.
bcb_edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>,
///
/// The iteration order of this map can affect the precise contents of MIR,
/// so we use `FxIndexMap` to avoid query stability hazards.
bcb_edge_counters: FxIndexMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>,
/// Tracks which BCBs have a counter associated with some incoming edge.
/// Only used by assertions, to verify that BCBs with incoming edge
/// counters do not have their own physical counters (expressions are allowed).
Expand All @@ -64,7 +67,7 @@ impl CoverageCounters {
Self {
next_counter_id: CounterId::START,
bcb_counters: IndexVec::from_elem_n(None, num_bcbs),
bcb_edge_counters: FxHashMap::default(),
bcb_edge_counters: FxIndexMap::default(),
bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs),
expressions: IndexVec::new(),
}
Expand Down

0 comments on commit 4b9a76c

Please sign in to comment.