From c3a606237d9c01e4622b267031596bf632567e6f Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 21 May 2024 20:12:07 -0400 Subject: [PATCH] PR feedback --- compiler/rustc_codegen_ssa/src/lib.rs | 2 +- compiler/rustc_driver_impl/src/lib.rs | 2 +- compiler/rustc_driver_impl/src/session_diagnostics.rs | 4 ++-- compiler/rustc_incremental/src/persist/load.rs | 6 +++--- compiler/rustc_metadata/src/locator.rs | 2 +- compiler/rustc_metadata/src/rmeta/decoder.rs | 7 +++++-- compiler/rustc_middle/src/query/on_disk_cache.rs | 7 +++++-- compiler/rustc_serialize/src/opaque.rs | 10 +++++----- compiler/rustc_serialize/tests/leb128.rs | 5 +++-- src/librustdoc/scrape_examples.rs | 2 +- 10 files changed, 27 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 9bf8fde55fc..66a7a2e090a 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -266,7 +266,7 @@ pub fn deserialize_rlink( }); } - let Some(mut decoder) = MemDecoder::new(&data[4..], 0) else { + let Ok(mut decoder) = MemDecoder::new(&data[4..], 0) else { return Err(CodegenErrors::CorruptFile); }; let rustc_version = decoder.read_str(); diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 30cc41ecea0..5532eff7be6 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -654,7 +654,7 @@ fn process_rlink(sess: &Session, compiler: &interface::Compiler) { }) } CodegenErrors::CorruptFile => { - dcx.emit_fatal(RlinkCorruptFile { file: file.display().to_string() }); + dcx.emit_fatal(RlinkCorruptFile { file }); } }; } diff --git a/compiler/rustc_driver_impl/src/session_diagnostics.rs b/compiler/rustc_driver_impl/src/session_diagnostics.rs index 864b122233a..449878f28c4 100644 --- a/compiler/rustc_driver_impl/src/session_diagnostics.rs +++ b/compiler/rustc_driver_impl/src/session_diagnostics.rs @@ -34,8 +34,8 @@ pub(crate) struct RLinkRustcVersionMismatch<'a> { #[derive(Diagnostic)] #[diag(driver_impl_rlink_corrupt_file)] -pub(crate) struct RlinkCorruptFile { - pub file: String, +pub(crate) struct RlinkCorruptFile<'a> { + pub file: &'a std::path::Path, } #[derive(Diagnostic)] diff --git a/compiler/rustc_incremental/src/persist/load.rs b/compiler/rustc_incremental/src/persist/load.rs index c96d22ce45d..9e6ce066785 100644 --- a/compiler/rustc_incremental/src/persist/load.rs +++ b/compiler/rustc_incremental/src/persist/load.rs @@ -115,7 +115,7 @@ fn load_dep_graph(sess: &Session) -> LoadResult<(Arc, WorkPr if let LoadResult::Ok { data: (work_products_data, start_pos) } = load_result { // Decode the list of work_products - let Some(mut work_product_decoder) = + let Ok(mut work_product_decoder) = MemDecoder::new(&work_products_data[..], start_pos) else { sess.dcx().emit_warn(errors::CorruptFile { path: &work_products_path }); @@ -150,7 +150,7 @@ fn load_dep_graph(sess: &Session) -> LoadResult<(Arc, WorkPr LoadResult::DataOutOfDate => LoadResult::DataOutOfDate, LoadResult::LoadDepGraph(path, err) => LoadResult::LoadDepGraph(path, err), LoadResult::Ok { data: (bytes, start_pos) } => { - let Some(mut decoder) = MemDecoder::new(&bytes, start_pos) else { + let Ok(mut decoder) = MemDecoder::new(&bytes, start_pos) else { sess.dcx().emit_warn(errors::CorruptFile { path: &path }); return LoadResult::DataOutOfDate; }; @@ -192,7 +192,7 @@ pub fn load_query_result_cache(sess: &Session) -> Option> { let path = query_cache_path(sess); match load_data(&path, sess) { LoadResult::Ok { data: (bytes, start_pos) } => { - let cache = OnDiskCache::new(sess, bytes, start_pos).unwrap_or_else(|| { + 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()) }); diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs index fa33eccd2a1..6ff19974c1e 100644 --- a/compiler/rustc_metadata/src/locator.rs +++ b/compiler/rustc_metadata/src/locator.rs @@ -853,7 +853,7 @@ fn get_metadata_section<'p>( slice_owned(mmap, Deref::deref) } }; - let Some(blob) = MetadataBlob::new(raw_bytes) else { + let Ok(blob) = MetadataBlob::new(raw_bytes) else { return Err(MetadataError::LoadFailure(format!( "corrupt metadata encountered in {}", filename.display() diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 1058f58da13..f91e121a240 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -54,10 +54,13 @@ fn deref(&self) -> &[u8] { } impl MetadataBlob { - pub fn new(slice: OwnedSlice) -> Option { - if MemDecoder::new(&*slice, 0).is_some() { Some(Self(slice)) } else { None } + /// Runs the [`MemDecoder`] validation and if it passes, constructs a new [`MetadataBlob`]. + pub fn new(slice: OwnedSlice) -> Result { + 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 } diff --git a/compiler/rustc_middle/src/query/on_disk_cache.rs b/compiler/rustc_middle/src/query/on_disk_cache.rs index 6815e4263d5..941911c2230 100644 --- a/compiler/rustc_middle/src/query/on_disk_cache.rs +++ b/compiler/rustc_middle/src/query/on_disk_cache.rs @@ -154,7 +154,10 @@ fn new(tcx: TyCtxt<'_>, file: &SourceFile) -> EncodedSourceFileId { impl<'sess> OnDiskCache<'sess> { /// Creates a new `OnDiskCache` instance from the serialized data in `data`. - pub fn new(sess: &'sess Session, data: Mmap, start_pos: usize) -> Option { + /// + /// The serialized cache has some basic integrity checks, if those checks indicate that the + /// on-disk data is corrupt, an error is returned. + pub fn new(sess: &'sess Session, data: Mmap, start_pos: usize) -> Result { assert!(sess.opts.incremental.is_some()); let mut decoder = MemDecoder::new(&data, start_pos)?; @@ -169,7 +172,7 @@ pub fn new(sess: &'sess Session, data: Mmap, start_pos: usize) -> Option { let footer: Footer = decoder.with_position(footer_pos, |decoder| decode_tagged(decoder, TAG_FILE_FOOTER)); - Some(Self { + Ok(Self { serialized_data: RwLock::new(Some(data)), file_index_to_stable_id: footer.file_index_to_stable_id, file_index_to_file: Default::default(), diff --git a/compiler/rustc_serialize/src/opaque.rs b/compiler/rustc_serialize/src/opaque.rs index 1b4b1d2436f..1dcb69920d7 100644 --- a/compiler/rustc_serialize/src/opaque.rs +++ b/compiler/rustc_serialize/src/opaque.rs @@ -17,7 +17,7 @@ pub type FileEncodeResult = Result; -const FOOTER: &[u8] = b"rust-end-file"; +pub const MAGIC_END_BYTES: &[u8] = b"rust-end-file"; /// The size of the buffer in `FileEncoder`. const BUF_SIZE: usize = 8192; @@ -183,7 +183,7 @@ fn panic_invalid_write(written: usize) { } pub fn finish(&mut self) -> FileEncodeResult { - self.write_all(FOOTER); + self.write_all(MAGIC_END_BYTES); self.flush(); #[cfg(debug_assertions)] { @@ -264,10 +264,10 @@ pub struct MemDecoder<'a> { impl<'a> MemDecoder<'a> { #[inline] - pub fn new(data: &'a [u8], position: usize) -> Option> { - let data = data.strip_suffix(FOOTER)?; + pub fn new(data: &'a [u8], position: usize) -> Result, ()> { + let data = data.strip_suffix(MAGIC_END_BYTES).ok_or(())?; let Range { start, end } = data.as_ptr_range(); - Some(MemDecoder { start, current: data[position..].as_ptr(), end, _marker: PhantomData }) + Ok(MemDecoder { start, current: data[position..].as_ptr(), end, _marker: PhantomData }) } #[inline] diff --git a/compiler/rustc_serialize/tests/leb128.rs b/compiler/rustc_serialize/tests/leb128.rs index 2bf4f38d3c3..fafe4b91a95 100644 --- a/compiler/rustc_serialize/tests/leb128.rs +++ b/compiler/rustc_serialize/tests/leb128.rs @@ -1,5 +1,6 @@ use rustc_serialize::leb128::*; use rustc_serialize::opaque::MemDecoder; +use rustc_serialize::opaque::MAGIC_END_BYTES; use rustc_serialize::Decoder; macro_rules! impl_test_unsigned_leb128 { @@ -27,7 +28,7 @@ fn $test_name() { stream.extend(&buf[..n]); } let stream_end = stream.len(); - stream.extend(b"rust-end-file"); + stream.extend(MAGIC_END_BYTES); let mut decoder = MemDecoder::new(&stream, 0).unwrap(); for &expected in &values { @@ -76,7 +77,7 @@ fn $test_name() { stream.extend(&buf[..n]); } let stream_end = stream.len(); - stream.extend(b"rust-end-file"); + stream.extend(MAGIC_END_BYTES); let mut decoder = MemDecoder::new(&stream, 0).unwrap(); for &expected in &values { diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index 0bf98886965..e9b380fdeac 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -344,7 +344,7 @@ pub(crate) fn load_call_locations( Ok(bytes) => bytes, Err(e) => dcx.fatal(format!("failed to load examples: {e}")), }; - let Some(mut decoder) = MemDecoder::new(&bytes, 0) else { + let Ok(mut decoder) = MemDecoder::new(&bytes, 0) else { dcx.fatal(format!("Corrupt metadata encountered in {path}")) }; let calls = AllCallLocations::decode(&mut decoder);