Skip to content
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

Fix ICE in incremental compilation when rust-src component is changed #83591

Closed
wants to merge 3 commits into from
Closed
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
3 changes: 2 additions & 1 deletion compiler/rustc_interface/src/queries.rs
Original file line number Diff line number Diff line change
@@ -345,7 +345,8 @@ impl<'tcx> Queries<'tcx> {
pub fn linker(&'tcx self) -> Result<Linker> {
let dep_graph = self.dep_graph()?;
let prepare_outputs = self.prepare_outputs()?;
let crate_hash = self.global_ctxt()?.peek_mut().enter(|tcx| tcx.crate_hash(LOCAL_CRATE));
let crate_hash =
self.global_ctxt()?.peek_mut().enter(|tcx| tcx.crate_hash(LOCAL_CRATE).svh);
let ongoing_codegen = self.ongoing_codegen()?;

let sess = self.session().clone();
158 changes: 83 additions & 75 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
@@ -1606,84 +1606,92 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
/// Proc macro crates don't currently export spans, so this function does not have
/// to work for them.
fn imported_source_files(&self, sess: &Session) -> &'a [ImportedSourceFile] {
// Translate the virtual `/rustc/$hash` prefix back to a real directory
// that should hold actual sources, where possible.
//
// NOTE: if you update this, you might need to also update bootstrap's code for generating
// the `rust-src` component in `Src::run` in `src/bootstrap/dist.rs`.
let virtual_rust_source_base_dir = option_env!("CFG_VIRTUAL_RUST_SOURCE_BASE_DIR")
.map(Path::new)
.filter(|_| {
// Only spend time on further checks if we have what to translate *to*.
sess.real_rust_source_base_dir.is_some()
})
.filter(|virtual_dir| {
// Don't translate away `/rustc/$hash` if we're still remapping to it,
// since that means we're still building `std`/`rustc` that need it,
// and we don't want the real path to leak into codegen/debuginfo.
!sess.opts.remap_path_prefix.iter().any(|(_from, to)| to == virtual_dir)
});
let try_to_translate_virtual_to_real = |name: &mut rustc_span::FileName| {
debug!(
"try_to_translate_virtual_to_real(name={:?}): \
virtual_rust_source_base_dir={:?}, real_rust_source_base_dir={:?}",
name, virtual_rust_source_base_dir, sess.real_rust_source_base_dir,
);
self.cdata.source_map_import_info.get_or_init(|| {
// Translate the virtual `/rustc/$hash` prefix back to a real directory
// that should hold actual sources, where possible.
//
// NOTE: if you update this, you might need to also update bootstrap's code for generating
// the `rust-src` component in `Src::run` in `src/bootstrap/dist.rs`.
let virtual_rust_source_base_dir = sess
.opts
.debugging_opts
.force_virtual_source_base
.as_deref()
.or(option_env!("CFG_VIRTUAL_RUST_SOURCE_BASE_DIR"))
.map(Path::new)
.filter(|_| {
// Only spend time on further checks if we have what to translate *to*.
sess.real_rust_source_base_dir.is_some()
})
.filter(|virtual_dir| {
// Don't translate away `/rustc/$hash` if we're still remapping to it,
// since that means we're still building `std`/`rustc` that need it,
// and we don't want the real path to leak into codegen/debuginfo.
!sess.opts.remap_path_prefix.iter().any(|(_from, to)| to == virtual_dir)
});
let try_to_translate_virtual_to_real =
|name: &mut rustc_span::FileName, name_hash: &mut u128| {
debug!(
"try_to_translate_virtual_to_real(name={:?}): \
virtual_rust_source_base_dir={:?}, real_rust_source_base_dir={:?}",
name, virtual_rust_source_base_dir, sess.real_rust_source_base_dir,
);

if let Some(virtual_dir) = virtual_rust_source_base_dir {
if let Some(real_dir) = &sess.real_rust_source_base_dir {
if let rustc_span::FileName::Real(old_name) = name {
if let rustc_span::RealFileName::Named(one_path) = old_name {
if let Ok(rest) = one_path.strip_prefix(virtual_dir) {
let virtual_name = one_path.clone();

// The std library crates are in
// `$sysroot/lib/rustlib/src/rust/library`, whereas other crates
// may be in `$sysroot/lib/rustlib/src/rust/` directly. So we
// detect crates from the std libs and handle them specially.
const STD_LIBS: &[&str] = &[
"core",
"alloc",
"std",
"test",
"term",
"unwind",
"proc_macro",
"panic_abort",
"panic_unwind",
"profiler_builtins",
"rtstartup",
"rustc-std-workspace-core",
"rustc-std-workspace-alloc",
"rustc-std-workspace-std",
"backtrace",
];
let is_std_lib = STD_LIBS.iter().any(|l| rest.starts_with(l));

let new_path = if is_std_lib {
real_dir.join("library").join(rest)
} else {
real_dir.join(rest)
};

debug!(
"try_to_translate_virtual_to_real: `{}` -> `{}`",
virtual_name.display(),
new_path.display(),
);
let new_name = rustc_span::RealFileName::Devirtualized {
local_path: new_path,
virtual_name,
};
*old_name = new_name;
if let Some(virtual_dir) = virtual_rust_source_base_dir {
if let Some(real_dir) = &sess.real_rust_source_base_dir {
if let rustc_span::FileName::Real(old_name) = name {
if let rustc_span::RealFileName::Named(one_path) = old_name {
if let Ok(rest) = one_path.strip_prefix(virtual_dir) {
let virtual_name = one_path.clone();

// The std library crates are in
// `$sysroot/lib/rustlib/src/rust/library`, whereas other crates
// may be in `$sysroot/lib/rustlib/src/rust/` directly. So we
// detect crates from the std libs and handle them specially.
const STD_LIBS: &[&str] = &[
"core",
"alloc",
"std",
"test",
"term",
"unwind",
"proc_macro",
"panic_abort",
"panic_unwind",
"profiler_builtins",
"rtstartup",
"rustc-std-workspace-core",
"rustc-std-workspace-alloc",
"rustc-std-workspace-std",
"backtrace",
];
let is_std_lib =
STD_LIBS.iter().any(|l| rest.starts_with(l));

let new_path = if is_std_lib {
real_dir.join("library").join(rest)
} else {
real_dir.join(rest)
};

debug!(
"try_to_translate_virtual_to_real: `{}` -> `{}`",
virtual_name.display(),
new_path.display(),
);
let new_name = rustc_span::RealFileName::Devirtualized {
local_path: new_path,
virtual_name,
};
*old_name = new_name;
*name_hash = name.name_hash();
}
}
}
}
}
}
}
};
};

self.cdata.source_map_import_info.get_or_init(|| {
let external_source_map = self.root.source_map.decode(self);

external_source_map
@@ -1700,7 +1708,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
mut multibyte_chars,
mut non_narrow_chars,
mut normalized_pos,
name_hash,
mut name_hash,
..
} = source_file_to_import;

@@ -1709,7 +1717,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
// on `try_to_translate_virtual_to_real`).
// FIXME(eddyb) we could check `name_was_remapped` here,
// but in practice it seems to be always `false`.
try_to_translate_virtual_to_real(&mut name);
try_to_translate_virtual_to_real(&mut name, &mut name_hash);

let source_length = (end_pos - start_pos).to_usize();

6 changes: 3 additions & 3 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ use rustc_hir::def::DefKind;
use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::definitions::{DefKey, DefPath, DefPathHash};
use rustc_middle::hir::exports::Export;
use rustc_middle::middle::cstore::ForeignModule;
use rustc_middle::middle::cstore::{CrateHashAndState, ForeignModule, OptCrateHashAndState};
use rustc_middle::middle::cstore::{CrateSource, CrateStore, EncodedMetadata};
use rustc_middle::middle::exported_symbols::ExportedSymbol;
use rustc_middle::middle::stability::DeprecationEntry;
@@ -199,8 +199,8 @@ provide! { <'tcx> tcx, def_id, other, cdata,
})
}
crate_disambiguator => { cdata.root.disambiguator }
crate_hash => { cdata.root.hash }
crate_host_hash => { cdata.host_hash }
crate_hash => { CrateHashAndState::new(tcx, cdata.root.hash) }
crate_host_hash => { OptCrateHashAndState::new(tcx, cdata.host_hash) }
original_crate_name => { cdata.root.name }

extra_filename => { cdata.root.extra_filename.clone() }
6 changes: 3 additions & 3 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
@@ -645,7 +645,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
name: tcx.crate_name(LOCAL_CRATE),
extra_filename: tcx.sess.opts.cg.extra_filename.clone(),
triple: tcx.sess.opts.target_triple.clone(),
hash: tcx.crate_hash(LOCAL_CRATE),
hash: tcx.crate_hash(LOCAL_CRATE).svh,
disambiguator: tcx.sess.local_crate_disambiguator(),
stable_crate_id: tcx.def_path_hash(LOCAL_CRATE.as_def_id()).stable_crate_id(),
panic_strategy: tcx.sess.panic_strategy(),
@@ -1646,8 +1646,8 @@ impl EncodeContext<'a, 'tcx> {
.map(|&cnum| {
let dep = CrateDep {
name: self.tcx.original_crate_name(cnum),
hash: self.tcx.crate_hash(cnum),
host_hash: self.tcx.crate_host_hash(cnum),
hash: self.tcx.crate_hash(cnum).svh,
host_hash: self.tcx.crate_host_hash(cnum).svh,
kind: self.tcx.dep_kind(cnum),
extra_filename: self.tcx.extra_filename(cnum),
};
12 changes: 6 additions & 6 deletions compiler/rustc_middle/src/hir/map/collector.rs
Original file line number Diff line number Diff line change
@@ -15,7 +15,6 @@ use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::*;
use rustc_index::vec::{Idx, IndexVec};
use rustc_session::{CrateDisambiguator, Session};
use rustc_span::source_map::SourceMap;
use rustc_span::{Span, Symbol, DUMMY_SP};

use std::iter::repeat;
@@ -27,8 +26,7 @@ pub(super) struct NodeCollector<'a, 'hir> {
/// The crate
krate: &'hir Crate<'hir>,

/// Source map
source_map: &'a SourceMap,
sess: &'a Session,

map: IndexVec<LocalDefId, HirOwnerData<'hir>>,

@@ -126,7 +124,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
let mut collector = NodeCollector {
arena,
krate,
source_map: sess.source_map(),
sess,
parent_node: hir::CRATE_HIR_ID,
current_dep_node_owner: LocalDefId { local_def_index: CRATE_DEF_INDEX },
definitions,
@@ -174,7 +172,8 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
// reproducible builds by compiling from the same directory. So we just
// hash the result of the mapping instead of the mapping itself.
let mut source_file_names: Vec<_> = self
.source_map
.sess
.source_map()
.files()
.iter()
.filter(|source_file| source_file.cnum == LOCAL_CRATE)
@@ -186,6 +185,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
let crate_hash_input = (
((node_hashes, upstream_crates), source_file_names),
(commandline_args_hash, crate_disambiguator.to_fingerprint()),
&self.sess.real_rust_source_base_dir,
);

let mut stable_hasher = StableHasher::new();
@@ -262,7 +262,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
span,
"inconsistent DepNode at `{:?}` for `{}`: \
current_dep_node_owner={} ({:?}), hir_id.owner={} ({:?})",
self.source_map.span_to_string(span),
self.sess.source_map().span_to_string(span),
node_str,
self.definitions
.def_path(self.current_dep_node_owner)
34 changes: 34 additions & 0 deletions compiler/rustc_middle/src/middle/cstore.rs
Original file line number Diff line number Diff line change
@@ -6,6 +6,8 @@ use crate::ty::TyCtxt;

use rustc_ast as ast;
use rustc_ast::expand::allocator::AllocatorKind;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::{self, MetadataRef};
use rustc_hir::def::DefKind;
@@ -255,3 +257,35 @@ pub fn used_crates(tcx: TyCtxt<'_>, prefer: LinkagePreference) -> Vec<(CrateNum,
libs.sort_by_cached_key(|&(a, _)| ordering.iter().position(|x| *x == a));
libs
}

/// Holds a crate hash, together with additional global state.
/// This is used as the return value of the `crate_hash` and `crate_host_hash`
/// queries. When the global state changes between compilation sessions,
/// the hash will change, causing the query system to re-run any queries
/// which depend on the crate hash.
#[derive(Clone, Debug, HashStable, Encodable, Decodable)]
pub struct HashAndState<T> {
pub svh: T,
/// The value of `tcx.sess.real_rust_source_base_dir` in the *current* session.
/// This global state is special - it influences how we decode foreign `SourceFile`s
/// (and therefore `Span`s). As a result, a foreign crate's hash can be unchanged across
/// two compilation sessions, but queries run on that crate can nevertheless return
/// different results across those compilation sessions.
/// This field is private, and never needs to be accessed by callers of `crate_hash`
/// or `host_crate_hash`. Its sole purpose is to be hashed as part of the
/// `HashStable` impl, so that its value influences the hash used by incremental compilation.
real_rust_source_base_dir: Fingerprint,
}

impl<T> HashAndState<T> {
pub fn new(tcx: TyCtxt<'tcx>, svh: T) -> HashAndState<T> {
let mut hasher = StableHasher::new();
let mut hcx = tcx.create_stable_hashing_context();
tcx.sess.real_rust_source_base_dir.hash_stable(&mut hcx, &mut hasher);
let real_rust_source_base_dir = hasher.finish();
HashAndState { svh, real_rust_source_base_dir }
}
}

pub type CrateHashAndState = HashAndState<Svh>;
pub type OptCrateHashAndState = HashAndState<Option<Svh>>;
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
@@ -1207,11 +1207,11 @@ rustc_queries! {
}
// The macro which defines `rustc_metadata::provide_extern` depends on this query's name.
// Changing the name should cause a compiler error, but in case that changes, be aware.
query crate_hash(_: CrateNum) -> Svh {
query crate_hash(_: CrateNum) -> rustc_middle::middle::cstore::CrateHashAndState {
eval_always
desc { "looking up the hash a crate" }
}
query crate_host_hash(_: CrateNum) -> Option<Svh> {
query crate_host_hash(_: CrateNum) -> rustc_middle::middle::cstore::OptCrateHashAndState {
eval_always
desc { "looking up the hash of a host version of a crate" }
}
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/ty/query/mod.rs
Original file line number Diff line number Diff line change
@@ -34,7 +34,6 @@ use crate::ty::{self, AdtSizedConstraint, CrateInherentImpls, ParamEnvAnd, Ty, T
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
use rustc_data_structures::stable_hasher::StableVec;
use rustc_data_structures::steal::Steal;
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::Lrc;
use rustc_errors::{ErrorReported, Handler};
use rustc_hir as hir;
2 changes: 2 additions & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
@@ -970,6 +970,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
(default: no)"),
force_overflow_checks: Option<bool> = (None, parse_opt_bool, [TRACKED],
"force overflow checks on or off"),
force_virtual_source_base: Option<String> = (None, parse_opt_string, [UNTRACKED],
"forces using the specified directory as the virtual prefix to remap in filenames"),
force_unstable_if_unmarked: bool = (false, parse_bool, [TRACKED],
"force all crates to be `rustc_private` unstable (default: no)"),
fuel: Option<(String, u64)> = (None, parse_optimization_fuel, [TRACKED],
3 changes: 3 additions & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
@@ -1394,6 +1394,9 @@ pub fn build_session(
// Only use this directory if it has a file we can expect to always find.
if candidate.join("library/std/src/lib.rs").is_file() { Some(candidate) } else { None }
};
if real_rust_source_base_dir.is_some() {
tracing::info!("Got base source dir: {:?}", real_rust_source_base_dir);
}

let asm_arch =
if target_cfg.allow_asm { InlineAsmArch::from_str(&target_cfg.arch).ok() } else { None };
14 changes: 9 additions & 5 deletions compiler/rustc_span/src/lib.rs
Original file line number Diff line number Diff line change
@@ -188,6 +188,14 @@ pub enum FileName {
InlineAsm(u64),
}

impl FileName {
pub fn name_hash(&self) -> u128 {
let mut hasher: StableHasher = StableHasher::new();
self.hash(&mut hasher);
hasher.finish::<u128>()
}
}

impl std::fmt::Display for FileName {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
use FileName::*;
@@ -1318,11 +1326,7 @@ impl SourceFile {
let src_hash = SourceFileHash::new(hash_kind, &src);
let normalized_pos = normalize_src(&mut src, start_pos);

let name_hash = {
let mut hasher: StableHasher = StableHasher::new();
name.hash(&mut hasher);
hasher.finish::<u128>()
};
let name_hash = name.name_hash();
let end_pos = start_pos.to_usize() + src.len();
assert!(end_pos <= u32::MAX as usize);

6 changes: 3 additions & 3 deletions compiler/rustc_ty_utils/src/ty.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::svh::Svh;
use rustc_hir as hir;
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE};
use rustc_middle::hir::map as hir_map;
use rustc_middle::middle::cstore::CrateHashAndState;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{
self, Binder, Predicate, PredicateKind, ToPredicate, Ty, TyCtxt, WithConstness,
@@ -400,8 +400,8 @@ fn original_crate_name(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Symbol {
tcx.crate_name
}

fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> Svh {
tcx.index_hir(crate_num).crate_hash
fn crate_hash(tcx: TyCtxt<'_>, crate_num: CrateNum) -> CrateHashAndState {
CrateHashAndState::new(tcx, tcx.index_hir(crate_num).crate_hash)
}

fn instance_def_size_estimate<'tcx>(
19 changes: 15 additions & 4 deletions src/test/run-make-fulldeps/incr-add-rust-src-component/Makefile
Original file line number Diff line number Diff line change
@@ -25,6 +25,13 @@ INCR=$(TMPDIR)/incr
# prefix, then loop on the next prefix. This way, we basically create a copy of
# the context around root/lib/rustlib/src, and can freely add/remove the src
# component itself.
#
# Normally, path remapping only occured when the compiler was build with
# `remap-debuginfo=true` in the `config.toml`.
# We don't do this on CI, and it would be very wasteful to rebuild the entire
# compiler for a single test. Instead, we use the `-Z force-virtual-source-base` option,
# which makes the compiler act as though it was built with debuginfo remapped to the
# specified directory.
all:
mkdir $(FAKEROOT)
ln -s $(SYSROOT)/* $(FAKEROOT)
@@ -38,7 +45,11 @@ all:
mkdir $(FAKEROOT)/lib/rustlib/src
ln -s $(SYSROOT)/lib/rustlib/src/* $(FAKEROOT)/lib/rustlib/src
rm -f $(FAKEROOT)/lib/rustlib/src/rust
$(RUSTC) --sysroot $(FAKEROOT) -C incremental=$(INCR) main.rs
mkdir -p $(FAKEROOT)/lib/rustlib/src/rust/src/libstd
touch $(FAKEROOT)/lib/rustlib/src/rust/src/libstd/lib.rs
$(RUSTC) --sysroot $(FAKEROOT) -C incremental=$(INCR) main.rs
$(RUSTC) --sysroot $(FAKEROOT) -C incremental=$(INCR) -Z force-virtual-source-base=$(S)/library main.rs
# Run the compiled binary, and check for a non-remapped path in the panic message
$(call RUN,main) 2>&1 | grep $(S)/library/core/src/sync/atomic.rs
mkdir -p $(FAKEROOT)/lib/rustlib/src/rust/library/std/src/
touch $(FAKEROOT)/lib/rustlib/src/rust/library/std/src/lib.rs
$(RUSTC) --sysroot $(FAKEROOT) -C incremental=$(INCR) -Z force-virtual-source-base=$(S)/library main.rs
# Run the newly compiled binary, and check that the path in the panic message is now remapped
$(call RUN,main) 2>&1 | grep $(FAKEROOT)/lib/rustlib/src/rust/library/core/src/sync/atomic.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
use std::sync::atomic::{AtomicUsize, Ordering};

fn main() {
println!("Hello World");
// This generates a panic pointing into libstd,
// so we don't have `#[track_caller]` on `AtomicUsize::load`.
// If `#[track_caller]` ever gets added to this method, the test
// will start failing, and we'll need to find a new method to call
AtomicUsize::new(0).load(Ordering::Release);
}