Auto merge of #115108 - ijackson:broken-wait-status, r=dtolnay

Fix exit status / wait status on non-Unix cfg(unix) platforms

Fixes #114593

Needs FCP due to behavioural changes (NB only on non-Unix `#[cfg(unix)]` platforms).

Also, I think this is likely to break in CI.  I have not been yet able to compile the new bits of `process_unsupported.rs`, although I have compiled the new module.  I'd like some help from people familiar with eg emscripten and fuchsia (which are going to be affected, I think).

Author: bors

library/std/src/sys/unix/process/process_common/tests.rs

@@ -159,3 +159,36 @@ fn test_program_kind() {
         );
     }
 }
+
+// Test that Rust std handles wait status values (`ExitStatus`) the way that Unix does,
+// at least for the values which represent a Unix exit status (`ExitCode`).
+// Should work on every #[cfg(unix)] platform.  However:
+#[cfg(not(any(
+    // Fuchsia is not Unix and has totally broken std::os::unix.
+    // https://github.com/rust-lang/rust/issues/58590#issuecomment-836535609
+    target_os = "fuchsia",
+)))]
+#[test]
+fn unix_exit_statuses() {
+    use crate::num::NonZeroI32;
+    use crate::os::unix::process::ExitStatusExt;
+    use crate::process::*;
+
+    for exit_code in 0..=0xff {
+        // FIXME impl From<ExitCode> for ExitStatus and then test that here too;
+        // the two ExitStatus values should be the same
+        let raw_wait_status = exit_code << 8;
+        let exit_status = ExitStatus::from_raw(raw_wait_status);
+
+        assert_eq!(exit_status.code(), Some(exit_code));
+
+        if let Ok(nz) = NonZeroI32::try_from(exit_code) {
+            assert!(!exit_status.success());
+            let es_error = exit_status.exit_ok().unwrap_err();
+            assert_eq!(es_error.code().unwrap(), i32::from(nz));
+        } else {
+            assert!(exit_status.success());
+            assert_eq!(exit_status.exit_ok(), Ok(()));
+        }
+    }
+}

library/std/src/sys/unix/process/process_unix.rs

@@ -1074,3 +1074,8 @@ impl crate::os::linux::process::ChildExt for crate::process::Child {
 #[cfg(test)]
 #[path = "process_unix/tests.rs"]
 mod tests;
+
+// See [`process_unsupported_wait_status::compare_with_linux`];
+#[cfg(all(test, target_os = "linux"))]
+#[path = "process_unsupported/wait_status.rs"]
+mod process_unsupported_wait_status;

library/std/src/sys/unix/process/process_unsupported.rs

@@ -55,56 +55,8 @@ impl Process {
     }
 }
 
-#[derive(PartialEq, Eq, Clone, Copy, Debug, Default)]
-pub struct ExitStatus(c_int);
-
-impl ExitStatus {
-    #[cfg_attr(target_os = "horizon", allow(unused))]
-    pub fn success(&self) -> bool {
-        self.code() == Some(0)
-    }
-
-    pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
-        Err(ExitStatusError(1.try_into().unwrap()))
-    }
-
-    pub fn code(&self) -> Option<i32> {
-        None
-    }
-
-    pub fn signal(&self) -> Option<i32> {
-        None
-    }
-
-    pub fn core_dumped(&self) -> bool {
-        false
-    }
-
-    pub fn stopped_signal(&self) -> Option<i32> {
-        None
-    }
-
-    pub fn continued(&self) -> bool {
-        false
-    }
-
-    pub fn into_raw(&self) -> c_int {
-        0
-    }
-}
-
-/// Converts a raw `c_int` to a type-safe `ExitStatus` by wrapping it without copying.
-impl From<c_int> for ExitStatus {
-    fn from(a: c_int) -> ExitStatus {
-        ExitStatus(a as i32)
-    }
-}
-
-impl fmt::Display for ExitStatus {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "exit code: {}", self.0)
-    }
-}
+mod wait_status;
+pub use wait_status::ExitStatus;
 
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 pub struct ExitStatusError(NonZero_c_int);

