Skip to content

Fix emit path hashing #86045

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

Merged
merged 6 commits into from
Jun 22, 2021
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
25 changes: 21 additions & 4 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ fn assert_different_hash(x: &Options, y: &Options) {
assert_same_clone(y);
}

fn assert_non_crate_hash_different(x: &Options, y: &Options) {
assert_eq!(x.dep_tracking_hash(true), y.dep_tracking_hash(true));
assert_ne!(x.dep_tracking_hash(false), y.dep_tracking_hash(false));
// Check clone
assert_same_clone(x);
assert_same_clone(y);
}

// When the user supplies --test we should implicitly supply --cfg test
#[test]
fn test_switch_implies_cfg_test() {
Expand Down Expand Up @@ -152,9 +160,9 @@ fn test_output_types_tracking_hash_different_paths() {
v2.output_types = OutputTypes::new(&[(OutputType::Exe, Some(PathBuf::from("/some/thing")))]);
v3.output_types = OutputTypes::new(&[(OutputType::Exe, None)]);

assert_different_hash(&v1, &v2);
assert_different_hash(&v1, &v3);
assert_different_hash(&v2, &v3);
assert_non_crate_hash_different(&v1, &v2);
assert_non_crate_hash_different(&v1, &v3);
assert_non_crate_hash_different(&v2, &v3);
}

#[test]
Expand Down Expand Up @@ -712,7 +720,6 @@ fn test_debugging_options_tracking_hash() {
tracked!(mir_opt_level, Some(4));
tracked!(mutable_noalias, Some(true));
tracked!(new_llvm_pass_manager, Some(true));
tracked!(no_codegen, true);
tracked!(no_generate_arange_section, true);
tracked!(no_link, true);
tracked!(osx_rpath_install_name, true);
Expand Down Expand Up @@ -747,6 +754,16 @@ fn test_debugging_options_tracking_hash() {
tracked!(use_ctors_section, Some(true));
tracked!(verify_llvm_ir, true);
tracked!(wasi_exec_model, Some(WasiExecModel::Reactor));

macro_rules! tracked_no_crate_hash {
($name: ident, $non_default_value: expr) => {
opts = reference.clone();
assert_ne!(opts.debugging_opts.$name, $non_default_value);
opts.debugging_opts.$name = $non_default_value;
assert_non_crate_hash_different(&reference, &opts);
};
}
tracked_no_crate_hash!(no_codegen, true);
}

#[test]
Expand Down
17 changes: 15 additions & 2 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,10 +601,23 @@ impl MetadataBlob {
}

crate fn list_crate_metadata(&self, out: &mut dyn io::Write) -> io::Result<()> {
write!(out, "=External Dependencies=\n")?;
let root = self.get_root();
writeln!(out, "Crate info:")?;
writeln!(out, "name {}{}", root.name, root.extra_filename)?;
writeln!(out, "hash {} stable_crate_id {:?}", root.hash, root.stable_crate_id)?;
writeln!(out, "proc_macro {:?}", root.proc_macro_data.is_some())?;
writeln!(out, "=External Dependencies=")?;
for (i, dep) in root.crate_deps.decode(self).enumerate() {
write!(out, "{} {}{}\n", i + 1, dep.name, dep.extra_filename)?;
writeln!(
out,
"{} {}{} hash {} host_hash {:?} kind {:?}",
i + 1,
dep.name,
dep.extra_filename,
dep.hash,
dep.host_hash,
dep.kind
)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

}
write!(out, "\n")?;
Ok(())
Expand Down
85 changes: 64 additions & 21 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use std::collections::btree_map::{
};
use std::collections::{BTreeMap, BTreeSet};
use std::fmt;
use std::hash::Hash;
use std::iter::{self, FromIterator};
use std::path::{Path, PathBuf};
use std::str::{self, FromStr};
Expand Down Expand Up @@ -325,12 +326,11 @@ impl Default for TrimmedDefPaths {

/// Use tree-based collections to cheaply get a deterministic `Hash` implementation.
/// *Do not* switch `BTreeMap` out for an unsorted container type! That would break
/// dependency tracking for command-line arguments.
#[derive(Clone, Hash, Debug)]
/// dependency tracking for command-line arguments. Also only hash keys, since tracking
/// should only depend on the output types, not the paths they're written to.
#[derive(Clone, Debug, Hash)]
pub struct OutputTypes(BTreeMap<OutputType, Option<PathBuf>>);

impl_stable_hash_via_hash!(OutputTypes);

impl OutputTypes {
pub fn new(entries: &[(OutputType, Option<PathBuf>)]) -> OutputTypes {
OutputTypes(BTreeMap::from_iter(entries.iter().map(|&(k, ref v)| (k, v.clone()))))
Expand Down Expand Up @@ -2426,8 +2426,8 @@ crate mod dep_tracking {
use super::LdImpl;
use super::{
CFGuard, CrateType, DebugInfo, ErrorOutputType, InstrumentCoverage, LinkerPluginLto,
LtoCli, OptLevel, OutputTypes, Passes, SourceFileHashAlgorithm, SwitchWithOptPath,
SymbolManglingVersion, TrimmedDefPaths,
LtoCli, OptLevel, OutputType, OutputTypes, Passes, SourceFileHashAlgorithm,
SwitchWithOptPath, SymbolManglingVersion, TrimmedDefPaths,
};
use crate::lint;
use crate::options::WasiExecModel;
Expand All @@ -2443,25 +2443,35 @@ crate mod dep_tracking {
use std::path::PathBuf;

pub trait DepTrackingHash {
fn hash(&self, hasher: &mut DefaultHasher, error_format: ErrorOutputType);
fn hash(
&self,
hasher: &mut DefaultHasher,
error_format: ErrorOutputType,
for_crate_hash: bool,
);
}

macro_rules! impl_dep_tracking_hash_via_hash {
($($t:ty),+ $(,)?) => {$(
impl DepTrackingHash for $t {
fn hash(&self, hasher: &mut DefaultHasher, _: ErrorOutputType) {
fn hash(&self, hasher: &mut DefaultHasher, _: ErrorOutputType, _for_crate_hash: bool) {
Hash::hash(self, hasher);
}
}
)+};
}

impl<T: DepTrackingHash> DepTrackingHash for Option<T> {
fn hash(&self, hasher: &mut DefaultHasher, error_format: ErrorOutputType) {
fn hash(
&self,
hasher: &mut DefaultHasher,
error_format: ErrorOutputType,
for_crate_hash: bool,
) {
match self {
Some(x) => {
Hash::hash(&1, hasher);
DepTrackingHash::hash(x, hasher, error_format);
DepTrackingHash::hash(x, hasher, error_format, for_crate_hash);
}
None => Hash::hash(&0, hasher),
}
Expand Down Expand Up @@ -2491,7 +2501,6 @@ crate mod dep_tracking {
LtoCli,
DebugInfo,
UnstableFeatures,
OutputTypes,
NativeLib,
NativeLibKind,
SanitizerSet,
Expand All @@ -2505,18 +2514,24 @@ crate mod dep_tracking {
SourceFileHashAlgorithm,
TrimmedDefPaths,
Option<LdImpl>,
OutputType,
);

impl<T1, T2> DepTrackingHash for (T1, T2)
where
T1: DepTrackingHash,
T2: DepTrackingHash,
{
fn hash(&self, hasher: &mut DefaultHasher, error_format: ErrorOutputType) {
fn hash(
&self,
hasher: &mut DefaultHasher,
error_format: ErrorOutputType,
for_crate_hash: bool,
) {
Hash::hash(&0, hasher);
DepTrackingHash::hash(&self.0, hasher, error_format);
DepTrackingHash::hash(&self.0, hasher, error_format, for_crate_hash);
Hash::hash(&1, hasher);
DepTrackingHash::hash(&self.1, hasher, error_format);
DepTrackingHash::hash(&self.1, hasher, error_format, for_crate_hash);
}
}

Expand All @@ -2526,22 +2541,49 @@ crate mod dep_tracking {
T2: DepTrackingHash,
T3: DepTrackingHash,
{
fn hash(&self, hasher: &mut DefaultHasher, error_format: ErrorOutputType) {
fn hash(
&self,
hasher: &mut DefaultHasher,
error_format: ErrorOutputType,
for_crate_hash: bool,
) {
Hash::hash(&0, hasher);
DepTrackingHash::hash(&self.0, hasher, error_format);
DepTrackingHash::hash(&self.0, hasher, error_format, for_crate_hash);
Hash::hash(&1, hasher);
DepTrackingHash::hash(&self.1, hasher, error_format);
DepTrackingHash::hash(&self.1, hasher, error_format, for_crate_hash);
Hash::hash(&2, hasher);
DepTrackingHash::hash(&self.2, hasher, error_format);
DepTrackingHash::hash(&self.2, hasher, error_format, for_crate_hash);
}
}

impl<T: DepTrackingHash> DepTrackingHash for Vec<T> {
fn hash(&self, hasher: &mut DefaultHasher, error_format: ErrorOutputType) {
fn hash(
&self,
hasher: &mut DefaultHasher,
error_format: ErrorOutputType,
for_crate_hash: bool,
) {
Hash::hash(&self.len(), hasher);
for (index, elem) in self.iter().enumerate() {
Hash::hash(&index, hasher);
DepTrackingHash::hash(elem, hasher, error_format);
DepTrackingHash::hash(elem, hasher, error_format, for_crate_hash);
}
}
}

impl DepTrackingHash for OutputTypes {
fn hash(
&self,
hasher: &mut DefaultHasher,
error_format: ErrorOutputType,
for_crate_hash: bool,
) {
Hash::hash(&self.0.len(), hasher);
for (key, val) in &self.0 {
DepTrackingHash::hash(key, hasher, error_format, for_crate_hash);
if !for_crate_hash {
DepTrackingHash::hash(val, hasher, error_format, for_crate_hash);
}
}
}
}
Expand All @@ -2551,13 +2593,14 @@ crate mod dep_tracking {
sub_hashes: BTreeMap<&'static str, &dyn DepTrackingHash>,
hasher: &mut DefaultHasher,
error_format: ErrorOutputType,
for_crate_hash: bool,
) {
for (key, sub_hash) in sub_hashes {
// Using Hash::hash() instead of DepTrackingHash::hash() is fine for
// the keys, as they are just plain strings
Hash::hash(&key.len(), hasher);
Hash::hash(key, hasher);
sub_hash.hash(hasher, error_format);
sub_hash.hash(hasher, error_format, for_crate_hash);
}
}
}
19 changes: 13 additions & 6 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ macro_rules! hash_substruct {
($opt_name:ident, $opt_expr:expr, $error_format:expr, $for_crate_hash:expr, $hasher:expr, [TRACKED_NO_CRATE_HASH]) => {{}};
($opt_name:ident, $opt_expr:expr, $error_format:expr, $for_crate_hash:expr, $hasher:expr, [SUBSTRUCT]) => {
use crate::config::dep_tracking::DepTrackingHash;
$opt_expr.dep_tracking_hash($for_crate_hash, $error_format).hash($hasher, $error_format);
$opt_expr.dep_tracking_hash($for_crate_hash, $error_format).hash(
$hasher,
$error_format,
$for_crate_hash,
);
};
}

Expand Down Expand Up @@ -79,7 +83,8 @@ macro_rules! top_level_options {
let mut hasher = DefaultHasher::new();
dep_tracking::stable_hash(sub_hashes,
&mut hasher,
self.error_format);
self.error_format,
for_crate_hash);
$({
hash_substruct!($opt,
&self.$opt,
Expand Down Expand Up @@ -236,19 +241,21 @@ macro_rules! options {
build_options(matches, $stat, $prefix, $outputname, error_format)
}

fn dep_tracking_hash(&self, _for_crate_hash: bool, error_format: ErrorOutputType) -> u64 {
fn dep_tracking_hash(&self, for_crate_hash: bool, error_format: ErrorOutputType) -> u64 {
let mut sub_hashes = BTreeMap::new();
$({
hash_opt!($opt,
&self.$opt,
&mut sub_hashes,
_for_crate_hash,
for_crate_hash,
[$dep_tracking_marker]);
})*
let mut hasher = DefaultHasher::new();
dep_tracking::stable_hash(sub_hashes,
&mut hasher,
error_format);
error_format,
for_crate_hash
);
hasher.finish()
}
}
Expand Down Expand Up @@ -1148,7 +1155,7 @@ options! {
"the directory the NLL facts are dumped into (default: `nll-facts`)"),
no_analysis: bool = (false, parse_no_flag, [UNTRACKED],
"parse and expand the source, but run no analysis"),
no_codegen: bool = (false, parse_no_flag, [TRACKED],
no_codegen: bool = (false, parse_no_flag, [TRACKED_NO_CRATE_HASH],
"run all passes except codegen; no output"),
no_generate_arange_section: bool = (false, parse_no_flag, [TRACKED],
"omit DWARF address ranges that give faster lookups"),
Expand Down
37 changes: 37 additions & 0 deletions src/test/run-make/emit-path-unhashed/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
-include ../../run-make-fulldeps/tools.mk

OUT=$(TMPDIR)/emit

# --emit KIND=PATH should not affect crate hash vs --emit KIND
all: $(OUT)/a/libfoo.rlib $(OUT)/b/libfoo.rlib $(OUT)/c/libfoo.rlib \
$(TMPDIR)/libfoo.rlib
$(RUSTC) -Zls $(TMPDIR)/libfoo.rlib > $(TMPDIR)/base.txt
$(RUSTC) -Zls $(OUT)/a/libfoo.rlib > $(TMPDIR)/a.txt
$(RUSTC) -Zls $(OUT)/b/libfoo.rlib > $(TMPDIR)/b.txt
$(RUSTC) -Zls $(OUT)/c/libfoo.rlib > $(TMPDIR)/c.txt

diff $(TMPDIR)/base.txt $(TMPDIR)/a.txt
diff $(TMPDIR)/base.txt $(TMPDIR)/b.txt

# Different KIND parameters do affect hash.
# diff exits 1 on difference, 2 on trouble
diff $(TMPDIR)/base.txt $(TMPDIR)/c.txt ; test "$$?" -eq 1

# Default output name
$(TMPDIR)/libfoo.rlib: foo.rs
$(RUSTC) --emit link foo.rs

# Output named with -o
$(OUT)/a/libfoo.rlib: foo.rs
mkdir -p $(OUT)/a
$(RUSTC) --emit link -o $@ foo.rs

# Output named with KIND=PATH
$(OUT)/b/libfoo.rlib: foo.rs
mkdir -p $(OUT)/b
$(RUSTC) --emit link=$@ foo.rs

# Output multiple kinds
$(OUT)/c/libfoo.rlib: foo.rs
mkdir -p $(OUT)/c
$(RUSTC) --emit link=$@,metadata foo.rs
1 change: 1 addition & 0 deletions src/test/run-make/emit-path-unhashed/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#![crate_type = "rlib"]