about summary refs log tree commit diff
path: root/compiler/rustc_mir_transform/src/coverage/mod.rs
diff options
context:
space:
mode:
authorLeón Orell Valerian Liehr <me@fmease.dev>2024-01-24 15:43:11 +0100
committerGitHub <noreply@github.com>2024-01-24 15:43:11 +0100
commit5a38754d23ef2d5b6ce16dcafdb9ac434726c7d9 (patch)
treed2dd0087f6c7495575e1cb400f5f10b5c263a994 /compiler/rustc_mir_transform/src/coverage/mod.rs
parent3529d45b7470c13eb1ab2bc03d660e1e1846ded8 (diff)
parent21e5beae3cc4ffd2adea9ae7b4a9d8b84a4bc0a8 (diff)
downloadrust-5a38754d23ef2d5b6ce16dcafdb9ac434726c7d9.tar.gz
rust-5a38754d23ef2d5b6ce16dcafdb9ac434726c7d9.zip
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
Diffstat (limited to 'compiler/rustc_mir_transform/src/coverage/mod.rs')
-rw-r--r--compiler/rustc_mir_transform/src/coverage/mod.rs35
1 files changed, 34 insertions, 1 deletions
diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
index a11d224e8f1..b6c06ac2d04 100644
--- a/compiler/rustc_mir_transform/src/coverage/mod.rs
+++ b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -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).