about summary refs log tree commit diff
path: root/compiler/rustc_mir_transform/src
diff options
context:
space:
mode:
authorZalathar <Zalathar@users.noreply.github.com>2024-11-29 14:52:41 +1100
committerZalathar <Zalathar@users.noreply.github.com>2024-11-29 14:57:01 +1100
commit9461f4296ffcbd75440ec211ead7bff6fb86012c (patch)
tree4855cb6a7cc7ddb6c3de368750840ac255f387be /compiler/rustc_mir_transform/src
parentd53f0b1d8e261f2f3535f1cd165c714fc0b0b298 (diff)
downloadrust-9461f4296ffcbd75440ec211ead7bff6fb86012c.tar.gz
rust-9461f4296ffcbd75440ec211ead7bff6fb86012c.zip
Revert "Rollup merge of #133418 - Zalathar:spans, r=jieyouxu"
This reverts commit adf9b5fcd1de43eaf0a779e10612caee8b47bede, reversing
changes made to af1ca153d4aed5ffe22445273aa388a8d3f8f4ae.

Reverting due to <https://github.com/rust-lang/rust/issues/133606>.
Diffstat (limited to 'compiler/rustc_mir_transform/src')
-rw-r--r--compiler/rustc_mir_transform/src/coverage/mod.rs166
1 files changed, 148 insertions, 18 deletions
diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
index 02fd289b6ef..e8b3d80be02 100644
--- a/compiler/rustc_mir_transform/src/coverage/mod.rs
+++ b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -13,15 +13,16 @@ use rustc_hir::intravisit::{Visitor, walk_expr};
 use rustc_middle::hir::map::Map;
 use rustc_middle::hir::nested_filter;
 use rustc_middle::mir::coverage::{
-    CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind,
+    CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, SourceRegion,
 };
 use rustc_middle::mir::{
     self, BasicBlock, BasicBlockData, SourceInfo, Statement, StatementKind, Terminator,
     TerminatorKind,
 };
 use rustc_middle::ty::TyCtxt;
-use rustc_span::Span;
 use rustc_span::def_id::LocalDefId;
+use rustc_span::source_map::SourceMap;
+use rustc_span::{BytePos, Pos, SourceFile, Span};
 use tracing::{debug, debug_span, trace};
 
 use crate::coverage::counters::{CounterIncrementSite, CoverageCounters};
@@ -96,7 +97,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
     let coverage_counters =
         CoverageCounters::make_bcb_counters(&basic_coverage_blocks, &bcbs_with_counter_mappings);
 
