From fbaa24ee35dffb044e4895ad68f01c3f06046275 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 27 Oct 2023 21:26:43 -0400 Subject: [PATCH] Call FileEncoder::finish in rmeta encoding --- Cargo.lock | 1 + compiler/rustc_codegen_ssa/src/lib.rs | 2 +- .../src/persist/file_format.rs | 4 +- .../rustc_incremental/src/persist/save.rs | 3 -- compiler/rustc_interface/Cargo.toml | 1 + compiler/rustc_interface/src/queries.rs | 10 ++++- compiler/rustc_metadata/messages.ftl | 5 +-- compiler/rustc_metadata/src/errors.rs | 9 +--- compiler/rustc_metadata/src/rmeta/encoder.rs | 34 +++++++++------ .../rustc_middle/src/query/on_disk_cache.rs | 3 +- compiler/rustc_middle/src/ty/context.rs | 4 ++ .../rustc_query_system/src/dep_graph/graph.rs | 2 +- compiler/rustc_serialize/src/opaque.rs | 43 ++++++++++++++++--- src/librustdoc/scrape_examples.rs | 2 +- 14 files changed, 82 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8c9b12028f0..6dc1ea75d8e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4041,6 +4041,7 @@ dependencies = [ "rustc_query_impl", "rustc_query_system", "rustc_resolve", + "rustc_serialize", "rustc_session", "rustc_span", "rustc_symbol_mangling", diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 8a82a37df9c..5514a32b23a 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -226,7 +226,7 @@ pub fn serialize_rlink( encoder.emit_raw_bytes(&RLINK_VERSION.to_be_bytes()); encoder.emit_str(sess.cfg_version); Encodable::encode(codegen_results, &mut encoder); - encoder.finish() + encoder.finish().map_err(|(_path, err)| err) } pub fn deserialize_rlink(sess: &Session, data: Vec) -> Result { diff --git a/compiler/rustc_incremental/src/persist/file_format.rs b/compiler/rustc_incremental/src/persist/file_format.rs index 25bf83f64a0..b5742b97d02 100644 --- a/compiler/rustc_incremental/src/persist/file_format.rs +++ b/compiler/rustc_incremental/src/persist/file_format.rs @@ -80,8 +80,8 @@ pub(crate) fn save_in(sess: &Session, path_buf: PathBuf, name: &str, encode: ); debug!("save: data written to disk successfully"); } - Err(err) => { - sess.emit_err(errors::WriteNew { name, path: path_buf, err }); + Err((path, err)) => { + sess.emit_err(errors::WriteNew { name, path, err }); } } } diff --git a/compiler/rustc_incremental/src/persist/save.rs b/compiler/rustc_incremental/src/persist/save.rs index fa21320be26..d6320d680e7 100644 --- a/compiler/rustc_incremental/src/persist/save.rs +++ b/compiler/rustc_incremental/src/persist/save.rs @@ -50,9 +50,6 @@ pub fn save_dep_graph(tcx: TyCtxt<'_>) { join( move || { sess.time("incr_comp_persist_dep_graph", || { - if let Err(err) = tcx.dep_graph.encode(&tcx.sess.prof) { - sess.emit_err(errors::WriteDepGraph { path: &staging_dep_graph_path, err }); - } if let Err(err) = fs::rename(&staging_dep_graph_path, &dep_graph_path) { sess.emit_err(errors::MoveDepGraph { from: &staging_dep_graph_path, diff --git a/compiler/rustc_interface/Cargo.toml b/compiler/rustc_interface/Cargo.toml index fd587e53f91..319e8175809 100644 --- a/compiler/rustc_interface/Cargo.toml +++ b/compiler/rustc_interface/Cargo.toml @@ -40,6 +40,7 @@ rustc_privacy = { path = "../rustc_privacy" } rustc_query_impl = { path = "../rustc_query_impl" } rustc_query_system = { path = "../rustc_query_system" } rustc_resolve = { path = "../rustc_resolve" } +rustc_serialize = { path = "../rustc_serialize" } rustc_session = { path = "../rustc_session" } rustc_span = { path = "../rustc_span" } rustc_symbol_mangling = { path = "../rustc_symbol_mangling" } diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index 1c65cf19cde..bee27dc2d69 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -1,6 +1,6 @@ use crate::errors::{FailedWritingFile, RustcErrorFatal, RustcErrorUnexpectedAnnotation}; use crate::interface::{Compiler, Result}; -use crate::{passes, util}; +use crate::{errors, passes, util}; use rustc_ast as ast; use rustc_codegen_ssa::traits::CodegenBackend; @@ -15,6 +15,7 @@ use rustc_middle::arena::Arena; use rustc_middle::dep_graph::DepGraph; use rustc_middle::ty::{GlobalCtxt, TyCtxt}; +use rustc_serialize::opaque::FileEncodeResult; use rustc_session::config::{self, CrateType, OutputFilenames, OutputType}; use rustc_session::cstore::Untracked; use rustc_session::output::find_crate_name; @@ -102,6 +103,10 @@ pub fn new(compiler: &'tcx Compiler) -> Queries<'tcx> { } } + pub fn finish(&self) -> FileEncodeResult { + if let Some(gcx) = self.gcx_cell.get() { gcx.finish() } else { Ok(0) } + } + pub fn parse(&self) -> Result> { self.parse.compute(|| { passes::parse(&self.compiler.sess).map_err(|mut parse_error| parse_error.emit()) @@ -317,6 +322,9 @@ pub fn enter(&self, f: F) -> T // The timer's lifetime spans the dropping of `queries`, which contains // the global context. _timer = Some(self.sess.timer("free_global_ctxt")); + if let Err((path, error)) = queries.finish() { + self.sess.emit_err(errors::FailedWritingFile { path: &path, error }); + } ret } diff --git a/compiler/rustc_metadata/messages.ftl b/compiler/rustc_metadata/messages.ftl index d1815717e22..44b235a6d3d 100644 --- a/compiler/rustc_metadata/messages.ftl +++ b/compiler/rustc_metadata/messages.ftl @@ -63,11 +63,8 @@ metadata_extern_location_not_file = metadata_fail_create_file_encoder = failed to create file encoder: {$err} -metadata_fail_seek_file = - failed to seek the file: {$err} - metadata_fail_write_file = - failed to write to the file: {$err} + failed to write to `{$path}`: {$err} metadata_failed_copy_to_stdout = failed to copy {$filename} to stdout: {$err} diff --git a/compiler/rustc_metadata/src/errors.rs b/compiler/rustc_metadata/src/errors.rs index 70daee291e7..edc8d8532d3 100644 --- a/compiler/rustc_metadata/src/errors.rs +++ b/compiler/rustc_metadata/src/errors.rs @@ -307,15 +307,10 @@ pub struct FailCreateFileEncoder { pub err: Error, } -#[derive(Diagnostic)] -#[diag(metadata_fail_seek_file)] -pub struct FailSeekFile { - pub err: Error, -} - #[derive(Diagnostic)] #[diag(metadata_fail_write_file)] -pub struct FailWriteFile { +pub struct FailWriteFile<'a> { + pub path: &'a Path, pub err: Error, } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 19add1ef1ac..44e53d07f00 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1,4 +1,4 @@ -use crate::errors::{FailCreateFileEncoder, FailSeekFile, FailWriteFile}; +use crate::errors::{FailCreateFileEncoder, FailWriteFile}; use crate::rmeta::def_path_hash_map::DefPathHashMapRef; use crate::rmeta::table::TableBuilder; use crate::rmeta::*; @@ -42,6 +42,7 @@ use rustc_span::{self, ExternalSource, FileName, SourceFile, Span, SpanData, SyntaxContext}; use std::borrow::Borrow; use std::collections::hash_map::Entry; +use std::fs::File; use std::hash::Hash; use std::io::{Read, Seek, Write}; use std::num::NonZeroUsize; @@ -2250,25 +2251,34 @@ fn encode_metadata_impl(tcx: TyCtxt<'_>, path: &Path) { // culminating in the `CrateRoot` which points to all of it. let root = ecx.encode_crate_root(); - ecx.opaque.flush(); + // Make sure we report any errors from writing to the file. + // If we forget this, compilation can succeed with an incomplete rmeta file, + // causing an ICE when the rmeta file is read by another compilation. + if let Err((path, err)) = ecx.opaque.finish() { + tcx.sess.emit_err(FailWriteFile { path: &path, err }); + } - let mut file = ecx.opaque.file(); + let file = ecx.opaque.file(); + if let Err(err) = encode_root_position(file, root.position.get()) { + tcx.sess.emit_err(FailWriteFile { path: ecx.opaque.path(), err }); + } + + // Record metadata size for self-profiling + tcx.prof.artifact_size("crate_metadata", "crate_metadata", file.metadata().unwrap().len()); +} + +fn encode_root_position(mut file: &File, pos: usize) -> Result<(), std::io::Error> { // We will return to this position after writing the root position. let pos_before_seek = file.stream_position().unwrap(); // Encode the root position. let header = METADATA_HEADER.len(); - file.seek(std::io::SeekFrom::Start(header as u64)) - .unwrap_or_else(|err| tcx.sess.emit_fatal(FailSeekFile { err })); - let pos = root.position.get(); - file.write_all(&[(pos >> 24) as u8, (pos >> 16) as u8, (pos >> 8) as u8, (pos >> 0) as u8]) - .unwrap_or_else(|err| tcx.sess.emit_fatal(FailWriteFile { err })); + file.seek(std::io::SeekFrom::Start(header as u64))?; + file.write_all(&[(pos >> 24) as u8, (pos >> 16) as u8, (pos >> 8) as u8, (pos >> 0) as u8])?; // Return to the position where we are before writing the root position. - file.seek(std::io::SeekFrom::Start(pos_before_seek)).unwrap(); - - // Record metadata size for self-profiling - tcx.prof.artifact_size("crate_metadata", "crate_metadata", file.metadata().unwrap().len()); + file.seek(std::io::SeekFrom::Start(pos_before_seek))?; + Ok(()) } pub fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_middle/src/query/on_disk_cache.rs b/compiler/rustc_middle/src/query/on_disk_cache.rs index ee6567f6cc4..f37cfe8b0a1 100644 --- a/compiler/rustc_middle/src/query/on_disk_cache.rs +++ b/compiler/rustc_middle/src/query/on_disk_cache.rs @@ -25,7 +25,6 @@ use rustc_span::{BytePos, ExpnData, ExpnHash, Pos, RelativeBytePos, SourceFile, Span}; use rustc_span::{CachingSourceMapView, Symbol}; use std::collections::hash_map::Entry; -use std::io; use std::mem; const TAG_FILE_FOOTER: u128 = 0xC0FFEE_C0FFEE_C0FFEE_C0FFEE_C0FFEE; @@ -862,7 +861,7 @@ pub fn encode_tagged, V: Encodable>(&mut self, tag: T, } #[inline] - fn finish(self) -> Result { + fn finish(mut self) -> FileEncodeResult { self.encoder.finish() } } diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 62b0536dabe..82080205c8d 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -597,6 +597,10 @@ pub fn enter<'a: 'tcx, F, R>(&'a self, f: F) -> R let icx = tls::ImplicitCtxt::new(self); tls::enter_context(&icx, || f(icx.tcx)) } + + pub fn finish(&self) -> FileEncodeResult { + self.dep_graph.finish_encoding(&self.sess.prof) + } } impl<'tcx> TyCtxt<'tcx> { diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index d720744a7a7..5acd012ef04 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -982,7 +982,7 @@ pub fn print_incremental_info(&self) { } } - pub fn encode(&self, profiler: &SelfProfilerRef) -> FileEncodeResult { + pub fn finish_encoding(&self, profiler: &SelfProfilerRef) -> FileEncodeResult { if let Some(data) = &self.data { data.current.encoder.steal().finish(profiler) } else { diff --git a/compiler/rustc_serialize/src/opaque.rs b/compiler/rustc_serialize/src/opaque.rs index 55255439051..cc8d1c25092 100644 --- a/compiler/rustc_serialize/src/opaque.rs +++ b/compiler/rustc_serialize/src/opaque.rs @@ -5,12 +5,13 @@ use std::marker::PhantomData; use std::ops::Range; use std::path::Path; +use std::path::PathBuf; // ----------------------------------------------------------------------------- // Encoder // ----------------------------------------------------------------------------- -pub type FileEncodeResult = Result; +pub type FileEncodeResult = Result; /// The size of the buffer in `FileEncoder`. const BUF_SIZE: usize = 8192; @@ -34,6 +35,9 @@ pub struct FileEncoder { // This is used to implement delayed error handling, as described in the // comment on `trait Encoder`. res: Result<(), io::Error>, + path: PathBuf, + #[cfg(debug_assertions)] + finished: bool, } impl FileEncoder { @@ -41,14 +45,18 @@ pub fn new>(path: P) -> io::Result { // File::create opens the file for writing only. When -Zmeta-stats is enabled, the metadata // encoder rewinds the file to inspect what was written. So we need to always open the file // for reading and writing. - let file = File::options().read(true).write(true).create(true).truncate(true).open(path)?; + let file = + File::options().read(true).write(true).create(true).truncate(true).open(&path)?; Ok(FileEncoder { buf: vec![0u8; BUF_SIZE].into_boxed_slice().try_into().unwrap(), + path: path.as_ref().into(), buffered: 0, flushed: 0, file, res: Ok(()), + #[cfg(debug_assertions)] + finished: false, }) } @@ -63,6 +71,10 @@ pub fn position(&self) -> usize { #[cold] #[inline(never)] pub fn flush(&mut self) { + #[cfg(debug_assertions)] + { + self.finished = false; + } if self.res.is_ok() { self.res = self.file.write_all(&self.buf[..self.buffered]); } @@ -74,6 +86,10 @@ pub fn file(&self) -> &File { &self.file } + pub fn path(&self) -> &Path { + &self.path + } + #[inline] fn buffer_empty(&mut self) -> &mut [u8] { // SAFETY: self.buffered is inbounds as an invariant of the type @@ -97,6 +113,10 @@ fn write_all_cold_path(&mut self, buf: &[u8]) { #[inline] fn write_all(&mut self, buf: &[u8]) { + #[cfg(debug_assertions)] + { + self.finished = false; + } if let Some(dest) = self.buffer_empty().get_mut(..buf.len()) { dest.copy_from_slice(buf); self.buffered += buf.len(); @@ -121,6 +141,10 @@ fn write_all(&mut self, buf: &[u8]) { /// with one instruction, so while this does in some sense do wasted work, we come out ahead. #[inline] pub fn write_with(&mut self, visitor: impl FnOnce(&mut [u8; N]) -> usize) { + #[cfg(debug_assertions)] + { + self.finished = false; + } let flush_threshold = const { BUF_SIZE.checked_sub(N).unwrap() }; if std::intrinsics::unlikely(self.buffered > flush_threshold) { self.flush(); @@ -152,20 +176,25 @@ fn panic_invalid_write(written: usize) { }) } - pub fn finish(mut self) -> Result { + pub fn finish(&mut self) -> FileEncodeResult { self.flush(); + #[cfg(debug_assertions)] + { + self.finished = true; + } match std::mem::replace(&mut self.res, Ok(())) { Ok(()) => Ok(self.position()), - Err(e) => Err(e), + Err(e) => Err((self.path.clone(), e)), } } } +#[cfg(debug_assertions)] impl Drop for FileEncoder { fn drop(&mut self) { - // Likely to be a no-op, because `finish` should have been called and - // it also flushes. But do it just in case. - self.flush(); + if !std::thread::panicking() { + assert!(self.finished); + } } } diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index dd52deef672..14680fdb064 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -325,7 +325,7 @@ pub(crate) fn run( // Save output to provided path let mut encoder = FileEncoder::new(options.output_path).map_err(|e| e.to_string())?; calls.encode(&mut encoder); - encoder.finish().map_err(|e| e.to_string())?; + encoder.finish().map_err(|(_path, e)| e.to_string())?; Ok(()) };