From a895069c5023a82e9316d21854a81172c69d1418 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 12 Aug 2021 15:30:40 -0500 Subject: [PATCH] Include (potentially remapped) working dir in crate hash Fixes #85019 A `SourceFile` created during compilation may have a relative path (e.g. if rustc itself is invoked with a relative path). When we write out crate metadata, we convert all relative paths to absolute paths using the current working direction. However, the working directory is not included in the crate hash. This means that the crate metadata can change while the crate hash remains the same. Among other problems, this can cause a fingerprint mismatch ICE, since incremental compilation uses the crate metadata hash to determine if a foreign query is green. This commit moves the field holding the working directory from `Session` to `Options`, including it as part of the crate hash. --- .../src/debuginfo/metadata.rs | 4 +-- compiler/rustc_metadata/src/rmeta/encoder.rs | 5 +++- .../rustc_save_analysis/src/dump_visitor.rs | 2 +- .../rustc_save_analysis/src/span_utils.rs | 1 + compiler/rustc_session/src/config.rs | 17 +++++++++++++ compiler/rustc_session/src/options.rs | 4 +++ compiler/rustc_session/src/session.rs | 15 +---------- .../issue-85019-moved-src-dir/Makefile | 25 +++++++++++++++++++ .../issue-85019-moved-src-dir/main.rs | 5 ++++ .../issue-85019-moved-src-dir/my_lib.rs | 1 + 10 files changed, 61 insertions(+), 18 deletions(-) create mode 100644 src/test/run-make/issue-85019-moved-src-dir/Makefile create mode 100644 src/test/run-make/issue-85019-moved-src-dir/main.rs create mode 100644 src/test/run-make/issue-85019-moved-src-dir/my_lib.rs diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 2cb126f1a7e4d..8ff2f1cc6520f 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -771,7 +771,7 @@ pub fn file_metadata(cx: &CodegenCx<'ll, '_>, source_file: &SourceFile) -> &'ll let hash = Some(&source_file.src_hash); let file_name = Some(source_file.name.prefer_remapped().to_string()); let directory = if source_file.is_real_file() && !source_file.is_imported() { - Some(cx.sess().working_dir.to_string_lossy(false).to_string()) + Some(cx.sess().opts.working_dir.to_string_lossy(false).to_string()) } else { // If the path comes from an upstream crate we assume it has been made // independent of the compiler's working directory one way or another. @@ -999,7 +999,7 @@ pub fn compile_unit_metadata( let producer = format!("clang LLVM ({})", rustc_producer); let name_in_debuginfo = name_in_debuginfo.to_string_lossy(); - let work_dir = tcx.sess.working_dir.to_string_lossy(false); + let work_dir = tcx.sess.opts.working_dir.to_string_lossy(false); let flags = "\0"; let output_filenames = tcx.output_filenames(()); let out_dir = &output_filenames.out_directory; diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index b7921c403bb7b..d9bbb03129975 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -503,7 +503,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { // Prepend path of working directory onto potentially // relative paths, because they could become relative // to a wrong directory. - let working_dir = &self.tcx.sess.working_dir; + // We include `working_dir` as part of the crate hash, + // so it's okay for us to use it as part of the encoded + // metadata. + let working_dir = &self.tcx.sess.opts.working_dir; match working_dir { RealFileName::LocalPath(absolute) => { // If working_dir has not been remapped, then we emit a diff --git a/compiler/rustc_save_analysis/src/dump_visitor.rs b/compiler/rustc_save_analysis/src/dump_visitor.rs index aea48bf3da8df..29068761d6d84 100644 --- a/compiler/rustc_save_analysis/src/dump_visitor.rs +++ b/compiler/rustc_save_analysis/src/dump_visitor.rs @@ -185,7 +185,7 @@ impl<'tcx> DumpVisitor<'tcx> { }; let data = CompilationOptions { - directory: self.tcx.sess.working_dir.remapped_path_if_available().into(), + directory: self.tcx.sess.opts.working_dir.remapped_path_if_available().into(), program, arguments, output: self.save_ctxt.compilation_output(crate_name), diff --git a/compiler/rustc_save_analysis/src/span_utils.rs b/compiler/rustc_save_analysis/src/span_utils.rs index 1947b04f441c3..8d6758f40f965 100644 --- a/compiler/rustc_save_analysis/src/span_utils.rs +++ b/compiler/rustc_save_analysis/src/span_utils.rs @@ -27,6 +27,7 @@ impl<'a> SpanUtils<'a> { .to_string() } else { self.sess + .opts .working_dir .remapped_path_if_available() .join(&path) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 96d896cb9e1da..123f47b430a17 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -21,6 +21,7 @@ use rustc_feature::UnstableFeatures; use rustc_span::edition::{Edition, DEFAULT_EDITION, EDITION_NAME_LIST, LATEST_STABLE_EDITION}; use rustc_span::source_map::{FileName, FilePathMapping}; use rustc_span::symbol::{sym, Symbol}; +use rustc_span::RealFileName; use rustc_span::SourceFileHashAlgorithm; use rustc_errors::emitter::HumanReadableErrorType; @@ -707,6 +708,7 @@ impl Default for Options { json_artifact_notifications: false, json_unused_externs: false, pretty: None, + working_dir: RealFileName::LocalPath(std::env::current_dir().unwrap()), } } } @@ -2132,6 +2134,18 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { if candidate.join("library/std/src/lib.rs").is_file() { Some(candidate) } else { None } }; + let working_dir = std::env::current_dir().unwrap_or_else(|e| { + early_error(error_format, &format!("Current directory is invalid: {}", e)); + }); + + let (path, remapped) = + FilePathMapping::new(remap_path_prefix.clone()).map_prefix(working_dir.clone()); + let working_dir = if remapped { + RealFileName::Remapped { local_path: Some(working_dir), virtual_name: path } + } else { + RealFileName::LocalPath(path) + }; + Options { crate_types, optimize: opt_level, @@ -2167,6 +2181,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { json_artifact_notifications, json_unused_externs, pretty, + working_dir, } } @@ -2413,6 +2428,7 @@ crate mod dep_tracking { use crate::utils::{NativeLib, NativeLibKind}; use rustc_feature::UnstableFeatures; use rustc_span::edition::Edition; + use rustc_span::RealFileName; use rustc_target::spec::{CodeModel, MergeFunctions, PanicStrategy, RelocModel}; use rustc_target::spec::{RelroLevel, SanitizerSet, SplitDebuginfo, TargetTriple, TlsModel}; use std::collections::hash_map::DefaultHasher; @@ -2494,6 +2510,7 @@ crate mod dep_tracking { TrimmedDefPaths, Option, OutputType, + RealFileName, ); impl DepTrackingHash for (T1, T2) diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 95a7b0994b8ad..481520122d25d 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -10,6 +10,7 @@ use rustc_target::spec::{RelocModel, RelroLevel, SplitDebuginfo, TargetTriple, T use rustc_feature::UnstableFeatures; use rustc_span::edition::Edition; +use rustc_span::RealFileName; use rustc_span::SourceFileHashAlgorithm; use std::collections::BTreeMap; @@ -203,6 +204,9 @@ top_level_options!( json_unused_externs: bool [UNTRACKED], pretty: Option [UNTRACKED], + + /// The (potentially remapped) working directory + working_dir: RealFileName [TRACKED], } ); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 0b1a99922d6d7..5b163603d5ffb 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -23,8 +23,8 @@ use rustc_errors::registry::Registry; use rustc_errors::{DiagnosticBuilder, DiagnosticId, ErrorReported}; use rustc_macros::HashStable_Generic; pub use rustc_span::def_id::StableCrateId; +use rustc_span::edition::Edition; use rustc_span::source_map::{FileLoader, MultiSpan, RealFileLoader, SourceMap, Span}; -use rustc_span::{edition::Edition, RealFileName}; use rustc_span::{sym, SourceFileHashAlgorithm, Symbol}; use rustc_target::asm::InlineAsmArch; use rustc_target::spec::{CodeModel, PanicStrategy, RelocModel, RelroLevel}; @@ -139,8 +139,6 @@ pub struct Session { /// The name of the root source file of the crate, in the local file system. /// `None` means that there is no source file. pub local_crate_source_file: Option, - /// The directory the compiler has been executed in - pub working_dir: RealFileName, /// Set of `(DiagnosticId, Option, message)` tuples tracking /// (sub)diagnostics that have been set once, but should not be set again, @@ -1304,16 +1302,6 @@ pub fn build_session( let print_fuel_crate = sopts.debugging_opts.print_fuel.clone(); let print_fuel = AtomicU64::new(0); - let working_dir = env::current_dir().unwrap_or_else(|e| { - parse_sess.span_diagnostic.fatal(&format!("Current directory is invalid: {}", e)).raise() - }); - let (path, remapped) = file_path_mapping.map_prefix(working_dir.clone()); - let working_dir = if remapped { - RealFileName::Remapped { local_path: Some(working_dir), virtual_name: path } - } else { - RealFileName::LocalPath(path) - }; - let cgu_reuse_tracker = if sopts.debugging_opts.query_dep_graph { CguReuseTracker::new() } else { @@ -1344,7 +1332,6 @@ pub fn build_session( parse_sess, sysroot, local_crate_source_file, - working_dir, one_time_diagnostics: Default::default(), crate_types: OnceCell::new(), stable_crate_id: OnceCell::new(), diff --git a/src/test/run-make/issue-85019-moved-src-dir/Makefile b/src/test/run-make/issue-85019-moved-src-dir/Makefile new file mode 100644 index 0000000000000..451e2b403c67c --- /dev/null +++ b/src/test/run-make/issue-85019-moved-src-dir/Makefile @@ -0,0 +1,25 @@ +include ../../run-make-fulldeps/tools.mk + +INCR=$(TMPDIR)/incr +FIRST_SRC=$(TMPDIR)/first_src +SECOND_SRC=$(TMPDIR)/second_src + +# Tests that we don't get an ICE when the working directory +# (but not the build directory!) changes between compilation +# sessions + +all: + mkdir $(INCR) + # Build from 'FIRST_SRC' + mkdir $(FIRST_SRC) + cp my_lib.rs $(FIRST_SRC)/my_lib.rs + cp main.rs $(FIRST_SRC)/main.rs + cd $(FIRST_SRC) && \ + $(RUSTC) -C incremental=$(INCR) --crate-type lib my_lib.rs && \ + $(RUSTC) -C incremental=$(INCR) --extern my_lib=$(TMPDIR)/libmy_lib.rlib main.rs + # Build from 'SECOND_SRC', keeping the output directory and incremental directory + # the same + mv $(FIRST_SRC) $(SECOND_SRC) + cd $(SECOND_SRC) && \ + $(RUSTC) -C incremental=$(INCR) --crate-type lib my_lib.rs && \ + $(RUSTC) -C incremental=$(INCR) --extern my_lib=$(TMPDIR)/libmy_lib.rlib main.rs diff --git a/src/test/run-make/issue-85019-moved-src-dir/main.rs b/src/test/run-make/issue-85019-moved-src-dir/main.rs new file mode 100644 index 0000000000000..543559a5c539f --- /dev/null +++ b/src/test/run-make/issue-85019-moved-src-dir/main.rs @@ -0,0 +1,5 @@ +extern crate my_lib; + +fn main() { + my_lib::my_fn("hi"); +} diff --git a/src/test/run-make/issue-85019-moved-src-dir/my_lib.rs b/src/test/run-make/issue-85019-moved-src-dir/my_lib.rs new file mode 100644 index 0000000000000..432875739affb --- /dev/null +++ b/src/test/run-make/issue-85019-moved-src-dir/my_lib.rs @@ -0,0 +1 @@ +pub fn my_fn(_val: T) {}