Unrolled build for rust-lang#119510

rust-timer · web-flow · commit b12ed75ed24a · 2024-01-03T15:36:41.000-05:00
Rollup merge of rust-lang#119510 - saethlin:fatal-io-errors, r=WaffleLapkin,Nilstrieb Report I/O errors from rmeta encoding with emit_fatal rust-lang#119456 reminded me that I never did systematic testing to provoke the out-of-disk ICEs so I grepped through a recent crater run (rust-lang#119440 (comment)) for more out-of-disk ICEs on current master and yep there's 2 in there. So I finally cooked up a way to provoke for these crashes. I wrote a little `cdylib` crate that has a `#[no_mangle] pub extern "C" fn write` which occasionally reports `ENOSPC`, and prints a backtrace when it does. <details><summary><strong>code for the dylib</strong></summary> <p> ```rust // cargo add libc rand backtrace use rand::Rng; #[no_mangle] pub extern "C" fn write( fd: libc::c_int, buf: *const libc::c_void, count: libc::size_t, ) -> libc::ssize_t { if fd > 2 && rand::thread_rng().gen::<u8>() == 0 { let mut count = 0; backtrace::trace(|frame| { backtrace::resolve_frame(frame, |symbol| { if let Some(name) = symbol.name() { if count > 3 { eprintln!("{}", name); } } count += 1; }); true }); unsafe { *libc::__errno_location() = libc::ENOSPC; } return -1; } else { unsafe { let res = libc::syscall(libc::SYS_write, fd as usize, buf as usize, count as usize) as isize; if res < 0 { *libc::__errno_location() = -res as i32; -1 } else { res } } } } ``` </p> </details> Then `LD_PRELOAD` that dylib and repeatedly build a big project until it ICEs, such as with this: ```bash while true; do cargo clean LD_PRELOAD=/home/ben/evil/target/release/libevil.so cargo +stage1 check 2> errors if grep "thread 'rustc' panicked" errors; then break fi done ``` My "big project" for testing was an otherwise-empty project with `cargo add axum`. Before this PR, the above procedure finds a crash in between 1 and 15 minutes. With this PR, I have not found a crash in 30 minutes, and I'll be leaving this to run overnight (starting now). (A night has now passed, no crashes were found) I believe the problem is that even though since rust-lang#117301 we correctly check `FileEncoder` for errors on all paths, we use `emit_err`, so there is a window of time between the call to `emit_err` and the full error reporting where rustc believes it has emitted a valid rmeta file and will permit Cargo to launch a build for a dependent crate. Changing these calls to `emit_fatal` closes that window. I think there are a number of other cases where `emit_err` has been used instead of the more-correct `emit_fatal` such as https://github.com/rust-lang/rust/blob/e51e98dde6a60637b6a71b8105245b629ac3fe77/compiler/rustc_codegen_ssa/src/back/write.rs#L542 but unlike rmeta encoding I am not aware of those cases of those causing problems. r? ``@WaffleLapkin``
diff --git a/compiler/rustc_incremental/src/persist/file_format.rs b/compiler/rustc_incremental/src/persist/file_format.rs
@@ -55,18 +55,12 @@ where
             debug!("save: remove old file");
         }
         Err(err) if err.kind() == io::ErrorKind::NotFound => (),
-        Err(err) => {
-            sess.dcx().emit_err(errors::DeleteOld { name, path: path_buf, err });
-            return;
-        }
+        Err(err) => sess.dcx().emit_fatal(errors::DeleteOld { name, path: path_buf, err }),
     }
 
     let mut encoder = match FileEncoder::new(&path_buf) {
         Ok(encoder) => encoder,
-        Err(err) => {
-            sess.dcx().emit_err(errors::CreateNew { name, path: path_buf, err });
-            return;
-        }
+        Err(err) => sess.dcx().emit_fatal(errors::CreateNew { name, path: path_buf, err }),
     };
 
     write_file_header(&mut encoder, sess);
@@ -80,9 +74,7 @@ where
             );
             debug!("save: data written to disk successfully");
         }
-        Err((path, err)) => {
-            sess.dcx().emit_err(errors::WriteNew { name, path, err });
-        }
+        Err((path, err)) => sess.dcx().emit_fatal(errors::WriteNew { name, path, err }),
     }
 }
 
diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs
@@ -332,7 +332,7 @@ impl Compiler {
         // the global context.
         _timer = Some(self.sess.timer("free_global_ctxt"));
         if let Err((path, error)) = queries.finish() {
-            self.sess.dcx().emit_err(errors::FailedWritingFile { path: &path, error });
+            self.sess.dcx().emit_fatal(errors::FailedWritingFile { path: &path, error });
         }
 
         ret
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -2255,12 +2255,12 @@ pub fn encode_metadata(tcx: TyCtxt<'_>, path: &Path) {
     // 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.dcx().emit_err(FailWriteFile { path: &path, err });
+        tcx.dcx().emit_fatal(FailWriteFile { path: &path, err });
     }
 
     let file = ecx.opaque.file();
     if let Err(err) = encode_root_position(file, root.position.get()) {
-        tcx.dcx().emit_err(FailWriteFile { path: ecx.opaque.path(), err });
+        tcx.dcx().emit_fatal(FailWriteFile { path: ecx.opaque.path(), err });
     }
 
     // Record metadata size for self-profiling

Original file line number	Diff line number	Diff line change
`@@ -55,18 +55,12 @@ where`
`55`	`55`	`debug!("save: remove old file");`
`56`	`56`	`}`
`57`	`57`	`Err(err) if err.kind() == io::ErrorKind::NotFound => (),`
`58`		`- Err(err) => {`
`59`		`- sess.dcx().emit_err(errors::DeleteOld { name, path: path_buf, err });`
`60`		`- return;`
`61`		`- }`
	`58`	`+ Err(err) => sess.dcx().emit_fatal(errors::DeleteOld { name, path: path_buf, err }),`
`62`	`59`	`}`
`63`	`60`
`64`	`61`	`let mut encoder = match FileEncoder::new(&path_buf) {`
`65`	`62`	`Ok(encoder) => encoder,`
`66`		`- Err(err) => {`
`67`		`- sess.dcx().emit_err(errors::CreateNew { name, path: path_buf, err });`
`68`		`- return;`
`69`		`- }`
	`63`	`+ Err(err) => sess.dcx().emit_fatal(errors::CreateNew { name, path: path_buf, err }),`
`70`	`64`	`};`
`71`	`65`
`72`	`66`	`write_file_header(&mut encoder, sess);`
`@@ -80,9 +74,7 @@ where`
`80`	`74`	`);`
`81`	`75`	`debug!("save: data written to disk successfully");`
`82`	`76`	`}`
`83`		`- Err((path, err)) => {`
`84`		`- sess.dcx().emit_err(errors::WriteNew { name, path, err });`
`85`		`- }`
	`77`	`+ Err((path, err)) => sess.dcx().emit_fatal(errors::WriteNew { name, path, err }),`
`86`	`78`	`}`
`87`	`79`	`}`
`88`	`80`
Original file line number	Diff line number	Diff line change
`@@ -332,7 +332,7 @@ impl Compiler {`
`332`	`332`	`// the global context.`
`333`	`333`	`_timer = Some(self.sess.timer("free_global_ctxt"));`
`334`	`334`	`if let Err((path, error)) = queries.finish() {`
`335`		`- self.sess.dcx().emit_err(errors::FailedWritingFile { path: &path, error });`
	`335`	`+ self.sess.dcx().emit_fatal(errors::FailedWritingFile { path: &path, error });`
`336`	`336`	`}`
`337`	`337`
`338`	`338`	`ret`
Original file line number	Diff line number	Diff line change
`@@ -2255,12 +2255,12 @@ pub fn encode_metadata(tcx: TyCtxt<'_>, path: &Path) {`
`2255`	`2255`	`// If we forget this, compilation can succeed with an incomplete rmeta file,`
`2256`	`2256`	`// causing an ICE when the rmeta file is read by another compilation.`
`2257`	`2257`	`if let Err((path, err)) = ecx.opaque.finish() {`
`2258`		`- tcx.dcx().emit_err(FailWriteFile { path: &path, err });`
	`2258`	`+ tcx.dcx().emit_fatal(FailWriteFile { path: &path, err });`
`2259`	`2259`	`}`
`2260`	`2260`
`2261`	`2261`	`let file = ecx.opaque.file();`
`2262`	`2262`	`if let Err(err) = encode_root_position(file, root.position.get()) {`
`2263`		`- tcx.dcx().emit_err(FailWriteFile { path: ecx.opaque.path(), err });`
	`2263`	`+ tcx.dcx().emit_fatal(FailWriteFile { path: ecx.opaque.path(), err });`
`2264`	`2264`	`}`
`2265`	`2265`
`2266`	`2266`	`// Record metadata size for self-profiling`