Skip to content

Commit

Permalink
Rollup merge of #119514 - Zalathar:query-stability, r=wesleywiser
Browse files Browse the repository at this point in the history
coverage: Avoid a query stability hazard in `function_coverage_map`

When #118865 started enforcing the `rustc::potential_query_instability` lint in `rustc_codegen_llvm`, it added an exemption for this site, arguing that the entries are only used to create a list of filenames that is later sorted.

However, the list of entries also gets traversed when creating the function coverage records in LLVM IR, which may be sensitive to hash-based ordering.

This patch therefore changes `function_coverage_map` to use `FxIndexMap`, which should avoid hash-based instability by iterating in insertion order.

cc ``@Enselic``
  • Loading branch information
fmease authored Jan 3, 2024
2 parents c2debec + 5e7c1b9 commit ea39f19
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 9 deletions.
5 changes: 0 additions & 5 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ pub fn finalize(cx: &CodegenCx<'_, '_>) {
return;
}

// The entries of the map are only used to get a list of all files with
// coverage info. In the end the list of files is passed into
// `GlobalFileTable::new()` which internally do `.sort_unstable_by()`, so
// the iteration order here does not matter.
#[allow(rustc::potential_query_instability)]
let function_coverage_entries = function_coverage_map
.into_iter()
.map(|(instance, function_coverage)| (instance, function_coverage.into_finished()))
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_codegen_ssa::traits::{
BaseTypeMethods, BuilderMethods, ConstMethods, CoverageInfoBuilderMethods, MiscMethods,
StaticMethods,
};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
use rustc_llvm::RustString;
use rustc_middle::bug;
use rustc_middle::mir::coverage::CoverageKind;
Expand All @@ -30,7 +30,7 @@ const VAR_ALIGN_BYTES: usize = 8;
pub struct CrateCoverageContext<'ll, 'tcx> {
/// Coverage data for each instrumented function identified by DefId.
pub(crate) function_coverage_map:
RefCell<FxHashMap<Instance<'tcx>, FunctionCoverageCollector<'tcx>>>,
RefCell<FxIndexMap<Instance<'tcx>, FunctionCoverageCollector<'tcx>>>,
pub(crate) pgo_func_name_var_map: RefCell<FxHashMap<Instance<'tcx>, &'ll llvm::Value>>,
}

Expand All @@ -44,8 +44,8 @@ impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> {

pub fn take_function_coverage_map(
&self,
) -> FxHashMap<Instance<'tcx>, FunctionCoverageCollector<'tcx>> {
self.function_coverage_map.replace(FxHashMap::default())
) -> FxIndexMap<Instance<'tcx>, FunctionCoverageCollector<'tcx>> {
self.function_coverage_map.replace(FxIndexMap::default())
}
}

Expand Down

0 comments on commit ea39f19

Please sign in to comment.