Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When a codegen worker has a FatalError, propagate it instead of ICE'ing. #67458

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/librustc_codegen_ssa/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ pub enum Message<B: WriteBackendMethods> {
worker_id: usize,
},
Done {
result: Result<CompiledModule, ()>,
result: Result<CompiledModule, Option<WorkerFatalError>>,
worker_id: usize,
},
CodegenDone {
Expand Down Expand Up @@ -1476,9 +1476,12 @@ fn start_executing_work<B: ExtraBackendMethods>(
main_thread_worker_state = MainThreadWorkerState::Idle;
}
// If the thread failed that means it panicked, so we abort immediately.
Message::Done { result: Err(()), worker_id: _ } => {
Message::Done { result: Err(None), worker_id: _ } => {
bug!("worker thread panicked");
}
Message::Done { result: Err(Some(WorkerFatalError)), worker_id: _ } => {
return Err(());
}
Message::CodegenItem => bug!("the coordinator should not receive codegen requests"),
}
}
Expand Down Expand Up @@ -1527,6 +1530,10 @@ fn start_executing_work<B: ExtraBackendMethods>(

pub const CODEGEN_WORKER_ID: usize = ::std::usize::MAX;

/// `FatalError` is explicitly not `Send`.
#[must_use]
pub struct WorkerFatalError;

fn spawn_work<B: ExtraBackendMethods>(cgcx: CodegenContext<B>, work: WorkItem<B>) {
let depth = time_depth();

Expand All @@ -1537,23 +1544,26 @@ fn spawn_work<B: ExtraBackendMethods>(cgcx: CodegenContext<B>, work: WorkItem<B>
// we exit.
struct Bomb<B: ExtraBackendMethods> {
coordinator_send: Sender<Box<dyn Any + Send>>,
result: Option<WorkItemResult<B>>,
result: Option<Result<WorkItemResult<B>, FatalError>>,
worker_id: usize,
}
impl<B: ExtraBackendMethods> Drop for Bomb<B> {
fn drop(&mut self) {
let worker_id = self.worker_id;
let msg = match self.result.take() {
Some(WorkItemResult::Compiled(m)) => {
Some(Ok(WorkItemResult::Compiled(m))) => {
Message::Done::<B> { result: Ok(m), worker_id }
}
Some(WorkItemResult::NeedsFatLTO(m)) => {
Some(Ok(WorkItemResult::NeedsFatLTO(m))) => {
Message::NeedsFatLTO::<B> { result: m, worker_id }
}
Some(WorkItemResult::NeedsThinLTO(name, thin_buffer)) => {
Some(Ok(WorkItemResult::NeedsThinLTO(name, thin_buffer))) => {
Message::NeedsThinLTO::<B> { name, thin_buffer, worker_id }
}
None => Message::Done::<B> { result: Err(()), worker_id },
Some(Err(FatalError)) => {
Message::Done::<B> { result: Err(Some(WorkerFatalError)), worker_id }
}
None => Message::Done::<B> { result: Err(None), worker_id },
};
drop(self.coordinator_send.send(Box::new(msg)));
}
Expand All @@ -1573,7 +1583,7 @@ fn spawn_work<B: ExtraBackendMethods>(cgcx: CodegenContext<B>, work: WorkItem<B>
// surface that there was an error in this worker.
bomb.result = {
let _prof_timer = cgcx.prof.generic_activity(work.profiling_event_id());
execute_work_item(&cgcx, work).ok()
Some(execute_work_item(&cgcx, work))
};
});
}
Expand Down
37 changes: 37 additions & 0 deletions src/test/ui/non-ice-error-on-worker-io-fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Issue #66530: We would ICE if someone compiled with `-o /dev/null`,
// because we would try to generate auxiliary files in `/dev/` (which
// at least the OS X file system rejects).
//
// An attempt to `-o` into a directory we cannot write into should indeed
// be an error; but not an ICE.

// compile-flags: -o /dev/null

// The error-pattern check occurs *before* normalization, and the error patterns
// are wildly different between build environments. So this is a cop-out (and we
// rely on the checking of the normalized stderr output as our actual
// "verification" of the diagnostic).

// error-pattern: error

// On Mac OS X, we get an error like the below
// normalize-stderr-test "failed to write bytecode to /dev/null.non_ice_error_on_worker_io_fail.*" -> "io error modifying /dev/"

// On Linux, we get an error like the below
// normalize-stderr-test "couldn't create a temp dir.*" -> "io error modifying /dev/"

// ignore-tidy-linelength
// ignore-windows - this is a unix-specific test
// ignore-emscripten - the file-system issues do not replicate here
// ignore-wasm - the file-system issues do not replicate here
// ignore-arm - the file-system issues do not replicate here, at least on armhf-gnu

#![crate_type="lib"]

#![cfg_attr(not(feature = "std"), no_std)]
pub mod task {
pub mod __internal {
use crate::task::Waker;
}
pub use core::task::Waker;
}
6 changes: 6 additions & 0 deletions src/test/ui/non-ice-error-on-worker-io-fail.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
warning: ignoring --out-dir flag due to -o flag

error: io error modifying /dev/

error: aborting due to previous error