Skip to content

Commit

Permalink
Auto merge of #93655 - matthiaskrgr:rollup-dm88b02, r=matthiaskrgr
Browse files Browse the repository at this point in the history
Rollup of 7 pull requests

Successful merges:

 - #90132 (Stabilize `-Z instrument-coverage` as `-C instrument-coverage`)
 - #91589 (impl `Arc::unwrap_or_clone`)
 - #93495 (kmc-solid: Fix off-by-one error in `SystemTime::now`)
 - #93576 (Emit more valid HTML from rustdoc)
 - #93608 (Clean up `find_library_crate`)
 - #93612 (doc: use U+2212 for minus sign in integer MIN/MAX text)
 - #93615 (Fix `isize` optimization in `StableHasher` for big-endian architectures)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Feb 5, 2022
2 parents 71226d7 + 2d62bd0 commit 4c55c83
Show file tree
Hide file tree
Showing 63 changed files with 408 additions and 338 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) {
// LLVM 12.
let version = coverageinfo::mapping_version();
if version < 4 {
tcx.sess.fatal("rustc option `-Z instrument-coverage` requires LLVM 12 or higher.");
tcx.sess.fatal("rustc option `-C instrument-coverage` requires LLVM 12 or higher.");
}

debug!("Generating coverage map for CodegenUnit: `{}`", cx.codegen_unit.name());
Expand Down Expand Up @@ -274,7 +274,7 @@ fn save_function_record(
/// (functions referenced by other "used" or public items). Any other functions considered unused,
/// or "Unreachable", were still parsed and processed through the MIR stage, but were not
/// codegenned. (Note that `-Clink-dead-code` can force some unused code to be codegenned, but
/// that flag is known to cause other errors, when combined with `-Z instrument-coverage`; and
/// that flag is known to cause other errors, when combined with `-C instrument-coverage`; and
/// `-Clink-dead-code` will not generate code for unused generic functions.)
///
/// We can find the unused functions (including generic functions) by the set difference of all MIR
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ pub trait CoverageInfoMethods<'tcx>: BackendTypes {

pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes {
/// Returns true if the function source hash was added to the coverage map (even if it had
/// already been added, for this instance). Returns false *only* if `-Z instrument-coverage` is
/// already been added, for this instance). Returns false *only* if `-C instrument-coverage` is
/// not enabled (a coverage map is not being generated).
fn set_function_source_hash(
&mut self,
instance: Instance<'tcx>,
function_source_hash: u64,
) -> bool;

/// Returns true if the counter was added to the coverage map; false if `-Z instrument-coverage`
/// Returns true if the counter was added to the coverage map; false if `-C instrument-coverage`
/// is not enabled (a coverage map is not being generated).
fn add_coverage_counter(
&mut self,
Expand All @@ -40,7 +40,7 @@ pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes {
) -> bool;

/// Returns true if the expression was added to the coverage map; false if
/// `-Z instrument-coverage` is not enabled (a coverage map is not being generated).
/// `-C instrument-coverage` is not enabled (a coverage map is not being generated).
fn add_coverage_counter_expression(
&mut self,
instance: Instance<'tcx>,
Expand All @@ -51,7 +51,7 @@ pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes {
region: Option<CodeRegion>,
) -> bool;

/// Returns true if the region was added to the coverage map; false if `-Z instrument-coverage`
/// Returns true if the region was added to the coverage map; false if `-C instrument-coverage`
/// is not enabled (a coverage map is not being generated).
fn add_coverage_unreachable(&mut self, instance: Instance<'tcx>, region: CodeRegion) -> bool;
}
10 changes: 7 additions & 3 deletions compiler/rustc_data_structures/src/stable_hasher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,18 +133,18 @@ impl Hasher for StableHasher {

#[inline]
fn write_isize(&mut self, i: isize) {
// Always treat isize as i64 so we get the same results on 32 and 64 bit
// Always treat isize as a 64-bit number so we get the same results on 32 and 64 bit
// platforms. This is important for symbol hashes when cross compiling,
// for example. Sign extending here is preferable as it means that the
// same negative number hashes the same on both 32 and 64 bit platforms.
let value = (i as i64).to_le() as u64;
let value = i as u64;

// Cold path
#[cold]
#[inline(never)]
fn hash_value(state: &mut SipHasher128, value: u64) {
state.write_u8(0xFF);
state.write_u64(value);
state.write_u64(value.to_le());
}

// `isize` values often seem to have a small (positive) numeric value in practice.
Expand All @@ -161,6 +161,10 @@ impl Hasher for StableHasher {
// 8 bytes. Since this prefix cannot occur when we hash a single byte, when we hash two
// `isize`s that fit within a different amount of bytes, they should always produce a different
// byte stream for the hasher.
//
// To ensure that this optimization hashes the exact same bytes on both little-endian and
// big-endian architectures, we compare the value with 0xFF before we convert the number
// into a unified representation (little-endian).
if value < 0xFF {
self.state.write_u8(value as u8);
} else {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_data_structures/src/stable_hasher/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,5 @@ fn test_isize_compression() {
check_hash(0xAAAA, 0xAAAAAA);
check_hash(0xAAAAAA, 0xAAAAAAAA);
check_hash(0xFF, 0xFFFFFFFFFFFFFFFF);
check_hash(u64::MAX /* -1 */, 1);
}
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,7 @@ fn test_codegen_options_tracking_hash() {
tracked!(force_frame_pointers, Some(false));
tracked!(force_unwind_tables, Some(true));
tracked!(inline_threshold, Some(0xf007ba11));
tracked!(instrument_coverage, Some(InstrumentCoverage::All));
tracked!(linker_plugin_lto, LinkerPluginLto::LinkerPluginAuto);
tracked!(link_dead_code, Some(true));
tracked!(llvm_args, vec![String::from("1"), String::from("2")]);
Expand Down
97 changes: 53 additions & 44 deletions compiler/rustc_metadata/src/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ use rustc_data_structures::sync::MetadataRef;
use rustc_errors::{struct_span_err, FatalError};
use rustc_session::config::{self, CrateType};
use rustc_session::cstore::{CrateSource, MetadataLoader};
use rustc_session::filesearch::{FileDoesntMatch, FileMatches, FileSearch};
use rustc_session::filesearch::FileSearch;
use rustc_session::search_paths::PathKind;
use rustc_session::utils::CanonicalizedPath;
use rustc_session::Session;
Expand Down Expand Up @@ -371,15 +371,20 @@ impl<'a> CrateLocator<'a> {
extra_prefix: &str,
seen_paths: &mut FxHashSet<PathBuf>,
) -> Result<Option<Library>, CrateError> {
// want: crate_name.dir_part() + prefix + crate_name.file_part + "-"
let dylib_prefix = format!("{}{}{}", self.target.dll_prefix, self.crate_name, extra_prefix);
let rlib_prefix = format!("lib{}{}", self.crate_name, extra_prefix);
let rmeta_prefix = &format!("lib{}{}", self.crate_name, extra_prefix);
let rlib_prefix = rmeta_prefix;
let dylib_prefix =
&format!("{}{}{}", self.target.dll_prefix, self.crate_name, extra_prefix);
let staticlib_prefix =
format!("{}{}{}", self.target.staticlib_prefix, self.crate_name, extra_prefix);
&format!("{}{}{}", self.target.staticlib_prefix, self.crate_name, extra_prefix);

let rmeta_suffix = ".rmeta";
let rlib_suffix = ".rlib";
let dylib_suffix = &self.target.dll_suffix;
let staticlib_suffix = &self.target.staticlib_suffix;

let mut candidates: FxHashMap<_, (FxHashMap<_, _>, FxHashMap<_, _>, FxHashMap<_, _>)> =
Default::default();
let mut staticlibs = vec![];

// First, find all possible candidate rlibs and dylibs purely based on
// the name of the files themselves. We're trying to match against an
Expand All @@ -394,46 +399,50 @@ impl<'a> CrateLocator<'a> {
// of the crate id (path/name/id).
//
// The goal of this step is to look at as little metadata as possible.
self.filesearch.search(|spf, kind| {
let file = match &spf.file_name_str {
None => return FileDoesntMatch,
Some(file) => file,
};
let (hash, found_kind) = if file.starts_with(&rlib_prefix) && file.ends_with(".rlib") {
(&file[(rlib_prefix.len())..(file.len() - ".rlib".len())], CrateFlavor::Rlib)
} else if file.starts_with(&rlib_prefix) && file.ends_with(".rmeta") {
(&file[(rlib_prefix.len())..(file.len() - ".rmeta".len())], CrateFlavor::Rmeta)
} else if file.starts_with(&dylib_prefix) && file.ends_with(&self.target.dll_suffix) {
(
&file[(dylib_prefix.len())..(file.len() - self.target.dll_suffix.len())],
CrateFlavor::Dylib,
)
} else {
if file.starts_with(&staticlib_prefix)
&& file.ends_with(&self.target.staticlib_suffix)
{
staticlibs
.push(CrateMismatch { path: spf.path.clone(), got: "static".to_string() });
}
return FileDoesntMatch;
};
// Unfortunately, the prefix-based matching sometimes is over-eager.
// E.g. if `rlib_suffix` is `libstd` it'll match the file
// `libstd_detect-8d6701fb958915ad.rlib` (incorrect) as well as
// `libstd-f3ab5b1dea981f17.rlib` (correct). But this is hard to avoid
// given that `extra_filename` comes from the `-C extra-filename`
// option and thus can be anything, and the incorrect match will be
// handled safely in `extract_one`.
for search_path in self.filesearch.search_paths() {
debug!("searching {}", search_path.dir.display());
for spf in search_path.files.iter() {
debug!("testing {}", spf.path.display());

let f = &spf.file_name_str;
let (hash, kind) = if f.starts_with(rlib_prefix) && f.ends_with(rlib_suffix) {
(&f[rlib_prefix.len()..(f.len() - rlib_suffix.len())], CrateFlavor::Rlib)
} else if f.starts_with(rmeta_prefix) && f.ends_with(rmeta_suffix) {
(&f[rmeta_prefix.len()..(f.len() - rmeta_suffix.len())], CrateFlavor::Rmeta)
} else if f.starts_with(dylib_prefix) && f.ends_with(dylib_suffix) {
(&f[dylib_prefix.len()..(f.len() - dylib_suffix.len())], CrateFlavor::Dylib)
} else {
if f.starts_with(staticlib_prefix) && f.ends_with(staticlib_suffix) {
self.crate_rejections.via_kind.push(CrateMismatch {
path: spf.path.clone(),
got: "static".to_string(),
});
}
continue;
};

info!("lib candidate: {}", spf.path.display());
info!("lib candidate: {}", spf.path.display());

let (rlibs, rmetas, dylibs) = candidates.entry(hash.to_string()).or_default();
let path = fs::canonicalize(&spf.path).unwrap_or_else(|_| spf.path.clone());
if seen_paths.contains(&path) {
return FileDoesntMatch;
};
seen_paths.insert(path.clone());
match found_kind {
CrateFlavor::Rlib => rlibs.insert(path, kind),
CrateFlavor::Rmeta => rmetas.insert(path, kind),
CrateFlavor::Dylib => dylibs.insert(path, kind),
};
FileMatches
});
self.crate_rejections.via_kind.extend(staticlibs);
let (rlibs, rmetas, dylibs) = candidates.entry(hash.to_string()).or_default();
let path = fs::canonicalize(&spf.path).unwrap_or_else(|_| spf.path.clone());
if seen_paths.contains(&path) {
continue;
};
seen_paths.insert(path.clone());
match kind {
CrateFlavor::Rlib => rlibs.insert(path, search_path.kind),
CrateFlavor::Rmeta => rmetas.insert(path, search_path.kind),
CrateFlavor::Dylib => dylibs.insert(path, search_path.kind),
};
}
}

// We have now collected all known libraries into a set of candidates
// keyed of the filename hash listed. For each filename, we also have a
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1577,7 +1577,7 @@ pub enum StatementKind<'tcx> {
/// - `Bivariant` -- no effect
AscribeUserType(Box<(Place<'tcx>, UserTypeProjection)>, ty::Variance),

/// Marks the start of a "coverage region", injected with '-Zinstrument-coverage'. A
/// Marks the start of a "coverage region", injected with '-Cinstrument-coverage'. A
/// `Coverage` statement carries metadata about the coverage region, used to inject a coverage
/// map into the binary. If `Coverage::kind` is a `Counter`, the statement also generates
/// executable code, to increment a counter variable at runtime, each time the code region is
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ pub struct DestructuredConst<'tcx> {
}

/// Coverage information summarized from a MIR if instrumented for source code coverage (see
/// compiler option `-Zinstrument-coverage`). This information is generated by the
/// compiler option `-Cinstrument-coverage`). This information is generated by the
/// `InstrumentCoverage` MIR pass and can be retrieved via the `coverageinfo` query.
#[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable)]
pub struct CoverageInfo {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ rustc_queries! {
}

/// Returns coverage summary info for a function, after executing the `InstrumentCoverage`
/// MIR pass (assuming the -Zinstrument-coverage option is enabled).
/// MIR pass (assuming the -Cinstrument-coverage option is enabled).
query coverageinfo(key: ty::InstanceDef<'tcx>) -> mir::CoverageInfo {
desc { |tcx| "retrieving coverage info from MIR for `{}`", tcx.def_path_str(key.def_id()) }
storage(ArenaCacheSelector<'tcx>)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/coverage/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//!
//! To enable coverage, include the rustc command line option:
//!
//! * `-Z instrument-coverage`
//! * `-C instrument-coverage`
//!
//! MIR Dump Files, with additional `CoverageGraph` graphviz and `CoverageSpan` spanview
//! ------------------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
/// evaluation: `if false { ... }`.
///
/// Those statements are bypassed by redirecting paths in the CFG around the
/// `dead blocks`; but with `-Z instrument-coverage`, the dead blocks usually
/// `dead blocks`; but with `-C instrument-coverage`, the dead blocks usually
/// include `Coverage` statements representing the Rust source code regions to
/// be counted at runtime. Without these `Coverage` statements, the regions are
/// lost, and the Rust source code will show no coverage information.
Expand Down
54 changes: 40 additions & 14 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,16 @@ pub enum MirSpanview {
Block,
}

/// The different settings that the `-Z instrument-coverage` flag can have.
/// The different settings that the `-C instrument-coverage` flag can have.
///
/// Coverage instrumentation now supports combining `-Z instrument-coverage`
/// Coverage instrumentation now supports combining `-C instrument-coverage`
/// with compiler and linker optimization (enabled with `-O` or `-C opt-level=1`
/// 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 `-Z instrument-coverage=except-unused-functions` or `-Z
/// instrument-coverage=except-unused-generics`.
/// 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
Expand All @@ -148,13 +148,13 @@ pub enum MirSpanview {
/// unless the function has type parameters.
#[derive(Clone, Copy, PartialEq, Hash, Debug)]
pub enum InstrumentCoverage {
/// Default `-Z instrument-coverage` or `-Z instrument-coverage=statement`
/// Default `-C instrument-coverage` or `-C instrument-coverage=statement`
All,
/// `-Z instrument-coverage=except-unused-generics`
/// `-Zunstable-options -C instrument-coverage=except-unused-generics`
ExceptUnusedGenerics,
/// `-Z instrument-coverage=except-unused-functions`
/// `-Zunstable-options -C instrument-coverage=except-unused-functions`
ExceptUnusedFunctions,
/// `-Z instrument-coverage=off` (or `no`, etc.)
/// `-C instrument-coverage=off` (or `no`, etc.)
Off,
}

Expand Down Expand Up @@ -2195,18 +2195,44 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
_ => {}
}

if debugging_opts.instrument_coverage.is_some()
&& debugging_opts.instrument_coverage != Some(InstrumentCoverage::Off)
{
// Handle both `-Z instrument-coverage` and `-C instrument-coverage`; the latter takes
// precedence.
match (cg.instrument_coverage, debugging_opts.instrument_coverage) {
(Some(ic_c), Some(ic_z)) if ic_c != ic_z => {
early_error(
error_format,
"incompatible values passed for `-C instrument-coverage` \
and `-Z instrument-coverage`",
);
}
(Some(InstrumentCoverage::Off | InstrumentCoverage::All), _) => {}
(Some(_), _) if !debugging_opts.unstable_options => {
early_error(
error_format,
"`-C instrument-coverage=except-*` requires `-Z unstable-options`",
);
}
(None, None) => {}
(None, ic) => {
early_warn(
error_format,
"`-Z instrument-coverage` is deprecated; use `-C instrument-coverage`",
);
cg.instrument_coverage = ic;
}
_ => {}
}

if cg.instrument_coverage.is_some() && cg.instrument_coverage != Some(InstrumentCoverage::Off) {
if cg.profile_generate.enabled() || cg.profile_use.is_some() {
early_error(
error_format,
"option `-Z instrument-coverage` is not compatible with either `-C profile-use` \
"option `-C instrument-coverage` is not compatible with either `-C profile-use` \
or `-C profile-generate`",
);
}

// `-Z instrument-coverage` implies `-C symbol-mangling-version=v0` - to ensure consistent
// `-C instrument-coverage` implies `-C 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.
Expand All @@ -2215,7 +2241,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
Some(SymbolManglingVersion::Legacy) => {
early_warn(
error_format,
"-Z instrument-coverage requires symbol mangling version `v0`, \
"-C instrument-coverage requires symbol mangling version `v0`, \
but `-C symbol-mangling-version=legacy` was specified",
);
}
Expand Down
Loading

0 comments on commit 4c55c83

Please sign in to comment.