diff options
16 files changed, 62 insertions, 154 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) } diff --git a/tests/coverage/async_closure.cov-map b/tests/coverage/async_closure.cov-map index 53128dd7a48..9f8dc8d6cbb 100644 --- a/tests/coverage/async_closure.cov-map +++ b/tests/coverage/async_closure.cov-map @@ -37,29 +37,32 @@ Number of file 0 mappings: 8 Highest counter ID seen: c0 Function name: async_closure::main::{closure#0} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24] +Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24] Number of files: 1 - file 0 => $DIR/async_closure.rs Number of expressions: 0 -Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36) +Number of file 0 mappings: 2 +- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35) +- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36) Highest counter ID seen: c0 Function name: async_closure::main::{closure#0} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24] +Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24] Number of files: 1 - file 0 => $DIR/async_closure.rs Number of expressions: 0 -Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36) +Number of file 0 mappings: 2 +- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35) +- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36) Highest counter ID seen: c0 Function name: async_closure::main::{closure#0}::{closure#0}::<i16> -Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24] +Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24] Number of files: 1 - file 0 => $DIR/async_closure.rs Number of expressions: 0 -Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36) +Number of file 0 mappings: 2 +- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 35) +- Code(Counter(0)) at (prev + 0, 35) to (start + 0, 36) Highest counter ID seen: c0 diff --git a/tests/coverage/unused-local-file.coverage b/tests/coverage/unused-local-file.coverage deleted file mode 100644 index 8f5a32f6d70..00000000000 --- a/tests/coverage/unused-local-file.coverage +++ /dev/null @@ -1,7 +0,0 @@ - LL| |//@ edition: 2021 - LL| | - LL| |// Force this function to be generated in its home crate, so that it ends up - LL| |// with normal coverage metadata. - LL| |#[inline(never)] - LL| 1|pub fn external_function() {} - diff --git a/tests/coverage/unused-local-file.rs b/tests/coverage/unused-local-file.rs deleted file mode 100644 index cf43c62d703..00000000000 --- a/tests/coverage/unused-local-file.rs +++ /dev/null @@ -1,22 +0,0 @@ -//! If we give LLVM a local file table for a function, but some of the entries -//! in that table have no associated mapping regions, then an assertion failure -//! will occur in LLVM. We therefore need to detect and skip any function that -//! would trigger that assertion. -//! -//! To test that this case is handled, even before adding code that could allow -//! it to happen organically (for expansion region support), we use a special -//! testing-only flag to force it to occur. - -//@ edition: 2024 -//@ compile-flags: -Zcoverage-options=inject-unused-local-file - -// The `llvm-cov` tool will complain if the test binary ends up having no -// coverage metadata at all. To prevent that, we also link to instrumented -// code in an auxiliary crate that doesn't have the special flag set. - -//@ aux-build: discard_all_helper.rs -extern crate discard_all_helper; - -fn main() { - discard_all_helper::external_function(); -} diff --git a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff index fa88211383a..d465b8bded2 100644 --- a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff @@ -40,7 +40,7 @@ + coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:17: 19:18 (#0); + coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:23: 19:30 (#0); + coverage Code { bcb: bcb5 } => $DIR/branch_match_arms.rs:19:31: 19:32 (#0); -+ coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:1: 21:2 (#0); ++ coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:2: 21:2 (#0); + bb0: { + Coverage::VirtualCounter(bcb0); diff --git a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff index 9b6d2b22087..cf6d85abd80 100644 --- a/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff @@ -6,7 +6,7 @@ + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:27:1: 27:17 (#0); + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:28:5: 28:9 (#0); -+ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:1: 29:2 (#0); ++ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:2: 29:2 (#0); + bb0: { + Coverage::VirtualCounter(bcb0); diff --git a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff index b2bb2375aee..980c5e202ff 100644 --- a/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff @@ -10,8 +10,8 @@ + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:13:1: 13:10 (#0); + coverage Code { bcb: bcb1 } => $DIR/instrument_coverage.rs:15:12: 15:15 (#0); + coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:16:13: 16:18 (#0); -+ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage.rs:17:9: 17:10 (#0); -+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:19:1: 19:2 (#0); ++ coverage Code { bcb: bcb3 } => $DIR/instrument_coverage.rs:17:10: 17:10 (#0); ++ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:19:2: 19:2 (#0); + bb0: { + Coverage::VirtualCounter(bcb0); diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff index 2eb78c08ee8..b707cd41788 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff @@ -10,8 +10,8 @@ coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:13:1: 13:10 (#0); coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); coverage Code { bcb: bcb3 } => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); - coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:38: 14:39 (#0); - coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:1: 15:2 (#0); + coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); + coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); coverage Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); bb0: { diff --git a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff index 0c1bc24b6dc..239b845c231 100644 --- a/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff +++ b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff @@ -10,8 +10,8 @@ + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:13:1: 13:10 (#0); + coverage Code { bcb: bcb0 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); + coverage Code { bcb: bcb3 } => $DIR/instrument_coverage_cleanup.rs:14:37: 14:39 (#0); -+ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:38: 14:39 (#0); -+ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:1: 15:2 (#0); ++ coverage Code { bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:39: 14:39 (#0); ++ coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#0); + coverage Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0); + bb0: { |