about summary refs log tree commit diff
diff options
context:
space:
mode:
authorLeón Orell Valerian Liehr <me@fmease.dev>2024-01-03 16:08:31 +0100
committerGitHub <noreply@github.com>2024-01-03 16:08:31 +0100
commita34754e7d54fad76ae4879fa055a358aedc4b921 (patch)
tree17a6a876d3ab90439a0806cb33c19f3ef0755159
parent8bce6fc35eeb9c0e4260766f2c734a89c1523f5d (diff)
parent94c43ccd876f87a556aacd8f87228b41632573b4 (diff)
downloadrust-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.rs14
-rw-r--r--compiler/rustc_interface/src/queries.rs2
-rw-r--r--compiler/rustc_metadata/src/rmeta/encoder.rs4
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