Reconsider guarantees of Pid and refactor (#657)

* Reconsider guarantees of `Pid` and refactor

1. `Pid` and `RawPid` use `NonZeroI32` and `i32` now. `Pid` are
   guaranteed to be in range `1..=i32::MAX`.

Notice that pid range should be enforced for all APIs, and negative
values are exploited by Linux for different functionalities (eg, kill(2)
a process group instead of a process). We should validate that range to
prevent accidental misfunction, no matter for `i32` or `u32`.

So here we consistently use a wrapped `i32` for both linux_raw and libc
backend. Two constructor `Pid::from_raw{,_unchecked}` are provided to
either do or bypass the check. These APIs also simplify code a lot.

2. `Pid::from_raw_nonzero` is removed.

As mentioned in (1), the range guarantee is stricter now and only the
nonzero property cannot help much.

This is a breaking change.

3. `Pid::from_raw` is safe now.

We are not possible to make any guarantee for an arbitrary non-children
pid at all, and an invalid pid causes nothing more than a logic error.

But since we use `NonZeroI32` internally, `Pid::from_raw_unchecked` is
still unsafe.

* Fix typo in doc of `Pid::is_init`

* Clean up unnecessary `unsafe` and `unwrap`

* Remove `unsafe` for `Pid::from_raw` on FreeBSD and Darwin

* Disable debug_assert in `Pid::from_raw_unchecked` due to MSRV

* Re-enable `debug_assert!` in a const fn, now that our MSRV is 1.63.

* Remove `RawNonZeroPid`.

* Make `get_reaper_status`'s `pid` result optional.

The documentation for the field is qualified with "if there are any
descendants". It appears if there are no descendents, the `rs_pid`
field is -1. Check for this case, to avoid trying to convert -1 to
a `Pid`.

---------

Co-authored-by: Dan Gohman <[email protected]>

Author: oxalica

src/backend/libc/pid/syscalls.rs

@@ -1,15 +1,14 @@
 //! libc syscalls for PIDs
 
 use crate::backend::c;
-use crate::pid::{Pid, RawNonZeroPid};
+use crate::pid::Pid;
 
 #[cfg(not(target_os = "wasi"))]
 #[inline]
 #[must_use]
 pub(crate) fn getpid() -> Pid {
     unsafe {
         let pid = c::getpid();
-        debug_assert_ne!(pid, 0);
-        Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid))
+        Pid::from_raw_unchecked(pid)
     }
 }

src/backend/libc/process/syscalls.rs

@@ -26,7 +26,7 @@ use crate::process::Uid;
 #[cfg(linux_kernel)]
 use crate::process::{Cpuid, MembarrierCommand, MembarrierQuery};
 #[cfg(not(target_os = "wasi"))]
-use crate::process::{Gid, Pid, RawNonZeroPid, RawPid, Signal, WaitOptions, WaitStatus};
+use crate::process::{Gid, Pid, RawPid, Signal, WaitOptions, WaitStatus};
 #[cfg(not(any(target_os = "fuchsia", target_os = "redox", target_os = "wasi")))]
 use crate::process::{Resource, Rlimit};
 #[cfg(not(any(target_os = "redox", target_os = "openbsd", target_os = "wasi")))]
@@ -109,8 +109,7 @@ pub(crate) fn getppid() -> Option<Pid> {
 pub(crate) fn getpgid(pid: Option<Pid>) -> io::Result<Pid> {
     unsafe {
         let pgid = ret_pid_t(c::getpgid(Pid::as_raw(pid) as _))?;
-        debug_assert_ne!(pgid, 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pgid)))
+        Ok(Pid::from_raw_unchecked(pgid))
     }
 }
 
@@ -126,8 +125,7 @@ pub(crate) fn setpgid(pid: Option<Pid>, pgid: Option<Pid>) -> io::Result<()> {
 pub(crate) fn getpgrp() -> Pid {
     unsafe {
         let pgid = c::getpgrp();
-        debug_assert_ne!(pgid, 0);
-        Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pgid))
+        Pid::from_raw_unchecked(pgid)
     }
 }
 
@@ -336,12 +334,7 @@ pub(crate) fn _waitpid(
     unsafe {
         let mut status: c::c_int = 0;
         let pid = ret_c_int(c::waitpid(pid as _, &mut status, waitopts.bits() as _))?;
-        Ok(RawNonZeroPid::new(pid).map(|non_zero| {
-            (
-                Pid::from_raw_nonzero(non_zero),
-                WaitStatus::new(status as _),
-            )
-        }))
+        Ok(Pid::from_raw(pid).map(|pid| (pid, WaitStatus::new(status as _))))
     }
 }
 
@@ -442,8 +435,7 @@ unsafe fn cvt_waitid_status(status: MaybeUninit<c::siginfo_t>) -> Option<WaitidS
 pub(crate) fn getsid(pid: Option<Pid>) -> io::Result<Pid> {
     unsafe {
         let pid = ret_pid_t(c::getsid(Pid::as_raw(pid) as _))?;
-        debug_assert_ne!(pid, 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid)))
+        Ok(Pid::from_raw_unchecked(pid))
     }
 }
 
