Call FileEncoder::finish in rmeta encoding

Author: saethlin

Cargo.lock

@@ -4041,6 +4041,7 @@ dependencies = [
  "rustc_query_impl",
  "rustc_query_system",
  "rustc_resolve",
+ "rustc_serialize",
  "rustc_session",
  "rustc_span",
  "rustc_symbol_mangling",

compiler/rustc_codegen_ssa/src/lib.rs

@@ -226,7 +226,7 @@ impl CodegenResults {
         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<u8>) -> Result<Self, CodegenErrors> {

compiler/rustc_incremental/src/persist/file_format.rs

@@ -80,8 +80,8 @@ where
             );
             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 });
         }
     }
 }

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,

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" }

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_metadata::creader::CStore;
 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 @@ impl<'tcx> 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<QueryResult<'_, ast::Crate>> {
         self.parse.compute(|| {
             passes::parse(&self.compiler.sess).map_err(|mut parse_error| parse_error.emit())
@@ -317,6 +322,9 @@ impl Compiler {
         // 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
     }

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}

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,
 }
 

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::symbol::{sym, Symbol};
 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 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());
+}
 
-    let mut file = ecx.opaque.file();
+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) {

compiler/rustc_middle/src/query/on_disk_cache.rs

@@ -25,7 +25,6 @@ use rustc_span::source_map::{SourceMap, StableSourceFileId};
 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 @@ impl<'a, 'tcx> CacheEncoder<'a, 'tcx> {
     }
 
     #[inline]
-    fn finish(self) -> Result<usize, io::Error> {
+    fn finish(mut self) -> FileEncodeResult {
         self.encoder.finish()
     }
 }