Skip to content

Commit 24e2b48

Browse files
committed
coverage: Infer instances_used from pgo_func_name_var_map
In obscure circumstances, we would sometimes emit a covfun record for a function with no physical coverage counters, causing `llvm-cov` to fail with the cryptic error message: malformed instrumentation profile data: function name is empty We can eliminate this mismatch by removing `instances_used` entirely, and instead inferring its contents from the keys of `pgo_func_name_var_map`. This makes it impossible for a "used" function to lack a PGO name entry.
1 parent 052114f commit 24e2b48

File tree

2 files changed

+21
-18
lines changed

2 files changed

+21
-18
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,21 +46,17 @@ pub(crate) fn finalize(cx: &mut CodegenCx<'_, '_>) {
4646
debug!("Generating coverage map for CodegenUnit: `{}`", cx.codegen_unit.name());
4747

4848
// FIXME(#132395): Can this be none even when coverage is enabled?
49-
let instances_used = match cx.coverage_cx {
50-
Some(ref cx) => cx.instances_used.borrow(),
51-
None => return,
52-
};
49+
let Some(ref coverage_cx) = cx.coverage_cx else { return };
5350

54-
let mut covfun_records = instances_used
55-
.iter()
56-
.copied()
51+
let mut covfun_records = coverage_cx
52+
.instances_used()
53+
.into_iter()
5754
// Sort by symbol name, so that the global file table is built in an
5855
// order that doesn't depend on the stable-hash-based order in which
5956
// instances were visited during codegen.
6057
.sorted_by_cached_key(|&instance| tcx.symbol_name(instance).name)
6158
.filter_map(|instance| prepare_covfun_record(tcx, instance, true))
6259
.collect::<Vec<_>>();
63-
drop(instances_used);
6460

6561
// In a single designated CGU, also prepare covfun records for functions
6662
// in this crate that were instrumented for coverage, but are unused.

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rustc_abi::Size;
55
use rustc_codegen_ssa::traits::{
66
BuilderMethods, ConstCodegenMethods, CoverageInfoBuilderMethods, MiscCodegenMethods,
77
};
8-
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
8+
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
99
use rustc_middle::mir::coverage::CoverageKind;
1010
use rustc_middle::ty::Instance;
1111
use tracing::{debug, instrument};
@@ -20,9 +20,14 @@ mod mapgen;
2020

2121
/// Extra per-CGU context/state needed for coverage instrumentation.
2222
pub(crate) struct CguCoverageContext<'ll, 'tcx> {
23-
/// Coverage data for each instrumented function identified by DefId.
24-
pub(crate) instances_used: RefCell<FxIndexSet<Instance<'tcx>>>,
25-
pub(crate) pgo_func_name_var_map: RefCell<FxHashMap<Instance<'tcx>, &'ll llvm::Value>>,
23+
/// Associates function instances with an LLVM global that holds the
24+
/// function's symbol name, as needed by LLVM coverage intrinsics.
25+
///
26+
/// Instances in this map are also considered "used" for the purposes of
27+
/// emitting covfun records. Every covfun record holds a hash of its
28+
/// symbol name, and `llvm-cov` will exit fatally if it can't resolve that
29+
/// hash back to an entry in the binary's `__llvm_prf_names` linker section.
30+
pub(crate) pgo_func_name_var_map: RefCell<FxIndexMap<Instance<'tcx>, &'ll llvm::Value>>,
2631
pub(crate) mcdc_condition_bitmap_map: RefCell<FxHashMap<Instance<'tcx>, Vec<&'ll llvm::Value>>>,
2732

2833
covfun_section_name: OnceCell<CString>,
@@ -31,7 +36,6 @@ pub(crate) struct CguCoverageContext<'ll, 'tcx> {
3136
impl<'ll, 'tcx> CguCoverageContext<'ll, 'tcx> {
3237
pub(crate) fn new() -> Self {
3338
Self {
34-
instances_used: RefCell::<FxIndexSet<_>>::default(),
3539
pgo_func_name_var_map: Default::default(),
3640
mcdc_condition_bitmap_map: Default::default(),
3741
covfun_section_name: Default::default(),
@@ -53,6 +57,14 @@ impl<'ll, 'tcx> CguCoverageContext<'ll, 'tcx> {
5357
.and_then(|bitmap_map| bitmap_map.get(decision_depth as usize))
5458
.copied() // Dereference Option<&&Value> to Option<&Value>
5559
}
60+
61+
/// Returns the list of instances considered "used" in this CGU, as
62+
/// inferred from the keys of `pgo_func_name_var_map`.
63+
pub(crate) fn instances_used(&self) -> Vec<Instance<'tcx>> {
64+
// Collecting into a Vec is way easier than trying to juggle RefCell
65+
// projections, and this should only run once per CGU anyway.
66+
self.pgo_func_name_var_map.borrow().keys().copied().collect::<Vec<_>>()
67+
}
5668
}
5769

5870
impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
@@ -151,11 +163,6 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
151163
return;
152164
};
153165

154-
// Mark the instance as used in this CGU, for coverage purposes.
155-
// This includes functions that were not partitioned into this CGU,
156-
// but were MIR-inlined into one of this CGU's functions.
157-
coverage_cx.instances_used.borrow_mut().insert(instance);
158-
159166
match *kind {
160167
CoverageKind::SpanMarker | CoverageKind::BlockMarker { .. } => unreachable!(
161168
"marker statement {kind:?} should have been removed by CleanupPostBorrowck"

0 commit comments

Comments
 (0)