-    let mappings = create_mappings(&extracted_mappings, &coverage_counters);
+    let mappings = create_mappings(tcx, &hir_info, &extracted_mappings, &coverage_counters);
     if mappings.is_empty() {
         // No spans could be converted into valid mappings, so skip this function.
         debug!("no spans could be converted into valid mappings; skipping");
@@ -135,12 +136,18 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
 ///
 /// Precondition: All BCBs corresponding to those spans have been given
 /// coverage counters.
-fn create_mappings(
+fn create_mappings<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    hir_info: &ExtractedHirInfo,
     extracted_mappings: &ExtractedMappings,
     coverage_counters: &CoverageCounters,
 ) -> Vec<Mapping> {
+    let source_map = tcx.sess.source_map();
+    let file = source_map.lookup_source_file(hir_info.body_span.lo());
+
     let term_for_bcb =
         |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters");
+    let region_for_span = |span: Span| make_source_region(source_map, hir_info, &file, span);
 
     // Fully destructure the mappings struct to make sure we don't miss any kinds.
     let ExtractedMappings {
@@ -153,20 +160,22 @@ fn create_mappings(
     } = extracted_mappings;
     let mut mappings = Vec::new();
 
-    mappings.extend(code_mappings.iter().map(
+    mappings.extend(code_mappings.iter().filter_map(
         // Ordinary code mappings are the simplest kind.
         |&mappings::CodeMapping { span, bcb }| {
+            let source_region = region_for_span(span)?;
             let kind = MappingKind::Code(term_for_bcb(bcb));
-            Mapping { kind, span }
+            Some(Mapping { kind, source_region })
         },
     ));
 
-    mappings.extend(branch_pairs.iter().map(
+    mappings.extend(branch_pairs.iter().filter_map(
         |&mappings::BranchPair { span, true_bcb, false_bcb }| {
             let true_term = term_for_bcb(true_bcb);
             let false_term = term_for_bcb(false_bcb);
             let kind = MappingKind::Branch { true_term, false_term };
-            Mapping { kind, span }
+            let source_region = region_for_span(span)?;
+            Some(Mapping { kind, source_region })
         },
     ));
 
@@ -174,7 +183,7 @@ fn create_mappings(
         |bcb| coverage_counters.term_for_bcb(bcb).expect("all BCBs with spans were given counters");
 
     // MCDC branch mappings are appended with their decisions in case decisions were ignored.
-    mappings.extend(mcdc_degraded_branches.iter().map(
+    mappings.extend(mcdc_degraded_branches.iter().filter_map(
         |&mappings::MCDCBranch {
              span,
              true_bcb,
@@ -183,9 +192,10 @@ fn create_mappings(
              true_index: _,
              false_index: _,
          }| {
+            let source_region = region_for_span(span)?;
             let true_term = term_for_bcb(true_bcb);
             let false_term = term_for_bcb(false_bcb);
-            Mapping { kind: MappingKind::Branch { true_term, false_term }, span }
+            Some(Mapping { kind: MappingKind::Branch { true_term, false_term }, source_region })
         },
     ));
 
@@ -193,7 +203,7 @@ fn create_mappings(
         let num_conditions = branches.len() as u16;
         let conditions = branches
             .into_iter()
-            .map(
+            .filter_map(
                 |&mappings::MCDCBranch {
                      span,
                      true_bcb,
@@ -202,29 +212,31 @@ fn create_mappings(
                      true_index: _,
                      false_index: _,
                  }| {
+                    let source_region = region_for_span(span)?;
                     let true_term = term_for_bcb(true_bcb);
                     let false_term = term_for_bcb(false_bcb);
-                    Mapping {
+                    Some(Mapping {
                         kind: MappingKind::MCDCBranch {
                             true_term,
                             false_term,
                             mcdc_params: condition_info,
                         },
-                        span,
-                    }
+                        source_region,
+                    })
                 },
             )
             .collect::<Vec<_>>();
 
-        if conditions.len() == num_conditions as usize {
+        if conditions.len() == num_conditions as usize
+            && let Some(source_region) = region_for_span(decision.span)
+        {
             // LLVM requires end index for counter mapping regions.
             let kind = MappingKind::MCDCDecision(DecisionInfo {
                 bitmap_idx: (decision.bitmap_idx + decision.num_test_vectors) as u32,
                 num_conditions,
             });
             mappings.extend(
-                std::iter::once(Mapping { kind, span: decision.span })
-                    .chain(conditions.into_iter()),
+                std::iter::once(Mapping { kind, source_region }).chain(conditions.into_iter()),
             );
         } else {
             mappings.extend(conditions.into_iter().map(|mapping| {
@@ -233,7 +245,10 @@ fn create_mappings(
                 else {
                     unreachable!("all mappings here are MCDCBranch as shown above");
                 };
-                Mapping { kind: MappingKind::Branch { true_term, false_term }, span: mapping.span }
+                Mapping {
+                    kind: MappingKind::Branch { true_term, false_term },
+                    source_region: mapping.source_region,
+                }
             }))
         }
     }
@@ -376,6 +391,121 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
     data.statements.insert(0, statement);
 }
 
+fn ensure_non_empty_span(
+    source_map: &SourceMap,
+    hir_info: &ExtractedHirInfo,
+    span: Span,
+) -> Option<Span> {
+    if !span.is_empty() {
+        return Some(span);
+    }
+
+    let lo = span.lo();
+    let hi = span.hi();
+
+    // The span is empty, so try to expand it to cover an adjacent '{' or '}',
+    // but only within the bounds of the body span.
+    let try_next = hi < hir_info.body_span.hi();
+    let try_prev = hir_info.body_span.lo() < lo;
+    if !(try_next || try_prev) {
+        return None;
+    }
+
+    source_map
+        .span_to_source(span, |src, start, end| try {
+            // We're only checking for specific ASCII characters, so we don't
+            // have to worry about multi-byte code points.
+            if try_next && src.as_bytes()[end] == b'{' {
+                Some(span.with_hi(hi + BytePos(1)))
+            } else if try_prev && src.as_bytes()[start - 1] == b'}' {
+                Some(span.with_lo(lo - BytePos(1)))
+            } else {
+                None
+            }
+        })
+        .ok()?
+}
+
+/// 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,
+/// 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.
+fn make_source_region(
+    source_map: &SourceMap,
+    hir_info: &ExtractedHirInfo,
+    file: &SourceFile,
+    span: Span,
+) -> Option<SourceRegion> {
+    let span = ensure_non_empty_span(source_map, hir_info, span)?;
+
+    let lo = span.lo();
+    let hi = span.hi();
+
+    // Column numbers need to be in bytes, so we can't use the more convenient
+    // `SourceMap` methods for looking up file coordinates.
+    let line_and_byte_column = |pos: BytePos| -> Option<(usize, usize)> {
+        let rpos = file.relative_position(pos);
+        let line_index = file.lookup_line(rpos)?;
+        let line_start = file.lines()[line_index];
+        // Line numbers and column numbers are 1-based, so add 1 to each.
+        Some((line_index + 1, (rpos - line_start).to_usize() + 1))
+    };
+
+    let (mut start_line, start_col) = line_and_byte_column(lo)?;
+    let (mut end_line, end_col) = line_and_byte_column(hi)?;
+
+    // Apply an offset so that code in doctests has correct line numbers.
+    // FIXME(#79417): Currently we have no way to offset doctest _columns_.
+    start_line = source_map.doctest_offset_line(&file.name, start_line);
+    end_line = source_map.doctest_offset_line(&file.name, end_line);
+
+    check_source_region(SourceRegion {
+        start_line: start_line as u32,
+        start_col: start_col as u32,
+        end_line: end_line as u32,
+        end_col: end_col as u32,
+    })
+}
+
+/// 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
+/// way that makes them improperly ordered.
+fn check_source_region(source_region: SourceRegion) -> Option<SourceRegion> {
+    let SourceRegion { start_line, start_col, end_line, end_col } = source_region;
+
+    // Line/column coordinates are supposed to be 1-based. If we ever emit
+    // coordinates of 0, `llvm-cov` might misinterpret them.
+    let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0);
+    // Coverage mappings use the high bit of `end_col` to indicate that a
+    // region is actually a "gap" region, so make sure it's unset.
+    let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0;
+    // If a region is improperly ordered (end < start), `llvm-cov` will exit
+    // with a fatal error, which is inconvenient for users and hard to debug.
+    let is_ordered = (start_line, start_col) <= (end_line, end_col);
+
+    if all_nonzero && end_col_has_high_bit_unset && is_ordered {
+        Some(source_region)
+    } else {
+        debug!(
+            ?source_region,
+            ?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: {source_region:?}");
+        None
+    }
+}
+
 /// Function information extracted from HIR by the coverage instrumentor.
 #[derive(Debug)]
 struct ExtractedHirInfo {