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

OnDiskCache cannot decode Spans with foreign files #92014

Closed
Aaron1011 opened this issue Dec 16, 2021 · 1 comment · Fixed by #92175
Closed

OnDiskCache cannot decode Spans with foreign files #92014

Aaron1011 opened this issue Dec 16, 2021 · 1 comment · Fixed by #92175
Labels
A-incr-comp Area: Incremental compilation

Comments

@Aaron1011
Copy link
Member

When we encode the incremental cache, we store information about every file that we've currently loaded:

let files = tcx.sess.source_map().files();
let mut file_to_file_index =
FxHashMap::with_capacity_and_hasher(files.len(), Default::default());
let mut file_index_to_stable_id =
FxHashMap::with_capacity_and_hasher(files.len(), Default::default());
for (index, file) in files.iter().enumerate() {
let index = SourceFileIndex(index as u32);
let file_ptr: *const SourceFile = &**file as *const _;
file_to_file_index.insert(file_ptr, index);
let source_file_id = EncodedSourceFileId::new(tcx, &file);
file_index_to_stable_id.insert(index, source_file_id);
}
(file_to_file_index, file_index_to_stable_id)

When we decode a Span, we assume that a file with the correct StableSourceFileId has already been loaded:

let stable_id = file_index_to_stable_id[&index].translate(tcx);
source_map
.source_file_by_stable_id(stable_id)

For files in the current crate, this is true - we load all used files in the current crate during expansion (before the incremental cache is loaded), and any stale StableSourceFileIds will be skipped due to the corresponding HIR depnodes being marked as red.

However, we may also end up ending a Span of a file from a different crate. Most queries only cache results for local DefIds, so this can usually open happen when macros are involved (e.g. a mir::Body containing identifiers expanded from a foreign macro). Since macros executions occur before the incremental cache is set up, we should always have the necessary foreign SourceFiles loaded before we try to decode any foreign Spans from the incremental cache.

As a result, I don't think it's actually possible to hit this in practice. However, trying to cache a query result for a foreign DefId can easily hit this - the foreign files associated with the spans we encode need not be loaded by any of the macros that we invoke. We should either make this a requirement of query caching (cache_on_disk_if { key.is_local() } must always be used when the result can contain foreign Span, or adjust the incremental cache to correctly load foreign files when decoding Spans.

@Aaron1011 Aaron1011 added the A-incr-comp Area: Incremental compilation label Dec 16, 2021
@Aaron1011
Copy link
Member Author

Apparently, it is possible to hit this in practice: #92163

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 31, 2021
…cjgillot

Import `SourceFile`s from crate before decoding foreign `Span`

Fixes rust-lang#92163
Fixes rust-lang#92014

When writing to the incremental cache, we encode all `Span`s
we encounter, regardless of whether or not their `SourceFile`
comes from the local crate, or from a foreign crate.

When we decode a `Span`, we use the `StableSourceFileId` we encoded
to locate the matching `SourceFile` in the current session. If this
id corresponds to a `SourceFile` from another crate, then we need to
have already imported that `SourceFile` into our current session.

This usually happens automatically during resolution / macro expansion,
when we try to resolve definitions from other crates. In certain cases,
however, we may try to load a `Span` from a transitive dependency
without having ever imported the `SourceFile`s from that crate, leading
to an ICE.

This PR fixes the issue by enconding the `SourceFile`'s `CrateNum`
when we encode a `Span`. During decoding, we call `imported_source_files()`
when we encounter a foreign `CrateNum`, which ensure that all
`SourceFile`s from that crate are imported into the current session.
@bors bors closed this as completed in d922092 Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant