Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix -Z instrument-coverage on MSVC #76002

Merged
merged 1 commit into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,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) => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir/src/monomorphize/partitioning/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down
17 changes: 4 additions & 13 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
48 changes: 34 additions & 14 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,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)
}
Expand Down Expand Up @@ -1428,20 +1462,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",
Expand Down