Auto merge of #67458 - pnkfelix:fix-66530-by-propagating-fatal-error-from-worker, r=matthewjasper

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

Fix #66530

Author: bors

src/librustc_codegen_ssa/back/write.rs

@@ -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 {
@@ -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"),
             }
         }
@@ -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)));
             }
@@ -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))
         };
     });
 }

src/test/ui/non-ice-error-on-worker-io-fail.rs

@@ -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;
+}

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
+