Rollup merge of #119460 - Zalathar:improper-region, r=wesleywiser

coverage: Never emit improperly-ordered coverage regions

If we emit a coverage region that is improperly ordered (end < start), `llvm-cov` will fail with `coveragemap_error::malformed`, which is inconvenient for users and also very hard to debug.

Ideally we would fix the root causes of these situations, but they tend to occur in very obscure edge-case scenarios (often involving nested macros), and we don't always have a good MCVE to work from. So it makes sense to also have a catch-all check that will prevent improperly-ordered regions from ever being emitted.

---

This is mainly aimed at resolving #119453. We don't have a specific way to reproduce it, which is why I haven't been able to add a test case in this PR. But based on the information provided in that issue, this change seems likely to avoid the error in `llvm-cov`.

`````@rustbot````` label +A-code-coverage
This commit is contained in:
León Orell Valerian Liehr 2024-01-24 15:43:11 +01:00 committed by GitHub
commit 5a38754d23
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -329,7 +329,7 @@ fn make_code_region(
start_line = source_map.doctest_offset_line(&file.name, start_line);
end_line = source_map.doctest_offset_line(&file.name, end_line);
Some(CodeRegion {
check_code_region(CodeRegion {
file_name,
start_line: start_line as u32,
start_col: start_col as u32,
@ -338,6 +338,39 @@ fn make_code_region(
})
}
/// If `llvm-cov` sees a code 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_code_region(code_region: CodeRegion) -> Option<CodeRegion> {
let CodeRegion { file_name: _, start_line, start_col, end_line, end_col } = code_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(code_region)
} else {
debug!(
?code_region,
?all_nonzero,
?end_col_has_high_bit_unset,
?is_ordered,
"Skipping code 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 code region: {code_region:?}");
None
}
}
fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
// Only instrument functions, methods, and closures (not constants since they are evaluated
// at compile time by Miri).