Auto merge of #124686 - saethlin:rust-file-footer, r=fmease
Add a footer in FileEncoder and check for it in MemDecoder We have a few reports of ICEs due to decoding failures, where the fault does not lie with the compiler. The goal of this PR is to add some very lightweight and on-by-default validation to the compiler's outputs. If validation fails, we emit a fatal error for rmeta files in general that mentions the path that didn't load, and for incremental compilation artifacts we emit a verbose warning that tries to explain the situation and treat the artifacts as outdated. The validation currently implemented here is very crude, and yet I think we have 11 ICE reports currently open (you can find them by searching issues for `1002111927320821928687967599834759150`) which this simple validation would have detected. The structure of the code changes here should permit the addition of further validation code, such as a checksum, if it is merited. I would like to have code to detect corruption such as reported in https://github.com/rust-lang/rust/issues/124719, but I'm not yet sure how to do that efficiently, and this PR is already a good size. The ICE reports I have in mind that this PR would have smoothed over are: https://github.com/rust-lang/rust/issues/124469 https://github.com/rust-lang/rust/issues/123352 https://github.com/rust-lang/rust/issues/123376 [^1] https://github.com/rust-lang/rust/issues/99763 https://github.com/rust-lang/rust/issues/93900. --- [^1]: This one might be a compiler bug, but even if it is I think the workflow described is pushing the envelope of what we can support. This issue is one of the reasons this warning still asks people to file an issue.
This commit is contained in:
commit
22f5bdc42b
16 changed files with 122 additions and 52 deletions
|
@ -853,7 +853,12 @@ fn get_metadata_section<'p>(
|
|||
slice_owned(mmap, Deref::deref)
|
||||
}
|
||||
};
|
||||
let blob = MetadataBlob(raw_bytes);
|
||||
let Ok(blob) = MetadataBlob::new(raw_bytes) else {
|
||||
return Err(MetadataError::LoadFailure(format!(
|
||||
"corrupt metadata encountered in {}",
|
||||
filename.display()
|
||||
)));
|
||||
};
|
||||
match blob.check_compatibility(cfg_version) {
|
||||
Ok(()) => Ok(blob),
|
||||
Err(None) => Err(MetadataError::LoadFailure(format!(
|
||||
|
|
|
@ -40,10 +40,9 @@ use rustc_span::hygiene::HygieneDecodeContext;
|
|||
mod cstore_impl;
|
||||
|
||||
/// A reference to the raw binary version of crate metadata.
|
||||
/// A `MetadataBlob` internally is just a reference counted pointer to
|
||||
/// the actual data, so cloning it is cheap.
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct MetadataBlob(pub(crate) OwnedSlice);
|
||||
/// This struct applies [`MemDecoder`]'s validation when constructed
|
||||
/// so that later constructions are guaranteed to succeed.
|
||||
pub(crate) struct MetadataBlob(OwnedSlice);
|
||||
|
||||
impl std::ops::Deref for MetadataBlob {
|
||||
type Target = [u8];
|
||||
|
@ -54,6 +53,19 @@ impl std::ops::Deref for MetadataBlob {
|
|||
}
|
||||
}
|
||||
|
||||
impl MetadataBlob {
|
||||
/// Runs the [`MemDecoder`] validation and if it passes, constructs a new [`MetadataBlob`].
|
||||
pub fn new(slice: OwnedSlice) -> Result<Self, ()> {
|
||||
if MemDecoder::new(&slice, 0).is_ok() { Ok(Self(slice)) } else { Err(()) }
|
||||
}
|
||||
|
||||
/// Since this has passed the validation of [`MetadataBlob::new`], this returns bytes which are
|
||||
/// known to pass the [`MemDecoder`] validation.
|
||||
pub fn bytes(&self) -> &OwnedSlice {
|
||||
&self.0
|
||||
}
|
||||
}
|
||||
|
||||
/// A map from external crate numbers (as decoded from some crate file) to
|
||||
/// local crate numbers (as generated during this session). Each external
|
||||
/// crate may refer to types in other external crates, and each has their
|
||||
|
@ -165,7 +177,14 @@ pub(super) trait Metadata<'a, 'tcx>: Copy {
|
|||
fn decoder(self, pos: usize) -> DecodeContext<'a, 'tcx> {
|
||||
let tcx = self.tcx();
|
||||
DecodeContext {
|
||||
opaque: MemDecoder::new(self.blob(), pos),
|
||||
// FIXME: This unwrap should never panic because we check that it won't when creating
|
||||
// `MetadataBlob`. Ideally we'd just have a `MetadataDecoder` and hand out subslices of
|
||||
// it as we do elsewhere in the compiler using `MetadataDecoder::split_at`. But we own
|
||||
// the data for the decoder so holding onto the `MemDecoder` too would make us a
|
||||
// self-referential struct which is downright goofy because `MetadataBlob` is already
|
||||
// self-referential. Probably `MemDecoder` should contain an `OwnedSlice`, but that
|
||||
// demands a significant refactoring due to our crate graph.
|
||||
opaque: MemDecoder::new(self.blob(), pos).unwrap(),
|
||||
cdata: self.cdata(),
|
||||
blob: self.blob(),
|
||||
sess: self.sess().or(tcx.map(|tcx| tcx.sess)),
|
||||
|
@ -393,7 +412,7 @@ impl<'a, 'tcx> TyDecoder for DecodeContext<'a, 'tcx> {
|
|||
where
|
||||
F: FnOnce(&mut Self) -> R,
|
||||
{
|
||||
let new_opaque = MemDecoder::new(self.opaque.data(), pos);
|
||||
let new_opaque = self.opaque.split_at(pos);
|
||||
let old_opaque = mem::replace(&mut self.opaque, new_opaque);
|
||||
let old_state = mem::replace(&mut self.lazy_state, LazyState::NoNode);
|
||||
let r = f(self);
|
||||
|
|
|
@ -48,7 +48,7 @@ impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for DefPathHashMapRef<'static>
|
|||
fn decode(d: &mut DecodeContext<'a, 'tcx>) -> DefPathHashMapRef<'static> {
|
||||
let len = d.read_usize();
|
||||
let pos = d.position();
|
||||
let o = d.blob().clone().0.slice(|blob| &blob[pos..pos + len]);
|
||||
let o = d.blob().bytes().clone().slice(|blob| &blob[pos..pos + len]);
|
||||
|
||||
// Although we already have the data we need via the `OwnedSlice`, we still need
|
||||
// to advance the `DecodeContext`'s position so it's in a valid state after
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue