diff options
| author | Zalathar <Zalathar@users.noreply.github.com> | 2024-11-29 14:52:41 +1100 |
|---|---|---|
| committer | Zalathar <Zalathar@users.noreply.github.com> | 2024-11-29 14:57:01 +1100 |
| commit | 9461f4296ffcbd75440ec211ead7bff6fb86012c (patch) | |
| tree | 4855cb6a7cc7ddb6c3de368750840ac255f387be /compiler/rustc_mir_transform/src | |
| parent | d53f0b1d8e261f2f3535f1cd165c714fc0b0b298 (diff) | |
| download | rust-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.rs | 166 |
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 { |
