From cef3ab75b12155e0582dd8b7710b7b901215fdd6 Mon Sep 17 00:00:00 2001 From: Jeremy Fitzhardinge Date: Sat, 5 Jun 2021 15:42:04 -0700 Subject: [PATCH 1/6] Print more crate details in -Zls Useful for debugging crate hash and resolution issues. --- compiler/rustc_metadata/src/rmeta/decoder.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 48900fecd3e1b..45972c0a0cd02 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -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 + )?; } write!(out, "\n")?; Ok(()) From a26d99f348de873e7a7b2b9ab6e78506278c0fee Mon Sep 17 00:00:00 2001 From: Jeremy Fitzhardinge Date: Sat, 5 Jun 2021 15:43:12 -0700 Subject: [PATCH 2/6] In --emit KIND=PATH options, only hash KIND The PATH has no material effect on the emitted artifact, and setting the patch via `-o` or `--out-dir` does not affect the hash. Closes https://github.com/rust-lang/rust/issues/86044 --- compiler/rustc_interface/src/tests.rs | 6 ++--- compiler/rustc_session/src/config.rs | 14 ++++++++-- src/test/run-make/emit-path-unhashed/Makefile | 26 +++++++++++++++++++ src/test/run-make/emit-path-unhashed/foo.rs | 1 + 4 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 src/test/run-make/emit-path-unhashed/Makefile create mode 100644 src/test/run-make/emit-path-unhashed/foo.rs diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index f486a82ef953e..865c281be9c81 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -152,9 +152,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_same_hash(&v1, &v2); + assert_same_hash(&v1, &v3); + assert_same_hash(&v2, &v3); } #[test] diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 5afa8e6a09a69..331817ad2d0a7 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -31,6 +31,7 @@ use std::collections::btree_map::{ }; use std::collections::{BTreeMap, BTreeSet}; use std::fmt; +use std::hash::{Hash, Hasher}; use std::iter::{self, FromIterator}; use std::path::{Path, PathBuf}; use std::str::{self, FromStr}; @@ -325,10 +326,19 @@ 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)] pub struct OutputTypes(BTreeMap>); +impl Hash for OutputTypes { + fn hash(&self, hasher: &mut H) { + for k in self.keys() { + k.hash(hasher); + } + } +} + impl_stable_hash_via_hash!(OutputTypes); impl OutputTypes { diff --git a/src/test/run-make/emit-path-unhashed/Makefile b/src/test/run-make/emit-path-unhashed/Makefile new file mode 100644 index 0000000000000..bd04b3aa423a2 --- /dev/null +++ b/src/test/run-make/emit-path-unhashed/Makefile @@ -0,0 +1,26 @@ +-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 $(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 + + diff $(TMPDIR)/base.txt $(TMPDIR)/a.txt + diff $(TMPDIR)/base.txt $(TMPDIR)/b.txt + +# 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 diff --git a/src/test/run-make/emit-path-unhashed/foo.rs b/src/test/run-make/emit-path-unhashed/foo.rs new file mode 100644 index 0000000000000..c1bfaa6cab5d9 --- /dev/null +++ b/src/test/run-make/emit-path-unhashed/foo.rs @@ -0,0 +1 @@ +#![crate_type = "rlib"] From 48921ce30095bcc356f2fe082e29edb004d1eda9 Mon Sep 17 00:00:00 2001 From: Jeremy Fitzhardinge Date: Thu, 10 Jun 2021 14:01:21 -0700 Subject: [PATCH 3/6] Implement assert_non_crate_hash_different for tests --- compiler/rustc_interface/src/tests.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 865c281be9c81..c210865a4ad8c 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -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() { From f1f7f2f508b78defcf9f22f1c35342c27765a1f0 Mon Sep 17 00:00:00 2001 From: Jeremy Fitzhardinge Date: Wed, 9 Jun 2021 16:51:58 -0700 Subject: [PATCH 4/6] make -Zno-codegen TRACKED_NO_CRATE_HASH --- compiler/rustc_interface/src/tests.rs | 11 ++++++++++- compiler/rustc_session/src/options.rs | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index c210865a4ad8c..26a7dd0745ce1 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -720,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); @@ -755,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] diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index ebf59bb4cc679..51bf1710db0aa 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1148,7 +1148,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"), From 99f652ff2247f21e3842f312ec0e8112774ebef0 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 19 Jun 2021 19:22:14 -0500 Subject: [PATCH 5/6] Only hash OutputTypes keys in non-crate-hash mode This effectively turns OutputTypes into a hybrid where keys (OutputType) are TRACKED and the values (optional paths) are TRACKED_NO_CRATE_HASH. --- compiler/rustc_interface/src/tests.rs | 6 +- compiler/rustc_session/src/config.rs | 91 ++++++++++++++++++--------- compiler/rustc_session/src/options.rs | 17 +++-- 3 files changed, 77 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 26a7dd0745ce1..52b3076a44396 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -160,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_same_hash(&v1, &v2); - assert_same_hash(&v1, &v3); - assert_same_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] diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 331817ad2d0a7..88eaa7fe32924 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -31,7 +31,7 @@ use std::collections::btree_map::{ }; use std::collections::{BTreeMap, BTreeSet}; use std::fmt; -use std::hash::{Hash, Hasher}; +use std::hash::Hash; use std::iter::{self, FromIterator}; use std::path::{Path, PathBuf}; use std::str::{self, FromStr}; @@ -328,19 +328,9 @@ impl Default for TrimmedDefPaths { /// *Do not* switch `BTreeMap` out for an unsorted container type! That would break /// 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)] +#[derive(Clone, Debug, Hash)] pub struct OutputTypes(BTreeMap>); -impl Hash for OutputTypes { - fn hash(&self, hasher: &mut H) { - for k in self.keys() { - k.hash(hasher); - } - } -} - -impl_stable_hash_via_hash!(OutputTypes); - impl OutputTypes { pub fn new(entries: &[(OutputType, Option)]) -> OutputTypes { OutputTypes(BTreeMap::from_iter(entries.iter().map(|&(k, ref v)| (k, v.clone())))) @@ -2436,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; @@ -2453,13 +2443,18 @@ 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); } } @@ -2467,11 +2462,16 @@ crate mod dep_tracking { } impl DepTrackingHash for Option { - 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), } @@ -2501,7 +2501,6 @@ crate mod dep_tracking { LtoCli, DebugInfo, UnstableFeatures, - OutputTypes, NativeLib, NativeLibKind, SanitizerSet, @@ -2515,6 +2514,7 @@ crate mod dep_tracking { SourceFileHashAlgorithm, TrimmedDefPaths, Option, + OutputType, ); impl DepTrackingHash for (T1, T2) @@ -2522,11 +2522,16 @@ crate mod dep_tracking { 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); } } @@ -2536,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 DepTrackingHash for Vec { - 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); + } } } } @@ -2561,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); } } } diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 51bf1710db0aa..f5dd8992c29f9 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -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, + ); }; } @@ -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, @@ -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() } } From 45146978e8d593d5ec713f54140407d64f620800 Mon Sep 17 00:00:00 2001 From: Jeremy Fitzhardinge Date: Mon, 21 Jun 2021 17:10:46 -0700 Subject: [PATCH 6/6] Add test showing different KIND parameters change hash --- src/test/run-make/emit-path-unhashed/Makefile | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/test/run-make/emit-path-unhashed/Makefile b/src/test/run-make/emit-path-unhashed/Makefile index bd04b3aa423a2..b6b2d8af64800 100644 --- a/src/test/run-make/emit-path-unhashed/Makefile +++ b/src/test/run-make/emit-path-unhashed/Makefile @@ -3,14 +3,20 @@ OUT=$(TMPDIR)/emit # --emit KIND=PATH should not affect crate hash vs --emit KIND -all: $(OUT)/a/libfoo.rlib $(OUT)/b/libfoo.rlib $(TMPDIR)/libfoo.rlib +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 @@ -24,3 +30,8 @@ $(OUT)/a/libfoo.rlib: foo.rs $(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