diff options
| author | León Orell Valerian Liehr <me@fmease.dev> | 2024-01-03 16:08:31 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-01-03 16:08:31 +0100 |
| commit | a34754e7d54fad76ae4879fa055a358aedc4b921 (patch) | |
| tree | 17a6a876d3ab90439a0806cb33c19f3ef0755159 | |
| parent | 8bce6fc35eeb9c0e4260766f2c734a89c1523f5d (diff) | |
| parent | 94c43ccd876f87a556aacd8f87228b41632573b4 (diff) | |
| download | rust-a34754e7d54fad76ae4879fa055a358aedc4b921.tar.gz rust-a34754e7d54fad76ae4879fa055a358aedc4b921.zip | |
Rollup merge of #119510 - saethlin:fatal-io-errors, r=WaffleLapkin,Nilstrieb
Report I/O errors from rmeta encoding with emit_fatal
https://github.com/rust-lang/rust/issues/119456 reminded me that I never did systematic testing to provoke the out-of-disk ICEs so I grepped through a recent crater run (https://github.com/rust-lang/rust/pull/119440#issuecomment-1873393963) 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 https://github.com/rust-lang/rust/pull/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``
| -rw-r--r-- | compiler/rustc_incremental/src/persist/file_format.rs | 14 | ||||
| -rw-r--r-- | compiler/rustc_interface/src/queries.rs | 2 | ||||
| -rw-r--r-- | compiler/rustc_metadata/src/rmeta/encoder.rs | 4 |
3 files changed, 6 insertions, 14 deletions
diff --git a/compiler/rustc_incremental/src/persist/file_format.rs b/compiler/rustc_incremental/src/persist/file_format.rs index e68195acee0..b459f82f23e 100644 --- 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 index 1ea3db26e21..07bbe78dc2d 100644 --- 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 index aca7a66596e..5b296c098bc 100644 --- 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 |