library/std/src/sys/unix/process/process_unsupported/wait_status.rs

@@ -0,0 +1,68 @@
+//! Emulated wait status for non-Unix #[cfg(unix) platforms
+//!
+//! Separate module to facilitate testing against a real Unix implementation.
+
+use crate::ffi::c_int;
+use crate::fmt;
+
+/// Emulated wait status for use by `process_unsupported.rs`
+///
+/// Uses the "traditional unix" encoding.  For use on platfors which are `#[cfg(unix)]`
+/// but do not actually support subprocesses at all.
+///
+/// These platforms aren't Unix, but are simply pretending to be for porting convenience.
+/// So, we provide a faithful pretence here.
+#[derive(PartialEq, Eq, Clone, Copy, Debug, Default)]
+pub struct ExitStatus {
+    wait_status: c_int,
+}
+
+/// Converts a raw `c_int` to a type-safe `ExitStatus` by wrapping it
+impl From<c_int> for ExitStatus {
+    fn from(wait_status: c_int) -> ExitStatus {
+        ExitStatus { wait_status }
+    }
+}
+
+impl fmt::Display for ExitStatus {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "emulated wait status: {}", self.wait_status)
+    }
+}
+
+impl ExitStatus {
+    pub fn code(&self) -> Option<i32> {
+        // Linux and FreeBSD both agree that values linux 0x80
+        // count as "WIFEXITED" even though this is quite mad.
+        // Likewise the macros disregard all the high bits, so are happy to declare
+        // out-of-range values to be WIFEXITED, WIFSTOPPED, etc.
+        let w = self.wait_status;
+        if (w & 0x7f) == 0 { Some((w & 0xff00) >> 8) } else { None }
+    }
+
+    pub fn signal(&self) -> Option<i32> {
+        let signal = self.wait_status & 0x007f;
+        if signal > 0 && signal < 0x7f { Some(signal) } else { None }
+    }
+
+    pub fn core_dumped(&self) -> bool {
+        self.signal().is_some() && (self.wait_status & 0x80) != 0
+    }
+
+    pub fn stopped_signal(&self) -> Option<i32> {
+        let w = self.wait_status;
+        if (w & 0xff) == 0x7f { Some((w & 0xff00) >> 8) } else { None }
+    }
+
+    pub fn continued(&self) -> bool {
+        self.wait_status == 0xffff
+    }
+
+    pub fn into_raw(&self) -> c_int {
+        self.wait_status
+    }
+}
+
+#[cfg(test)]
+#[path = "wait_status/tests.rs"] // needed because of strange layout of process_unsupported
+mod tests;

library/std/src/sys/unix/process/process_unsupported/wait_status/tests.rs

@@ -0,0 +1,36 @@
+// Note that tests in this file are run on Linux as well as on platforms using process_unsupported
+
+// Test that our emulation exactly matches Linux
+//
+// This test runs *on Linux* but it tests
+// the implementation used on non-Unix `#[cfg(unix)]` platforms.
+//
+// I.e. we're using Linux as a proxy for "trad unix".
+#[cfg(target_os = "linux")]
+#[test]
+fn compare_with_linux() {
+    use super::ExitStatus as Emulated;
+    use crate::os::unix::process::ExitStatusExt as _;
+    use crate::process::ExitStatus as Real;
+
+    // Check that we handle out-of-range values similarly, too.
+    for wstatus in -0xf_ffff..0xf_ffff {
+        let emulated = Emulated::from(wstatus);
+        let real = Real::from_raw(wstatus);
+
+        macro_rules! compare { { $method:ident } => {
+            assert_eq!(
+                emulated.$method(),
+                real.$method(),
+                "{wstatus:#x}.{}()",
+                stringify!($method),
+            );
+        } }
+        compare!(code);
+        compare!(signal);
+        compare!(core_dumped);
+        compare!(stopped_signal);
+        compare!(continued);
+        compare!(into_raw);
+    }
+}