From 0435c1b0a5fbc0cbaef8cb9f1711d3e30c944781 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 20 Dec 2019 20:34:24 -0500 Subject: [PATCH 1/3] When a codegen worker has a FatalError, propagate it instead of ICE'ing. --- src/librustc_codegen_ssa/back/write.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index a5a90980c3a23..9f577ba83d2b7 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -903,7 +903,7 @@ pub enum Message { worker_id: usize, }, Done { - result: Result, + result: Result>, worker_id: usize, }, CodegenDone { @@ -1476,9 +1476,12 @@ fn start_executing_work( 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"), } } @@ -1527,6 +1530,10 @@ fn start_executing_work( pub const CODEGEN_WORKER_ID: usize = ::std::usize::MAX; +/// `FatalError` is explicitly not `Send`. +#[must_use] +pub struct WorkerFatalError; + fn spawn_work(cgcx: CodegenContext, work: WorkItem) { let depth = time_depth(); @@ -1537,23 +1544,26 @@ fn spawn_work(cgcx: CodegenContext, work: WorkItem // we exit. struct Bomb { coordinator_send: Sender>, - result: Option>, + result: Option, FatalError>>, worker_id: usize, } impl Drop for Bomb { 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:: { result: Ok(m), worker_id } } - Some(WorkItemResult::NeedsFatLTO(m)) => { + Some(Ok(WorkItemResult::NeedsFatLTO(m))) => { Message::NeedsFatLTO:: { result: m, worker_id } } - Some(WorkItemResult::NeedsThinLTO(name, thin_buffer)) => { + Some(Ok(WorkItemResult::NeedsThinLTO(name, thin_buffer))) => { Message::NeedsThinLTO:: { name, thin_buffer, worker_id } } - None => Message::Done:: { result: Err(()), worker_id }, + Some(Err(FatalError)) => { + Message::Done:: { result: Err(Some(WorkerFatalError)), worker_id } + } + None => Message::Done:: { result: Err(None), worker_id }, }; drop(self.coordinator_send.send(Box::new(msg))); } @@ -1573,7 +1583,7 @@ fn spawn_work(cgcx: CodegenContext, work: WorkItem // 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)) }; }); } From 7c5cff717ffb13c8d0b9eb4d287bd2c565a9d7f4 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 20 Dec 2019 20:55:12 -0500 Subject: [PATCH 2/3] regression test for 66530. it uses normalize-stderr-test because not all targets hit the same OS error number nor message ... ... and ignores tidy since I dont know how to make the normalize line shorter ... and has effectively a no-op for its error-pattern because the targets' error messages are so wildly different (and the error-pattern check occurs *before* stderr normalization.) --- .../ui/non-ice-error-on-worker-io-fail.rs | 36 +++++++++++++++++++ .../ui/non-ice-error-on-worker-io-fail.stderr | 6 ++++ 2 files changed, 42 insertions(+) create mode 100644 src/test/ui/non-ice-error-on-worker-io-fail.rs create mode 100644 src/test/ui/non-ice-error-on-worker-io-fail.stderr diff --git a/src/test/ui/non-ice-error-on-worker-io-fail.rs b/src/test/ui/non-ice-error-on-worker-io-fail.rs new file mode 100644 index 0000000000000..deffb2969b60e --- /dev/null +++ b/src/test/ui/non-ice-error-on-worker-io-fail.rs @@ -0,0 +1,36 @@ +// 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 + +#![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; +} diff --git a/src/test/ui/non-ice-error-on-worker-io-fail.stderr b/src/test/ui/non-ice-error-on-worker-io-fail.stderr new file mode 100644 index 0000000000000..f5601ad03d5d8 --- /dev/null +++ b/src/test/ui/non-ice-error-on-worker-io-fail.stderr @@ -0,0 +1,6 @@ +warning: ignoring --out-dir flag due to -o flag + +error: io error modifying /dev/ + +error: aborting due to previous error + From 31938366d2da5434344011f20fc2564e03228cd1 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 9 Jan 2020 05:20:06 +0100 Subject: [PATCH 3/3] Keep plugging holes in the test to placate CI. If this happens again, I'm going to change tack and make this an `// only-linux` test or something along those lines. --- src/test/ui/non-ice-error-on-worker-io-fail.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/non-ice-error-on-worker-io-fail.rs b/src/test/ui/non-ice-error-on-worker-io-fail.rs index deffb2969b60e..8af17742850da 100644 --- a/src/test/ui/non-ice-error-on-worker-io-fail.rs +++ b/src/test/ui/non-ice-error-on-worker-io-fail.rs @@ -24,6 +24,7 @@ // 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"]