Skip to content

Commit 20bf8be

Browse files
committed
coverage: Detect unused local file IDs to avoid an LLVM assertion
This case can't actually happen yet (other than via a testing flag), because currently all of a function's spans must belong to the same file and expansion. But this will be an important edge case when adding expansion region support.
1 parent 553a2a4 commit 20bf8be

File tree

8 files changed

+87
-6
lines changed

8 files changed

+87
-6
lines changed

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,20 @@ pub(crate) struct Regions {
155155
impl Regions {
156156
/// Returns true if none of this structure's tables contain any regions.
157157
pub(crate) fn has_no_regions(&self) -> bool {
158+
// Every region has a span, so if there are no spans then there are no regions.
159+
self.all_cov_spans().next().is_none()
160+
}
161+
162+
pub(crate) fn all_cov_spans(&self) -> impl Iterator<Item = &CoverageSpan> {
163+
macro_rules! iter_cov_spans {
164+
( $( $regions:expr ),* $(,)? ) => {
165+
std::iter::empty()
166+
$(
167+
.chain( $regions.iter().map(|region| &region.cov_span) )
168+
)*
169+
}
170+
}
171+
158172
let Self {
159173
code_regions,
160174
expansion_regions,
@@ -163,11 +177,13 @@ impl Regions {
163177
mcdc_decision_regions,
164178
} = self;
165179

166-
code_regions.is_empty()
167-
&& expansion_regions.is_empty()
168-
&& branch_regions.is_empty()
169-
&& mcdc_branch_regions.is_empty()
170-
&& mcdc_decision_regions.is_empty()
180+
iter_cov_spans!(
181+
code_regions,
182+
expansion_regions,
183+
branch_regions,
184+
mcdc_branch_regions,
185+
mcdc_decision_regions,
186+
)
171187
}
172188
}
173189

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use rustc_abi::Align;
1111
use rustc_codegen_ssa::traits::{
1212
BaseTypeCodegenMethods as _, ConstCodegenMethods, StaticCodegenMethods,
1313
};
14+
use rustc_index::IndexVec;
1415
use rustc_middle::mir::coverage::{
1516
BasicCoverageBlock, CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping,
1617
MappingKind, Op,
@@ -125,6 +126,12 @@ fn fill_region_tables<'tcx>(
125126

126127
let local_file_id = covfun.virtual_file_mapping.push_file(&source_file);
127128

129+
// If this testing flag is set, add an extra unused entry to the local
130+
// file table, to help test the code for detecting unused file IDs.
131+
if tcx.sess.coverage_inject_unused_local_file() {
132+
covfun.virtual_file_mapping.push_file(&source_file);
133+
}
134+
128135
// In rare cases, _all_ of a function's spans are discarded, and coverage
129136
// codegen needs to handle that gracefully to avoid #133606.
130137
// It's hard for tests to trigger this organically, so instead we set
@@ -177,6 +184,19 @@ fn fill_region_tables<'tcx>(
177184
}
178185
}
179186

187+
/// LLVM requires all local file IDs to have at least one mapping region.
188+
/// If that's not the case, skip this function, to avoid an assertion failure
189+
/// (or worse) in LLVM.
190+
fn check_local_file_table(covfun: &CovfunRecord<'_>) -> bool {
191+
let mut local_file_id_seen =
192+
IndexVec::<u32, _>::from_elem_n(false, covfun.virtual_file_mapping.local_file_table.len());
193+
for cov_span in covfun.regions.all_cov_spans() {
194+
local_file_id_seen[cov_span.file_id] = true;
195+
}
196+
197+
local_file_id_seen.into_iter().all(|seen| seen)
198+
}
199+
180200
/// Generates the contents of the covfun record for this function, which
181201
/// contains the function's coverage mapping data. The record is then stored
182202
/// as a global variable in the `__llvm_covfun` section.
@@ -185,6 +205,10 @@ pub(crate) fn generate_covfun_record<'tcx>(
185205
global_file_table: &GlobalFileTable,
186206
covfun: &CovfunRecord<'tcx>,
187207
) {
208+
if !check_local_file_table(covfun) {
209+
return;
210+
}
211+
188212
let &CovfunRecord {
189213
mangled_function_name,
190214
source_hash,

compiler/rustc_interface/src/tests.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,8 @@ fn test_unstable_options_tracking_hash() {
776776
CoverageOptions {
777777
level: CoverageLevel::Mcdc,
778778
no_mir_spans: true,
779-
discard_all_spans_in_codegen: true
779+
discard_all_spans_in_codegen: true,
780+
inject_unused_local_file: true,
780781
}
781782
);
782783
tracked!(crate_attr, vec!["abc".to_string()]);

compiler/rustc_session/src/config.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,11 @@ pub struct CoverageOptions {
195195
/// regression tests for #133606, because we don't have an easy way to
196196
/// reproduce it from actual source code.
197197
pub discard_all_spans_in_codegen: bool,
198+
199+
/// `-Zcoverage-options=inject-unused-local-file`: During codegen, add an
200+
/// extra dummy entry to each function's local file table, to exercise the
201+
/// code that checks for local file IDs with no mapping regions.
202+
pub inject_unused_local_file: bool,
198203
}
199204

200205
/// Controls whether branch coverage or MC/DC coverage is enabled.

compiler/rustc_session/src/options.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,7 @@ pub mod parse {
14131413
"mcdc" => slot.level = CoverageLevel::Mcdc,
14141414
"no-mir-spans" => slot.no_mir_spans = true,
14151415
"discard-all-spans-in-codegen" => slot.discard_all_spans_in_codegen = true,
1416+
"inject-unused-local-file" => slot.inject_unused_local_file = true,
14161417
_ => return false,
14171418
}
14181419
}

compiler/rustc_session/src/session.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,11 @@ impl Session {
371371
self.opts.unstable_opts.coverage_options.discard_all_spans_in_codegen
372372
}
373373

374+
/// True if testing flag `-Zcoverage-options=inject-unused-local-file` was passed.
375+
pub fn coverage_inject_unused_local_file(&self) -> bool {
376+
self.opts.unstable_opts.coverage_options.inject_unused_local_file
377+
}
378+
374379
pub fn is_sanitizer_cfi_enabled(&self) -> bool {
375380
self.opts.unstable_opts.sanitizer.contains(SanitizerSet::CFI)
376381
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
LL| |//@ edition: 2021
2+
LL| |
3+
LL| |// Force this function to be generated in its home crate, so that it ends up
4+
LL| |// with normal coverage metadata.
5+
LL| |#[inline(never)]
6+
LL| 1|pub fn external_function() {}
7+

tests/coverage/unused-local-file.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//! If we give LLVM a local file table for a function, but some of the entries
2+
//! in that table have no associated mapping regions, then an assertion failure
3+
//! will occur in LLVM. We therefore need to detect and skip any function that
4+
//! would trigger that assertion.
5+
//!
6+
//! To test that this case is handled, even before adding code that could allow
7+
//! it to happen organically (for expansion region support), we use a special
8+
//! testing-only flag to force it to occur.
9+
10+
//@ edition: 2024
11+
//@ compile-flags: -Zcoverage-options=inject-unused-local-file
12+
13+
// The `llvm-cov` tool will complain if the test binary ends up having no
14+
// coverage metadata at all. To prevent that, we also link to instrumented
15+
// code in an auxiliary crate that doesn't have the special flag set.
16+
17+
//@ aux-build: discard_all_helper.rs
18+
extern crate discard_all_helper;
19+
20+
fn main() {
21+
discard_all_helper::external_function();
22+
}

0 commit comments

Comments
 (0)