Auto merge of #141243 - Zalathar:rollup-x5xt80l, r=Zalathar

Rollup of 5 pull requests

Successful merges:

 - #140847 (coverage: Detect unused local file IDs to avoid an LLVM assertion)
 - #141117 (opt-dist: fix deprecated BOLT -icf=1 option)
 - #141225 (more ice tests)
 - #141239 (dladdr cannot leave dli_fname to be null)
 - #141242 (in `tests/ui/asm/aarch64/parse-error.rs`, only test cases specific to that target)

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

Author: bors

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

@@ -155,6 +155,20 @@ pub(crate) struct Regions {
 impl Regions {
     /// Returns true if none of this structure's tables contain any regions.
     pub(crate) fn has_no_regions(&self) -> bool {
+        // Every region has a span, so if there are no spans then there are no regions.
+        self.all_cov_spans().next().is_none()
+    }
+
+    pub(crate) fn all_cov_spans(&self) -> impl Iterator<Item = &CoverageSpan> {
+        macro_rules! iter_cov_spans {
+            ( $( $regions:expr ),* $(,)? ) => {
+                std::iter::empty()
+                $(
+                    .chain( $regions.iter().map(|region| &region.cov_span) )
+                )*
+            }
+        }
+
         let Self {
             code_regions,
             expansion_regions,
@@ -163,11 +177,13 @@ impl Regions {
             mcdc_decision_regions,
         } = self;
 
-        code_regions.is_empty()
-            && expansion_regions.is_empty()
-            && branch_regions.is_empty()
-            && mcdc_branch_regions.is_empty()
-            && mcdc_decision_regions.is_empty()
+        iter_cov_spans!(
+            code_regions,
+            expansion_regions,
+            branch_regions,
+            mcdc_branch_regions,
+            mcdc_decision_regions,
+        )
     }
 }
 

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs

@@ -11,6 +11,7 @@ use rustc_abi::Align;
 use rustc_codegen_ssa::traits::{
     BaseTypeCodegenMethods as _, ConstCodegenMethods, StaticCodegenMethods,
 };
+use rustc_index::IndexVec;
 use rustc_middle::mir::coverage::{
     BasicCoverageBlock, CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping,
     MappingKind, Op,
@@ -104,6 +105,16 @@ fn fill_region_tables<'tcx>(
     ids_info: &'tcx CoverageIdsInfo,
     covfun: &mut CovfunRecord<'tcx>,
 ) {
+    // If this function is unused, replace all counters with zero.
+    let counter_for_bcb = |bcb: BasicCoverageBlock| -> ffi::Counter {
+        let term = if covfun.is_used {
+            ids_info.term_for_bcb[bcb].expect("every BCB in a mapping was given a term")
+        } else {
+            CovTerm::Zero
+        };
+        ffi::Counter::from_term(term)
+    };
+
     // Currently a function's mappings must all be in the same file, so use the
     // first mapping's span to determine the file.
     let source_map = tcx.sess.source_map();
@@ -115,6 +126,12 @@ fn fill_region_tables<'tcx>(
 
     let local_file_id = covfun.virtual_file_mapping.push_file(&source_file);
 
+    // If this testing flag is set, add an extra unused entry to the local
+    // file table, to help test the code for detecting unused file IDs.
+    if tcx.sess.coverage_inject_unused_local_file() {
+        covfun.virtual_file_mapping.push_file(&source_file);
+    }
+
     // In rare cases, _all_ of a function's spans are discarded, and coverage
     // codegen needs to handle that gracefully to avoid #133606.
     // It's hard for tests to trigger this organically, so instead we set
@@ -135,16 +152,6 @@ fn fill_region_tables<'tcx>(
     // For each counter/region pair in this function+file, convert it to a
     // form suitable for FFI.
     for &Mapping { ref kind, span } in &fn_cov_info.mappings {
-        // If this function is unused, replace all counters with zero.
-        let counter_for_bcb = |bcb: BasicCoverageBlock| -> ffi::Counter {
-            let term = if covfun.is_used {
-                ids_info.term_for_bcb[bcb].expect("every BCB in a mapping was given a term")
-            } else {
-                CovTerm::Zero
-            };
-            ffi::Counter::from_term(term)
-        };
-
         let Some(coords) = make_coords(span) else { continue };
         let cov_span = coords.make_coverage_span(local_file_id);
 
@@ -177,6 +184,19 @@ fn fill_region_tables<'tcx>(
     }
 }
 
+/// LLVM requires all local file IDs to have at least one mapping region.
+/// If that's not the case, skip this function, to avoid an assertion failure
+/// (or worse) in LLVM.
+fn check_local_file_table(covfun: &CovfunRecord<'_>) -> bool {
+    let mut local_file_id_seen =
+        IndexVec::<u32, _>::from_elem_n(false, covfun.virtual_file_mapping.local_file_table.len());
+    for cov_span in covfun.regions.all_cov_spans() {
+        local_file_id_seen[cov_span.file_id] = true;
+    }
+
+    local_file_id_seen.into_iter().all(|seen| seen)
+}
+
 /// Generates the contents of the covfun record for this function, which
 /// contains the function's coverage mapping data. The record is then stored
 /// as a global variable in the `__llvm_covfun` section.
@@ -185,6 +205,10 @@ pub(crate) fn generate_covfun_record<'tcx>(
     global_file_table: &GlobalFileTable,
     covfun: &CovfunRecord<'tcx>,
 ) {
+    if !check_local_file_table(covfun) {
+        return;
+    }
+
     let &CovfunRecord {
         mangled_function_name,
         source_hash,

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs

@@ -39,7 +39,10 @@ impl Coords {
 /// 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.
 pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span) -> Option<Coords> {
-    let span = ensure_non_empty_span(source_map, span)?;
+    if span.is_empty() {
+        debug_assert!(false, "can't make coords from empty span: {span:?}");
+        return None;
+    }
 
     let lo = span.lo();
     let hi = span.hi();
@@ -70,29 +73,6 @@ pub(crate) fn make_coords(source_map: &SourceMap, file: &SourceFile, span: Span)
     })
 }
 
-fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> {
-    if !span.is_empty() {
-        return Some(span);
-    }
-
-    // The span is empty, so try to enlarge it to cover an adjacent '{' or '}'.
-    source_map
-        .span_to_source(span, |src, start, end| try {
-            // Adjusting span endpoints by `BytePos(1)` is normally a bug,
-            // but in this case we have specifically checked that the character
-            // we're skipping over is one of two specific ASCII characters, so
-            // adjusting by exactly 1 byte is correct.
-            if src.as_bytes().get(end).copied() == Some(b'{') {
-                Some(span.with_hi(span.hi() + BytePos(1)))
-            } else if start > 0 && src.as_bytes()[start - 1] == b'}' {
-                Some(span.with_lo(span.lo() - BytePos(1)))
-            } else {
-                None
-            }
-        })
-        .ok()?
-}
-
 /// 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

compiler/rustc_interface/src/tests.rs

@@ -776,7 +776,8 @@ fn test_unstable_options_tracking_hash() {
         CoverageOptions {
             level: CoverageLevel::Mcdc,
             no_mir_spans: true,
-            discard_all_spans_in_codegen: true
+            discard_all_spans_in_codegen: true,
+            inject_unused_local_file: true,
         }
     );
     tracked!(crate_attr, vec!["abc".to_string()]);

compiler/rustc_mir_transform/src/coverage/spans.rs

@@ -1,7 +1,8 @@
 use rustc_data_structures::fx::FxHashSet;
 use rustc_middle::mir;
 use rustc_middle::ty::TyCtxt;
-use rustc_span::{DesugaringKind, ExpnKind, MacroKind, Span};
+use rustc_span::source_map::SourceMap;
+use rustc_span::{BytePos, DesugaringKind, ExpnKind, MacroKind, Span};
 use tracing::instrument;
 
 use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
@@ -83,8 +84,18 @@ pub(super) fn extract_refined_covspans<'tcx>(
     // Discard any span that overlaps with a hole.
     discard_spans_overlapping_holes(&mut covspans, &holes);
 
-    // Perform more refinement steps after holes have been dealt with.
+    // Discard spans that overlap in unwanted ways.
     let mut covspans = remove_unwanted_overlapping_spans(covspans);
+
+    // For all empty spans, either enlarge them to be non-empty, or discard them.
+    let source_map = tcx.sess.source_map();
+    covspans.retain_mut(|covspan| {
+        let Some(span) = ensure_non_empty_span(source_map, covspan.span) else { return false };
+        covspan.span = span;
+        true
+    });
+
+    // Merge covspans that can be merged.
     covspans.dedup_by(|b, a| a.merge_if_eligible(b));
 
     code_mappings.extend(covspans.into_iter().map(|Covspan { span, bcb }| {
@@ -230,3 +241,26 @@ fn compare_spans(a: Span, b: Span) -> std::cmp::Ordering {
         // - Both have the same start and span A extends further right
         .then_with(|| Ord::cmp(&a.hi(), &b.hi()).reverse())
 }
+
+fn ensure_non_empty_span(source_map: &SourceMap, span: Span) -> Option<Span> {
+    if !span.is_empty() {
+        return Some(span);
+    }
+
+    // The span is empty, so try to enlarge it to cover an adjacent '{' or '}'.
+    source_map
+        .span_to_source(span, |src, start, end| try {
+            // Adjusting span endpoints by `BytePos(1)` is normally a bug,
+            // but in this case we have specifically checked that the character
+            // we're skipping over is one of two specific ASCII characters, so
+            // adjusting by exactly 1 byte is correct.
+            if src.as_bytes().get(end).copied() == Some(b'{') {
+                Some(span.with_hi(span.hi() + BytePos(1)))
+            } else if start > 0 && src.as_bytes()[start - 1] == b'}' {
+                Some(span.with_lo(span.lo() - BytePos(1)))
+            } else {
+                None
+            }
+        })
+        .ok()?
+}

compiler/rustc_session/src/config.rs

@@ -195,6 +195,11 @@ pub struct CoverageOptions {
     /// regression tests for #133606, because we don't have an easy way to
     /// reproduce it from actual source code.
     pub discard_all_spans_in_codegen: bool,
+
+    /// `-Zcoverage-options=inject-unused-local-file`: During codegen, add an
+    /// extra dummy entry to each function's local file table, to exercise the
+    /// code that checks for local file IDs with no mapping regions.
+    pub inject_unused_local_file: bool,
 }
 
 /// Controls whether branch coverage or MC/DC coverage is enabled.

compiler/rustc_session/src/filesearch.rs

@@ -82,9 +82,7 @@ fn current_dll_path() -> Result<PathBuf, String> {
                 let fname_ptr = info.dli_fname.as_ptr();
                 #[cfg(not(target_os = "cygwin"))]
                 let fname_ptr = {
-                    if info.dli_fname.is_null() {
-                        return Err("dladdr returned null pointer".into());
-                    }
+                    assert!(!info.dli_fname.is_null(), "the docs do not allow dladdr to be null");
                     info.dli_fname
                 };
                 let bytes = CStr::from_ptr(fname_ptr).to_bytes();

compiler/rustc_session/src/options.rs

@@ -1413,6 +1413,7 @@ pub mod parse {
                 "mcdc" => slot.level = CoverageLevel::Mcdc,
                 "no-mir-spans" => slot.no_mir_spans = true,
                 "discard-all-spans-in-codegen" => slot.discard_all_spans_in_codegen = true,
+                "inject-unused-local-file" => slot.inject_unused_local_file = true,
                 _ => return false,
             }
         }

compiler/rustc_session/src/session.rs

@@ -371,6 +371,11 @@ impl Session {
         self.opts.unstable_opts.coverage_options.discard_all_spans_in_codegen
     }
 
+    /// True if testing flag `-Zcoverage-options=inject-unused-local-file` was passed.
+    pub fn coverage_inject_unused_local_file(&self) -> bool {
+        self.opts.unstable_opts.coverage_options.inject_unused_local_file
+    }
+
     pub fn is_sanitizer_cfi_enabled(&self) -> bool {
         self.opts.unstable_opts.sanitizer.contains(SanitizerSet::CFI)
     }

src/tools/miri/src/shims/native_lib.rs

@@ -92,8 +92,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
     fn get_func_ptr_explicitly_from_lib(&mut self, link_name: Symbol) -> Option<CodePtr> {
         let this = self.eval_context_mut();
         // Try getting the function from the shared library.
-        // On windows `_lib_path` will be unused, hence the name starting with `_`.
-        let (lib, _lib_path) = this.machine.native_lib.as_ref().unwrap();
+        let (lib, lib_path) = this.machine.native_lib.as_ref().unwrap();
         let func: libloading::Symbol<'_, unsafe extern "C" fn()> = unsafe {
             match lib.get(link_name.as_str().as_bytes()) {
                 Ok(x) => x,
@@ -114,16 +113,17 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // This code is a reimplementation of the mechanism for getting `dli_fname` in `libloading`,
         // from: https://docs.rs/libloading/0.7.3/src/libloading/os/unix/mod.rs.html#411
         // using the `libc` crate where this interface is public.
-        let mut info = std::mem::MaybeUninit::<libc::Dl_info>::uninit();
+        let mut info = std::mem::MaybeUninit::<libc::Dl_info>::zeroed();
         unsafe {
             if libc::dladdr(*func.deref() as *const _, info.as_mut_ptr()) != 0 {
                 let info = info.assume_init();
                 #[cfg(target_os = "cygwin")]
                 let fname_ptr = info.dli_fname.as_ptr();
                 #[cfg(not(target_os = "cygwin"))]
                 let fname_ptr = info.dli_fname;
+                assert!(!fname_ptr.is_null());
                 if std::ffi::CStr::from_ptr(fname_ptr).to_str().unwrap()
-                    != _lib_path.to_str().unwrap()
+                    != lib_path.to_str().unwrap()
                 {
                     return None;
                 }