Rollup merge of #79958 - richkadel:llvm-coverage-counters-2.2.0, r=tmandry

Fixes reported bugs in Rust Coverage

Fixes: #79569

Fixes: #79566
Fixes: #79565

For the first issue (#79569), I got hit a `debug_assert!()` before
encountering the reported error message (because I have `debug = true`
enabled in my config.toml).

The assertion showed me that some `SwitchInt`s can have more than one
target pointing to the same `BasicBlock`.

I had thought that was invalid, but since it seems to be possible, I'm
allowing this now.

I added a new test for this.

----

In the last two cases above, both tests (intentionally) fail to compile,
but the `InstrumentCoverage` pass is invoked anyway.

The MIR starts with an `Unreachable` `BasicBlock`, which I hadn't
encountered before. (I had assumed the `InstrumentCoverage` pass
would only be invoked with MIRs from successful compilations.)

I don't have test infrastructure set up to test coverage on files that
fail to compile, so I didn't add a new test.

r? `@tmandry`
FYI: `@wesleywiser`

compiler/rustc_interface/src/tests.rs

@@ -587,7 +587,7 @@ fn test_debugging_options_tracking_hash() {
     tracked!(share_generics, Some(true));
     tracked!(show_span, Some(String::from("abc")));
     tracked!(src_hash_algorithm, Some(SourceFileHashAlgorithm::Sha1));
-    tracked!(symbol_mangling_version, SymbolManglingVersion::V0);
+    tracked!(symbol_mangling_version, Some(SymbolManglingVersion::V0));
     tracked!(teach, true);
     tracked!(thinlto, Some(true));
     tracked!(tune_cpu, Some(String::from("abc")));

compiler/rustc_metadata/src/creader.rs

@@ -706,7 +706,7 @@ impl<'a> CrateLoader<'a> {
         self.inject_dependency_if(cnum, "a panic runtime", &|data| data.needs_panic_runtime());
     }
 
-    fn inject_profiler_runtime(&mut self) {
+    fn inject_profiler_runtime(&mut self, krate: &ast::Crate) {
         if (self.sess.opts.debugging_opts.instrument_coverage
             || self.sess.opts.debugging_opts.profile
             || self.sess.opts.cg.profile_generate.enabled())
@@ -714,6 +714,13 @@ impl<'a> CrateLoader<'a> {
         {
             info!("loading profiler");
 
+            if self.sess.contains_name(&krate.attrs, sym::no_core) {
+                self.sess.err(
+                    "`profiler_builtins` crate (required by compiler options) \
+                               is not compatible with crate attribute `#![no_core]`",
+                );
+            }
+
             let name = sym::profiler_builtins;
             let cnum = self.resolve_crate(name, DUMMY_SP, CrateDepKind::Implicit, None);
             let data = self.cstore.get_crate_data(cnum);
@@ -879,7 +886,7 @@ impl<'a> CrateLoader<'a> {
     }
 
     pub fn postprocess(&mut self, krate: &ast::Crate) {
-        self.inject_profiler_runtime();
+        self.inject_profiler_runtime(krate);
         self.inject_allocator_crate(krate);
         self.inject_panic_runtime(krate);
 

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -663,7 +663,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             no_builtins: tcx.sess.contains_name(&attrs, sym::no_builtins),
             panic_runtime: tcx.sess.contains_name(&attrs, sym::panic_runtime),
             profiler_runtime: tcx.sess.contains_name(&attrs, sym::profiler_runtime),
-            symbol_mangling_version: tcx.sess.opts.debugging_opts.symbol_mangling_version,
+            symbol_mangling_version: tcx.sess.opts.debugging_opts.get_symbol_mangling_version(),
 
             crate_deps,
             dylib_dependency_formats,

compiler/rustc_mir/src/transform/coverage/graph.rs

@@ -32,24 +32,28 @@ impl CoverageGraph {
 
         // Pre-transform MIR `BasicBlock` successors and predecessors into the BasicCoverageBlock
         // equivalents. Note that since the BasicCoverageBlock graph has been fully simplified, the
-        // each predecessor of a BCB leader_bb should be in a unique BCB, and each successor of a
-        // BCB last_bb should be in its own unique BCB. Therefore, collecting the BCBs using
-        // `bb_to_bcb` should work without requiring a deduplication step.
+        // each predecessor of a BCB leader_bb should be in a unique BCB. It is possible for a
+        // `SwitchInt` to have multiple targets to the same destination `BasicBlock`, so
+        // de-duplication is required. This is done without reordering the successors.
 
+        let bcbs_len = bcbs.len();
+        let mut seen = IndexVec::from_elem_n(false, bcbs_len);
         let successors = IndexVec::from_fn_n(
             |bcb| {
+                for b in seen.iter_mut() {
+                    *b = false;
+                }
                 let bcb_data = &bcbs[bcb];
-                let bcb_successors =
+                let mut bcb_successors = Vec::new();
+                for successor in
                     bcb_filtered_successors(&mir_body, &bcb_data.terminator(mir_body).kind)
                         .filter_map(|&successor_bb| bb_to_bcb[successor_bb])
-                        .collect::<Vec<_>>();
-                debug_assert!({
-                    let mut sorted = bcb_successors.clone();
-                    sorted.sort_unstable();
-                    let initial_len = sorted.len();
-                    sorted.dedup();
-                    sorted.len() == initial_len
-                });
+                {
+                    if !seen[successor] {
+                        seen[successor] = true;
+                        bcb_successors.push(successor);
+                    }
+                }
                 bcb_successors
             },
             bcbs.len(),

compiler/rustc_mir/src/transform/coverage/mod.rs

@@ -78,6 +78,14 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage {
             return;
         }
 
+        match mir_body.basic_blocks()[mir::START_BLOCK].terminator().kind {
+            TerminatorKind::Unreachable => {
+                trace!("InstrumentCoverage skipped for unreachable `START_BLOCK`");
+                return;
+            }
+            _ => {}
+        }
+
         trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
         Instrumentor::new(&self.name(), tcx, mir_body).inject_counters();
         trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());

compiler/rustc_mir/src/transform/inline.rs

@@ -41,14 +41,6 @@ impl<'tcx> MirPass<'tcx> for Inline {
             return;
         }
 
-        if tcx.sess.opts.debugging_opts.instrument_coverage {
-            // The current implementation of source code coverage injects code region counters
-            // into the MIR, and assumes a 1-to-1 correspondence between MIR and source-code-
-            // based function.
-            debug!("function inlining is disabled when compiling with `instrument_coverage`");
-            return;
-        }
-
         if inline(tcx, body) {
             debug!("running simplify cfg on {:?}", body.source);
             CfgSimplifier::new(body).simplify();

compiler/rustc_session/src/config.rs

@@ -692,6 +692,10 @@ impl DebuggingOptions {
             deduplicate_diagnostics: self.deduplicate_diagnostics,
         }
     }
+
+    pub fn get_symbol_mangling_version(&self) -> SymbolManglingVersion {
+        self.symbol_mangling_version.unwrap_or(SymbolManglingVersion::Legacy)
+    }
 }
 
 // The type of entry function, so users can have their own entry functions
@@ -1757,7 +1761,30 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
         // and reversible name mangling. Note, LLVM coverage tools can analyze coverage over
         // multiple runs, including some changes to source code; so mangled names must be consistent
         // across compilations.
-        debugging_opts.symbol_mangling_version = SymbolManglingVersion::V0;
+        match debugging_opts.symbol_mangling_version {
+            None => {
+                debugging_opts.symbol_mangling_version = Some(SymbolManglingVersion::V0);
+            }
+            Some(SymbolManglingVersion::Legacy) => {
+                early_warn(
+                    error_format,
+                    "-Z instrument-coverage requires symbol mangling version `v0`, \
+                    but `-Z symbol-mangling-version=legacy` was specified",
+                );
+            }
+            Some(SymbolManglingVersion::V0) => {}
+        }
+
+        if debugging_opts.mir_opt_level > 1 {
+            early_warn(
+                error_format,
+                &format!(
+                    "`-Z mir-opt-level={}` (any level > 1) enables function inlining, which \
+                    limits the effectiveness of `-Z instrument-coverage`.",
+                    debugging_opts.mir_opt_level,
+                ),
+            );
+        }
     }
 
     if let Ok(graphviz_font) = std::env::var("RUSTC_GRAPHVIZ_FONT") {
@@ -2162,7 +2189,7 @@ crate mod dep_tracking {
     impl_dep_tracking_hash_via_hash!(Edition);
     impl_dep_tracking_hash_via_hash!(LinkerPluginLto);
     impl_dep_tracking_hash_via_hash!(SwitchWithOptPath);
-    impl_dep_tracking_hash_via_hash!(SymbolManglingVersion);
+    impl_dep_tracking_hash_via_hash!(Option<SymbolManglingVersion>);
     impl_dep_tracking_hash_via_hash!(Option<SourceFileHashAlgorithm>);
     impl_dep_tracking_hash_via_hash!(TrimmedDefPaths);
 

compiler/rustc_session/src/options.rs

@@ -677,12 +677,12 @@ macro_rules! options {
         }
 
         fn parse_symbol_mangling_version(
-            slot: &mut SymbolManglingVersion,
+            slot: &mut Option<SymbolManglingVersion>,
             v: Option<&str>,
         ) -> bool {
             *slot = match v {
-                Some("legacy") => SymbolManglingVersion::Legacy,
-                Some("v0") => SymbolManglingVersion::V0,
+                Some("legacy") => Some(SymbolManglingVersion::Legacy),
+                Some("v0") => Some(SymbolManglingVersion::V0),
                 _ => return false,
             };
             true
@@ -1088,9 +1088,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
         "hash algorithm of source files in debug info (`md5`, `sha1`, or `sha256`)"),
     strip: Strip = (Strip::None, parse_strip, [UNTRACKED],
         "tell the linker which information to strip (`none` (default), `debuginfo` or `symbols`)"),
-    symbol_mangling_version: SymbolManglingVersion = (SymbolManglingVersion::Legacy,
+    symbol_mangling_version: Option<SymbolManglingVersion> = (None,
         parse_symbol_mangling_version, [TRACKED],
-        "which mangling version to use for symbol names"),
+        "which mangling version to use for symbol names ('legacy' (default) or 'v0')"),
     teach: bool = (false, parse_bool, [TRACKED],
         "show extended diagnostic help (default: no)"),
     terminal_width: Option<usize> = (None, parse_opt_uint, [UNTRACKED],

compiler/rustc_symbol_mangling/src/lib.rs

@@ -245,7 +245,7 @@ fn compute_symbol_name(
     // 2. we favor `instantiating_crate` where possible (i.e. when `Some`)
     let mangling_version_crate = instantiating_crate.unwrap_or(def_id.krate);
     let mangling_version = if mangling_version_crate == LOCAL_CRATE {
-        tcx.sess.opts.debugging_opts.symbol_mangling_version
+        tcx.sess.opts.debugging_opts.get_symbol_mangling_version()
     } else {
         tcx.symbol_mangling_version(mangling_version_crate)
     };