Add a footer in FileEncoder and check for it in MemDecoder

Author: saethlin

compiler/rustc_codegen_ssa/src/lib.rs

@@ -195,6 +195,7 @@ pub enum CodegenErrors {
     EmptyVersionNumber,
     EncodingVersionMismatch { version_array: String, rlink_version: u32 },
     RustcVersionMismatch { rustc_version: String },
+    CorruptFile,
 }
 
 pub fn provide(providers: &mut Providers) {
@@ -265,7 +266,9 @@ impl CodegenResults {
             });
         }
 
-        let mut decoder = MemDecoder::new(&data[4..], 0);
+        let Some(mut decoder) = MemDecoder::new(&data[4..], 0) else {
+            return Err(CodegenErrors::CorruptFile);
+        };
         let rustc_version = decoder.read_str();
         if rustc_version != sess.cfg_version {
             return Err(CodegenErrors::RustcVersionMismatch {

compiler/rustc_driver_impl/messages.ftl

@@ -10,6 +10,8 @@ driver_impl_ice_path_error = the ICE couldn't be written to `{$path}`: {$error}
 driver_impl_ice_path_error_env = the environment variable `RUSTC_ICE` is set to `{$env_var}`
 driver_impl_ice_version = rustc {$version} running on {$triple}
 
+driver_impl_rlink_corrupt_file = corrupt metadata encountered in `{$file}`
+
 driver_impl_rlink_empty_version_number = The input does not contain version number
 
 driver_impl_rlink_encoding_version_mismatch = .rlink file was produced with encoding version `{$version_array}`, but the current version is `{$rlink_version}`

compiler/rustc_driver_impl/src/lib.rs

@@ -95,7 +95,7 @@ mod signal_handler {
 
 use crate::session_diagnostics::{
     RLinkEmptyVersionNumber, RLinkEncodingVersionMismatch, RLinkRustcVersionMismatch,
-    RLinkWrongFileType, RlinkNotAFile, RlinkUnableToRead,
+    RLinkWrongFileType, RlinkCorruptFile, RlinkNotAFile, RlinkUnableToRead,
 };
 
 rustc_fluent_macro::fluent_messages! { "../messages.ftl" }
@@ -644,15 +644,17 @@ fn process_rlink(sess: &Session, compiler: &interface::Compiler) {
                 match err {
                     CodegenErrors::WrongFileType => dcx.emit_fatal(RLinkWrongFileType),
                     CodegenErrors::EmptyVersionNumber => dcx.emit_fatal(RLinkEmptyVersionNumber),
-                    CodegenErrors::EncodingVersionMismatch { version_array, rlink_version } => sess
-                        .dcx()
+                    CodegenErrors::EncodingVersionMismatch { version_array, rlink_version } => dcx
                         .emit_fatal(RLinkEncodingVersionMismatch { version_array, rlink_version }),
                     CodegenErrors::RustcVersionMismatch { rustc_version } => {
                         dcx.emit_fatal(RLinkRustcVersionMismatch {
                             rustc_version,
                             current_version: sess.cfg_version,
                         })
                     }
+                    CodegenErrors::CorruptFile => {
+                        dcx.emit_fatal(RlinkCorruptFile { file: file.display().to_string() });
+                    }
                 };
             }
         };

compiler/rustc_driver_impl/src/session_diagnostics.rs

@@ -32,6 +32,12 @@ pub(crate) struct RLinkRustcVersionMismatch<'a> {
 #[diag(driver_impl_rlink_no_a_file)]
 pub(crate) struct RlinkNotAFile;
 
+#[derive(Diagnostic)]
+#[diag(driver_impl_rlink_corrupt_file)]
+pub(crate) struct RlinkCorruptFile {
+    pub file: String,
+}
+
 #[derive(Diagnostic)]
 #[diag(driver_impl_ice)]
 pub(crate) struct Ice;

compiler/rustc_incremental/messages.ftl

@@ -21,6 +21,8 @@ incremental_cargo_help_2 =
 incremental_copy_workproduct_to_cache =
     error copying object file `{$from}` to incremental directory as `{$to}`: {$err}
 
+incremental_corrupt_file = corrupt incremental compilation artifact found at `{$path}`. This file will automatically be ignored and deleted. If you see this message repeatedly or can provoke it without manually manipulating the compiler's artifacts, please file an issue. The incremental compilation system relies on hardlinks and filesystem locks behaving correctly, and may not deal well with OS crashes, so whatever information you can provide about your filesystem or other state may be very relevant.
+
 incremental_create_dep_graph = failed to create dependency graph at `{$path}`: {$err}
 
 incremental_create_incr_comp_dir =

compiler/rustc_incremental/src/errors.rs

@@ -306,3 +306,9 @@ pub struct DeleteWorkProduct<'a> {
     pub path: &'a Path,
     pub err: std::io::Error,
 }
+
+#[derive(Diagnostic)]
+#[diag(incremental_corrupt_file)]
+pub struct CorruptFile<'a> {
+    pub path: &'a Path,
+}

compiler/rustc_incremental/src/persist/load.rs

@@ -115,7 +115,12 @@ fn load_dep_graph(sess: &Session) -> LoadResult<(Arc<SerializedDepGraph>, WorkPr
 
         if let LoadResult::Ok { data: (work_products_data, start_pos) } = load_result {
             // Decode the list of work_products
-            let mut work_product_decoder = MemDecoder::new(&work_products_data[..], start_pos);
+            let Some(mut work_product_decoder) =
+                MemDecoder::new(&work_products_data[..], start_pos)
+            else {
+                sess.dcx().emit_warn(errors::CorruptFile { path: &work_products_path });
+                return LoadResult::DataOutOfDate;
+            };
             let work_products: Vec<SerializedWorkProduct> =
                 Decodable::decode(&mut work_product_decoder);
 
@@ -145,7 +150,10 @@ fn load_dep_graph(sess: &Session) -> LoadResult<(Arc<SerializedDepGraph>, WorkPr
         LoadResult::DataOutOfDate => LoadResult::DataOutOfDate,
         LoadResult::LoadDepGraph(path, err) => LoadResult::LoadDepGraph(path, err),
         LoadResult::Ok { data: (bytes, start_pos) } => {
-            let mut decoder = MemDecoder::new(&bytes, start_pos);
+            let Some(mut decoder) = MemDecoder::new(&bytes, start_pos) else {
+                sess.dcx().emit_warn(errors::CorruptFile { path: &path });
+                return LoadResult::DataOutOfDate;
+            };
             let prev_commandline_args_hash = u64::decode(&mut decoder);
 
             if prev_commandline_args_hash != expected_hash {
@@ -181,9 +189,14 @@ pub fn load_query_result_cache(sess: &Session) -> Option<OnDiskCache<'_>> {
 
     let _prof_timer = sess.prof.generic_activity("incr_comp_load_query_result_cache");
 
-    match load_data(&query_cache_path(sess), sess) {
+    let path = query_cache_path(sess);
+    match load_data(&path, sess) {
         LoadResult::Ok { data: (bytes, start_pos) } => {
-            Some(OnDiskCache::new(sess, bytes, start_pos))
+            let cache = OnDiskCache::new(sess, bytes, start_pos).unwrap_or_else(|| {
+                sess.dcx().emit_warn(errors::CorruptFile { path: &path });
+                OnDiskCache::new_empty(sess.source_map())
+            });
+            Some(cache)
         }
         _ => Some(OnDiskCache::new_empty(sess.source_map())),
     }

compiler/rustc_metadata/src/locator.rs

@@ -853,7 +853,12 @@ fn get_metadata_section<'p>(
             slice_owned(mmap, Deref::deref)
         }
     };
-    let blob = MetadataBlob(raw_bytes);
+    let Some(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!(

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -39,10 +39,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];
@@ -53,6 +52,16 @@ impl std::ops::Deref for MetadataBlob {
     }
 }
 
+impl MetadataBlob {
+    pub fn new(slice: OwnedSlice) -> Option<Self> {
+        if MemDecoder::new(&*slice, 0).is_some() { Some(Self(slice)) } else { None }
+    }
+
+    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
@@ -164,7 +173,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)),
@@ -392,7 +408,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);

compiler/rustc_metadata/src/rmeta/def_path_hash_map.rs

@@ -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