Skip to content

Commit

Permalink
Auto merge of rust-lang#67458 - pnkfelix:fix-66530-by-propagating-fat…
Browse files Browse the repository at this point in the history
…al-error-from-worker, r=matthewjasper

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

Fix rust-lang#66530
  • Loading branch information
bors committed Jan 11, 2020
2 parents 1389494 + 3193836 commit f363745
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 8 deletions.
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 @@ -902,7 +902,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 @@ -1474,9 +1474,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 @@ -1520,29 +1523,36 @@ 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>) {
thread::spawn(move || {
// Set up a destructor which will fire off a message that we're done as
// 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 @@ -1562,7 +1572,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

0 comments on commit f363745

Please sign in to comment.