Improve warnings on incompatible options involving -Zinstrument-coverage

richkadel · richkadel · commit 4f550f1f930b · 2020-12-14T12:55:46.000-08:00
Adds checks for:

* `no_core` attribute
* explicitly-enabled `legacy` symbol mangling
* mir_opt_level &gt; 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.
diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs
@@ -561,7 +561,7 @@ fn test_debugging_options_tracking_hash() {
     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 @@ 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")));
diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs
@@ -706,14 +706,21 @@ 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())
             && !self.sess.opts.debugging_opts.no_profiler_runtime
         {
             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);
 
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -663,7 +663,12 @@ 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
+                .symbol_mangling_version
+                .unwrap_or(SymbolManglingVersion::default()),
 
             crate_deps,
             dylib_dependency_formats,
diff --git a/compiler/rustc_mir/src/transform/const_prop.rs b/compiler/rustc_mir/src/transform/const_prop.rs
@@ -22,6 +22,7 @@ use rustc_middle::ty::subst::{InternalSubsts, Subst};
 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 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
             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 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
 
     /// 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 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
 
         // 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)
         }
     }
diff --git a/compiler/rustc_mir/src/transform/dest_prop.rs b/compiler/rustc_mir/src/transform/dest_prop.rs
@@ -115,6 +115,7 @@ use rustc_middle::mir::{
     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;
         }
 
diff --git a/compiler/rustc_mir/src/transform/early_otherwise_branch.rs b/compiler/rustc_mir/src/transform/early_otherwise_branch.rs
@@ -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 @@ pub struct EarlyOtherwiseBranch;
 
 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);
diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs
@@ -9,6 +9,7 @@ use rustc_middle::mir::visit::*;
 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;
         }
 
diff --git a/compiler/rustc_mir/src/transform/match_branches.rs b/compiler/rustc_mir/src/transform/match_branches.rs
@@ -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 @@ pub struct MatchBranchSimplification;
 
 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;
         }
 
diff --git a/compiler/rustc_mir/src/transform/mod.rs b/compiler/rustc_mir/src/transform/mod.rs
@@ -10,6 +10,7 @@ use rustc_middle::mir::visit::Visitor as _;
 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
diff --git a/compiler/rustc_mir/src/transform/multiple_return_terminators.rs b/compiler/rustc_mir/src/transform/multiple_return_terminators.rs
@@ -5,12 +5,13 @@ use crate::transform::{simplify, MirPass};
 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;
         }
 
diff --git a/compiler/rustc_mir/src/transform/nrvo.rs b/compiler/rustc_mir/src/transform/nrvo.rs
@@ -5,6 +5,7 @@ use rustc_index::bit_set::HybridBitSet;
 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 @@ pub struct RenameReturnPlace;
 
 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;
         }
 
diff --git a/compiler/rustc_mir/src/transform/unreachable_prop.rs b/compiler/rustc_mir/src/transform/unreachable_prop.rs
@@ -7,12 +7,13 @@ use crate::transform::MirPass;
 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;
diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs
@@ -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 @@ 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);
 
diff --git a/compiler/rustc_session/src/options.rs b/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
@@ -970,7 +970,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
     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 @@ 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],
diff --git a/compiler/rustc_symbol_mangling/src/lib.rs b/compiler/rustc_symbol_mangling/src/lib.rs
@@ -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)
     };
diff --git a/src/tools/clippy/src/driver.rs b/src/tools/clippy/src/driver.rs