@@ -452,8 +444,7 @@ pub(crate) fn getsid(pid: Option<Pid>) -> io::Result<Pid> {
 pub(crate) fn setsid() -> io::Result<Pid> {
     unsafe {
         let pid = ret_c_int(c::setsid())?;
-        debug_assert_ne!(pid, 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid)))
+        Ok(Pid::from_raw_unchecked(pid))
     }
 }
 

src/backend/libc/termios/syscalls.rs

@@ -14,7 +14,7 @@ use core::mem::MaybeUninit;
 #[cfg(not(target_os = "wasi"))]
 use {
     crate::io,
-    crate::pid::{Pid, RawNonZeroPid},
+    crate::pid::Pid,
     crate::termios::{Action, OptionalActions, QueueSelector, Speed, Termios, Winsize},
 };
 
@@ -52,8 +52,7 @@ pub(crate) fn tcgetattr2(fd: BorrowedFd<'_>) -> io::Result<crate::termios::Termi
 pub(crate) fn tcgetpgrp(fd: BorrowedFd<'_>) -> io::Result<Pid> {
     unsafe {
         let pid = ret_pid_t(c::tcgetpgrp(borrowed_fd(fd)))?;
-        debug_assert_ne!(pid, 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid)))
+        Ok(Pid::from_raw_unchecked(pid))
     }
 }
 
@@ -128,8 +127,7 @@ pub(crate) fn tcflow(fd: BorrowedFd, action: Action) -> io::Result<()> {
 pub(crate) fn tcgetsid(fd: BorrowedFd) -> io::Result<Pid> {
     unsafe {
         let pid = ret_pid_t(c::tcgetsid(borrowed_fd(fd)))?;
-        debug_assert_ne!(pid, 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid)))
+        Ok(Pid::from_raw_unchecked(pid))
     }
 }
 

src/backend/libc/thread/syscalls.rs

@@ -15,7 +15,7 @@ use core::mem::MaybeUninit;
 use {
     crate::backend::conv::{borrowed_fd, ret_c_int, syscall_ret},
     crate::fd::BorrowedFd,
-    crate::pid::{Pid, RawNonZeroPid},
+    crate::pid::Pid,
     crate::utils::as_mut_ptr,
 };
 #[cfg(not(any(
@@ -285,8 +285,7 @@ pub(crate) fn gettid() -> Pid {
 
     unsafe {
         let tid = gettid();
-        debug_assert_ne!(tid, 0);
-        Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(tid))
+        Pid::from_raw_unchecked(tid)
     }
 }
 

src/backend/linux_raw/pid/syscalls.rs

@@ -7,13 +7,13 @@
 #![allow(clippy::undocumented_unsafe_blocks)]
 
 use crate::backend::conv::ret_usize_infallible;
-use crate::pid::{Pid, RawNonZeroPid, RawPid};
+use crate::pid::{Pid, RawPid};
 
 #[inline]
 pub(crate) fn getpid() -> Pid {
     unsafe {
         let pid = ret_usize_infallible(syscall_readonly!(__NR_getpid)) as RawPid;
         debug_assert!(pid > 0);
-        Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid as _))
+        Pid::from_raw_unchecked(pid)
     }
 }

src/backend/linux_raw/process/syscalls.rs

@@ -19,7 +19,7 @@ use crate::fd::{AsRawFd, BorrowedFd, OwnedFd, RawFd};
 #[cfg(feature = "fs")]
 use crate::ffi::CStr;
 use crate::io;
-use crate::pid::{RawNonZeroPid, RawPid};
+use crate::pid::RawPid;
 use crate::process::{
     Cpuid, Gid, MembarrierCommand, MembarrierQuery, Pid, PidfdFlags, PidfdGetfdFlags, Resource,
     Rlimit, Uid, WaitId, WaitOptions, WaitStatus, WaitidOptions, WaitidStatus,
@@ -105,7 +105,7 @@ pub(crate) fn getpgid(pid: Option<Pid>) -> io::Result<Pid> {
     unsafe {
         let pgid = ret_c_int(syscall_readonly!(__NR_getpgid, c_int(Pid::as_raw(pid))))?;
         debug_assert!(pgid > 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pgid)))
+        Ok(Pid::from_raw_unchecked(pgid))
     }
 }
 
@@ -127,15 +127,15 @@ pub(crate) fn getpgrp() -> Pid {
     unsafe {
         let pgid = ret_c_int_infallible(syscall_readonly!(__NR_getpgrp));
         debug_assert!(pgid > 0);
-        Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pgid))
+        Pid::from_raw_unchecked(pgid)
     }
 
     // Otherwise use `getpgrp` and pass it zero.
     #[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))]
     unsafe {
         let pgid = ret_c_int_infallible(syscall_readonly!(__NR_getpgid, c_uint(0)));
         debug_assert!(pgid > 0);
-        Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pgid))
+        Pid::from_raw_unchecked(pgid)
     }
 }
 
