diff options
| author | Zalathar <Zalathar@users.noreply.github.com> | 2025-05-27 23:25:57 +1000 |
|---|---|---|
| committer | Zalathar <Zalathar@users.noreply.github.com> | 2025-05-27 23:33:29 +1000 |
| commit | 3f526eeec4efe8a31154b7b20b64d99c1badce9e (patch) | |
| tree | 37bc901383513aa0d281bfb99d135d27f9984984 /compiler | |
| parent | d76fe154029e03aeb64af721beafdcef856d576a (diff) | |
| download | rust-3f526eeec4efe8a31154b7b20b64d99c1badce9e.tar.gz rust-3f526eeec4efe8a31154b7b20b64d99c1badce9e.zip | |
coverage: Revert "unused local file IDs" due to empty function names
This reverts commit 3b22c21dd8c30f499051fe7a758ca0e5d81eb638, reversing changes made to 5f292eea6d63abbd26f1e6e00a0b8cf21d828d7d.
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs | 26 | ||||
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs | 44 | ||||
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs | 28 | ||||
| -rw-r--r-- | compiler/rustc_interface/src/tests.rs | 3 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/spans.rs | 38 | ||||
| -rw-r--r-- | compiler/rustc_session/src/config.rs | 5 | ||||
| -rw-r--r-- | compiler/rustc_session/src/options.rs | 1 | ||||
| -rw-r--r-- | compiler/rustc_session/src/session.rs | 5 |
8 files changed, 42 insertions, 108 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index c207df2fb0b..f6000e72840 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -155,20 +155,6 @@ pub(crate) struct Regions { impl Regions { /// Returns true if none of this structure's tables contain any regions. pub(crate) fn has_no_regions(&self) -> bool { - // Every region has a span, so if there are no spans then there are no regions. - self.all_cov_spans().next().is_none() - } - - pub(crate) fn all_cov_spans(&self) -> impl Iterator<Item = &CoverageSpan> { - macro_rules! iter_cov_spans { - ( $( $regions:expr ),* $(,)? ) => { - std::iter::empty() - $( - .chain( $regions.iter().map(|region| ®ion.cov_span) ) - )* - } - } - let Self { code_regions, expansion_regions, @@ -177,13 +163,11 @@ impl Regions { mcdc_decision_regions, } = self; - iter_cov_spans!( - code_regions, - expansion_regions, - branch_regions, - mcdc_branch_regions, - mcdc_decision_regions, - ) + code_regions.is_empty() + && expansion_regions.is_empty() + && branch_regions.is_empty() + && mcdc_branch_regions.is_empty() + && mcdc_decision_regions.is_empty() } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index d3a815fabe7..7bdbc685952 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -11,7 +11,6 @@ use rustc_abi::Align; use rustc_codegen_ssa::traits::{ BaseTypeCodegenMethods as _, ConstCodegenMethods, StaticCodegenMethods, }; -use rustc_index::IndexVec; use rustc_middle::mir::coverage::{ BasicCoverageBlock, CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping, MappingKind, Op, @@ -105,16 +104,6 @@ fn fill_region_tables<'tcx>( ids_info: &'tcx CoverageIdsInfo, covfun: &mut CovfunRecord<'tcx>, ) { - // If this function is unused, replace all counters with zero. - let counter_for_bcb = |bcb: BasicCoverageBlock| -> ffi::Counter { - let term = if covfun.is_used { - ids_info.term_for_bcb[bcb].expect("every BCB in a mapping was given a term") - } else { - CovTerm::Zero - }; - ffi::Counter::from_term(term) - }; - // Currently a function's mappings must all be in the same file, so use the // first mapping's span to determine the file. let source_map = tcx.sess.source_map(); @@ -126,12 +115,6 @@ fn fill_region_tables<'tcx>( let local_file_id = covfun.virtual_file_mapping.push_file(&source_file); - // If this testing flag is set, add an extra unused entry to the local - // file table, to help test the code for detecting unused file IDs. - if tcx.sess.coverage_inject_unused_local_file() { - covfun.virtual_file_mapping.push_file(&source_file); - } - // In rare cases, _all_ of a function's spans are discarded, and coverage // codegen needs to handle that gracefully to avoid #133606. // It's hard for tests to trigger this organically, so instead we set @@ -152,6 +135,16 @@ fn fill_region_tables<'tcx>( // For each counter/region pair in this function+file, convert it to a // form suitable for FFI. for &Mapping { ref kind, span } in &fn_cov_info.mappings { + // If this function is unused, replace all counters with zero. + let counter_for_bcb = |bcb: BasicCoverageBlock| -> ffi::Counter { + let term = if covfun.is_used { + ids_info.term_for_bcb[bcb].expect("every BCB in a mapping was given a term") + } else { + CovTerm::Zero + }; + ffi::Counter::from_term(term) + }; + let Some(coords) = make_coords(span) else { continue }; let cov_span = coords.make_coverage_span(local_file_id); @@ -184,19 +177,6 @@ fn fill_region_tables<'tcx>( } } -/// LLVM requires all local file IDs to have at least one mapping region. -/// If that's not the case, skip this function, to avoid an assertion failure -/// (or worse) in LLVM. -fn check_local_file_table(covfun: &CovfunRecord<'_>) -> bool { - let mut local_file_id_seen = - IndexVec::<u32, _>::from_elem_n(false, covfun.virtual_file_mapping.local_file_table.len()); - for cov_span in covfun.regions.all_cov_spans() { - local_file_id_seen[cov_span.file_id] = true; - } - - local_file_id_seen.into_iter().all(|seen| seen) -} - /// Generates the contents of the covfun record for this function, which /// contains the function's coverage mapping data. The record is then stored /// as a global variable in the `__llvm_covfun` section. @@ -205,10 +185,6 @@ pub(crate) fn generate_covfun_record<'tcx>( global_file_table: &GlobalFileTable, covfun: &CovfunRecord<'tcx>, ) { - if !check_local_file_table(covfun) { - return; - } - let &CovfunRecord { mangled_function_name, source_hash, diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs index 574463be7ff..39a59560c9d 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs @@ -39,10 +39,7 @@ impl Coords { /// or other expansions), and if it does happen then skipping a span or function is /// better than an ICE or `llvm-cov` failure that the user might have no way to avoid. pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) -> Option<Coords> { - if span.is_empty() { - debug_assert!(false, "can't make coords from empty span: {span:?}"); - return None; - } + let span = ensure_non_empty_span(source_map, span)?; let lo = span.lo(); let hi = span.hi(); @@ -73,6 +70,29 @@ pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) }) } +fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> { + if !span.is_empty() { + return Some(span); + } + + // The span is empty, so try to enlarge it to cover an adjacent '{' or '}'. + source_map + .span_to_source(span, |src, start, end| try { + // Adjusting span endpoints by `BytePos(1)` is normally a bug, + // but in this case we have specifically checked that the character + // we're skipping over is one of two specific ASCII characters, so + // adjusting by exactly 1 byte is correct. + if src.as_bytes().get(end).copied() == Some(b'{') { + Some(span.with_hi(span.hi() + BytePos(1))) + } else if start > 0 && src.as_bytes()[start - 1] == b'}' { + Some(span.with_lo(span.lo() - BytePos(1))) + } else { + None + } + }) + .ok()? +} + /// If `llvm-cov` sees a source region that is improperly ordered (end < start), /// it will immediately exit with a fatal error. To prevent that from happening, /// discard regions that are improperly ordered, or might be interpreted in a diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 20e081d3360..068d96c860f 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -776,8 +776,7 @@ fn test_unstable_options_tracking_hash() { CoverageOptions { level: CoverageLevel::Mcdc, no_mir_spans: true, - discard_all_spans_in_codegen: true, - inject_unused_local_file: true, + discard_all_spans_in_codegen: true } ); tracked!(crate_attr, vec!["abc".to_string()]); diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index ddeae093df5..ec76076020e 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,8 +1,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir; use rustc_middle::ty::TyCtxt; -use rustc_span::source_map::SourceMap; -use rustc_span::{BytePos, DesugaringKind, ExpnKind, MacroKind, Span}; +use rustc_span::{DesugaringKind, ExpnKind, MacroKind, Span}; use tracing::instrument; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph}; @@ -84,18 +83,8 @@ pub(super) fn extract_refined_covspans<'tcx>( // Discard any span that overlaps with a hole. discard_spans_overlapping_holes(&mut covspans, &holes); - // Discard spans that overlap in unwanted ways. + // Perform more refinement steps after holes have been dealt with. let mut covspans = remove_unwanted_overlapping_spans(covspans); - - // For all empty spans, either enlarge them to be non-empty, or discard them. - let source_map = tcx.sess.source_map(); - covspans.retain_mut(|covspan| { - let Some(span) = ensure_non_empty_span(source_map, covspan.span) else { return false }; - covspan.span = span; - true - }); - - // Merge covspans that can be merged. covspans.dedup_by(|b, a| a.merge_if_eligible(b)); code_mappings.extend(covspans.into_iter().map(|Covspan { span, bcb }| { @@ -241,26 +230,3 @@ fn compare_spans(a: Span, b: Span) -> std::cmp::Ordering { // - Both have the same start and span A extends further right .then_with(|| Ord::cmp(&a.hi(), &b.hi()).reverse()) } - -fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> { - if !span.is_empty() { - return Some(span); - } - - // The span is empty, so try to enlarge it to cover an adjacent '{' or '}'. - source_map - .span_to_source(span, |src, start, end| try { - // Adjusting span endpoints by `BytePos(1)` is normally a bug, - // but in this case we have specifically checked that the character - // we're skipping over is one of two specific ASCII characters, so - // adjusting by exactly 1 byte is correct. - if src.as_bytes().get(end).copied() == Some(b'{') { - Some(span.with_hi(span.hi() + BytePos(1))) - } else if start > 0 && src.as_bytes()[start - 1] == b'}' { - Some(span.with_lo(span.lo() - BytePos(1))) - } else { - None - } - }) - .ok()? -} diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 144aeb5c369..60e1b465ba9 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -195,11 +195,6 @@ pub struct CoverageOptions { /// regression tests for #133606, because we don't have an easy way to /// reproduce it from actual source code. pub discard_all_spans_in_codegen: bool, - - /// `-Zcoverage-options=inject-unused-local-file`: During codegen, add an - /// extra dummy entry to each function's local file table, to exercise the - /// code that checks for local file IDs with no mapping regions. - pub inject_unused_local_file: bool, } /// Controls whether branch coverage or MC/DC coverage is enabled. diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 3d9fdcbc7b1..5b4068740a1 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1413,7 +1413,6 @@ pub mod parse { "mcdc" => slot.level = CoverageLevel::Mcdc, "no-mir-spans" => slot.no_mir_spans = true, "discard-all-spans-in-codegen" => slot.discard_all_spans_in_codegen = true, - "inject-unused-local-file" => slot.inject_unused_local_file = true, _ => return false, } } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 34ac37d6378..010ae42c280 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -371,11 +371,6 @@ impl Session { self.opts.unstable_opts.coverage_options.discard_all_spans_in_codegen } - /// True if testing flag `-Zcoverage-options=inject-unused-local-file` was passed. - pub fn coverage_inject_unused_local_file(&self) -> bool { - self.opts.unstable_opts.coverage_options.inject_unused_local_file - } - pub fn is_sanitizer_cfi_enabled(&self) -> bool { self.opts.unstable_opts.sanitizer.contains(SanitizerSet::CFI) } |
