Skip to content

Commit

Permalink
coverage: llvm-cov expects column numbers to be bytes, not code points
Browse files Browse the repository at this point in the history
  • Loading branch information
Zalathar committed Dec 17, 2023
1 parent 5db359c commit ae031ef
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 24 deletions.
59 changes: 46 additions & 13 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rustc_middle::mir::{
use rustc_middle::ty::TyCtxt;
use rustc_span::def_id::LocalDefId;
use rustc_span::source_map::SourceMap;
use rustc_span::{ExpnKind, Span, Symbol};
use rustc_span::{BytePos, ExpnKind, Pos, RelativeBytePos, Span, Symbol};

/// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected
/// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen
Expand Down Expand Up @@ -258,6 +258,10 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb

/// Convert the Span into its file name, 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. There is no known example
/// of code that would cause this to happen, but it's hard to rule out entirely
/// (especially in the presence of complex macros or other expansions), and if it does
Expand All @@ -276,20 +280,49 @@ fn make_code_region(
source_map.span_to_diagnostic_string(body_span)
);

let (file, mut start_line, mut start_col, mut end_line, mut end_col) =
source_map.span_to_location_info(span);
if span.hi() == span.lo() {
// Extend an empty span by one character so the region will be counted.
if span.hi() == body_span.hi() {
start_col = start_col.saturating_sub(1);
} else {
end_col = start_col + 1;
}
let lo = span.lo();
let hi = span.hi();

let file = source_map.lookup_source_file(lo);

// Column numbers need to be in bytes, so we can't use the more convenient
// `SourceMap` methods for looking up file coordinates.
let rpos_and_line_and_byte_column = |pos: BytePos| -> Option<(RelativeBytePos, 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((rpos, line_index + 1, (rpos - line_start).to_usize() + 1))
};
if let Some(file) = file {
start_line = source_map.doctest_offset_line(&file.name, start_line);
end_line = source_map.doctest_offset_line(&file.name, end_line);

let (lo_rpos, mut start_line, mut start_col) = rpos_and_line_and_byte_column(lo)?;
let (hi_rpos, mut end_line, mut end_col) = rpos_and_line_and_byte_column(hi)?;

// If the span is empty, try to expand it by one character so that it is
// more visible in `llvm-cov` reports. (LLVM measures columns in bytes,
// so "one character" might be multiple bytes.)
if lo == hi
&& let Some(src) = &file.src
{
// Prefer to expand the end position, if it won't go outside the body span.
if hi < body_span.hi() {
let hi_rpos = hi_rpos.to_usize();
let nudge_bytes = src.ceil_char_boundary(hi_rpos + 1) - hi_rpos;
end_col += nudge_bytes;
} else if lo > body_span.lo() {
let lo_rpos = lo_rpos.to_usize();
let nudge_bytes = lo_rpos - src.floor_char_boundary(lo_rpos - 1);
// Subtract the nudge, but don't go below column 1.
start_col = start_col.saturating_sub(nudge_bytes).max(1);
}
// If neither nudge could be applied, stick with the empty span coordinates.
}

// 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);

Some(CodeRegion {
file_name,
start_line: start_line as u32,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#![feature(min_specialization)]
#![feature(never_type)]
#![feature(option_get_or_insert_default)]
#![feature(round_char_boundary)]
#![feature(trusted_step)]
#![feature(try_blocks)]
#![feature(yeet_expr)]
Expand Down
18 changes: 9 additions & 9 deletions tests/coverage/unicode.cov-map
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Function name: unicode::main
Raw bytes (67): 0x[01, 01, 09, 01, 05, 03, 05, 1e, 0d, 22, 09, 03, 05, 11, 1b, 1e, 0d, 22, 09, 03, 05, 09, 01, 0c, 01, 00, 0b, 05, 01, 09, 00, 0b, 03, 00, 0f, 00, 18, 05, 00, 19, 00, 24, 22, 02, 08, 00, 13, 09, 00, 17, 00, 22, 11, 00, 23, 02, 06, 1b, 02, 06, 00, 07, 17, 02, 05, 01, 02]
Raw bytes (67): 0x[01, 01, 09, 01, 05, 03, 05, 1e, 0d, 22, 09, 03, 05, 11, 1b, 1e, 0d, 22, 09, 03, 05, 09, 01, 0c, 01, 00, 0b, 05, 01, 09, 00, 0c, 03, 00, 10, 00, 1b, 05, 00, 1c, 00, 28, 22, 02, 08, 00, 25, 09, 00, 29, 00, 46, 11, 00, 47, 02, 06, 1b, 02, 06, 00, 07, 17, 02, 05, 01, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 9
Expand All @@ -14,26 +14,26 @@ Number of expressions: 9
- expression 8 operands: lhs = Expression(0, Add), rhs = Counter(1)
Number of file 0 mappings: 9
- Code(Counter(0)) at (prev + 12, 1) to (start + 0, 11)
- Code(Counter(1)) at (prev + 1, 9) to (start + 0, 11)
- Code(Expression(0, Add)) at (prev + 0, 15) to (start + 0, 24)
- Code(Counter(1)) at (prev + 1, 9) to (start + 0, 12)
- Code(Expression(0, Add)) at (prev + 0, 16) to (start + 0, 27)
= (c0 + c1)
- Code(Counter(1)) at (prev + 0, 25) to (start + 0, 36)
- Code(Expression(8, Sub)) at (prev + 2, 8) to (start + 0, 19)
- Code(Counter(1)) at (prev + 0, 28) to (start + 0, 40)
- Code(Expression(8, Sub)) at (prev + 2, 8) to (start + 0, 37)
= ((c0 + c1) - c1)
- Code(Counter(2)) at (prev + 0, 23) to (start + 0, 34)
- Code(Counter(4)) at (prev + 0, 35) to (start + 2, 6)
- Code(Counter(2)) at (prev + 0, 41) to (start + 0, 70)
- Code(Counter(4)) at (prev + 0, 71) to (start + 2, 6)
- Code(Expression(6, Add)) at (prev + 2, 6) to (start + 0, 7)
= ((((c0 + c1) - c1) - c2) + c3)
- Code(Expression(5, Add)) at (prev + 2, 5) to (start + 1, 2)
= (c4 + ((((c0 + c1) - c1) - c2) + c3))

Function name: unicode::サビ
Raw bytes (9): 0x[01, 01, 00, 01, 01, 1c, 12, 00, 14]
Raw bytes (9): 0x[01, 01, 00, 01, 01, 1c, 14, 00, 18]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 28, 18) to (start + 0, 20)
- Code(Counter(0)) at (prev + 28, 20) to (start + 0, 24)

Function name: unicode::申し訳ございません
Raw bytes (9): 0x[01, 01, 00, 01, 01, 16, 01, 02, 02]
Expand Down
4 changes: 2 additions & 2 deletions tests/coverage/unicode.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
LL| |
LL| 1|fn main() {
LL| 33| for _İ in 'А'..='Я' { /* Я */ }
^32 ^32
^32 ^32
LL| |
LL| 1| if 申し訳ございません() && 申し訳ございません() {
^0
^0
LL| 0| println!("true");
LL| 1| }
LL| |
Expand Down

0 comments on commit ae031ef

Please sign in to comment.