Skip to content

Commit

Permalink
Rollup merge of rust-lang#119510 - saethlin:fatal-io-errors, r=Waffle…
Browse files Browse the repository at this point in the history
…Lapkin,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`
  • Loading branch information
fmease authored Jan 3, 2024
2 parents b2673fa + 94c43cc commit 462505d
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 14 deletions.
14 changes: 3 additions & 11 deletions compiler/rustc_incremental/src/persist/file_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 }),
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 462505d

Please sign in to comment.