Rollup merge of rust-lang#76002 - richkadel:llvm-coverage-map-gen-6b.3, r=tmandry

Fix `-Z instrument-coverage` on MSVC

Found that `-C link-dead-code` (which was enabled automatically
under `-Z instrument-coverage`) was causing the linking error that
resulted in segmentation faults in coverage instrumented binaries. Link
dead code is now disabled under MSVC, allowing `-Z instrument-coverage`
to be enabled under MSVC for the first time.

More details are included in Issue rust-lang#76038 .

Note this PR makes it possible to support `Z instrument-coverage` but
does not enable instrument coverage for MSVC in existing tests. It will be
enabled in another PR to follow this one (both PRs coming from original
PR rust-lang#75828).

r? @tmandry
FYI: @wesleywiser

Author: tmandry

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -1668,7 +1668,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
     // FIXME: Order dependent, applies to the following objects. Where should it be placed?
     // Try to strip as much out of the generated object by removing unused
     // sections if possible. See more comments in linker.rs
-    if sess.opts.cg.link_dead_code != Some(true) {
+    if !sess.link_dead_code() {
         let keep_metadata = crate_type == CrateType::Dylib;
         cmd.gc_sections(keep_metadata);
     }

compiler/rustc_middle/src/mir/mono.rs

@@ -85,7 +85,7 @@ impl<'tcx> MonoItem<'tcx> {
             .debugging_opts
             .inline_in_all_cgus
             .unwrap_or_else(|| tcx.sess.opts.optimize != OptLevel::No)
-            && tcx.sess.opts.cg.link_dead_code != Some(true);
+            && !tcx.sess.link_dead_code();
 
         match *self {
             MonoItem::Fn(ref instance) => {

compiler/rustc_mir/src/monomorphize/partitioning/mod.rs

@@ -190,7 +190,7 @@ pub fn partition<'tcx>(
 
     // Next we try to make as many symbols "internal" as possible, so LLVM has
     // more freedom to optimize.
-    if tcx.sess.opts.cg.link_dead_code != Some(true) {
+    if !tcx.sess.link_dead_code() {
         let _prof_timer = tcx.prof.generic_activity("cgu_partitioning_internalize_symbols");
         partitioner.internalize_symbols(tcx, &mut post_inlining, inlining_map);
     }
@@ -327,7 +327,7 @@ fn collect_and_partition_mono_items<'tcx>(
             }
         }
         None => {
-            if tcx.sess.opts.cg.link_dead_code == Some(true) {
+            if tcx.sess.link_dead_code() {
                 MonoItemCollectionMode::Eager
             } else {
                 MonoItemCollectionMode::Lazy

compiler/rustc_session/src/config.rs

@@ -1718,20 +1718,11 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
             );
         }
 
-        // `-Z instrument-coverage` implies:
-        //   * `-Z symbol-mangling-version=v0` - to ensure consistent 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.
-        //   * `-C link-dead-code` - so unexecuted code is still counted as zero, rather than be
-        //     optimized out. Note that instrumenting dead code can be explicitly disabled with:
-        //         `-Z instrument-coverage -C link-dead-code=no`.
+        // `-Z instrument-coverage` implies `-Z symbol-mangling-version=v0` - to ensure consistent
+        // 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;
-        if cg.link_dead_code == None {
-            // FIXME(richkadel): Investigate if the `instrument-coverage` implementation can
-            // inject ["zero counters"](https://llvm.org/docs/CoverageMappingFormat.html#counter)
-            // in the coverage map when "dead code" is removed, rather than forcing `link-dead-code`.
-            cg.link_dead_code = Some(true);
-        }
     }
 
     if !cg.embed_bitcode {

compiler/rustc_session/src/options.rs

@@ -885,9 +885,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
         "instrument the generated code to support LLVM source-based code coverage \
         reports (note, the compiler build config must include `profiler = true`, \
         and is mutually exclusive with `-C profile-generate`/`-C profile-use`); \
-        implies `-C link-dead-code` (unless explicitly disabled)` and \
-        `-Z symbol-mangling-version=v0`; and disables/overrides some optimization \
-        options (default: no)"),
+        implies `-C link-dead-code` (unless targeting MSVC, or explicitly disabled) \
+        and `-Z symbol-mangling-version=v0`; disables/overrides some Rust \
+        optimizations (default: no)"),
     instrument_mcount: bool = (false, parse_bool, [TRACKED],
         "insert function instrument code for mcount-based tracing (default: no)"),
     keep_hygiene_data: bool = (false, parse_bool, [UNTRACKED],

compiler/rustc_session/src/session.rs

@@ -1024,6 +1024,40 @@ impl Session {
         || self.opts.debugging_opts.sanitizer.intersects(SanitizerSet::ADDRESS | SanitizerSet::MEMORY)
     }
 
+    pub fn link_dead_code(&self) -> bool {
+        match self.opts.cg.link_dead_code {
+            Some(explicitly_set) => explicitly_set,
+            None => {
+                self.opts.debugging_opts.instrument_coverage
+                    && !self.target.target.options.is_like_msvc
+                // Issue #76038: (rustc `-Clink-dead-code` causes MSVC linker to produce invalid
+                // binaries when LLVM InstrProf counters are enabled). As described by this issue,
+                // the "link dead code" option produces incorrect binaries when compiled and linked
+                // under MSVC. The resulting Rust programs typically crash with a segmentation
+                // fault, or produce an empty "*.profraw" file (profiling counter results normally
+                // generated during program exit).
+                //
+                // If not targeting MSVC, `-Z instrument-coverage` implies `-C link-dead-code`, so
+                // unexecuted code is still counted as zero, rather than be optimized out. Note that
+                // instrumenting dead code can be explicitly disabled with:
+                //
+                //     `-Z instrument-coverage -C link-dead-code=no`.
+                //
+                // FIXME(richkadel): Investigate if `instrument-coverage` implementation can inject
+                // [zero counters](https://llvm.org/docs/CoverageMappingFormat.html#counter) in the
+                // coverage map when "dead code" is removed, rather than forcing `link-dead-code`.
+                // This may not be possible, however, if (as it seems to appear) the "dead code"
+                // that would otherwise not be linked is only identified as "dead" by the native
+                // linker. If that's the case, I believe it is too late for the Rust compiler to
+                // leverage any information it might be able to get from the linker regarding what
+                // code is dead, to be able to add those counters.
+                //
+                // On the other hand, if any Rust compiler passes are optimizing out dead code blocks
+                // we should inject "zero" counters for those code regions.
+            }
+        }
+    }
+
     pub fn mark_attr_known(&self, attr: &Attribute) {
         self.known_attrs.lock().mark(attr)
     }
@@ -1432,20 +1466,6 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
         );
     }
 
-    // FIXME(richkadel): See `src/test/run-make-fulldeps/instrument-coverage/Makefile`. After
-    // compiling with `-Zinstrument-coverage`, the resulting binary generates a segfault during
-    // the program's exit process (likely while attempting to generate the coverage stats in
-    // the "*.profraw" file). An investigation to resolve the problem on Windows is ongoing,
-    // but until this is resolved, the option is disabled on Windows, and the test is skipped
-    // when targeting `MSVC`.
-    if sess.opts.debugging_opts.instrument_coverage && sess.target.target.options.is_like_msvc {
-        sess.warn(
-            "Rust source-based code coverage instrumentation (with `-Z instrument-coverage`) \
-            is not yet supported on Windows when targeting MSVC. The resulting binaries will \
-            still be instrumented for experimentation purposes, but may not execute correctly.",
-        );
-    }
-
     const ASAN_SUPPORTED_TARGETS: &[&str] = &[
         "aarch64-fuchsia",
         "aarch64-unknown-linux-gnu",