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

coverage: Remove or migrate all unstable values of -Cinstrument-coverage #122226

Merged
merged 2 commits into from
Mar 13, 2024
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
13 changes: 6 additions & 7 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,21 +355,20 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) {

let tcx = cx.tcx;

let ignore_unused_generics = tcx.sess.instrument_coverage_except_unused_generics();

let eligible_def_ids = tcx.mir_keys(()).iter().filter_map(|local_def_id| {
let def_id = local_def_id.to_def_id();
let kind = tcx.def_kind(def_id);
// `mir_keys` will give us `DefId`s for all kinds of things, not
// just "functions", like consts, statics, etc. Filter those out.
// If `ignore_unused_generics` was specified, filter out any
// generic functions from consideration as well.
if !matches!(kind, DefKind::Fn | DefKind::AssocFn | DefKind::Closure) {
return None;
}
if ignore_unused_generics && tcx.generics_of(def_id).requires_monomorphization(tcx) {
return None;
}

// FIXME(79651): Consider trying to filter out dummy instantiations of
// unused generic functions from library crates, because they can produce
// "unused instantiation" in coverage reports even when they are actually
// used by some downstream crate in the same binary.

Some(local_def_id.to_def_id())
});

Expand Down
12 changes: 7 additions & 5 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ use rustc_data_structures::profiling::TimePassesFormat;
use rustc_errors::{emitter::HumanReadableErrorType, registry, ColorConfig};
use rustc_session::config::{
build_configuration, build_session_options, rustc_optgroups, BranchProtection, CFGuard, Cfg,
CollapseMacroDebuginfo, DebugInfo, DumpMonoStatsFormat, ErrorOutputType, ExternEntry,
ExternLocation, Externs, FunctionReturn, InliningThreshold, Input, InstrumentCoverage,
InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli, NextSolverConfig,
OomStrategy, Options, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet, Passes, Polonius,
ProcMacroExecutionStrategy, Strip, SwitchWithOptPath, SymbolManglingVersion, WasiExecModel,
CollapseMacroDebuginfo, CoverageOptions, DebugInfo, DumpMonoStatsFormat, ErrorOutputType,
ExternEntry, ExternLocation, Externs, FunctionReturn, InliningThreshold, Input,
InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli,
NextSolverConfig, OomStrategy, Options, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet,
Passes, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath, SymbolManglingVersion,
WasiExecModel,
};
use rustc_session::lint::Level;
use rustc_session::search_paths::SearchPath;
Expand Down Expand Up @@ -750,6 +751,7 @@ fn test_unstable_options_tracking_hash() {
);
tracked!(codegen_backend, Some("abc".to_string()));
tracked!(collapse_macro_debuginfo, CollapseMacroDebuginfo::Yes);
tracked!(coverage_options, CoverageOptions { branch: true });
tracked!(crate_attr, vec!["abc".to_string()]);
tracked!(cross_crate_inline_threshold, InliningThreshold::Always);
tracked!(debug_info_for_profiling, true);
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_monomorphize/src/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,7 @@ where
}

// Mark one CGU for dead code, if necessary.
let instrument_dead_code =
tcx.sess.instrument_coverage() && !tcx.sess.instrument_coverage_except_unused_functions();
if instrument_dead_code {
if tcx.sess.instrument_coverage() {
mark_code_coverage_dead_code_cgu(&mut codegen_units);
}

Expand Down
64 changes: 21 additions & 43 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,31 +134,26 @@ pub enum LtoCli {
/// and higher). Nevertheless, there are many variables, depending on options
/// selected, code structure, and enabled attributes. If errors are encountered,
/// either while compiling or when generating `llvm-cov show` reports, consider
/// lowering the optimization level, including or excluding `-C link-dead-code`,
/// or using `-Zunstable-options -C instrument-coverage=except-unused-functions`
/// or `-Zunstable-options -C instrument-coverage=except-unused-generics`.
///
/// Note that `ExceptUnusedFunctions` means: When `mapgen.rs` generates the
/// coverage map, it will not attempt to generate synthetic functions for unused
/// (and not code-generated) functions (whether they are generic or not). As a
/// result, non-codegenned functions will not be included in the coverage map,
/// and will not appear, as covered or uncovered, in coverage reports.
///
/// `ExceptUnusedGenerics` will add synthetic functions to the coverage map,
/// unless the function has type parameters.
/// lowering the optimization level, or including/excluding `-C link-dead-code`.
#[derive(Clone, Copy, PartialEq, Hash, Debug)]
pub enum InstrumentCoverage {
/// `-C instrument-coverage=no` (or `off`, `false` etc.)
No,
/// `-C instrument-coverage` or `-C instrument-coverage=yes`
Yes,
/// Additionally, instrument branches and output branch coverage.
/// `-Zunstable-options -C instrument-coverage=branch`
Branch,
/// `-Zunstable-options -C instrument-coverage=except-unused-generics`
ExceptUnusedGenerics,
/// `-Zunstable-options -C instrument-coverage=except-unused-functions`
ExceptUnusedFunctions,
}

/// Individual flag values controlled by `-Z coverage-options`.
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub struct CoverageOptions {
/// Add branch coverage instrumentation (placeholder flag; not yet implemented).
pub branch: bool,
}

impl Default for CoverageOptions {
fn default() -> Self {
Self { branch: false }
}
}

/// Settings for `-Z instrument-xray` flag.
Expand Down Expand Up @@ -2718,24 +2713,6 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
}
}

