Unrolled build for rust-lang#140847

Rollup merge of rust-lang#140847 - Zalathar:unused-local-file, r=SparrowLii

coverage: Detect unused local file IDs to avoid an LLVM assertion

Each function's coverage metadata contains a *local file table* that maps local file IDs (used by the function's mapping regions) to global file IDs (shared by all functions in the same CGU).

LLVM requires all local file IDs to have at least one mapping region, and has an assertion that will fail if it detects a local file ID with no regions. To make sure that assertion doesn't fire, we need to detect and skip functions whose metadata would trigger it.

(This can't actually happen yet, 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.)

Author: rust-timer

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

@@ -155,6 +155,20 @@ 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| &region.cov_span) )
+                )*
+            }
+        }
+
         let Self {
             code_regions,
             expansion_regions,
@@ -163,11 +177,13 @@ impl Regions {
             mcdc_decision_regions,
         } = self;
 
-        code_regions.is_empty()
-            && expansion_regions.is_empty()
-            && branch_regions.is_empty()
-            && mcdc_branch_regions.is_empty()
-            && mcdc_decision_regions.is_empty()
+        iter_cov_spans!(
+            code_regions,
+            expansion_regions,
+            branch_regions,
+            mcdc_branch_regions,
+            mcdc_decision_regions,
+        )
     }
 }
 

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

@@ -11,6 +11,7 @@ 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,
@@ -104,6 +105,16 @@ 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();
@@ -115,6 +126,12 @@ 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
@@ -135,16 +152,6 @@ 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);
 
@@ -177,6 +184,19 @@ 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.
@@ -185,6 +205,10 @@ 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,

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

@@ -39,7 +39,10 @@ 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> {
-    let span = ensure_non_empty_span(source_map, span)?;
+    if span.is_empty() {
+        debug_assert!(false, "can't make coords from empty span: {span:?}");
+        return None;
+    }
 
     let lo = span.lo();
     let hi = span.hi();
@@ -70,29 +73,6 @@ 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

compiler/rustc_interface/src/tests.rs

@@ -776,7 +776,8 @@ fn test_unstable_options_tracking_hash() {
         CoverageOptions {
             level: CoverageLevel::Mcdc,
             no_mir_spans: true,
-            discard_all_spans_in_codegen: true
+            discard_all_spans_in_codegen: true,
+            inject_unused_local_file: true,
         }
     );
     tracked!(crate_attr, vec!["abc".to_string()]);

compiler/rustc_mir_transform/src/coverage/spans.rs

@@ -1,7 +1,8 @@
 use rustc_data_structures::fx::FxHashSet;
 use rustc_middle::mir;
 use rustc_middle::ty::TyCtxt;
-use rustc_span::{DesugaringKind, ExpnKind, MacroKind, Span};
+use rustc_span::source_map::SourceMap;
+use rustc_span::{BytePos, DesugaringKind, ExpnKind, MacroKind, Span};
 use tracing::instrument;
 
 use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
@@ -83,8 +84,18 @@ pub(super) fn extract_refined_covspans<'tcx>(
     // Discard any span that overlaps with a hole.
     discard_spans_overlapping_holes(&mut covspans, &holes);
 
-    // Perform more refinement steps after holes have been dealt with.
+    // Discard spans that overlap in unwanted ways.
     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 }| {
@@ -230,3 +241,26 @@ 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()?
+}

compiler/rustc_session/src/config.rs

@@ -195,6 +195,11 @@ 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.

compiler/rustc_session/src/options.rs

@@ -1413,6 +1413,7 @@ 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,
             }
         }

compiler/rustc_session/src/session.rs

@@ -371,6 +371,11 @@ 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)
     }

tests/coverage/async_closure.cov-map

@@ -37,32 +37,29 @@ Number of file 0 mappings: 8
 Highest counter ID seen: c0
 
 Function name: async_closure::main::{closure#0}
-Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
 Number of files: 1
 - file 0 => $DIR/async_closure.rs
 Number of expressions: 0
-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)
+Number of file 0 mappings: 1
+- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
 Highest counter ID seen: c0
 
 Function name: async_closure::main::{closure#0}
-Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
 Number of files: 1
 - file 0 => $DIR/async_closure.rs
 Number of expressions: 0
-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)
+Number of file 0 mappings: 1
+- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
 Highest counter ID seen: c0
 
 Function name: async_closure::main::{closure#0}::{closure#0}::<i16>
-Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 22, 00, 23, 01, 00, 23, 00, 24]
+Raw bytes (9): 0x[01, 01, 00, 01, 01, 0b, 22, 00, 24]
 Number of files: 1
 - file 0 => $DIR/async_closure.rs
 Number of expressions: 0
-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)
+Number of file 0 mappings: 1
+- Code(Counter(0)) at (prev + 11, 34) to (start + 0, 36)
 Highest counter ID seen: c0
 

tests/coverage/unused-local-file.coverage

@@ -0,0 +1,7 @@
+   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() {}
+

tests/coverage/unused-local-file.rs

@@ -0,0 +1,22 @@
+//! 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();
+}

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:2: 21:2 (#0);
++     coverage Code { bcb: bcb2 } => $DIR/branch_match_arms.rs:21:1: 21:2 (#0);
 + 
       bb0: {
 +         Coverage::VirtualCounter(bcb0);

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:2: 29:2 (#0);
++     coverage Code { bcb: bcb0 } => $DIR/instrument_coverage.rs:29:1: 29:2 (#0);
 + 
       bb0: {
 +         Coverage::VirtualCounter(bcb0);

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:10: 17:10 (#0);
-+     coverage Code { bcb: bcb2 } => $DIR/instrument_coverage.rs:19:2: 19:2 (#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);
 + 
       bb0: {
 +         Coverage::VirtualCounter(bcb0);

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:39: 14:39 (#0);
-      coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#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 Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
 
       bb0: {

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:39: 14:39 (#0);
-+     coverage Code { bcb: bcb2 } => $DIR/instrument_coverage_cleanup.rs:15:2: 15:2 (#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 Branch { true_bcb: bcb3, false_bcb: bcb1 } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0);
 + 
       bb0: {