From 1e857b46f8a283f4b16e471a3a82b8cf92a8017c Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Sun, 16 Jan 2022 07:35:49 -0800 Subject: [PATCH 1/4] Port cargo's libc usage to rustix. This factors out several `unsafe` blocks, and replaces several instances of manual error handling with `Result`s. And it replaces several places that previously manipulated raw file descriptors to use the I/O safety owning and borrowing types and traits instead. This also eliminates the need for a special case for musl, allowing cargo to behave the same way on glibc and musl systems. --- Cargo.toml | 2 +- crates/cargo-util/Cargo.toml | 3 +- crates/cargo-util/src/process_error.rs | 31 ++++++++-------- crates/cargo-util/src/read2.rs | 50 ++++++++++++------------- src/cargo/core/shell.rs | 8 +--- src/cargo/util/config/mod.rs | 2 +- src/cargo/util/flock.rs | 51 +++++++++++--------------- src/cargo/util/job.rs | 2 +- tests/testsuite/death.rs | 9 +++-- 9 files changed, 73 insertions(+), 85 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ab6a6406846..23eb32ebed5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ ignore = "0.4.7" lazy_static = "1.2.0" jobserver = "0.1.24" lazycell = "1.2.0" -libc = "0.2" +rustix = "0.32.0" log = "0.4.6" libgit2-sys = "0.12.24" memchr = "2.1.3" diff --git a/crates/cargo-util/Cargo.toml b/crates/cargo-util/Cargo.toml index 70c286a007f..d230cb0a822 100644 --- a/crates/cargo-util/Cargo.toml +++ b/crates/cargo-util/Cargo.toml @@ -13,7 +13,8 @@ crypto-hash = "0.3.1" filetime = "0.2.9" hex = "0.4.2" jobserver = "0.1.21" -libc = "0.2.88" +rustix = "0.32.0" +io-lifetimes = "0.4.4" log = "0.4.6" same-file = "1.0.6" shell-escape = "0.1.4" diff --git a/crates/cargo-util/src/process_error.rs b/crates/cargo-util/src/process_error.rs index 57feffbef67..99b45659ff3 100644 --- a/crates/cargo-util/src/process_error.rs +++ b/crates/cargo-util/src/process_error.rs @@ -104,25 +104,26 @@ pub fn exit_status_to_string(status: ExitStatus) -> String { #[cfg(unix)] fn status_to_string(status: ExitStatus) -> String { + use rustix::process::Signal; use std::os::unix::process::*; if let Some(signal) = status.signal() { - let name = match signal as libc::c_int { - libc::SIGABRT => ", SIGABRT: process abort signal", - libc::SIGALRM => ", SIGALRM: alarm clock", - libc::SIGFPE => ", SIGFPE: erroneous arithmetic operation", - libc::SIGHUP => ", SIGHUP: hangup", - libc::SIGILL => ", SIGILL: illegal instruction", - libc::SIGINT => ", SIGINT: terminal interrupt signal", - libc::SIGKILL => ", SIGKILL: kill", - libc::SIGPIPE => ", SIGPIPE: write on a pipe with no one to read", - libc::SIGQUIT => ", SIGQUIT: terminal quit signal", - libc::SIGSEGV => ", SIGSEGV: invalid memory reference", - libc::SIGTERM => ", SIGTERM: termination signal", - libc::SIGBUS => ", SIGBUS: access to undefined memory", + let name = match Signal::from_raw(signal) { + Some(Signal::Abort) => ", SIGABRT: process abort signal", + Some(Signal::Alarm) => ", SIGALRM: alarm clock", + Some(Signal::Fpe) => ", SIGFPE: erroneous arithmetic operation", + Some(Signal::Hup) => ", SIGHUP: hangup", + Some(Signal::Ill) => ", SIGILL: illegal instruction", + Some(Signal::Int) => ", SIGINT: terminal interrupt signal", + Some(Signal::Kill) => ", SIGKILL: kill", + Some(Signal::Pipe) => ", SIGPIPE: write on a pipe with no one to read", + Some(Signal::Quit) => ", SIGQUIT: terminal quit signal", + Some(Signal::Segv) => ", SIGSEGV: invalid memory reference", + Some(Signal::Term) => ", SIGTERM: termination signal", + Some(Signal::Bus) => ", SIGBUS: access to undefined memory", #[cfg(not(target_os = "haiku"))] - libc::SIGSYS => ", SIGSYS: bad system call", - libc::SIGTRAP => ", SIGTRAP: trace/breakpoint trap", + Some(Signal::Sys) => ", SIGSYS: bad system call", + Some(Signal::Trap) => ", SIGTRAP: trace/breakpoint trap", _ => "", }; format!("signal: {}{}", signal, name) diff --git a/crates/cargo-util/src/read2.rs b/crates/cargo-util/src/read2.rs index 53322a51d70..b413cb0c46c 100644 --- a/crates/cargo-util/src/read2.rs +++ b/crates/cargo-util/src/read2.rs @@ -2,52 +2,50 @@ pub use self::imp::read2; #[cfg(unix)] mod imp { + use io_lifetimes::AsFilelike; + use rustix::io::{PollFd, PollFlags}; + use std::fs::File; use std::io; use std::io::prelude::*; - use std::mem; - use std::os::unix::prelude::*; use std::process::{ChildStderr, ChildStdout}; pub fn read2( - mut out_pipe: ChildStdout, - mut err_pipe: ChildStderr, + out_pipe: ChildStdout, + err_pipe: ChildStderr, data: &mut dyn FnMut(bool, &mut Vec, bool), ) -> io::Result<()> { - unsafe { - libc::fcntl(out_pipe.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK); - libc::fcntl(err_pipe.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK); - } + rustix::fs::fcntl_setfl(&out_pipe, rustix::fs::OFlags::NONBLOCK)?; + rustix::fs::fcntl_setfl(&err_pipe, rustix::fs::OFlags::NONBLOCK)?; let mut out_done = false; let mut err_done = false; let mut out = Vec::new(); let mut err = Vec::new(); - let mut fds: [libc::pollfd; 2] = unsafe { mem::zeroed() }; - fds[0].fd = out_pipe.as_raw_fd(); - fds[0].events = libc::POLLIN; - fds[1].fd = err_pipe.as_raw_fd(); - fds[1].events = libc::POLLIN; + let mut fds = [ + PollFd::new(&out_pipe, PollFlags::IN), + PollFd::new(&err_pipe, PollFlags::IN), + ]; let mut nfds = 2; let mut errfd = 1; while nfds > 0 { - // wait for either pipe to become readable using `select` - let r = unsafe { libc::poll(fds.as_mut_ptr(), nfds, -1) }; - if r == -1 { - let err = io::Error::last_os_error(); - if err.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(err); - } + // wait for either pipe to become readable using `poll` + match rustix::io::poll(&mut fds, -1) { + Ok(_num) => (), + Err(rustix::io::Error::INTR) => continue, + Err(err) => return Err(err.into()), + }; // Read as much as we can from each pipe, ignoring EWOULDBLOCK or // EAGAIN. If we hit EOF, then this will happen because the underlying // reader will return Ok(0), in which case we'll see `Ok` ourselves. In // this case we flip the other fd back into blocking mode and read // whatever's leftover on that file descriptor. - let handle = |res: io::Result<_>| match res { + let handle = |poll_fd: &PollFd, buf: &mut Vec| match poll_fd + .as_filelike_view::() + .read_to_end(buf) + { Ok(_) => Ok(true), Err(e) => { if e.kind() == io::ErrorKind::WouldBlock { @@ -57,14 +55,14 @@ mod imp { } } }; - if !err_done && fds[errfd].revents != 0 && handle(err_pipe.read_to_end(&mut err))? { + if !err_done && !fds[errfd].revents().is_empty() && handle(&fds[errfd], &mut err)? { err_done = true; nfds -= 1; } data(false, &mut err, err_done); - if !out_done && fds[0].revents != 0 && handle(out_pipe.read_to_end(&mut out))? { + if !out_done && !fds[0].revents().is_empty() && handle(&fds[0], &mut out)? { out_done = true; - fds[0].fd = err_pipe.as_raw_fd(); + fds[0].set_fd(&err_pipe); errfd = 0; nfds -= 1; } diff --git a/src/cargo/core/shell.rs b/src/cargo/core/shell.rs index 887b8967d25..04004cde6f0 100644 --- a/src/cargo/core/shell.rs +++ b/src/cargo/core/shell.rs @@ -460,16 +460,10 @@ impl ColorChoice { #[cfg(unix)] mod imp { use super::{Shell, TtyWidth}; - use std::mem; pub fn stderr_width() -> TtyWidth { unsafe { - let mut winsize: libc::winsize = mem::zeroed(); - // The .into() here is needed for FreeBSD which defines TIOCGWINSZ - // as c_uint but ioctl wants c_ulong. - if libc::ioctl(libc::STDERR_FILENO, libc::TIOCGWINSZ.into(), &mut winsize) < 0 { - return TtyWidth::NoTty; - } + let winsize = rustix::io::ioctl_tiocgwinsz(&rustix::io::stderr()).unwrap(); if winsize.ws_col > 0 { TtyWidth::Known(winsize.ws_col as usize) } else { diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index fdf981f051f..750403d95d7 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -1604,7 +1604,7 @@ impl Config { } #[cfg(unix)] - return io.raw_os_error() == Some(libc::EROFS); + return io.raw_os_error() == Some(rustix::io::Error::ROFS.raw_os_error()); } false diff --git a/src/cargo/util/flock.rs b/src/cargo/util/flock.rs index 755bcdcd910..37a9b8b0604 100644 --- a/src/cargo/util/flock.rs +++ b/src/cargo/util/flock.rs @@ -278,8 +278,8 @@ fn acquire( config: &Config, msg: &str, path: &Path, - lock_try: &dyn Fn() -> io::Result<()>, - lock_block: &dyn Fn() -> io::Result<()>, + lock_try: &dyn Fn() -> rustix::io::Result<()>, + lock_block: &dyn Fn() -> rustix::io::Result<()>, ) -> CargoResult<()> { // File locking on Unix is currently implemented via `flock`, which is known // to be broken on NFS. We could in theory just ignore errors that happen on @@ -317,10 +317,9 @@ fn acquire( lock_block().with_context(|| format!("failed to lock file: {}", path.display()))?; return Ok(()); - #[cfg(all(target_os = "linux", not(target_env = "musl")))] + #[cfg(target_os = "linux")] fn is_on_nfs_mount(path: &Path) -> bool { use std::ffi::CString; - use std::mem; use std::os::unix::prelude::*; let path = match CString::new(path.as_os_str().as_bytes()) { @@ -328,15 +327,14 @@ fn acquire( Err(_) => return false, }; - unsafe { - let mut buf: libc::statfs = mem::zeroed(); - let r = libc::statfs(path.as_ptr(), &mut buf); - - r == 0 && buf.f_type as u32 == libc::NFS_SUPER_MAGIC as u32 + if let Ok(buf) = rustix::fs::statfs(path) { + buf.f_type as u32 == rustix::fs::NFS_SUPER_MAGIC as u32 + } else { + false } } - #[cfg(any(not(target_os = "linux"), target_env = "musl"))] + #[cfg(not(target_os = "linux"))] fn is_on_nfs_mount(_path: &Path) -> bool { false } @@ -344,58 +342,53 @@ fn acquire( #[cfg(unix)] mod sys { + use rustix::fs::FlockOperation; + use rustix::io::{Error, Result}; use std::fs::File; - use std::io::{Error, Result}; - use std::os::unix::io::AsRawFd; pub(super) fn lock_shared(file: &File) -> Result<()> { - flock(file, libc::LOCK_SH) + flock(file, FlockOperation::LockShared) } pub(super) fn lock_exclusive(file: &File) -> Result<()> { - flock(file, libc::LOCK_EX) + flock(file, FlockOperation::LockExclusive) } pub(super) fn try_lock_shared(file: &File) -> Result<()> { - flock(file, libc::LOCK_SH | libc::LOCK_NB) + flock(file, FlockOperation::NonBlockingLockShared) } pub(super) fn try_lock_exclusive(file: &File) -> Result<()> { - flock(file, libc::LOCK_EX | libc::LOCK_NB) + flock(file, FlockOperation::NonBlockingLockExclusive) } pub(super) fn unlock(file: &File) -> Result<()> { - flock(file, libc::LOCK_UN) + flock(file, FlockOperation::Unlock) } pub(super) fn error_contended(err: &Error) -> bool { - err.raw_os_error().map_or(false, |x| x == libc::EWOULDBLOCK) + *err == rustix::io::Error::WOULDBLOCK } pub(super) fn error_unsupported(err: &Error) -> bool { - match err.raw_os_error() { + match *err { // Unfortunately, depending on the target, these may or may not be the same. // For targets in which they are the same, the duplicate pattern causes a warning. #[allow(unreachable_patterns)] - Some(libc::ENOTSUP | libc::EOPNOTSUPP) => true, + rustix::io::Error::NOTSUP | rustix::io::Error::OPNOTSUPP => true, #[cfg(target_os = "linux")] - Some(libc::ENOSYS) => true, + rustix::io::Error::NOSYS => true, _ => false, } } #[cfg(not(target_os = "solaris"))] - fn flock(file: &File, flag: libc::c_int) -> Result<()> { - let ret = unsafe { libc::flock(file.as_raw_fd(), flag) }; - if ret < 0 { - Err(Error::last_os_error()) - } else { - Ok(()) - } + fn flock(file: &File, flag: FlockOperation) -> Result<()> { + rustix::fs::flock(file, flag) } #[cfg(target_os = "solaris")] - fn flock(file: &File, flag: libc::c_int) -> Result<()> { + fn flock(file: &File, flag: FlockOperation) -> Result<()> { // Solaris lacks flock(), so simply succeed with a no-op Ok(()) } diff --git a/src/cargo/util/job.rs b/src/cargo/util/job.rs index 7a9cd1bcaed..8332d59b9c7 100644 --- a/src/cargo/util/job.rs +++ b/src/cargo/util/job.rs @@ -33,7 +33,7 @@ mod imp { // one cargo spawned to become its own session leader, so we do that // here. if env::var("__CARGO_TEST_SETSID_PLEASE_DONT_USE_ELSEWHERE").is_ok() { - libc::setsid(); + rustix::process::setsid().unwrap(); } Some(()) } diff --git a/tests/testsuite/death.rs b/tests/testsuite/death.rs index d140534d5b1..6ddb5b5c302 100644 --- a/tests/testsuite/death.rs +++ b/tests/testsuite/death.rs @@ -89,10 +89,11 @@ fn ctrl_c_kills_everyone() { #[cfg(unix)] pub fn ctrl_c(child: &mut Child) { - let r = unsafe { libc::kill(-(child.id() as i32), libc::SIGINT) }; - if r < 0 { - panic!("failed to kill: {}", io::Error::last_os_error()); - } + rustix::process::kill_process_group( + rustix::process::Pid::from_child(child), + rustix::process::Signal::Int, + ) + .expect("failed to kill"); } #[cfg(windows)] From 364da25489a0e8659061aae05417f5128f60c808 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 19 Jan 2022 17:51:38 -0800 Subject: [PATCH 2/4] Update to rustix 0.32.1, which has a fix for non-Linux targets. --- Cargo.toml | 2 +- crates/cargo-util/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 23eb32ebed5..764508a66cd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ ignore = "0.4.7" lazy_static = "1.2.0" jobserver = "0.1.24" lazycell = "1.2.0" -rustix = "0.32.0" +rustix = "0.32.1" log = "0.4.6" libgit2-sys = "0.12.24" memchr = "2.1.3" diff --git a/crates/cargo-util/Cargo.toml b/crates/cargo-util/Cargo.toml index d230cb0a822..8d363d11094 100644 --- a/crates/cargo-util/Cargo.toml +++ b/crates/cargo-util/Cargo.toml @@ -13,7 +13,7 @@ crypto-hash = "0.3.1" filetime = "0.2.9" hex = "0.4.2" jobserver = "0.1.21" -rustix = "0.32.0" +rustix = "0.32.1" io-lifetimes = "0.4.4" log = "0.4.6" same-file = "1.0.6" From b4436dbdd9bd87c4ad26f82c666133c7e17f59a8 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 19 Jan 2022 18:53:02 -0800 Subject: [PATCH 3/4] Use `std::io::Error` in code shared with Windows. --- src/cargo/util/flock.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/cargo/util/flock.rs b/src/cargo/util/flock.rs index 37a9b8b0604..7dcc8570ac1 100644 --- a/src/cargo/util/flock.rs +++ b/src/cargo/util/flock.rs @@ -278,8 +278,8 @@ fn acquire( config: &Config, msg: &str, path: &Path, - lock_try: &dyn Fn() -> rustix::io::Result<()>, - lock_block: &dyn Fn() -> rustix::io::Result<()>, + lock_try: &dyn Fn() -> std::io::Result<()>, + lock_block: &dyn Fn() -> std::io::Result<()>, ) -> CargoResult<()> { // File locking on Unix is currently implemented via `flock`, which is known // to be broken on NFS. We could in theory just ignore errors that happen on @@ -343,8 +343,8 @@ fn acquire( #[cfg(unix)] mod sys { use rustix::fs::FlockOperation; - use rustix::io::{Error, Result}; use std::fs::File; + use std::io::{Error, Result}; pub(super) fn lock_shared(file: &File) -> Result<()> { flock(file, FlockOperation::LockShared) @@ -367,24 +367,25 @@ mod sys { } pub(super) fn error_contended(err: &Error) -> bool { - *err == rustix::io::Error::WOULDBLOCK + rustix::io::Error::from_io_error(err).map_or(false, |x| x == rustix::io::Error::WOULDBLOCK) } pub(super) fn error_unsupported(err: &Error) -> bool { - match *err { + match rustix::io::Error::from_io_error(err) { // Unfortunately, depending on the target, these may or may not be the same. // For targets in which they are the same, the duplicate pattern causes a warning. #[allow(unreachable_patterns)] - rustix::io::Error::NOTSUP | rustix::io::Error::OPNOTSUPP => true, + Some(rustix::io::Error::NOTSUP) | Some(rustix::io::Error::OPNOTSUPP) => true, #[cfg(target_os = "linux")] - rustix::io::Error::NOSYS => true, + Some(rustix::io::Error::NOSYS) => true, _ => false, } } #[cfg(not(target_os = "solaris"))] fn flock(file: &File, flag: FlockOperation) -> Result<()> { - rustix::fs::flock(file, flag) + rustix::fs::flock(file, flag)?; + Ok(()) } #[cfg(target_os = "solaris")] From 2a1a5ced6a870f02220af05efbf97b69f8001afd Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 21 Jan 2022 11:19:05 -0800 Subject: [PATCH 4/4] Fix the number of file descriptors being waited for with `poll`. `fds` here is an array with an explicit `nfds` count of how many elements are active, rather than a vec, so only pass the subrange `&fds[..nfds]` to `poll`. --- crates/cargo-util/src/read2.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cargo-util/src/read2.rs b/crates/cargo-util/src/read2.rs index b413cb0c46c..2319f8ee9fa 100644 --- a/crates/cargo-util/src/read2.rs +++ b/crates/cargo-util/src/read2.rs @@ -31,7 +31,7 @@ mod imp { while nfds > 0 { // wait for either pipe to become readable using `poll` - match rustix::io::poll(&mut fds, -1) { + match rustix::io::poll(&mut fds[..nfds], -1) { Ok(_num) => (), Err(rustix::io::Error::INTR) => continue, Err(err) => return Err(err.into()),