// Check for unstable values of `-C instrument-coverage`.
// This is what prevents them from being used on stable compilers.
match cg.instrument_coverage {
// Stable values:
InstrumentCoverage::Yes | InstrumentCoverage::No => {}
// Unstable values:
InstrumentCoverage::Branch
| InstrumentCoverage::ExceptUnusedFunctions
| InstrumentCoverage::ExceptUnusedGenerics => {
if !unstable_opts.unstable_options {
early_dcx.early_fatal(
"`-C instrument-coverage=branch` and `-C instrument-coverage=except-*` \
require `-Z unstable-options`",
);
}
}
}

if cg.instrument_coverage != InstrumentCoverage::No {
if cg.profile_generate.enabled() || cg.profile_use.is_some() {
early_dcx.early_fatal(
Expand Down Expand Up @@ -3204,12 +3181,12 @@ pub enum WasiExecModel {
/// how the hash should be calculated when adding a new command-line argument.
pub(crate) mod dep_tracking {
use super::{
BranchProtection, CFGuard, CFProtection, CollapseMacroDebuginfo, CrateType, DebugInfo,
DebugInfoCompression, ErrorOutputType, FunctionReturn, InliningThreshold,
InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail, LtoCli,
NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes, Polonius,
RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, SplitDwarfKind,
SwitchWithOptPath, SymbolManglingVersion, WasiExecModel,
BranchProtection, CFGuard, CFProtection, CollapseMacroDebuginfo, CoverageOptions,
CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FunctionReturn,
InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail,
LtoCli, NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes,
Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm,
SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, WasiExecModel,
};
use crate::lint;
use crate::utils::NativeLib;
Expand Down Expand Up @@ -3279,6 +3256,7 @@ pub(crate) mod dep_tracking {
CodeModel,
TlsModel,
InstrumentCoverage,
CoverageOptions,
InstrumentXRay,
CrateType,
MergeFunctions,
Expand Down
43 changes: 27 additions & 16 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,8 @@ mod desc {
pub const parse_linker_flavor: &str = ::rustc_target::spec::LinkerFlavorCli::one_of();
pub const parse_optimization_fuel: &str = "crate=integer";
pub const parse_dump_mono_stats: &str = "`markdown` (default) or `json`";
pub const parse_instrument_coverage: &str = "either a boolean (`yes`, `no`, `on`, `off`, etc) or (unstable) one of `branch`, `except-unused-generics`, `except-unused-functions`";
pub const parse_instrument_coverage: &str = parse_bool;
pub const parse_coverage_options: &str = "`branch` or `no-branch`";
pub const parse_instrument_xray: &str = "either a boolean (`yes`, `no`, `on`, `off`, etc), or a comma separated list of settings: `always` or `never` (mutually exclusive), `ignore-loops`, `instruction-threshold=N`, `skip-entry`, `skip-exit`";
pub const parse_unpretty: &str = "`string` or `string=string`";
pub const parse_treat_err_as_bug: &str = "either no value or a non-negative number";
Expand Down Expand Up @@ -928,21 +929,34 @@ mod parse {
return true;
};

// Parse values that have historically been accepted by stable compilers,
// even though they're currently just aliases for boolean values.
*slot = match v {
"all" => InstrumentCoverage::Yes,
"branch" => InstrumentCoverage::Branch,
"except-unused-generics" | "except_unused_generics" => {
InstrumentCoverage::ExceptUnusedGenerics
}
"except-unused-functions" | "except_unused_functions" => {
InstrumentCoverage::ExceptUnusedFunctions
}
"0" => InstrumentCoverage::No,
_ => return false,
};
true
}

pub(crate) fn parse_coverage_options(slot: &mut CoverageOptions, v: Option<&str>) -> bool {
let Some(v) = v else { return true };

for option in v.split(',') {
let (option, enabled) = match option.strip_prefix("no-") {
Some(without_no) => (without_no, false),
None => (option, true),
};
let slot = match option {
"branch" => &mut slot.branch,
_ => return false,
};
*slot = enabled;
}

true
}

pub(crate) fn parse_instrument_xray(
slot: &mut Option<InstrumentXRay>,
v: Option<&str>,
Expand Down Expand Up @@ -1445,14 +1459,9 @@ options! {
"set the threshold for inlining a function"),
#[rustc_lint_opt_deny_field_access("use `Session::instrument_coverage` instead of this field")]
instrument_coverage: InstrumentCoverage = (InstrumentCoverage::No, parse_instrument_coverage, [TRACKED],
"instrument the generated code to support LLVM source-based code coverage \
reports (note, the compiler build config must include `profiler = true`); \
implies `-C symbol-mangling-version=v0`. Optional values are:
`=no` `=n` `=off` `=false` (default)
`=yes` `=y` `=on` `=true` (implicit value)
`=branch` (unstable)
`=except-unused-generics` (unstable)
`=except-unused-functions` (unstable)"),
"instrument the generated code to support LLVM source-based code coverage reports \
(note, the compiler build config must include `profiler = true`); \
implies `-C symbol-mangling-version=v0`"),
link_arg: (/* redirected to link_args */) = ((), parse_string_push, [UNTRACKED],
"a single extra argument to append to the linker invocation (can be used several times)"),
link_args: Vec<String> = (Vec::new(), parse_list, [UNTRACKED],
Expand Down Expand Up @@ -1574,6 +1583,8 @@ options! {
"set option to collapse debuginfo for macros"),
combine_cgu: bool = (false, parse_bool, [TRACKED],
"combine CGUs into a single one"),
coverage_options: CoverageOptions = (CoverageOptions::default(), parse_coverage_options, [TRACKED],
"control details of coverage instrumentation"),
crate_attr: Vec<String> = (Vec::new(), parse_string_push, [TRACKED],
"inject the given attribute in the crate"),
cross_crate_inline_threshold: InliningThreshold = (InliningThreshold::Sometimes(100), parse_inlining_threshold, [TRACKED],
Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,15 +353,7 @@ impl Session {
}

pub fn instrument_coverage_branch(&self) -> bool {
self.opts.cg.instrument_coverage() == InstrumentCoverage::Branch
}

pub fn instrument_coverage_except_unused_generics(&self) -> bool {
self.opts.cg.instrument_coverage() == InstrumentCoverage::ExceptUnusedGenerics
}

pub fn instrument_coverage_except_unused_functions(&self) -> bool {
self.opts.cg.instrument_coverage() == InstrumentCoverage::ExceptUnusedFunctions
self.instrument_coverage() && self.opts.unstable_opts.coverage_options.branch
}

pub fn is_sanitizer_cfi_enabled(&self) -> bool {
Expand Down
15 changes: 7 additions & 8 deletions src/doc/rustc/src/instrument-coverage.md
Original file line number Diff line number Diff line change
Expand Up @@ -346,14 +346,13 @@ $ llvm-cov report \
more fine-grained coverage options are added.
Using this value is currently not recommended.

### Unstable values

- `-Z unstable-options -C instrument-coverage=branch`:
Placeholder for potential branch coverage support in the future.
- `-Z unstable-options -C instrument-coverage=except-unused-generics`:
Instrument all functions except unused generics.
- `-Z unstable-options -C instrument-coverage=except-unused-functions`:
Instrument only used (called) functions and instantiated generic functions.
## `-Z coverage-options=<options>`

This unstable option provides finer control over some aspects of coverage
instrumentation. Pass one or more of the following values, separated by commas.

- `branch` or `no-branch`
- Placeholder for potential branch coverage support in the future.

## Other references

Expand Down
8 changes: 8 additions & 0 deletions src/doc/unstable-book/src/compiler-flags/coverage-options.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# `coverage-options`

This option controls details of the coverage instrumentation performed by
`-C instrument-coverage`.

Multiple options can be passed, separated by commas. Valid options are:

- `branch` or `no-branch`: Placeholder for future branch coverage support.
15 changes: 5 additions & 10 deletions tests/coverage/auxiliary/used_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,8 @@ fn use_this_lib_crate() {
// ```
//
// The notice is triggered because the function is unused by the library itself,
// and when the library is compiled, a synthetic function is generated, so
// unused function coverage can be reported. Coverage can be skipped for unused
// generic functions with:
//
// ```shell
// $ `rustc -Zunstable-options -C instrument-coverage=except-unused-generics ...`
// ```
// so when the library is compiled, an "unused" set of mappings for that function
// is included in the library's coverage metadata.
//
// Even though this function is used by `uses_crate.rs` (and
// counted), with substitutions for `T`, those instantiations are only generated
Expand All @@ -98,6 +93,6 @@ fn use_this_lib_crate() {
// another binary that never used this generic function, then it would be valid
// to show the unused generic, with unknown substitution (`_`).
//
// The alternative is to exclude all generics from being included in the "unused
// functions" list, which would then omit coverage results for
// `unused_generic_function<T>()`, below.
// The alternative would be to exclude all generics from being included in the
// "unused functions" list, which would then omit coverage results for
// `unused_generic_function<T>()`.
15 changes: 5 additions & 10 deletions tests/coverage/uses_crate.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,8 @@ $DIR/auxiliary/used_crate.rs:
LL| |// ```
LL| |//
LL| |// The notice is triggered because the function is unused by the library itself,
LL| |// and when the library is compiled, a synthetic function is generated, so
LL| |// unused function coverage can be reported. Coverage can be skipped for unused
LL| |// generic functions with:
LL| |//
LL| |// ```shell
LL| |// $ `rustc -Zunstable-options -C instrument-coverage=except-unused-generics ...`
LL| |// ```
LL| |// so when the library is compiled, an "unused" set of mappings for that function
LL| |// is included in the library's coverage metadata.
LL| |//
LL| |// Even though this function is used by `uses_crate.rs` (and
LL| |// counted), with substitutions for `T`, those instantiations are only generated
Expand All @@ -146,9 +141,9 @@ $DIR/auxiliary/used_crate.rs:
LL| |// another binary that never used this generic function, then it would be valid
LL| |// to show the unused generic, with unknown substitution (`_`).
LL| |//
LL| |// The alternative is to exclude all generics from being included in the "unused
LL| |// functions" list, which would then omit coverage results for
LL| |// `unused_generic_function<T>()`, below.
LL| |// The alternative would be to exclude all generics from being included in the
LL| |// "unused functions" list, which would then omit coverage results for
LL| |// `unused_generic_function<T>()`.

$DIR/uses_crate.rs:
LL| |// This test was failing on Linux for a while due to #110393 somehow making
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/instrument-coverage/bad-value.bad.stderr
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
error: incorrect value `bad-value` for codegen option `instrument-coverage` - either a boolean (`yes`, `no`, `on`, `off`, etc) or (unstable) one of `branch`, `except-unused-generics`, `except-unused-functions` was expected
error: incorrect value `bad-value` for codegen option `instrument-coverage` - one of: `y`, `yes`, `on`, `true`, `n`, `no`, `off` or `false` was expected

2 changes: 1 addition & 1 deletion tests/ui/instrument-coverage/bad-value.blank.stderr
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
error: incorrect value `` for codegen option `instrument-coverage` - either a boolean (`yes`, `no`, `on`, `off`, etc) or (unstable) one of `branch`, `except-unused-generics`, `except-unused-functions` was expected
error: incorrect value `` for codegen option `instrument-coverage` - one of: `y`, `yes`, `on`, `true`, `n`, `no`, `off` or `false` was expected

2 changes: 2 additions & 0 deletions tests/ui/instrument-coverage/coverage-options.bad.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
error: incorrect value `bad` for unstable option `coverage-options` - `branch` or `no-branch` was expected

14 changes: 14 additions & 0 deletions tests/ui/instrument-coverage/coverage-options.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ needs-profiler-support
//@ revisions: branch no-branch bad
//@ compile-flags -Cinstrument-coverage

//@ [branch] check-pass
//@ [branch] compile-flags: -Zcoverage-options=branch

//@ [no-branch] check-pass
//@ [no-branch] compile-flags: -Zcoverage-options=no-branch

//@ [bad] check-fail
//@ [bad] compile-flags: -Zcoverage-options=bad

fn main() {}
2 changes: 0 additions & 2 deletions tests/ui/instrument-coverage/unstable.branch.stderr

This file was deleted.

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions tests/ui/instrument-coverage/unstable.rs

This file was deleted.

Loading