Improve warnings on incompatible options involving -Zinstrument-coverage

Adds checks for:

* `no_core` attribute
* explicitly-enabled `legacy` symbol mangling
* mir_opt_level > 1 (which enables inlining)

I removed code from the `Inline` MIR pass that forcibly disabled
inlining if `-Zinstrument-coverage` was set. The default `mir_opt_level`
does not enable inlining anyway. But if the level is explicitly set and
is greater than 1, I issue a warning.

The new warnings show up in tests, which is much better for diagnosing
potential option conflicts in these cases.
This commit is contained in:
Rich Kadel 2020-12-14 00:25:29 -08:00
parent eb963ffe45
commit 4f550f1f93
16 changed files with 103 additions and 49 deletions

View File

@ -561,7 +561,7 @@ macro_rules! tracked {
tracked!(link_only, true);
tracked!(merge_functions, Some(MergeFunctions::Disabled));
tracked!(mir_emit_retag, true);
tracked!(mir_opt_level, 3);
tracked!(mir_opt_level, Some(3));
tracked!(mutable_noalias, true);
tracked!(new_llvm_pass_manager, true);
tracked!(no_codegen, true);
@ -587,7 +587,7 @@ macro_rules! tracked {
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")));

View File

@ -706,7 +706,7 @@ fn inject_panic_runtime(&mut self, krate: &ast::Crate) {
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 @@ fn inject_profiler_runtime(&mut self) {
{
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 @@ fn report_unused_deps(&mut self, krate: &ast::Crate) {
}
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);

View File

@ -663,7 +663,12 @@ fn encode_crate_root(&mut self) -> Lazy<CrateRoot<'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
.symbol_mangling_version
.unwrap_or(SymbolManglingVersion::default()),
crate_deps,
dylib_dependency_formats,

View File

@ -22,6 +22,7 @@
use rustc_middle::ty::{
self, ConstInt, ConstKind, Instance, ParamEnv, ScalarInt, Ty, TyCtxt, TypeFoldable,
};
use rustc_session::config::MIR_OPT_LEVEL_DEFAULT;
use rustc_session::lint;
use rustc_span::{def_id::DefId, Span};
use rustc_target::abi::{HasDataLayout, LayoutOf, Size, TargetDataLayout};
@ -708,7 +709,7 @@ fn const_prop(
return None;
}
if self.tcx.sess.opts.debugging_opts.mir_opt_level >= 3 {
if self.tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT) >= 3 {
self.eval_rvalue_with_identities(rvalue, place)
} else {
self.use_ecx(|this| this.ecx.eval_rvalue_into_place(rvalue, place))
@ -886,7 +887,8 @@ fn replace_with_const(
/// Returns `true` if and only if this `op` should be const-propagated into.
fn should_const_prop(&mut self, op: OpTy<'tcx>) -> bool {
let mir_opt_level = self.tcx.sess.opts.debugging_opts.mir_opt_level;
let mir_opt_level =
self.tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT);
if mir_opt_level == 0 {
return false;
@ -1056,7 +1058,7 @@ fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) {
// Only const prop copies and moves on `mir_opt_level=2` as doing so
// currently slightly increases compile time in some cases.
if self.tcx.sess.opts.debugging_opts.mir_opt_level >= 2 {
if self.tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT) >= 2 {
self.propagate_operand(operand)
}
}

View File

@ -115,6 +115,7 @@
Rvalue, Statement, StatementKind, Terminator, TerminatorKind,
};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::config::MIR_OPT_LEVEL_DEFAULT;
// Empirical measurements have resulted in some observations:
// - Running on a body with a single block and 500 locals takes barely any time
@ -129,7 +130,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
// Only run at mir-opt-level=2 or higher for now (we don't fix up debuginfo and remove
// storage statements at the moment).
if tcx.sess.opts.debugging_opts.mir_opt_level <= 1 {
if tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT) <= 1 {
return;
}

View File

@ -1,6 +1,7 @@
use crate::{transform::MirPass, util::patch::MirPatch};
use rustc_middle::mir::*;
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_session::config::MIR_OPT_LEVEL_DEFAULT;
use std::fmt::Debug;
use super::simplify::simplify_cfg;
@ -26,7 +27,7 @@
impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
if tcx.sess.opts.debugging_opts.mir_opt_level < 2 {
if tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT) < 2 {
return;
}
trace!("running EarlyOtherwiseBranch on {:?}", body.source);

View File

@ -9,6 +9,7 @@
use rustc_middle::mir::*;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_session::config::MIR_OPT_LEVEL_DEFAULT;
use rustc_span::{hygiene::ExpnKind, ExpnData, Span};
use rustc_target::spec::abi::Abi;
@ -37,15 +38,7 @@ struct CallSite<'tcx> {
impl<'tcx> MirPass<'tcx> for Inline {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
if tcx.sess.opts.debugging_opts.mir_opt_level < 2 {
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`");
if tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT) < 2 {
return;
}

View File

@ -1,6 +1,7 @@
use crate::transform::MirPass;
use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::MIR_OPT_LEVEL_DEFAULT;
pub struct MatchBranchSimplification;
@ -38,7 +39,7 @@
impl<'tcx> MirPass<'tcx> for MatchBranchSimplification {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
if tcx.sess.opts.debugging_opts.mir_opt_level <= 1 {
if tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT) <= 1 {
return;
}

View File

@ -10,6 +10,7 @@
use rustc_middle::mir::{traversal, Body, ConstQualifs, MirPhase, Promoted};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, TyCtxt, TypeFoldable};
use rustc_session::config::MIR_OPT_LEVEL_DEFAULT;
use rustc_span::{Span, Symbol};
use std::borrow::Cow;
@ -373,7 +374,7 @@ fn run_post_borrowck_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tc
}
fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let mir_opt_level = tcx.sess.opts.debugging_opts.mir_opt_level;
let mir_opt_level = tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT);
// Lowering generator control-flow and variables has to happen before we do anything else
// to them. We run some optimizations before that, because they may be harder to do on the state

View File

@ -5,12 +5,13 @@
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::MIR_OPT_LEVEL_DEFAULT;
pub struct MultipleReturnTerminators;
impl<'tcx> MirPass<'tcx> for MultipleReturnTerminators {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
if tcx.sess.opts.debugging_opts.mir_opt_level < 3 {
if tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT) < 3 {
return;
}

View File

@ -5,6 +5,7 @@
use rustc_middle::mir::visit::{MutVisitor, NonUseContext, PlaceContext, Visitor};
use rustc_middle::mir::{self, BasicBlock, Local, Location};
use rustc_middle::ty::TyCtxt;
use rustc_session::config::MIR_OPT_LEVEL_DEFAULT;
use crate::transform::MirPass;
@ -34,7 +35,7 @@
impl<'tcx> MirPass<'tcx> for RenameReturnPlace {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut mir::Body<'tcx>) {
if tcx.sess.opts.debugging_opts.mir_opt_level == 0 {
if tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT) == 0 {
return;
}

View File

@ -7,12 +7,13 @@
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::MIR_OPT_LEVEL_DEFAULT;
pub struct UnreachablePropagation;
impl MirPass<'_> for UnreachablePropagation {
fn run_pass<'tcx>(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
if tcx.sess.opts.debugging_opts.mir_opt_level < 3 {
if tcx.sess.opts.debugging_opts.mir_opt_level.unwrap_or(MIR_OPT_LEVEL_DEFAULT) < 3 {
// Enable only under -Zmir-opt-level=3 as in some cases (check the deeply-nested-opt
// perf benchmark) LLVM may spend quite a lot of time optimizing the generated code.
return;

View File

@ -174,6 +174,8 @@ pub enum MirSpanview {
Block,
}
pub const MIR_OPT_LEVEL_DEFAULT: usize = 1;
#[derive(Clone, PartialEq, Hash)]
pub enum LinkerPluginLto {
LinkerPlugin(PathBuf),
@ -212,6 +214,12 @@ pub enum SymbolManglingVersion {
V0,
}
impl SymbolManglingVersion {
pub fn default() -> Self {
SymbolManglingVersion::Legacy
}
}
impl_stable_hash_via_hash!(SymbolManglingVersion);
#[derive(Clone, Copy, Debug, PartialEq, Hash)]
@ -1757,7 +1765,33 @@ 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) => {}
}
match debugging_opts.mir_opt_level {
Some(level) if 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`.",
level,
),
);
}
_ => {}
}
}
if let Ok(graphviz_font) = std::env::var("RUSTC_GRAPHVIZ_FONT") {
@ -2162,7 +2196,7 @@ fn hash(&self, hasher: &mut DefaultHasher, error_format: ErrorOutputType) {
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);

View File

@ -677,12 +677,12 @@ fn parse_tls_model(slot: &mut Option<TlsModel>, v: Option<&str>) -> bool {
}
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
@ -970,7 +970,7 @@ fn parse_target_feature(slot: &mut String, v: Option<&str>) -> bool {
mir_emit_retag: bool = (false, parse_bool, [TRACKED],
"emit Retagging MIR statements, interpreted e.g., by miri; implies -Zmir-opt-level=0 \
(default: no)"),
mir_opt_level: usize = (1, parse_uint, [TRACKED],
mir_opt_level: Option<usize> = (None, parse_opt_uint, [TRACKED],
"MIR optimization level (0-3; default: 1)"),
mutable_noalias: bool = (false, parse_bool, [TRACKED],
"emit noalias metadata for mutable references (default: no)"),
@ -1088,9 +1088,9 @@ fn parse_target_feature(slot: &mut String, v: Option<&str>) -> bool {
"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],

View File

@ -245,7 +245,11 @@ 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
.symbol_mangling_version
.unwrap_or(SymbolManglingVersion::default())
} else {
tcx.symbol_mangling_version(mangling_version_crate)
};

View File

@ -41,7 +41,7 @@ fn arg_value<'a, T: Deref<Target = str>>(
match arg.next().or_else(|| args.next()) {
Some(v) if pred(v) => return Some(v),
_ => {},
_ => {}
}
}
None
@ -85,7 +85,7 @@ fn config(&mut self, config: &mut interface::Config) {
// run on the unoptimized MIR. On the other hand this results in some false negatives. If
// MIR passes can be enabled / disabled separately, we should figure out, what passes to
// use for Clippy.
config.opts.debugging_opts.mir_opt_level = 0;
config.opts.debugging_opts.mir_opt_level = Some(0);
}
}
@ -121,11 +121,12 @@ fn display_help() {
const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/new";
static ICE_HOOK: SyncLazy<Box<dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static>> = SyncLazy::new(|| {
let hook = panic::take_hook();
panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL)));
hook
});
static ICE_HOOK: SyncLazy<Box<dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static>> =
SyncLazy::new(|| {
let hook = panic::take_hook();
panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL)));
hook
});
fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) {
// Invoke our ICE handler, which prints the actual panic message and optionally a backtrace
@ -257,14 +258,17 @@ pub fn main() {
// Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument.
// We're invoking the compiler programmatically, so we ignore this/
let wrapper_mode = orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref());
let wrapper_mode =
orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref());
if wrapper_mode {
// we still want to be able to invoke it normally though
orig_args.remove(1);
}
if !wrapper_mode && (orig_args.iter().any(|a| a == "--help" || a == "-h") || orig_args.len() == 1) {
if !wrapper_mode
&& (orig_args.iter().any(|a| a == "--help" || a == "-h") || orig_args.len() == 1)
{
display_help();
exit(0);
}
@ -285,13 +289,11 @@ pub fn main() {
if clippy_enabled {
args.extend(vec!["--cfg".into(), r#"feature="cargo-clippy""#.into()]);
if let Ok(extra_args) = env::var("CLIPPY_ARGS") {
args.extend(extra_args.split("__CLIPPY_HACKERY__").filter_map(|s| {
if s.is_empty() {
None
} else {
Some(s.to_string())
}
}));
args.extend(
extra_args
.split("__CLIPPY_HACKERY__")
.filter_map(|s| if s.is_empty() { None } else { Some(s.to_string()) }),
);
}
}
let mut clippy = ClippyCallbacks;