Auto merge of #120309 - fmease:rollup-kr7wqy6, r=fmease

Rollup of 9 pull requests

Successful merges:

 - #114764 ([style edition 2024] Combine all delimited exprs as last argument)
 - #118326 (Add `NonZero*::count_ones`)
 - #119460 (coverage: Never emit improperly-ordered coverage regions)
 - #119616 (Add a new `wasm32-wasi-preview2` target)
 - #120185 (coverage: Don't instrument `#[automatically_derived]` functions)
 - #120265 (Remove no-system-llvm)
 - #120284 (privacy: Refactor top-level visiting in `TypePrivacyVisitor`)
 - #120285 (Remove extra # from url in suggestion)
 - #120299 (Add mw to review rotation and add some owner assignments)

r? `@ghost`
`@rustbot` modify labels: rollup

1 files changed, 45 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..316f7d678c6 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).
@@ -351,7 +384,18 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
         return false;
     }
 
+    // Don't instrument functions with `#[automatically_derived]` on their
+    // enclosing impl block, on the assumption that most users won't care about
+    // coverage for derived impls.
+    if let Some(impl_of) = tcx.impl_of_method(def_id.to_def_id())
+        && tcx.is_automatically_derived(impl_of)
+    {
+        trace!("InstrumentCoverage skipped for {def_id:?} (automatically derived)");
+        return false;
+    }
+
     if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
+        trace!("InstrumentCoverage skipped for {def_id:?} (`#[coverage(off)]`)");
         return false;
     }