@@ -453,12 +453,7 @@ pub(crate) fn _waitpid(
             c_int(waitopts.bits() as _),
             zero()
         ))?;
-        Ok(RawNonZeroPid::new(pid).map(|non_zero| {
-            (
-                Pid::from_raw_nonzero(non_zero),
-                WaitStatus::new(status.assume_init()),
-            )
-        }))
+        Ok(Pid::from_raw(pid).map(|pid| (pid, WaitStatus::new(status.assume_init()))))
     }
 }
 
@@ -551,7 +546,7 @@ pub(crate) fn getsid(pid: Option<Pid>) -> io::Result<Pid> {
     unsafe {
         let pid = ret_c_int(syscall_readonly!(__NR_getsid, c_int(Pid::as_raw(pid))))?;
         debug_assert!(pid > 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid)))
+        Ok(Pid::from_raw_unchecked(pid))
     }
 }
 
@@ -560,7 +555,7 @@ pub(crate) fn setsid() -> io::Result<Pid> {
     unsafe {
         let pid = ret_c_int(syscall_readonly!(__NR_setsid))?;
         debug_assert!(pid > 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid)))
+        Ok(Pid::from_raw_unchecked(pid))
     }
 }
 

src/backend/linux_raw/runtime/syscalls.rs

@@ -18,7 +18,7 @@ use crate::ffi::CStr;
 #[cfg(feature = "fs")]
 use crate::fs::AtFlags;
 use crate::io;
-use crate::pid::{Pid, RawNonZeroPid};
+use crate::pid::Pid;
 use crate::runtime::{How, Sigaction, Siginfo, Sigset, Stack};
 use crate::signal::Signal;
 use crate::timespec::Timespec;
@@ -101,7 +101,7 @@ pub(crate) mod tls {
     pub(crate) unsafe fn set_tid_address(data: *mut c::c_void) -> Pid {
         let tid: i32 = ret_c_int_infallible(syscall_readonly!(__NR_set_tid_address, data));
         debug_assert_ne!(tid, 0);
-        Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(tid))
+        Pid::from_raw_unchecked(tid)
     }
 
     #[inline]

src/backend/linux_raw/termios/syscalls.rs

@@ -10,7 +10,7 @@ use crate::backend::c;
 use crate::backend::conv::{by_ref, c_uint, ret};
 use crate::fd::BorrowedFd;
 use crate::io;
-use crate::pid::{Pid, RawNonZeroPid};
+use crate::pid::Pid;
 #[cfg(feature = "procfs")]
 use crate::procfs;
 use crate::termios::{
@@ -75,7 +75,7 @@ pub(crate) fn tcgetpgrp(fd: BorrowedFd<'_>) -> io::Result<Pid> {
         ret(syscall!(__NR_ioctl, fd, c_uint(TIOCGPGRP), &mut result))?;
         let pid = result.assume_init();
         debug_assert!(pid > 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid)))
+        Ok(Pid::from_raw_unchecked(pid))
     }
 }
 
@@ -169,7 +169,7 @@ pub(crate) fn tcgetsid(fd: BorrowedFd) -> io::Result<Pid> {
         ret(syscall!(__NR_ioctl, fd, c_uint(TIOCGSID), &mut result))?;
         let pid = result.assume_init();
         debug_assert!(pid > 0);
-        Ok(Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(pid)))
+        Ok(Pid::from_raw_unchecked(pid))
     }
 }
 

src/backend/linux_raw/thread/syscalls.rs

@@ -13,7 +13,7 @@ use crate::backend::conv::{
 };
 use crate::fd::BorrowedFd;
 use crate::io;
-use crate::pid::{Pid, RawNonZeroPid};
+use crate::pid::Pid;
 use crate::thread::{ClockId, FutexFlags, FutexOperation, NanosleepRelativeResult, Timespec};
 use core::mem::MaybeUninit;
 #[cfg(target_pointer_width = "32")]
@@ -201,7 +201,7 @@ pub(crate) fn gettid() -> Pid {
     unsafe {
         let tid = ret_c_int_infallible(syscall_readonly!(__NR_gettid));
         debug_assert_ne!(tid, 0);
-        Pid::from_raw_nonzero(RawNonZeroPid::new_unchecked(tid))
+        Pid::from_raw_unchecked(tid)
     }
 }
 

src/event/kqueue.rs

@@ -113,7 +113,7 @@ impl Event {
                 flags: VnodeEvents::from_bits_truncate(self.inner.fflags),
             },
             c::EVFILT_PROC => EventFilter::Proc {
-                pid: unsafe { Pid::from_raw(self.inner.ident as _) }.unwrap(),
+                pid: Pid::from_raw(self.inner.ident as _).unwrap(),
                 flags: ProcessEvents::from_bits_truncate(self.inner.fflags),
             },
             c::EVFILT_SIGNAL => EventFilter::Signal {