Rollup merge of rust-lang#138731 - Zalathar:llvm-expansion, r=jieyouxu

coverage: Add LLVM plumbing for expansion regions

This is currently unused, but paves the way for future work on expansion regions without having to worry about the FFI parts.

The span conversion refactoring is only loosely related, but I've included it here because it would conflict with the main changes in `fill_region_tables`, and is pretty straightforward on its own.

Author: matthiaskrgr

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

@@ -146,6 +146,7 @@ pub(crate) struct CoverageSpan {
 #[derive(Clone, Debug, Default)]
 pub(crate) struct Regions {
     pub(crate) code_regions: Vec<CodeRegion>,
+    pub(crate) expansion_regions: Vec<ExpansionRegion>,
     pub(crate) branch_regions: Vec<BranchRegion>,
     pub(crate) mcdc_branch_regions: Vec<MCDCBranchRegion>,
     pub(crate) mcdc_decision_regions: Vec<MCDCDecisionRegion>,
@@ -154,10 +155,16 @@ 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 {
-        let Self { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } =
-            self;
+        let Self {
+            code_regions,
+            expansion_regions,
+            branch_regions,
+            mcdc_branch_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()
@@ -172,6 +179,14 @@ pub(crate) struct CodeRegion {
     pub(crate) counter: Counter,
 }
 
+/// Must match the layout of `LLVMRustCoverageExpansionRegion`.
+#[derive(Clone, Debug)]
+#[repr(C)]
+pub(crate) struct ExpansionRegion {
+    pub(crate) cov_span: CoverageSpan,
+    pub(crate) expanded_file_id: u32,
+}
+
 /// Must match the layout of `LLVMRustCoverageBranchRegion`.
 #[derive(Clone, Debug)]
 #[repr(C)]

compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs

@@ -63,8 +63,18 @@ pub(crate) fn write_function_mappings_to_buffer(
     expressions: &[ffi::CounterExpression],
     regions: &ffi::Regions,
 ) -> Vec<u8> {
-    let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } =
-        regions;
+    let ffi::Regions {
+        code_regions,
+        expansion_regions,
+        branch_regions,
+        mcdc_branch_regions,
+        mcdc_decision_regions,
+    } = regions;
+
+    // SAFETY:
+    // - All types are FFI-compatible and have matching representations in Rust/C++.
+    // - For pointer/length pairs, the pointer and length come from the same vector or slice.
+    // - C++ code does not retain any pointers after the call returns.
     llvm::build_byte_buffer(|buffer| unsafe {
         llvm::LLVMRustCoverageWriteFunctionMappingsToBuffer(
             virtual_file_mapping.as_ptr(),
@@ -73,6 +83,8 @@ pub(crate) fn write_function_mappings_to_buffer(
             expressions.len(),
             code_regions.as_ptr(),
             code_regions.len(),
+            expansion_regions.as_ptr(),
+            expansion_regions.len(),
             branch_regions.as_ptr(),
             branch_regions.len(),
             mcdc_branch_regions.as_ptr(),

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

@@ -120,12 +120,22 @@ fn fill_region_tables<'tcx>(
     // Associate that global file ID with a local file ID for this function.
     let local_file_id = covfun.virtual_file_mapping.local_id_for_global(global_file_id);
 
-    let ffi::Regions { code_regions, branch_regions, mcdc_branch_regions, mcdc_decision_regions } =
-        &mut covfun.regions;
-
-    let make_cov_span =
-        |span: Span| spans::make_coverage_span(local_file_id, source_map, &source_file, span);
+    // 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
+    // `-Zcoverage-options=discard-all-spans-in-codegen` to force it to occur.
     let discard_all = tcx.sess.coverage_discard_all_spans_in_codegen();
+    let make_coords = |span: Span| {
+        if discard_all { None } else { spans::make_coords(source_map, &source_file, span) }
+    };
+
+    let ffi::Regions {
+        code_regions,
+        expansion_regions: _, // FIXME(Zalathar): Fill out support for expansion regions
+        branch_regions,
+        mcdc_branch_regions,
+        mcdc_decision_regions,
+    } = &mut covfun.regions;
 
     // For each counter/region pair in this function+file, convert it to a
     // form suitable for FFI.
@@ -140,17 +150,8 @@ fn fill_region_tables<'tcx>(
             ffi::Counter::from_term(term)
         };
 
-        // Convert the `Span` into coordinates that we can pass to LLVM, or
-        // discard the span if conversion fails. In rare, cases _all_ of a
-        // function's spans are discarded, and the rest of coverage codegen
-        // needs to handle that gracefully to avoid a repeat of #133606.
-        // We don't have a good test case for triggering that organically, so
-        // instead we set `-Zcoverage-options=discard-all-spans-in-codegen`
-        // to force it to occur.
-        let Some(cov_span) = make_cov_span(span) else { continue };
-        if discard_all {
-            continue;
-        }
+        let Some(coords) = make_coords(span) else { continue };
+        let cov_span = coords.make_coverage_span(local_file_id);
 
         match *kind {
             MappingKind::Code { bcb } => {

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

@@ -5,22 +5,40 @@ use tracing::debug;
 use crate::coverageinfo::ffi;
 use crate::coverageinfo::mapgen::LocalFileId;
 
+/// Line and byte-column coordinates of a source code span within some file.
+/// The file itself must be tracked separately.
+#[derive(Clone, Copy, Debug)]
+pub(crate) struct Coords {
+    /// 1-based starting line of the source code span.
+    pub(crate) start_line: u32,
+    /// 1-based starting column (in bytes) of the source code span.
+    pub(crate) start_col: u32,
+    /// 1-based ending line of the source code span.
+    pub(crate) end_line: u32,
+    /// 1-based ending column (in bytes) of the source code span. High bit must be unset.
+    pub(crate) end_col: u32,
+}
+
+impl Coords {
+    /// Attaches a local file ID to these coordinates to produce an `ffi::CoverageSpan`.
+    pub(crate) fn make_coverage_span(&self, local_file_id: LocalFileId) -> ffi::CoverageSpan {
+        let &Self { start_line, start_col, end_line, end_col } = self;
+        let file_id = local_file_id.as_u32();
+        ffi::CoverageSpan { file_id, start_line, start_col, end_line, end_col }
+    }
+}
+
 /// Converts the span into its start line and column, and end line and column.
 ///
 /// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by
 /// the compiler, these column numbers are denoted in **bytes**, because that's what
 /// LLVM's `llvm-cov` tool expects to see in coverage maps.
 ///
-/// Returns `None` if the conversion failed for some reason. This shouldn't happen,
+/// Returns `None` if the conversion failed for some reason. This should be uncommon,
 /// but it's hard to rule out entirely (especially in the presence of complex macros
 /// 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_coverage_span(
-    file_id: LocalFileId,
-    source_map: &SourceMap,
-    file: &SourceFile,
-    span: Span,
-) -> Option<ffi::CoverageSpan> {
+pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) -> Option<Coords> {
     let span = ensure_non_empty_span(source_map, span)?;
 
     let lo = span.lo();
@@ -44,8 +62,7 @@ pub(crate) fn make_coverage_span(
     start_line = source_map.doctest_offset_line(&file.name, start_line);
     end_line = source_map.doctest_offset_line(&file.name, end_line);
 
-    check_coverage_span(ffi::CoverageSpan {
-        file_id: file_id.as_u32(),
+    check_coords(Coords {
         start_line: start_line as u32,
         start_col: start_col as u32,
         end_line: end_line as u32,
@@ -80,8 +97,8 @@ fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> {
 /// 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
 /// way that makes them improperly ordered.
-fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option<ffi::CoverageSpan> {
-    let ffi::CoverageSpan { file_id: _, start_line, start_col, end_line, end_col } = cov_span;
+fn check_coords(coords: Coords) -> Option<Coords> {
+    let Coords { start_line, start_col, end_line, end_col } = coords;
 
     // Line/column coordinates are supposed to be 1-based. If we ever emit
     // coordinates of 0, `llvm-cov` might misinterpret them.
@@ -94,17 +111,17 @@ fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option<ffi::CoverageSpan>
     let is_ordered = (start_line, start_col) <= (end_line, end_col);
 
     if all_nonzero && end_col_has_high_bit_unset && is_ordered {
-        Some(cov_span)
+        Some(coords)
     } else {
         debug!(
-            ?cov_span,
+            ?coords,
             ?all_nonzero,
             ?end_col_has_high_bit_unset,
             ?is_ordered,
             "Skipping source region that would be misinterpreted or rejected by LLVM"
         );
         // If this happens in a debug build, ICE to make it easier to notice.
-        debug_assert!(false, "Improper source region: {cov_span:?}");
+        debug_assert!(false, "Improper source region: {coords:?}");
         None
     }
 }

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -2019,6 +2019,8 @@ unsafe extern "C" {
         NumExpressions: size_t,
         CodeRegions: *const crate::coverageinfo::ffi::CodeRegion,
         NumCodeRegions: size_t,
+        ExpansionRegions: *const crate::coverageinfo::ffi::ExpansionRegion,
+        NumExpansionRegions: size_t,
         BranchRegions: *const crate::coverageinfo::ffi::BranchRegion,
         NumBranchRegions: size_t,
         MCDCBranchRegions: *const crate::coverageinfo::ffi::MCDCBranchRegion,

compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp

@@ -77,6 +77,13 @@ struct LLVMRustCoverageCodeRegion {
   LLVMRustCounter Count;
 };
 
+// Must match the layout of
+// `rustc_codegen_llvm::coverageinfo::ffi::ExpansionRegion`.
+struct LLVMRustCoverageExpansionRegion {
+  LLVMRustCoverageSpan Span;
+  uint32_t ExpandedFileID;
+};
+
 // Must match the layout of
 // `rustc_codegen_llvm::coverageinfo::ffi::BranchRegion`.
 struct LLVMRustCoverageBranchRegion {
@@ -151,6 +158,8 @@ extern "C" void LLVMRustCoverageWriteFunctionMappingsToBuffer(
     const unsigned *VirtualFileMappingIDs, size_t NumVirtualFileMappingIDs,
     const LLVMRustCounterExpression *RustExpressions, size_t NumExpressions,
     const LLVMRustCoverageCodeRegion *CodeRegions, size_t NumCodeRegions,
+    const LLVMRustCoverageExpansionRegion *ExpansionRegions,
+    size_t NumExpansionRegions,
     const LLVMRustCoverageBranchRegion *BranchRegions, size_t NumBranchRegions,
     const LLVMRustCoverageMCDCBranchRegion *MCDCBranchRegions,
     size_t NumMCDCBranchRegions,
@@ -179,6 +188,13 @@ extern "C" void LLVMRustCoverageWriteFunctionMappingsToBuffer(
         Region.Span.ColumnStart, Region.Span.LineEnd, Region.Span.ColumnEnd));
   }
 
+  // Expansion regions:
+  for (const auto &Region : ArrayRef(ExpansionRegions, NumExpansionRegions)) {
+    MappingRegions.push_back(coverage::CounterMappingRegion::makeExpansion(
+        Region.Span.FileID, Region.ExpandedFileID, Region.Span.LineStart,
+        Region.Span.ColumnStart, Region.Span.LineEnd, Region.Span.ColumnEnd));
+  }
+
   // Branch regions:
   for (const auto &Region : ArrayRef(BranchRegions, NumBranchRegions)) {
     MappingRegions.push_back(coverage::CounterMappingRegion::makeBranchRegion(