From f45fe9493ba06cd57fc2f7317a39305f79a1a6e6 Mon Sep 17 00:00:00 2001 From: "Charles E. Lehner" Date: Sat, 20 Feb 2021 13:18:01 -0500 Subject: [PATCH 01/20] Add license metadata for std dependencies --- library/alloc/Cargo.toml | 3 +++ library/core/Cargo.toml | 3 +++ library/panic_abort/Cargo.toml | 3 +++ library/panic_unwind/Cargo.toml | 3 +++ library/unwind/Cargo.toml | 2 ++ 5 files changed, 14 insertions(+) diff --git a/library/alloc/Cargo.toml b/library/alloc/Cargo.toml index d95b5b7f17f42..4f97c95bcb9ea 100644 --- a/library/alloc/Cargo.toml +++ b/library/alloc/Cargo.toml @@ -2,6 +2,9 @@ authors = ["The Rust Project Developers"] name = "alloc" version = "0.0.0" +license = "MIT OR Apache-2.0" +repository = "https://github.com/rust-lang/rust.git" +description = "The Rust core allocation and collections library" autotests = false autobenches = false edition = "2018" diff --git a/library/core/Cargo.toml b/library/core/Cargo.toml index c1596012eac24..4a7dbb91822e2 100644 --- a/library/core/Cargo.toml +++ b/library/core/Cargo.toml @@ -2,6 +2,9 @@ authors = ["The Rust Project Developers"] name = "core" version = "0.0.0" +license = "MIT OR Apache-2.0" +repository = "https://github.com/rust-lang/rust.git" +description = "The Rust Core Library" autotests = false autobenches = false edition = "2018" diff --git a/library/panic_abort/Cargo.toml b/library/panic_abort/Cargo.toml index b15919fad75e7..caa89aa30d0bb 100644 --- a/library/panic_abort/Cargo.toml +++ b/library/panic_abort/Cargo.toml @@ -2,6 +2,9 @@ authors = ["The Rust Project Developers"] name = "panic_abort" version = "0.0.0" +license = "MIT OR Apache-2.0" +repository = "https://github.com/rust-lang/rust.git" +description = "Implementation of Rust panics via process aborts" edition = "2018" [lib] diff --git a/library/panic_unwind/Cargo.toml b/library/panic_unwind/Cargo.toml index d27ba9876416d..533f059a85e45 100644 --- a/library/panic_unwind/Cargo.toml +++ b/library/panic_unwind/Cargo.toml @@ -2,6 +2,9 @@ authors = ["The Rust Project Developers"] name = "panic_unwind" version = "0.0.0" +license = "MIT OR Apache-2.0" +repository = "https://github.com/rust-lang/rust.git" +description = "Implementation of Rust panics via stack unwinding" edition = "2018" [lib] diff --git a/library/unwind/Cargo.toml b/library/unwind/Cargo.toml index 4f7a304a59f68..69128591e0672 100644 --- a/library/unwind/Cargo.toml +++ b/library/unwind/Cargo.toml @@ -2,6 +2,8 @@ authors = ["The Rust Project Developers"] name = "unwind" version = "0.0.0" +license = "MIT OR Apache-2.0" +repository = "https://github.com/rust-lang/rust.git" edition = "2018" include = [ '/libunwind/*', From 2cbea9f98e6d93b85d5f603be311313d41b87525 Mon Sep 17 00:00:00 2001 From: Christiaan Dirkx Date: Wed, 24 Feb 2021 23:11:02 +0100 Subject: [PATCH 02/20] Reuse `std::sys::unsupported::pipe` on `hermit` --- library/std/src/sys/hermit/mod.rs | 1 + library/std/src/sys/hermit/pipe.rs | 38 ------------------------------ 2 files changed, 1 insertion(+), 38 deletions(-) delete mode 100644 library/std/src/sys/hermit/pipe.rs diff --git a/library/std/src/sys/hermit/mod.rs b/library/std/src/sys/hermit/mod.rs index f185635b7a0a6..17a51abeb0ef6 100644 --- a/library/std/src/sys/hermit/mod.rs +++ b/library/std/src/sys/hermit/mod.rs @@ -32,6 +32,7 @@ pub mod mutex; pub mod net; pub mod os; pub mod path; +#[path = "../unsupported/pipe.rs"] pub mod pipe; #[path = "../unsupported/process.rs"] pub mod process; diff --git a/library/std/src/sys/hermit/pipe.rs b/library/std/src/sys/hermit/pipe.rs deleted file mode 100644 index 10d0925823eb9..0000000000000 --- a/library/std/src/sys/hermit/pipe.rs +++ /dev/null @@ -1,38 +0,0 @@ -use crate::io::{self, IoSlice, IoSliceMut}; -use crate::sys::Void; - -pub struct AnonPipe(Void); - -impl AnonPipe { - pub fn read(&self, _buf: &mut [u8]) -> io::Result { - match self.0 {} - } - - pub fn read_vectored(&self, _bufs: &mut [IoSliceMut<'_>]) -> io::Result { - match self.0 {} - } - - pub fn is_read_vectored(&self) -> bool { - match self.0 {} - } - - pub fn write(&self, _buf: &[u8]) -> io::Result { - match self.0 {} - } - - pub fn write_vectored(&self, _bufs: &[IoSlice<'_>]) -> io::Result { - match self.0 {} - } - - pub fn is_write_vectored(&self) -> bool { - match self.0 {} - } - - pub fn diverge(&self) -> ! { - match self.0 {} - } -} - -pub fn read2(p1: AnonPipe, _v1: &mut Vec, _p2: AnonPipe, _v2: &mut Vec) -> io::Result<()> { - match p1.0 {} -} From 8c718bd3f657003f603ea6d4435f8d83752f0751 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 4 Mar 2021 10:43:24 +0100 Subject: [PATCH 03/20] Attempt to gather similar stats as rusage on Windows --- src/bootstrap/Cargo.toml | 2 +- src/bootstrap/bin/rustc.rs | 72 +++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index e04128d1b0b25..c14ad6fa5fff4 100644 --- a/src/bootstrap/Cargo.toml +++ b/src/bootstrap/Cargo.toml @@ -53,7 +53,7 @@ merge = "0.1.0" [target.'cfg(windows)'.dependencies.winapi] version = "0.3" -features = ["fileapi", "ioapiset", "jobapi2", "handleapi", "winioctl"] +features = ["fileapi", "ioapiset", "jobapi2", "handleapi", "winioctl", "psapi", "impl-default"] [dev-dependencies] pretty_assertions = "0.6" diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index 6b1be0ca09d0d..fb9f21bfd3c1f 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -156,9 +156,11 @@ fn main() { } let start = Instant::now(); - let status = { + let (child, status) = { let errmsg = format!("\nFailed to run:\n{:?}\n-------------", cmd); - cmd.status().expect(&errmsg) + let mut child = cmd.spawn().expect(&errmsg); + let status = child.wait().expect(&errmsg); + (child, status) }; if env::var_os("RUSTC_PRINT_STEP_TIMINGS").is_some() @@ -169,8 +171,19 @@ fn main() { let is_test = args.iter().any(|a| a == "--test"); // If the user requested resource usage data, then // include that in addition to the timing output. - let rusage_data = - env::var_os("RUSTC_PRINT_STEP_RUSAGE").and_then(|_| format_rusage_data()); + let rusage_data = env::var_os("RUSTC_PRINT_STEP_RUSAGE").and_then(|_| { + #[cfg(windows)] + { + use std::os::windows::io::AsRawHandle; + let handle = child.as_raw_handle(); + format_rusage_data(handle) + } + #[cfg(not(windows))] + { + let _child = child; + format_rusage_data() + } + }); eprintln!( "[RUSTC-TIMING] {} test:{} {}.{:03}{}{}", crate_name, @@ -207,13 +220,62 @@ fn main() { } } -#[cfg(not(unix))] +#[cfg(all(not(unix), not(windows)))] /// getrusage is not available on non-unix platforms. So for now, we do not /// bother trying to make a shim for it. fn format_rusage_data() -> Option { None } +#[cfg(windows)] +fn format_rusage_data(handle: std::os::windows::raw::HANDLE) -> Option { + macro_rules! try_bool { + ($e:expr) => { + if $e != 1 { + return None; + } + }; + } + unsafe { + let mut _filetime = winapi::shared::minwindef::FILETIME::default(); + let mut user_filetime = winapi::shared::minwindef::FILETIME::default(); + let mut kernel_filetime = winapi::shared::minwindef::FILETIME::default(); + try_bool!(winapi::um::processthreadsapi::GetProcessTimes( + handle, + &mut _filetime, + &mut _filetime, + &mut kernel_filetime, + &mut user_filetime, + )); + let mut memory_counters = winapi::um::psapi::PROCESS_MEMORY_COUNTERS_EX::default(); + try_bool!(winapi::um::psapi::GetProcessMemoryInfo( + handle as _, + &mut memory_counters as *mut _ as _, + std::mem::size_of::() as u32, + )); + let mut user_time = winapi::um::minwinbase::SYSTEMTIME::default(); + try_bool!(winapi::um::timezoneapi::FileTimeToSystemTime(&user_filetime, &mut user_time)); + let mut kernel_time = winapi::um::minwinbase::SYSTEMTIME::default(); + try_bool!(winapi::um::timezoneapi::FileTimeToSystemTime( + &kernel_filetime, + &mut kernel_time + )); + let maxrss = memory_counters.PeakWorkingSetSize / 1024; + Some(format!( + "user: {USER_SEC}.{USER_USEC:03} \ + sys: {SYS_SEC}.{SYS_USEC:03} \ + max rss (kb): {MAXRSS} \ + page faults: {PAGE_FAULTS}", + USER_SEC = user_time.wSecond + (user_time.wMinute * 60), + USER_USEC = user_time.wMilliseconds, + SYS_SEC = kernel_time.wSecond + (kernel_time.wMinute * 60), + SYS_USEC = kernel_time.wMilliseconds, + MAXRSS = maxrss, + PAGE_FAULTS = memory_counters.PageFaultCount, + )) + } +} + #[cfg(unix)] /// Tries to build a string with human readable data for several of the rusage /// fields. Note that we are focusing mainly on data that we believe to be From 0201e2bbde001aced41352d5437028a915e5556e Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 5 Mar 2021 14:38:52 +0100 Subject: [PATCH 04/20] Add more windows specific numbers --- src/bootstrap/bin/rustc.rs | 70 +++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index fb9f21bfd3c1f..97cea74fc5372 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -229,6 +229,7 @@ fn format_rusage_data() -> Option { #[cfg(windows)] fn format_rusage_data(handle: std::os::windows::raw::HANDLE) -> Option { + use winapi::um::{processthreadsapi, psapi, timezoneapi}; macro_rules! try_bool { ($e:expr) => { if $e != 1 { @@ -236,44 +237,57 @@ fn format_rusage_data(handle: std::os::windows::raw::HANDLE) -> Option { } }; } + + let mut user_filetime = Default::default(); + let mut user_time = Default::default(); + let mut kernel_filetime = Default::default(); + let mut kernel_time = Default::default(); + let mut memory_counters = psapi::PROCESS_MEMORY_COUNTERS::default(); + unsafe { - let mut _filetime = winapi::shared::minwindef::FILETIME::default(); - let mut user_filetime = winapi::shared::minwindef::FILETIME::default(); - let mut kernel_filetime = winapi::shared::minwindef::FILETIME::default(); - try_bool!(winapi::um::processthreadsapi::GetProcessTimes( + try_bool!(processthreadsapi::GetProcessTimes( handle, - &mut _filetime, - &mut _filetime, + &mut Default::default(), + &mut Default::default(), &mut kernel_filetime, &mut user_filetime, )); - let mut memory_counters = winapi::um::psapi::PROCESS_MEMORY_COUNTERS_EX::default(); - try_bool!(winapi::um::psapi::GetProcessMemoryInfo( + try_bool!(timezoneapi::FileTimeToSystemTime(&user_filetime, &mut user_time)); + try_bool!(timezoneapi::FileTimeToSystemTime(&kernel_filetime, &mut kernel_time)); + + // Unlike on Linux with RUSAGE_CHILDREN, this will only return memory information for the process + // with the given handle and none of that process's children. + try_bool!(psapi::GetProcessMemoryInfo( handle as _, &mut memory_counters as *mut _ as _, - std::mem::size_of::() as u32, - )); - let mut user_time = winapi::um::minwinbase::SYSTEMTIME::default(); - try_bool!(winapi::um::timezoneapi::FileTimeToSystemTime(&user_filetime, &mut user_time)); - let mut kernel_time = winapi::um::minwinbase::SYSTEMTIME::default(); - try_bool!(winapi::um::timezoneapi::FileTimeToSystemTime( - &kernel_filetime, - &mut kernel_time + std::mem::size_of::() as u32, )); - let maxrss = memory_counters.PeakWorkingSetSize / 1024; - Some(format!( - "user: {USER_SEC}.{USER_USEC:03} \ + } + + // Guide on interpreting these numbers: + // https://docs.microsoft.com/en-us/windows/win32/psapi/process-memory-usage-information + let peak_working_set = memory_counters.PeakWorkingSetSize / 1024; + let peak_page_file = memory_counters.PeakPagefileUsage / 1024; + let peak_paged_pool = memory_counters.QuotaPeakPagedPoolUsage / 1024; + let peak_nonpaged_pool = memory_counters.QuotaPeakNonPagedPoolUsage / 1024; + Some(format!( + "user: {USER_SEC}.{USER_USEC:03} \ sys: {SYS_SEC}.{SYS_USEC:03} \ - max rss (kb): {MAXRSS} \ + peak working set (kb): {PEAK_WORKING_SET} \ + peak page file usage (kb): {PEAK_PAGE_FILE} \ + peak paged pool usage (kb): {PEAK_PAGED_POOL} \ + peak non-paged pool usage (kb): {PEAK_NONPAGED_POOL} \ page faults: {PAGE_FAULTS}", - USER_SEC = user_time.wSecond + (user_time.wMinute * 60), - USER_USEC = user_time.wMilliseconds, - SYS_SEC = kernel_time.wSecond + (kernel_time.wMinute * 60), - SYS_USEC = kernel_time.wMilliseconds, - MAXRSS = maxrss, - PAGE_FAULTS = memory_counters.PageFaultCount, - )) - } + USER_SEC = user_time.wSecond + (user_time.wMinute * 60), + USER_USEC = user_time.wMilliseconds, + SYS_SEC = kernel_time.wSecond + (kernel_time.wMinute * 60), + SYS_USEC = kernel_time.wMilliseconds, + PEAK_WORKING_SET = peak_working_set, + PEAK_PAGE_FILE = peak_page_file, + PEAK_PAGED_POOL = peak_paged_pool, + PEAK_NONPAGED_POOL = peak_nonpaged_pool, + PAGE_FAULTS = memory_counters.PageFaultCount, + )) } #[cfg(unix)] From a2571cfc8b6f5cf5d8d2e7075ac4809aceae9541 Mon Sep 17 00:00:00 2001 From: Josh Cotton Date: Fri, 5 Mar 2021 11:27:58 -0500 Subject: [PATCH 05/20] Implement String::remove_matches --- library/alloc/src/string.rs | 56 +++++++++++++++++++++++++++++++++++ library/alloc/tests/lib.rs | 1 + library/alloc/tests/string.rs | 27 +++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index b567d0a2fe2d9..45adebf77cbf6 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -1202,6 +1202,62 @@ impl String { ch } + /// Remove all matches of pattern `pat` in the `String`. + /// + /// # Examples + /// + /// ``` + /// #![feature(string_remove_matches)] + /// let mut s = String::from("Trees are not green, the sky is not blue."); + /// s.remove_matches("not "); + /// assert_eq!("Trees are green, the sky is blue.", s); + /// ``` + /// + /// Matches will be detected and removed iteratively, so in cases where + /// patterns overlap, only the first pattern will be removed: + /// + /// ``` + /// #![feature(string_remove_matches)] + /// let mut s = String::from("banana"); + /// s.remove_matches("ana"); + /// assert_eq!("bna", s); + /// ``` + #[unstable(feature = "string_remove_matches", reason = "new API", issue = "72826")] + pub fn remove_matches<'a, P>(&'a mut self, pat: P) + where + P: for<'x> Pattern<'x>, + { + use core::str::pattern::Searcher; + + let matches = { + let mut searcher = pat.into_searcher(self); + let mut matches = Vec::new(); + + while let Some(m) = searcher.next_match() { + matches.push(m); + } + + matches + }; + + let len = self.len(); + let mut shrunk_by = 0; + + // SAFETY: start and end will be on utf8 byte boundaries per + // the Searcher docs + unsafe { + for (start, end) in matches { + ptr::copy( + self.vec.as_mut_ptr().add(end - shrunk_by), + self.vec.as_mut_ptr().add(start - shrunk_by), + len - end, + ); + shrunk_by += end - start; + } + self.vec.set_len(len - shrunk_by); + } + } + /// Retains only the characters specified by the predicate. /// /// In other words, remove all characters `c` such that `f(c)` returns `false`. diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index 799499b9b771f..feeb17e87da8b 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -21,6 +21,7 @@ #![feature(slice_group_by)] #![feature(vec_extend_from_within)] #![feature(vec_spare_capacity)] +#![feature(string_remove_matches)] use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; diff --git a/library/alloc/tests/string.rs b/library/alloc/tests/string.rs index f3d74e0514d29..9ec0ee97ab926 100644 --- a/library/alloc/tests/string.rs +++ b/library/alloc/tests/string.rs @@ -365,6 +365,33 @@ fn remove_bad() { "ศ".to_string().remove(1); } +#[test] +fn test_remove_matches() { + let mut s = "abc".to_string(); + + s.remove_matches('b'); + assert_eq!(s, "ac"); + s.remove_matches('b'); + assert_eq!(s, "ac"); + + let mut s = "abcb".to_string(); + + s.remove_matches('b'); + assert_eq!(s, "ac"); + + let mut s = "ศไทย中华Việt Nam; foobarศ".to_string(); + s.remove_matches('ศ'); + assert_eq!(s, "ไทย中华Việt Nam; foobar"); + + let mut s = "".to_string(); + s.remove_matches(""); + assert_eq!(s, ""); + + let mut s = "aaaaa".to_string(); + s.remove_matches('a'); + assert_eq!(s, ""); +} + #[test] fn test_retain() { let mut s = String::from("α_β_γ"); From 302867cf48db284cc666fff7c2953f6f94f30aac Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 11 Mar 2021 16:23:25 +0100 Subject: [PATCH 06/20] Clean up handling of child process --- src/bootstrap/bin/rustc.rs | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index 97cea74fc5372..269f810192eb5 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -17,7 +17,7 @@ use std::env; use std::path::PathBuf; -use std::process::Command; +use std::process::{Child, Command}; use std::str::FromStr; use std::time::Instant; @@ -171,19 +171,8 @@ fn main() { let is_test = args.iter().any(|a| a == "--test"); // If the user requested resource usage data, then // include that in addition to the timing output. - let rusage_data = env::var_os("RUSTC_PRINT_STEP_RUSAGE").and_then(|_| { - #[cfg(windows)] - { - use std::os::windows::io::AsRawHandle; - let handle = child.as_raw_handle(); - format_rusage_data(handle) - } - #[cfg(not(windows))] - { - let _child = child; - format_rusage_data() - } - }); + let rusage_data = + env::var_os("RUSTC_PRINT_STEP_RUSAGE").and_then(|_| format_rusage_data(child)); eprintln!( "[RUSTC-TIMING] {} test:{} {}.{:03}{}{}", crate_name, @@ -221,15 +210,16 @@ fn main() { } #[cfg(all(not(unix), not(windows)))] -/// getrusage is not available on non-unix platforms. So for now, we do not -/// bother trying to make a shim for it. -fn format_rusage_data() -> Option { +// In the future we can add this for more platforms +fn format_rusage_data(_child: Child) -> Option { None } #[cfg(windows)] -fn format_rusage_data(handle: std::os::windows::raw::HANDLE) -> Option { +fn format_rusage_data(child: Child) -> Option { + use std::os::windows::io::AsRawHandle; use winapi::um::{processthreadsapi, psapi, timezoneapi}; + let handle = child.as_raw_handle(); macro_rules! try_bool { ($e:expr) => { if $e != 1 { @@ -295,7 +285,7 @@ fn format_rusage_data(handle: std::os::windows::raw::HANDLE) -> Option { /// fields. Note that we are focusing mainly on data that we believe to be /// supplied on Linux (the `rusage` struct has other fields in it but they are /// currently unsupported by Linux). -fn format_rusage_data() -> Option { +fn format_rusage_data(_child: Child) -> Option { let rusage: libc::rusage = unsafe { let mut recv = std::mem::zeroed(); // -1 is RUSAGE_CHILDREN, which means to get the rusage for all children From 5e788f2fa0532cff429b2574a61e348e0bc52e0a Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 15 Sep 2020 23:35:08 -0400 Subject: [PATCH 07/20] Add Linux-specific pidfd process extensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Background: Over the last year, pidfd support was added to the Linux kernel. This allows interacting with other processes. In particular, this allows waiting on a child process with a timeout in a race-free way, bypassing all of the awful signal-handler tricks that are usually required. Pidfds can be obtained for a child process (as well as any other process) via the `pidfd_open` syscall. Unfortunately, this requires several conditions to hold in order to be race-free (i.e. the pid is not reused). Per `man pidfd_open`: ``` · the disposition of SIGCHLD has not been explicitly set to SIG_IGN (see sigaction(2)); · the SA_NOCLDWAIT flag was not specified while establishing a han‐ dler for SIGCHLD or while setting the disposition of that signal to SIG_DFL (see sigaction(2)); and · the zombie process was not reaped elsewhere in the program (e.g., either by an asynchronously executed signal handler or by wait(2) or similar in another thread). If any of these conditions does not hold, then the child process (along with a PID file descriptor that refers to it) should instead be created using clone(2) with the CLONE_PIDFD flag. ``` Sadly, these conditions are impossible to guarantee once any libraries are used. For example, C code runnng in a different thread could call `wait()`, which is impossible to detect from Rust code trying to open a pidfd. While pid reuse issues should (hopefully) be rare in practice, we can do better. By passing the `CLONE_PIDFD` flag to `clone()` or `clone3()`, we can obtain a pidfd for the child process in a guaranteed race-free manner. This PR: This PR adds Linux-specific process extension methods to allow obtaining pidfds for processes spawned via the standard `Command` API. Other than being made available to user code, the standard library does not make use of these pidfds in any way. In particular, the implementation of `Child::wait` is completely unchanged. Two Linux-specific helper methods are added: `CommandExt::create_pidfd` and `ChildExt::pidfd`. These methods are intended to serve as a building block for libraries to build higher-level abstractions - in particular, waiting on a process with a timeout. I've included a basic test, which verifies that pidfds are created iff the `create_pidfd` method is used. This test is somewhat special - it should always succeed on systems with the `clone3` system call available, and always fail on systems without `clone3` available. I'm not sure how to best ensure this programatically. This PR relies on the newer `clone3` system call to pass the `CLONE_FD`, rather than the older `clone` system call. `clone3` was added to Linux in the same release as pidfds, so this shouldn't unnecessarily limit the kernel versions that this code supports. Unresolved questions: * What should the name of the feature gate be for these newly added methods? * Should the `pidfd` method distinguish between an error occurring and `create_pidfd` not being called? --- library/std/src/os/linux/mod.rs | 1 + library/std/src/os/linux/process.rs | 47 ++++++++ library/std/src/process.rs | 2 +- .../src/sys/unix/process/process_common.rs | 6 + .../std/src/sys/unix/process/process_unix.rs | 109 ++++++++++++++++-- src/test/ui/command/command-create-pidfd.rs | 27 +++++ 6 files changed, 184 insertions(+), 8 deletions(-) create mode 100644 library/std/src/os/linux/process.rs create mode 100644 src/test/ui/command/command-create-pidfd.rs diff --git a/library/std/src/os/linux/mod.rs b/library/std/src/os/linux/mod.rs index f179a524336fc..f7bb63df0c1ff 100644 --- a/library/std/src/os/linux/mod.rs +++ b/library/std/src/os/linux/mod.rs @@ -3,4 +3,5 @@ #![stable(feature = "raw_ext", since = "1.1.0")] pub mod fs; +pub mod process; pub mod raw; diff --git a/library/std/src/os/linux/process.rs b/library/std/src/os/linux/process.rs new file mode 100644 index 0000000000000..661d3cef7a03a --- /dev/null +++ b/library/std/src/os/linux/process.rs @@ -0,0 +1,47 @@ +//! Linux-specific extensions to primitives in the `std::process` module. + +#![unstable(feature = "linux_pidfd", issue = "none")] + +use crate::process; +use crate::sys_common::AsInnerMut; +use crate::io::Result; + +/// Os-specific extensions to [`process::Child`] +/// +/// [`process::Child`]: crate::process::Child +pub trait ChildExt { + /// Obtains the pidfd created for this child process, if available. + /// + /// A pidfd will only ever be available if `create_pidfd(true)` was called + /// when the corresponding `Command` was created. + /// + /// Even if `create_pidfd(true)` is called, a pidfd may not be available + /// due to an older version of Linux being in use, or if + /// some other error occured. + /// + /// See `man pidfd_open` for more details about pidfds. + fn pidfd(&self) -> Result; +} + +/// Os-specific extensions to [`process::Command`] +/// +/// [`process::Command`]: crate::process::Command +pub trait CommandExt { + /// Sets whether or this `Command` will attempt to create a pidfd + /// for the child. If this method is never called, a pidfd will + /// not be crated. + /// + /// The pidfd can be retrieved from the child via [`ChildExt::pidfd`] + /// + /// A pidfd will only be created if it is possible to do so + /// in a guaranteed race-free manner (e.g. if the `clone3` system call is + /// supported). Otherwise, [`ChildExit::pidfd`] will return an error. + fn create_pidfd(&mut self, val: bool) -> &mut process::Command; +} + +impl CommandExt for process::Command { + fn create_pidfd(&mut self, val: bool) -> &mut process::Command { + self.as_inner_mut().create_pidfd(val); + self + } +} diff --git a/library/std/src/process.rs b/library/std/src/process.rs index f9cfd11e90650..940029adfcf69 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -165,7 +165,7 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; /// [`wait`]: Child::wait #[stable(feature = "process", since = "1.0.0")] pub struct Child { - handle: imp::Process, + pub(crate) handle: imp::Process, /// The handle for writing to the child's standard input (stdin), if it has /// been captured. To avoid partially moving diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index b9dcc4e4b9e38..38a5915e62b97 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -79,6 +79,7 @@ pub struct Command { stdin: Option, stdout: Option, stderr: Option, + pub(crate) make_pidfd: bool, } // Create a new type for argv, so that we can make it `Send` and `Sync` @@ -141,6 +142,7 @@ impl Command { stdin: None, stdout: None, stderr: None, + make_pidfd: false, } } @@ -176,6 +178,10 @@ impl Command { pub fn groups(&mut self, groups: &[gid_t]) { self.groups = Some(Box::from(groups)); } + + pub fn create_pidfd(&mut self, val: bool) { + self.make_pidfd = val; + } pub fn saw_nul(&self) -> bool { self.saw_nul diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 47aaca82af946..ad7567965cb40 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -6,6 +6,7 @@ use crate::ptr; use crate::sys; use crate::sys::cvt; use crate::sys::process::process_common::*; +use crate::sync::atomic::{AtomicBool, Ordering}; #[cfg(target_os = "vxworks")] use libc::RTP_ID as pid_t; @@ -48,7 +49,8 @@ impl Command { // a lock any more because the parent won't do anything and the child is // in its own process. Thus the parent drops the lock guard while the child // forgets it to avoid unlocking it on a new thread, which would be invalid. - let (env_lock, result) = unsafe { (sys::os::env_read_lock(), cvt(libc::fork())?) }; + let env_lock = sys::os::env_read_lock(); + let (result, pidfd) = self.do_fork()?; let pid = unsafe { match result { @@ -76,12 +78,12 @@ impl Command { } n => { drop(env_lock); - n + n as pid_t } } }; - let mut p = Process { pid, status: None }; + let mut p = Process { pid, status: None, pidfd }; drop(output); let mut bytes = [0; 8]; @@ -114,6 +116,85 @@ impl Command { } } + // Attempts to fork the process. If successful, returns + // Ok((0, -1)) in the child, and Ok((child_pid, child_pidfd)) in the parent. + fn do_fork(&mut self) -> Result<(libc::c_long, libc::pid_t), io::Error> { + // If we fail to create a pidfd for any reason, this will + // stay as -1, which indicates an error + let mut pidfd: libc::pid_t = -1; + + // On Linux, attempt to use the `clone3` syscall, which + // supports more argument (in prarticular, the ability to create a pidfd). + // If this fails, we will fall through this block to a call to `fork()` + cfg_if::cfg_if! { + if #[cfg(target_os = "linux")] { + static HAS_CLONE3: AtomicBool = AtomicBool::new(true); + + const CLONE_PIDFD: u64 = 0x00001000; + + #[repr(C)] + struct clone_args { + flags: u64, + pidfd: u64, + child_tid: u64, + parent_tid: u64, + exit_signal: u64, + stack: u64, + stack_size: u64, + tls: u64, + set_tid: u64, + set_tid_size: u64, + cgroup: u64, + } + + syscall! { + fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long + } + + if HAS_CLONE3.load(Ordering::Relaxed) { + let mut flags = 0; + if self.make_pidfd { + flags |= CLONE_PIDFD; + } + + let mut args = clone_args { + flags, + pidfd: &mut pidfd as *mut libc::pid_t as u64, + child_tid: 0, + parent_tid: 0, + exit_signal: libc::SIGCHLD as u64, + stack: 0, + stack_size: 0, + tls: 0, + set_tid: 0, + set_tid_size: 0, + cgroup: 0 + }; + + let args_ptr = &mut args as *mut clone_args; + let args_size = crate::mem::size_of::(); + + let res = cvt(unsafe { clone3(args_ptr, args_size) }); + match res { + Ok(n) => return Ok((n, pidfd)), + Err(e) => match e.raw_os_error() { + // Multiple threads can race to execute this store, + // but that's fine - that just means that multiple threads + // will have tried and failed to execute the same syscall, + // with no other side effects. + Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), + _ => return Err(e) + } + } + } + } + } + // If we get here, we are either not on Linux, + // or we are on Linux and the 'clone3' syscall does not exist + cvt(unsafe { libc::fork() }.into()).map(|res| (res, pidfd)) + } + + pub fn exec(&mut self, default: Stdio) -> io::Error { let envp = self.capture_env(); @@ -265,8 +346,6 @@ impl Command { #[cfg(not(any( target_os = "macos", target_os = "freebsd", - all(target_os = "linux", target_env = "gnu"), - all(target_os = "linux", target_env = "musl"), )))] fn posix_spawn( &mut self, @@ -281,8 +360,6 @@ impl Command { #[cfg(any( target_os = "macos", target_os = "freebsd", - all(target_os = "linux", target_env = "gnu"), - all(target_os = "linux", target_env = "musl"), ))] fn posix_spawn( &mut self, @@ -430,6 +507,12 @@ impl Command { pub struct Process { pid: pid_t, status: Option, + // On Linux, stores the pidfd created for this child. + // This is -1 if the user did not request pidfd creation, + // or if the pidfd could not be created for some reason + // (e.g. the `clone3` syscall was not available). + #[cfg(target_os = "linux")] + pidfd: libc::c_int, } impl Process { @@ -546,6 +629,18 @@ impl fmt::Display for ExitStatus { } } +#[cfg(target_os = "linux")] +#[unstable(feature = "linux_pidfd", issue = "none")] +impl crate::os::linux::process::ChildExt for crate::process::Child { + fn pidfd(&self) -> crate::io::Result { + if self.handle.pidfd > 0 { + Ok(self.handle.pidfd) + } else { + Err(crate::io::Error::from(crate::io::ErrorKind::Other)) + } + } +} + #[cfg(test)] #[path = "process_unix/tests.rs"] mod tests; diff --git a/src/test/ui/command/command-create-pidfd.rs b/src/test/ui/command/command-create-pidfd.rs new file mode 100644 index 0000000000000..248ae3457d715 --- /dev/null +++ b/src/test/ui/command/command-create-pidfd.rs @@ -0,0 +1,27 @@ +// run-pass +// linux-only - pidfds are a linux-specific concept + +#![feature(linux_pidfd)] +use std::os::linux::process::{CommandExt, ChildExt}; +use std::process::Command; + +fn main() { + // We don't assert the precise value, since the standard libarary + // may be opened other file descriptors before our code ran. + let _ = Command::new("echo") + .create_pidfd(true) + .spawn() + .unwrap() + .pidfd().expect("failed to obtain pidfd"); + + let _ = Command::new("echo") + .create_pidfd(false) + .spawn() + .unwrap() + .pidfd().expect_err("pidfd should not have been created when create_pid(false) is set"); + + let _ = Command::new("echo") + .spawn() + .unwrap() + .pidfd().expect_err("pidfd should not have been created"); +} From 3426bf746e95e5e396f62dcec8433f5708124935 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 17 Oct 2020 20:10:58 -0700 Subject: [PATCH 08/20] Typo fix Co-authored-by: bjorn3 --- library/std/src/sys/unix/process/process_unix.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index ad7567965cb40..5af9f8769b5c4 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -124,7 +124,7 @@ impl Command { let mut pidfd: libc::pid_t = -1; // On Linux, attempt to use the `clone3` syscall, which - // supports more argument (in prarticular, the ability to create a pidfd). + // supports more argument (in particular, the ability to create a pidfd). // If this fails, we will fall through this block to a call to `fork()` cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { From 704d6c5de3bdc7127112c07f1da01a15aaef23e4 Mon Sep 17 00:00:00 2001 From: Dominik Stolz Date: Sat, 6 Feb 2021 14:15:49 +0100 Subject: [PATCH 09/20] Add PidFd type and seal traits Improve docs --- library/std/src/os/linux/process.rs | 146 +++++++++++--- .../src/sys/unix/process/process_common.rs | 8 +- .../std/src/sys/unix/process/process_unix.rs | 178 ++++++++++-------- src/test/ui/command/command-create-pidfd.rs | 4 +- 4 files changed, 233 insertions(+), 103 deletions(-) diff --git a/library/std/src/os/linux/process.rs b/library/std/src/os/linux/process.rs index 661d3cef7a03a..435c0227c7eca 100644 --- a/library/std/src/os/linux/process.rs +++ b/library/std/src/os/linux/process.rs @@ -2,40 +2,142 @@ #![unstable(feature = "linux_pidfd", issue = "none")] -use crate::process; -use crate::sys_common::AsInnerMut; use crate::io::Result; +use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; +use crate::process; +use crate::sys::fd::FileDesc; +use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; -/// Os-specific extensions to [`process::Child`] +/// This type represents a file descriptor that refers to a process. +/// +/// A `PidFd` can be obtained by setting the corresponding option on [`Command`] +/// with [`create_pidfd`]. Subsequently, the created pidfd can be retrieved +/// from the [`Child`] by calling [`pidfd`] or [`take_pidfd`]. +/// +/// Example: +/// ```no_run +/// #![feature(linux_pidfd)] +/// use std::os::linux::process::{CommandExt, ChildExt}; +/// use std::process::Command; +/// +/// let mut child = Command::new("echo") +/// .create_pidfd(true) +/// .spawn() +/// .expect("Failed to spawn child"); /// -/// [`process::Child`]: crate::process::Child -pub trait ChildExt { - /// Obtains the pidfd created for this child process, if available. +/// let pidfd = child +/// .take_pidfd() +/// .expect("Failed to retrieve pidfd"); +/// +/// // The file descriptor will be closed when `pidfd` is dropped. +/// ``` +/// Refer to the man page of `pidfd_open(2)` for further details. +/// +/// [`Command`]: process::Command +/// [`create_pidfd`]: CommandExt::create_pidfd +/// [`Child`]: process::Child +/// [`pidfd`]: fn@ChildExt::pidfd +/// [`take_pidfd`]: ChildExt::take_pidfd +#[derive(Debug)] +pub struct PidFd { + inner: FileDesc, +} + +impl AsInner for PidFd { + fn as_inner(&self) -> &FileDesc { + &self.inner + } +} + +impl FromInner for PidFd { + fn from_inner(inner: FileDesc) -> PidFd { + PidFd { inner } + } +} + +impl IntoInner for PidFd { + fn into_inner(self) -> FileDesc { + self.inner + } +} + +impl AsRawFd for PidFd { + fn as_raw_fd(&self) -> RawFd { + self.as_inner().raw() + } +} + +impl FromRawFd for PidFd { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + Self::from_inner(FileDesc::new(fd)) + } +} + +impl IntoRawFd for PidFd { + fn into_raw_fd(self) -> RawFd { + self.into_inner().into_raw() + } +} + +mod private_child_ext { + pub trait Sealed {} + impl Sealed for crate::process::Child {} +} + +/// Os-specific extensions for [`Child`] +/// +/// [`Child`]: process::Child +pub trait ChildExt: private_child_ext::Sealed { + /// Obtains a reference to the [`PidFd`] created for this [`Child`], if available. + /// + /// A pidfd will only be available if its creation was requested with + /// [`create_pidfd`] when the corresponding [`Command`] was created. /// - /// A pidfd will only ever be available if `create_pidfd(true)` was called - /// when the corresponding `Command` was created. + /// Even if requested, a pidfd may not be available due to an older + /// version of Linux being in use, or if some other error occurred. + /// + /// [`Command`]: process::Command + /// [`create_pidfd`]: CommandExt::create_pidfd + /// [`Child`]: process::Child + fn pidfd(&self) -> Result<&PidFd>; + + /// Takes ownership of the [`PidFd`] created for this [`Child`], if available. /// - /// Even if `create_pidfd(true)` is called, a pidfd may not be available - /// due to an older version of Linux being in use, or if - /// some other error occured. + /// A pidfd will only be available if its creation was requested with + /// [`create_pidfd`] when the corresponding [`Command`] was created. /// - /// See `man pidfd_open` for more details about pidfds. - fn pidfd(&self) -> Result; + /// Even if requested, a pidfd may not be available due to an older + /// version of Linux being in use, or if some other error occurred. + /// + /// [`Command`]: process::Command + /// [`create_pidfd`]: CommandExt::create_pidfd + /// [`Child`]: process::Child + fn take_pidfd(&mut self) -> Result; +} + +mod private_command_ext { + pub trait Sealed {} + impl Sealed for crate::process::Command {} } -/// Os-specific extensions to [`process::Command`] +/// Os-specific extensions for [`Command`] /// -/// [`process::Command`]: crate::process::Command -pub trait CommandExt { - /// Sets whether or this `Command` will attempt to create a pidfd - /// for the child. If this method is never called, a pidfd will - /// not be crated. +/// [`Command`]: process::Command +pub trait CommandExt: private_command_ext::Sealed { + /// Sets whether a [`PidFd`](struct@PidFd) should be created for the [`Child`] + /// spawned by this [`Command`]. + /// By default, no pidfd will be created. /// - /// The pidfd can be retrieved from the child via [`ChildExt::pidfd`] + /// The pidfd can be retrieved from the child with [`pidfd`] or [`take_pidfd`]. /// /// A pidfd will only be created if it is possible to do so - /// in a guaranteed race-free manner (e.g. if the `clone3` system call is - /// supported). Otherwise, [`ChildExit::pidfd`] will return an error. + /// in a guaranteed race-free manner (e.g. if the `clone3` system call + /// is supported). Otherwise, [`pidfd`] will return an error. + /// + /// [`Command`]: process::Command + /// [`Child`]: process::Child + /// [`pidfd`]: fn@ChildExt::pidfd + /// [`take_pidfd`]: ChildExt::take_pidfd fn create_pidfd(&mut self, val: bool) -> &mut process::Command; } diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index 38a5915e62b97..9f6e120134a95 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -79,7 +79,7 @@ pub struct Command { stdin: Option, stdout: Option, stderr: Option, - pub(crate) make_pidfd: bool, + pub(crate) create_pidfd: bool, } // Create a new type for argv, so that we can make it `Send` and `Sync` @@ -142,7 +142,7 @@ impl Command { stdin: None, stdout: None, stderr: None, - make_pidfd: false, + create_pidfd: false, } } @@ -178,9 +178,9 @@ impl Command { pub fn groups(&mut self, groups: &[gid_t]) { self.groups = Some(Box::from(groups)); } - + pub fn create_pidfd(&mut self, val: bool) { - self.make_pidfd = val; + self.create_pidfd = val; } pub fn saw_nul(&self) -> bool { diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 5af9f8769b5c4..a95c595dc39a9 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -3,16 +3,20 @@ use crate::fmt; use crate::io::{self, Error, ErrorKind}; use crate::mem; use crate::ptr; +use crate::sync::atomic::{AtomicBool, Ordering}; use crate::sys; use crate::sys::cvt; use crate::sys::process::process_common::*; -use crate::sync::atomic::{AtomicBool, Ordering}; +use crate::sys_common::FromInner; + +#[cfg(target_os = "linux")] +use crate::os::linux::process::PidFd; #[cfg(target_os = "vxworks")] use libc::RTP_ID as pid_t; #[cfg(not(target_os = "vxworks"))] -use libc::{c_int, gid_t, pid_t, uid_t}; +use libc::{c_int, c_long, gid_t, pid_t, uid_t}; //////////////////////////////////////////////////////////////////////////////// // Command @@ -78,12 +82,12 @@ impl Command { } n => { drop(env_lock); - n as pid_t + n } } }; - let mut p = Process { pid, status: None, pidfd }; + let mut p = Process::new(pid, pidfd); drop(output); let mut bytes = [0; 8]; @@ -118,83 +122,85 @@ impl Command { // Attempts to fork the process. If successful, returns // Ok((0, -1)) in the child, and Ok((child_pid, child_pidfd)) in the parent. - fn do_fork(&mut self) -> Result<(libc::c_long, libc::pid_t), io::Error> { + fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { // If we fail to create a pidfd for any reason, this will // stay as -1, which indicates an error - let mut pidfd: libc::pid_t = -1; + let mut pidfd: pid_t = -1; // On Linux, attempt to use the `clone3` syscall, which - // supports more argument (in particular, the ability to create a pidfd). + // supports more arguments (in particular, the ability to create a pidfd). // If this fails, we will fall through this block to a call to `fork()` - cfg_if::cfg_if! { - if #[cfg(target_os = "linux")] { - static HAS_CLONE3: AtomicBool = AtomicBool::new(true); - - const CLONE_PIDFD: u64 = 0x00001000; - - #[repr(C)] - struct clone_args { - flags: u64, - pidfd: u64, - child_tid: u64, - parent_tid: u64, - exit_signal: u64, - stack: u64, - stack_size: u64, - tls: u64, - set_tid: u64, - set_tid_size: u64, - cgroup: u64, - } + #[cfg(target_os = "linux")] + { + static HAS_CLONE3: AtomicBool = AtomicBool::new(true); + + const CLONE_PIDFD: u64 = 0x00001000; + + #[repr(C)] + struct clone_args { + flags: u64, + pidfd: u64, + child_tid: u64, + parent_tid: u64, + exit_signal: u64, + stack: u64, + stack_size: u64, + tls: u64, + set_tid: u64, + set_tid_size: u64, + cgroup: u64, + } - syscall! { - fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long - } + syscall! { + fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> c_long + } - if HAS_CLONE3.load(Ordering::Relaxed) { - let mut flags = 0; - if self.make_pidfd { - flags |= CLONE_PIDFD; - } + if HAS_CLONE3.load(Ordering::Relaxed) { + let mut flags = 0; + if self.create_pidfd { + flags |= CLONE_PIDFD; + } - let mut args = clone_args { - flags, - pidfd: &mut pidfd as *mut libc::pid_t as u64, - child_tid: 0, - parent_tid: 0, - exit_signal: libc::SIGCHLD as u64, - stack: 0, - stack_size: 0, - tls: 0, - set_tid: 0, - set_tid_size: 0, - cgroup: 0 - }; - - let args_ptr = &mut args as *mut clone_args; - let args_size = crate::mem::size_of::(); - - let res = cvt(unsafe { clone3(args_ptr, args_size) }); - match res { - Ok(n) => return Ok((n, pidfd)), - Err(e) => match e.raw_os_error() { - // Multiple threads can race to execute this store, - // but that's fine - that just means that multiple threads - // will have tried and failed to execute the same syscall, - // with no other side effects. - Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), - _ => return Err(e) - } - } + let mut args = clone_args { + flags, + pidfd: &mut pidfd as *mut pid_t as u64, + child_tid: 0, + parent_tid: 0, + exit_signal: libc::SIGCHLD as u64, + stack: 0, + stack_size: 0, + tls: 0, + set_tid: 0, + set_tid_size: 0, + cgroup: 0, + }; + + let args_ptr = &mut args as *mut clone_args; + let args_size = crate::mem::size_of::(); + + let res = cvt(unsafe { clone3(args_ptr, args_size) }); + match res { + Ok(n) => return Ok((n as pid_t, pidfd)), + Err(e) => match e.raw_os_error() { + // Multiple threads can race to execute this store, + // but that's fine - that just means that multiple threads + // will have tried and failed to execute the same syscall, + // with no other side effects. + Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), + // Fallback to fork if `EPERM` is returned. (e.g. blocked by seccomp) + Some(libc::EPERM) => {} + _ => return Err(e), + }, } } } + // If we get here, we are either not on Linux, // or we are on Linux and the 'clone3' syscall does not exist - cvt(unsafe { libc::fork() }.into()).map(|res| (res, pidfd)) + // or we do not have permission to call it + cvt(unsafe { libc::fork() }).map(|res| (res, pidfd)) } - pub fn exec(&mut self, default: Stdio) -> io::Error { let envp = self.capture_env(); @@ -346,6 +352,8 @@ impl Command { #[cfg(not(any( target_os = "macos", target_os = "freebsd", + all(target_os = "linux", target_env = "gnu"), + all(target_os = "linux", target_env = "musl"), )))] fn posix_spawn( &mut self, @@ -360,6 +368,8 @@ impl Command { #[cfg(any( target_os = "macos", target_os = "freebsd", + all(target_os = "linux", target_env = "gnu"), + all(target_os = "linux", target_env = "musl"), ))] fn posix_spawn( &mut self, @@ -374,6 +384,7 @@ impl Command { || (self.env_saw_path() && !self.program_is_path()) || !self.get_closures().is_empty() || self.get_groups().is_some() + || self.create_pidfd { return Ok(None); } @@ -418,7 +429,7 @@ impl Command { None => None, }; - let mut p = Process { pid: 0, status: None }; + let mut p = Process::new(0, -1); struct PosixSpawnFileActions<'a>(&'a mut MaybeUninit); @@ -508,14 +519,25 @@ pub struct Process { pid: pid_t, status: Option, // On Linux, stores the pidfd created for this child. - // This is -1 if the user did not request pidfd creation, + // This is None if the user did not request pidfd creation, // or if the pidfd could not be created for some reason // (e.g. the `clone3` syscall was not available). #[cfg(target_os = "linux")] - pidfd: libc::c_int, + pidfd: Option, } impl Process { + #[cfg(target_os = "linux")] + fn new(pid: pid_t, pidfd: pid_t) -> Self { + let pidfd = (pidfd >= 0).then(|| PidFd::from_inner(sys::fd::FileDesc::new(pidfd))); + Process { pid, status: None, pidfd } + } + + #[cfg(not(target_os = "linux"))] + fn new(pid: pid_t, _pidfd: pid_t) -> Self { + Process { pid, status: None } + } + pub fn id(&self) -> u32 { self.pid as u32 } @@ -632,12 +654,18 @@ impl fmt::Display for ExitStatus { #[cfg(target_os = "linux")] #[unstable(feature = "linux_pidfd", issue = "none")] impl crate::os::linux::process::ChildExt for crate::process::Child { - fn pidfd(&self) -> crate::io::Result { - if self.handle.pidfd > 0 { - Ok(self.handle.pidfd) - } else { - Err(crate::io::Error::from(crate::io::ErrorKind::Other)) - } + fn pidfd(&self) -> io::Result<&PidFd> { + self.handle + .pidfd + .as_ref() + .ok_or_else(|| Error::new(ErrorKind::Other, "No pidfd was created.")) + } + + fn take_pidfd(&mut self) -> io::Result { + self.handle + .pidfd + .take() + .ok_or_else(|| Error::new(ErrorKind::Other, "No pidfd was created.")) } } diff --git a/src/test/ui/command/command-create-pidfd.rs b/src/test/ui/command/command-create-pidfd.rs index 248ae3457d715..2e08eb1be540a 100644 --- a/src/test/ui/command/command-create-pidfd.rs +++ b/src/test/ui/command/command-create-pidfd.rs @@ -6,8 +6,8 @@ use std::os::linux::process::{CommandExt, ChildExt}; use std::process::Command; fn main() { - // We don't assert the precise value, since the standard libarary - // may be opened other file descriptors before our code ran. + // We don't assert the precise value, since the standard library + // might have opened other file descriptors before our code runs. let _ = Command::new("echo") .create_pidfd(true) .spawn() From 766cc259cc84e6036014944df352cbda2297f993 Mon Sep 17 00:00:00 2001 From: Dominik Stolz Date: Wed, 10 Mar 2021 11:21:01 +0100 Subject: [PATCH 10/20] Add tracking issue and link to man-page --- library/std/src/os/linux/process.rs | 5 +++-- library/std/src/sys/unix/process/process_unix.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/library/std/src/os/linux/process.rs b/library/std/src/os/linux/process.rs index 435c0227c7eca..17b9ae82bce4d 100644 --- a/library/std/src/os/linux/process.rs +++ b/library/std/src/os/linux/process.rs @@ -1,6 +1,6 @@ //! Linux-specific extensions to primitives in the `std::process` module. -#![unstable(feature = "linux_pidfd", issue = "none")] +#![unstable(feature = "linux_pidfd", issue = "82971")] use crate::io::Result; use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; @@ -31,13 +31,14 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; /// /// // The file descriptor will be closed when `pidfd` is dropped. /// ``` -/// Refer to the man page of `pidfd_open(2)` for further details. +/// Refer to the man page of [`pidfd_open(2)`] for further details. /// /// [`Command`]: process::Command /// [`create_pidfd`]: CommandExt::create_pidfd /// [`Child`]: process::Child /// [`pidfd`]: fn@ChildExt::pidfd /// [`take_pidfd`]: ChildExt::take_pidfd +/// [`pidfd_open(2)`]: https://man7.org/linux/man-pages/man2/pidfd_open.2.html #[derive(Debug)] pub struct PidFd { inner: FileDesc, diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index a95c595dc39a9..c6f8c446a0bd8 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -652,7 +652,7 @@ impl fmt::Display for ExitStatus { } #[cfg(target_os = "linux")] -#[unstable(feature = "linux_pidfd", issue = "none")] +#[unstable(feature = "linux_pidfd", issue = "82971")] impl crate::os::linux::process::ChildExt for crate::process::Child { fn pidfd(&self) -> io::Result<&PidFd> { self.handle From d850f6b3746aee0505e21288cbef209050d118d0 Mon Sep 17 00:00:00 2001 From: Dominik Stolz Date: Mon, 15 Mar 2021 21:17:23 +0100 Subject: [PATCH 11/20] Update libc dependency to 0.2.89 --- Cargo.lock | 4 ++-- library/std/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 203d8acb5b470..779ab7cd401a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1889,9 +1889,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.88" +version = "0.2.89" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03b07a082330a35e43f63177cc01689da34fbffa0105e1246cf0311472cac73a" +checksum = "538c092e5586f4cdd7dd8078c4a79220e3e168880218124dcbce860f0ea938c6" dependencies = [ "rustc-std-workspace-core", ] diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index f0f5558fd1608..f84c5edef0fd0 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -16,7 +16,7 @@ cfg-if = { version = "0.1.8", features = ['rustc-dep-of-std'] } panic_unwind = { path = "../panic_unwind", optional = true } panic_abort = { path = "../panic_abort" } core = { path = "../core" } -libc = { version = "0.2.88", default-features = false, features = ['rustc-dep-of-std'] } +libc = { version = "0.2.89", default-features = false, features = ['rustc-dep-of-std'] } compiler_builtins = { version = "0.1.39" } profiler_builtins = { path = "../profiler_builtins", optional = true } unwind = { path = "../unwind" } From 5ac8a31fbdf590cba0411ba26a74c7d6740f05ac Mon Sep 17 00:00:00 2001 From: Dominik Stolz Date: Tue, 16 Mar 2021 10:03:27 +0100 Subject: [PATCH 12/20] Fix test header and imports --- library/std/src/sys/unix/process/process_unix.rs | 8 ++++---- src/test/ui/command/command-create-pidfd.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index c6f8c446a0bd8..e5b00973e23a8 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -3,11 +3,9 @@ use crate::fmt; use crate::io::{self, Error, ErrorKind}; use crate::mem; use crate::ptr; -use crate::sync::atomic::{AtomicBool, Ordering}; use crate::sys; use crate::sys::cvt; use crate::sys::process::process_common::*; -use crate::sys_common::FromInner; #[cfg(target_os = "linux")] use crate::os::linux::process::PidFd; @@ -16,7 +14,7 @@ use crate::os::linux::process::PidFd; use libc::RTP_ID as pid_t; #[cfg(not(target_os = "vxworks"))] -use libc::{c_int, c_long, gid_t, pid_t, uid_t}; +use libc::{c_int, gid_t, pid_t, uid_t}; //////////////////////////////////////////////////////////////////////////////// // Command @@ -132,6 +130,7 @@ impl Command { // If this fails, we will fall through this block to a call to `fork()` #[cfg(target_os = "linux")] { + use crate::sync::atomic::{AtomicBool, Ordering}; static HAS_CLONE3: AtomicBool = AtomicBool::new(true); const CLONE_PIDFD: u64 = 0x00001000; @@ -152,7 +151,7 @@ impl Command { } syscall! { - fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> c_long + fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long } if HAS_CLONE3.load(Ordering::Relaxed) { @@ -529,6 +528,7 @@ pub struct Process { impl Process { #[cfg(target_os = "linux")] fn new(pid: pid_t, pidfd: pid_t) -> Self { + use crate::sys_common::FromInner; let pidfd = (pidfd >= 0).then(|| PidFd::from_inner(sys::fd::FileDesc::new(pidfd))); Process { pid, status: None, pidfd } } diff --git a/src/test/ui/command/command-create-pidfd.rs b/src/test/ui/command/command-create-pidfd.rs index 2e08eb1be540a..db728be09dfbc 100644 --- a/src/test/ui/command/command-create-pidfd.rs +++ b/src/test/ui/command/command-create-pidfd.rs @@ -1,5 +1,5 @@ // run-pass -// linux-only - pidfds are a linux-specific concept +// only-linux - pidfds are a linux-specific concept #![feature(linux_pidfd)] use std::os::linux::process::{CommandExt, ChildExt}; From a266bd820143d2daa112e2cb3f3205e82108037e Mon Sep 17 00:00:00 2001 From: Dominik Stolz Date: Tue, 16 Mar 2021 11:27:24 +0100 Subject: [PATCH 13/20] Split do_fork into two --- .../std/src/sys/unix/process/process_unix.rs | 142 +++++++++--------- 1 file changed, 73 insertions(+), 69 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index e5b00973e23a8..24618bdcb582c 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -118,84 +118,88 @@ impl Command { } } - // Attempts to fork the process. If successful, returns - // Ok((0, -1)) in the child, and Ok((child_pid, child_pidfd)) in the parent. + // Attempts to fork the process. If successful, returns Ok((0, -1)) + // in the child, and Ok((child_pid, -1)) in the parent. + #[cfg(not(target_os = "linux"))] + fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { + cvt(unsafe { libc::fork() }).map(|res| (res, -1)) + } + + // Attempts to fork the process. If successful, returns Ok((0, -1)) + // in the child, and Ok((child_pid, child_pidfd)) in the parent. + #[cfg(target_os = "linux")] fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { + use crate::sync::atomic::{AtomicBool, Ordering}; + + static HAS_CLONE3: AtomicBool = AtomicBool::new(true); + const CLONE_PIDFD: u64 = 0x00001000; + + #[repr(C)] + struct clone_args { + flags: u64, + pidfd: u64, + child_tid: u64, + parent_tid: u64, + exit_signal: u64, + stack: u64, + stack_size: u64, + tls: u64, + set_tid: u64, + set_tid_size: u64, + cgroup: u64, + } + + syscall! { + fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long + } + // If we fail to create a pidfd for any reason, this will - // stay as -1, which indicates an error + // stay as -1, which indicates an error. let mut pidfd: pid_t = -1; - // On Linux, attempt to use the `clone3` syscall, which - // supports more arguments (in particular, the ability to create a pidfd). - // If this fails, we will fall through this block to a call to `fork()` - #[cfg(target_os = "linux")] - { - use crate::sync::atomic::{AtomicBool, Ordering}; - static HAS_CLONE3: AtomicBool = AtomicBool::new(true); - - const CLONE_PIDFD: u64 = 0x00001000; - - #[repr(C)] - struct clone_args { - flags: u64, - pidfd: u64, - child_tid: u64, - parent_tid: u64, - exit_signal: u64, - stack: u64, - stack_size: u64, - tls: u64, - set_tid: u64, - set_tid_size: u64, - cgroup: u64, - } - - syscall! { - fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long + // Attempt to use the `clone3` syscall, which supports more arguments + // (in particular, the ability to create a pidfd). If this fails, + // we will fall through this block to a call to `fork()` + if HAS_CLONE3.load(Ordering::Relaxed) { + let mut flags = 0; + if self.create_pidfd { + flags |= CLONE_PIDFD; } - if HAS_CLONE3.load(Ordering::Relaxed) { - let mut flags = 0; - if self.create_pidfd { - flags |= CLONE_PIDFD; - } - - let mut args = clone_args { - flags, - pidfd: &mut pidfd as *mut pid_t as u64, - child_tid: 0, - parent_tid: 0, - exit_signal: libc::SIGCHLD as u64, - stack: 0, - stack_size: 0, - tls: 0, - set_tid: 0, - set_tid_size: 0, - cgroup: 0, - }; - - let args_ptr = &mut args as *mut clone_args; - let args_size = crate::mem::size_of::(); - - let res = cvt(unsafe { clone3(args_ptr, args_size) }); - match res { - Ok(n) => return Ok((n as pid_t, pidfd)), - Err(e) => match e.raw_os_error() { - // Multiple threads can race to execute this store, - // but that's fine - that just means that multiple threads - // will have tried and failed to execute the same syscall, - // with no other side effects. - Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), - // Fallback to fork if `EPERM` is returned. (e.g. blocked by seccomp) - Some(libc::EPERM) => {} - _ => return Err(e), - }, - } + let mut args = clone_args { + flags, + pidfd: &mut pidfd as *mut pid_t as u64, + child_tid: 0, + parent_tid: 0, + exit_signal: libc::SIGCHLD as u64, + stack: 0, + stack_size: 0, + tls: 0, + set_tid: 0, + set_tid_size: 0, + cgroup: 0, + }; + + let args_ptr = &mut args as *mut clone_args; + let args_size = crate::mem::size_of::(); + + let res = cvt(unsafe { clone3(args_ptr, args_size) }); + match res { + Ok(n) => return Ok((n as pid_t, pidfd)), + Err(e) => match e.raw_os_error() { + // Multiple threads can race to execute this store, + // but that's fine - that just means that multiple threads + // will have tried and failed to execute the same syscall, + // with no other side effects. + Some(libc::ENOSYS) => HAS_CLONE3.store(false, Ordering::Relaxed), + // Fallback to fork if `EPERM` is returned. (e.g. blocked by seccomp) + Some(libc::EPERM) => {} + _ => return Err(e), + }, } } - // If we get here, we are either not on Linux, - // or we are on Linux and the 'clone3' syscall does not exist + // If we get here, the 'clone3' syscall does not exist // or we do not have permission to call it cvt(unsafe { libc::fork() }).map(|res| (res, pidfd)) } From cfb2d726098e6c0e3c8639894313ac39ff40a811 Mon Sep 17 00:00:00 2001 From: Dominik Stolz Date: Tue, 16 Mar 2021 17:30:14 +0100 Subject: [PATCH 14/20] Make do_fork unsafe --- library/std/src/sys/unix/process/process_unix.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 24618bdcb582c..47d8e0cd771d0 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -52,7 +52,7 @@ impl Command { // in its own process. Thus the parent drops the lock guard while the child // forgets it to avoid unlocking it on a new thread, which would be invalid. let env_lock = sys::os::env_read_lock(); - let (result, pidfd) = self.do_fork()?; + let (result, pidfd) = unsafe { self.do_fork()? }; let pid = unsafe { match result { @@ -121,14 +121,14 @@ impl Command { // Attempts to fork the process. If successful, returns Ok((0, -1)) // in the child, and Ok((child_pid, -1)) in the parent. #[cfg(not(target_os = "linux"))] - fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { - cvt(unsafe { libc::fork() }).map(|res| (res, -1)) + unsafe fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { + cvt(libc::fork()).map(|res| (res, -1)) } // Attempts to fork the process. If successful, returns Ok((0, -1)) // in the child, and Ok((child_pid, child_pidfd)) in the parent. #[cfg(target_os = "linux")] - fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { + unsafe fn do_fork(&mut self) -> Result<(pid_t, pid_t), io::Error> { use crate::sync::atomic::{AtomicBool, Ordering}; static HAS_CLONE3: AtomicBool = AtomicBool::new(true); @@ -183,7 +183,7 @@ impl Command { let args_ptr = &mut args as *mut clone_args; let args_size = crate::mem::size_of::(); - let res = cvt(unsafe { clone3(args_ptr, args_size) }); + let res = cvt(clone3(args_ptr, args_size)); match res { Ok(n) => return Ok((n as pid_t, pidfd)), Err(e) => match e.raw_os_error() { @@ -201,7 +201,7 @@ impl Command { // If we get here, the 'clone3' syscall does not exist // or we do not have permission to call it - cvt(unsafe { libc::fork() }).map(|res| (res, pidfd)) + cvt(libc::fork()).map(|res| (res, pidfd)) } pub fn exec(&mut self, default: Stdio) -> io::Error { From 620ecc01a2eb28848f5f3d8039bdb1f23d8cc21f Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 17 Mar 2021 10:28:52 -0400 Subject: [PATCH 15/20] Move some test-only code to test files This also relaxes the bounds on some structs and moves them to the impl block instead. --- compiler/rustc_arena/src/lib.rs | 16 ------- compiler/rustc_arena/src/tests.rs | 18 ++++++++ .../rustc_data_structures/src/tiny_list.rs | 14 +----- .../src/tiny_list/tests.rs | 11 +++++ .../src/transitive_relation.rs | 10 +---- .../src/transitive_relation/tests.rs | 8 ++++ compiler/rustc_span/src/source_map.rs | 42 ------------------ compiler/rustc_span/src/source_map/tests.rs | 44 +++++++++++++++++++ 8 files changed, 85 insertions(+), 78 deletions(-) diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs index 721cfdd4459e5..f99c05ce91fc8 100644 --- a/compiler/rustc_arena/src/lib.rs +++ b/compiler/rustc_arena/src/lib.rs @@ -297,22 +297,6 @@ impl TypedArena { } } - /// Clears the arena. Deallocates all but the longest chunk which may be reused. - pub fn clear(&mut self) { - unsafe { - // Clear the last chunk, which is partially filled. - let mut chunks_borrow = self.chunks.borrow_mut(); - if let Some(mut last_chunk) = chunks_borrow.last_mut() { - self.clear_last_chunk(&mut last_chunk); - let len = chunks_borrow.len(); - // If `T` is ZST, code below has no effect. - for mut chunk in chunks_borrow.drain(..len - 1) { - chunk.destroy(chunk.entries); - } - } - } - } - // Drops the contents of the last chunk. The last chunk is partially empty, unlike all other // chunks. fn clear_last_chunk(&self, last_chunk: &mut TypedArenaChunk) { diff --git a/compiler/rustc_arena/src/tests.rs b/compiler/rustc_arena/src/tests.rs index e8a1f2db1a16b..911e577c1edc7 100644 --- a/compiler/rustc_arena/src/tests.rs +++ b/compiler/rustc_arena/src/tests.rs @@ -11,6 +11,24 @@ struct Point { z: i32, } +impl TypedArena { + /// Clears the arena. Deallocates all but the longest chunk which may be reused. + fn clear(&mut self) { + unsafe { + // Clear the last chunk, which is partially filled. + let mut chunks_borrow = self.chunks.borrow_mut(); + if let Some(mut last_chunk) = chunks_borrow.last_mut() { + self.clear_last_chunk(&mut last_chunk); + let len = chunks_borrow.len(); + // If `T` is ZST, code below has no effect. + for mut chunk in chunks_borrow.drain(..len - 1) { + chunk.destroy(chunk.entries); + } + } + } + } +} + #[test] pub fn test_unused() { let arena: TypedArena = TypedArena::default(); diff --git a/compiler/rustc_data_structures/src/tiny_list.rs b/compiler/rustc_data_structures/src/tiny_list.rs index e94a0c6eb5943..f88bcc2948150 100644 --- a/compiler/rustc_data_structures/src/tiny_list.rs +++ b/compiler/rustc_data_structures/src/tiny_list.rs @@ -15,7 +15,7 @@ mod tests; #[derive(Clone)] -pub struct TinyList { +pub struct TinyList { head: Option>, } @@ -56,20 +56,10 @@ impl TinyList { } false } - - #[inline] - pub fn len(&self) -> usize { - let (mut elem, mut count) = (self.head.as_ref(), 0); - while let Some(ref e) = elem { - count += 1; - elem = e.next.as_deref(); - } - count - } } #[derive(Clone)] -struct Element { +struct Element { data: T, next: Option>>, } diff --git a/compiler/rustc_data_structures/src/tiny_list/tests.rs b/compiler/rustc_data_structures/src/tiny_list/tests.rs index a8ae2bc872789..c0334d2e23e55 100644 --- a/compiler/rustc_data_structures/src/tiny_list/tests.rs +++ b/compiler/rustc_data_structures/src/tiny_list/tests.rs @@ -3,6 +3,17 @@ use super::*; extern crate test; use test::{black_box, Bencher}; +impl TinyList { + fn len(&self) -> usize { + let (mut elem, mut count) = (self.head.as_ref(), 0); + while let Some(ref e) = elem { + count += 1; + elem = e.next.as_deref(); + } + count + } +} + #[test] fn test_contains_and_insert() { fn do_insert(i: u32) -> bool { diff --git a/compiler/rustc_data_structures/src/transitive_relation.rs b/compiler/rustc_data_structures/src/transitive_relation.rs index 2e1512b3929ca..ccf8bd69ebd06 100644 --- a/compiler/rustc_data_structures/src/transitive_relation.rs +++ b/compiler/rustc_data_structures/src/transitive_relation.rs @@ -9,7 +9,7 @@ use std::mem; mod tests; #[derive(Clone, Debug)] -pub struct TransitiveRelation { +pub struct TransitiveRelation { // List of elements. This is used to map from a T to a usize. elements: FxIndexSet, @@ -49,7 +49,7 @@ struct Edge { target: Index, } -impl TransitiveRelation { +impl TransitiveRelation { pub fn is_empty(&self) -> bool { self.edges.is_empty() } @@ -322,12 +322,6 @@ impl TransitiveRelation { .collect() } - /// A "best" parent in some sense. See `parents` and - /// `postdom_upper_bound` for more details. - pub fn postdom_parent(&self, a: &T) -> Option<&T> { - self.mutual_immediate_postdominator(self.parents(a)) - } - fn with_closure(&self, op: OP) -> R where OP: FnOnce(&BitMatrix) -> R, diff --git a/compiler/rustc_data_structures/src/transitive_relation/tests.rs b/compiler/rustc_data_structures/src/transitive_relation/tests.rs index ca90ba176ae1a..9fa7224376c1c 100644 --- a/compiler/rustc_data_structures/src/transitive_relation/tests.rs +++ b/compiler/rustc_data_structures/src/transitive_relation/tests.rs @@ -1,5 +1,13 @@ use super::*; +impl TransitiveRelation { + /// A "best" parent in some sense. See `parents` and + /// `postdom_upper_bound` for more details. + fn postdom_parent(&self, a: &T) -> Option<&T> { + self.mutual_immediate_postdominator(self.parents(a)) + } +} + #[test] fn test_one_step() { let mut relation = TransitiveRelation::default(); diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index b7eb6d5b3790b..f612d1425b932 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -453,41 +453,6 @@ impl SourceMap { } } - /// Returns `Some(span)`, a union of the LHS and RHS span. The LHS must precede the RHS. If - /// there are gaps between LHS and RHS, the resulting union will cross these gaps. - /// For this to work, - /// - /// * the syntax contexts of both spans much match, - /// * the LHS span needs to end on the same line the RHS span begins, - /// * the LHS span must start at or before the RHS span. - pub fn merge_spans(&self, sp_lhs: Span, sp_rhs: Span) -> Option { - // Ensure we're at the same expansion ID. - if sp_lhs.ctxt() != sp_rhs.ctxt() { - return None; - } - - let lhs_end = match self.lookup_line(sp_lhs.hi()) { - Ok(x) => x, - Err(_) => return None, - }; - let rhs_begin = match self.lookup_line(sp_rhs.lo()) { - Ok(x) => x, - Err(_) => return None, - }; - - // If we must cross lines to merge, don't merge. - if lhs_end.line != rhs_begin.line { - return None; - } - - // Ensure these follow the expected order and that we don't overlap. - if (sp_lhs.lo() <= sp_rhs.lo()) && (sp_lhs.hi() <= sp_rhs.lo()) { - Some(sp_lhs.to(sp_rhs)) - } else { - None - } - } - pub fn span_to_string(&self, sp: Span) -> String { if self.files.borrow().source_files.is_empty() && sp.is_dummy() { return "no-location".to_string(); @@ -931,13 +896,6 @@ impl SourceMap { SourceFileAndBytePos { sf, pos: offset } } - /// Converts an absolute `BytePos` to a `CharPos` relative to the `SourceFile`. - pub fn bytepos_to_file_charpos(&self, bpos: BytePos) -> CharPos { - let idx = self.lookup_source_file_idx(bpos); - let sf = &(*self.files.borrow().source_files)[idx]; - sf.bytepos_to_file_charpos(bpos) - } - // Returns the index of the `SourceFile` (in `self.files`) that contains `pos`. // This index is guaranteed to be valid for the lifetime of this `SourceMap`, // since `source_files` is a `MonotonicVec` diff --git a/compiler/rustc_span/src/source_map/tests.rs b/compiler/rustc_span/src/source_map/tests.rs index 0aca677248b72..7d814f1d82c11 100644 --- a/compiler/rustc_span/src/source_map/tests.rs +++ b/compiler/rustc_span/src/source_map/tests.rs @@ -10,6 +10,50 @@ fn init_source_map() -> SourceMap { sm } +impl SourceMap { + /// Returns `Some(span)`, a union of the LHS and RHS span. The LHS must precede the RHS. If + /// there are gaps between LHS and RHS, the resulting union will cross these gaps. + /// For this to work, + /// + /// * the syntax contexts of both spans much match, + /// * the LHS span needs to end on the same line the RHS span begins, + /// * the LHS span must start at or before the RHS span. + fn merge_spans(&self, sp_lhs: Span, sp_rhs: Span) -> Option { + // Ensure we're at the same expansion ID. + if sp_lhs.ctxt() != sp_rhs.ctxt() { + return None; + } + + let lhs_end = match self.lookup_line(sp_lhs.hi()) { + Ok(x) => x, + Err(_) => return None, + }; + let rhs_begin = match self.lookup_line(sp_rhs.lo()) { + Ok(x) => x, + Err(_) => return None, + }; + + // If we must cross lines to merge, don't merge. + if lhs_end.line != rhs_begin.line { + return None; + } + + // Ensure these follow the expected order and that we don't overlap. + if (sp_lhs.lo() <= sp_rhs.lo()) && (sp_lhs.hi() <= sp_rhs.lo()) { + Some(sp_lhs.to(sp_rhs)) + } else { + None + } + } + + /// Converts an absolute `BytePos` to a `CharPos` relative to the `SourceFile`. + fn bytepos_to_file_charpos(&self, bpos: BytePos) -> CharPos { + let idx = self.lookup_source_file_idx(bpos); + let sf = &(*self.files.borrow().source_files)[idx]; + sf.bytepos_to_file_charpos(bpos) + } +} + /// Tests `lookup_byte_offset`. #[test] fn t3() { From b1de9d4b648b841cb6762baead98b627b9731215 Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Tue, 16 Mar 2021 17:14:01 +0100 Subject: [PATCH 16/20] Fix gitattibutes for old git versions --- .gitattributes | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitattributes b/.gitattributes index 4038db6f7dab1..d29c15fe712f3 100644 --- a/.gitattributes +++ b/.gitattributes @@ -7,11 +7,11 @@ *.fixed linguist-language=Rust *.mir linguist-language=Rust src/etc/installer/gfx/* binary -*.woff binary -*.woff2 binary src/vendor/** -text Cargo.lock linguist-generated=false -# Older git versions try to fix line endings on images, this prevents it. +# Older git versions try to fix line endings on images and fonts, this prevents it. *.png binary *.ico binary +*.woff binary +*.woff2 binary From cfb4ad4f2abf42ff603d5f9e89f1352cb79a451c Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 13:06:01 +0100 Subject: [PATCH 17/20] Remove unwrap_none/expect_none from compiler/. --- compiler/rustc_codegen_ssa/src/lib.rs | 2 +- compiler/rustc_middle/src/ich/impls_syntax.rs | 6 +++++- compiler/rustc_middle/src/lib.rs | 2 +- .../rustc_middle/src/mir/interpret/allocation.rs | 2 +- compiler/rustc_mir/src/interpret/memory.rs | 14 +++++++++++--- compiler/rustc_mir/src/lib.rs | 3 ++- compiler/rustc_mir/src/transform/coverage/debug.rs | 12 +++++------- .../rustc_mir/src/transform/deduplicate_blocks.rs | 2 +- compiler/rustc_span/src/hygiene.rs | 14 +++++++++----- compiler/rustc_span/src/lib.rs | 4 ++-- 10 files changed, 38 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 2c2330409fd70..dd04d3e548f8c 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -1,6 +1,6 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] +#![feature(assert_matches)] #![feature(bool_to_option)] -#![feature(option_expect_none)] #![feature(box_patterns)] #![feature(drain_filter)] #![feature(try_blocks)] diff --git a/compiler/rustc_middle/src/ich/impls_syntax.rs b/compiler/rustc_middle/src/ich/impls_syntax.rs index bfbe15749ee1c..31374429940ca 100644 --- a/compiler/rustc_middle/src/ich/impls_syntax.rs +++ b/compiler/rustc_middle/src/ich/impls_syntax.rs @@ -45,7 +45,11 @@ impl<'ctx> rustc_ast::HashStableContext for StableHashingContext<'ctx> { item.hash_stable(self, hasher); style.hash_stable(self, hasher); span.hash_stable(self, hasher); - tokens.as_ref().expect_none("Tokens should have been removed during lowering!"); + assert_matches!( + tokens.as_ref(), + None, + "Tokens should have been removed during lowering!" + ); } else { unreachable!(); } diff --git a/compiler/rustc_middle/src/lib.rs b/compiler/rustc_middle/src/lib.rs index d9e8826505002..2d807591bfdd2 100644 --- a/compiler/rustc_middle/src/lib.rs +++ b/compiler/rustc_middle/src/lib.rs @@ -24,6 +24,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] #![feature(array_windows)] +#![feature(assert_matches)] #![feature(assoc_char_funcs)] #![feature(backtrace)] #![feature(bool_to_option)] @@ -38,7 +39,6 @@ #![feature(extern_types)] #![feature(nll)] #![feature(once_cell)] -#![feature(option_expect_none)] #![feature(or_patterns)] #![feature(min_specialization)] #![feature(trusted_len)] diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 48595f2badc8f..766d6a06f7e59 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -339,7 +339,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { for dest in bytes { *dest = src.next().expect("iterator was shorter than it said it would be"); } - src.next().expect_none("iterator was longer than it said it would be"); + assert_matches!(src.next(), None, "iterator was longer than it said it would be"); Ok(()) } diff --git a/compiler/rustc_mir/src/interpret/memory.rs b/compiler/rustc_mir/src/interpret/memory.rs index f3e373813ca53..fe5ebf0b6fe97 100644 --- a/compiler/rustc_mir/src/interpret/memory.rs +++ b/compiler/rustc_mir/src/interpret/memory.rs @@ -854,7 +854,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { Some(ptr) => ptr, None => { // zero-sized access - src.next().expect_none("iterator said it was empty but returned an element"); + assert_matches!( + src.next(), + None, + "iterator said it was empty but returned an element" + ); return Ok(()); } }; @@ -880,7 +884,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { Some(ptr) => ptr, None => { // zero-sized access - src.next().expect_none("iterator said it was empty but returned an element"); + assert_matches!( + src.next(), + None, + "iterator said it was empty but returned an element" + ); return Ok(()); } }; @@ -894,7 +902,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let offset_ptr = ptr.offset(Size::from_bytes(idx) * 2, &tcx)?; // `Size` multiplication allocation.write_scalar(&tcx, offset_ptr, val.into(), Size::from_bytes(2))?; } - src.next().expect_none("iterator was longer than it said it would be"); + assert_matches!(src.next(), None, "iterator was longer than it said it would be"); Ok(()) } diff --git a/compiler/rustc_mir/src/lib.rs b/compiler/rustc_mir/src/lib.rs index 194bc74c0fb1a..f73d5dc0c116d 100644 --- a/compiler/rustc_mir/src/lib.rs +++ b/compiler/rustc_mir/src/lib.rs @@ -7,6 +7,7 @@ Rust MIR: a lowered representation of Rust. #![feature(nll)] #![feature(in_band_lifetimes)] #![feature(array_windows)] +#![feature(assert_matches)] #![feature(bindings_after_at)] #![feature(bool_to_option)] #![feature(box_patterns)] @@ -18,13 +19,13 @@ Rust MIR: a lowered representation of Rust. #![feature(exact_size_is_empty)] #![feature(exhaustive_patterns)] #![feature(never_type)] +#![feature(map_try_insert)] #![feature(min_specialization)] #![feature(trusted_len)] #![feature(try_blocks)] #![feature(associated_type_defaults)] #![feature(stmt_expr_attributes)] #![feature(trait_alias)] -#![feature(option_expect_none)] #![feature(option_get_or_insert_default)] #![feature(or_patterns)] #![feature(once_cell)] diff --git a/compiler/rustc_mir/src/transform/coverage/debug.rs b/compiler/rustc_mir/src/transform/coverage/debug.rs index 2cd0dc6b1f2fd..aabfee53acb29 100644 --- a/compiler/rustc_mir/src/transform/coverage/debug.rs +++ b/compiler/rustc_mir/src/transform/coverage/debug.rs @@ -285,10 +285,8 @@ impl DebugCounters { ), }; counters - .insert(id, DebugCounter::new(counter_kind.clone(), some_block_label)) - .expect_none( - "attempt to add the same counter_kind to DebugCounters more than once", - ); + .try_insert(id, DebugCounter::new(counter_kind.clone(), some_block_label)) + .expect("attempt to add the same counter_kind to DebugCounters more than once"); } } @@ -479,9 +477,9 @@ impl GraphvizData { counter_kind: &CoverageKind, ) { if let Some(edge_to_counter) = self.some_edge_to_counter.as_mut() { - edge_to_counter.insert((from_bcb, to_bb), counter_kind.clone()).expect_none( - "invalid attempt to insert more than one edge counter for the same edge", - ); + edge_to_counter + .try_insert((from_bcb, to_bb), counter_kind.clone()) + .expect("invalid attempt to insert more than one edge counter for the same edge"); } } diff --git a/compiler/rustc_mir/src/transform/deduplicate_blocks.rs b/compiler/rustc_mir/src/transform/deduplicate_blocks.rs index c4b51099f5389..e102512e1f37f 100644 --- a/compiler/rustc_mir/src/transform/deduplicate_blocks.rs +++ b/compiler/rustc_mir/src/transform/deduplicate_blocks.rs @@ -86,7 +86,7 @@ fn find_duplicates<'a, 'tcx>(body: &'a Body<'tcx>) -> FxHashMap {:?}", bb, value); - duplicates.insert(bb, value).expect_none("key was already inserted"); + duplicates.try_insert(bb, value).expect("key was already inserted"); } Entry::Vacant(vacant) => { vacant.insert(bb); diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs index e67a4ca8fb26b..eb5b7c4a74a1d 100644 --- a/compiler/rustc_span/src/hygiene.rs +++ b/compiler/rustc_span/src/hygiene.rs @@ -118,7 +118,8 @@ impl ExpnId { HygieneData::with(|data| { let old_expn_data = &mut data.expn_data[self.0 as usize]; assert!(old_expn_data.is_none(), "expansion data is reset for an expansion ID"); - expn_data.orig_id.replace(self.as_u32()).expect_none("orig_id should be None"); + assert_eq!(expn_data.orig_id, None); + expn_data.orig_id = Some(self.as_u32()); *old_expn_data = Some(expn_data); }); update_disambiguator(self) @@ -202,7 +203,8 @@ impl HygieneData { fn fresh_expn(&mut self, mut expn_data: Option) -> ExpnId { let raw_id = self.expn_data.len() as u32; if let Some(data) = expn_data.as_mut() { - data.orig_id.replace(raw_id).expect_none("orig_id should be None"); + assert_eq!(data.orig_id, None); + data.orig_id = Some(raw_id); } self.expn_data.push(expn_data); ExpnId(raw_id) @@ -1410,9 +1412,11 @@ fn update_disambiguator(expn_id: ExpnId) { let new_hash: Fingerprint = hasher.finish(); HygieneData::with(|data| { - data.expn_data_disambiguators - .get(&new_hash) - .expect_none("Hash collision after disambiguator update!"); + assert_eq!( + data.expn_data_disambiguators.get(&new_hash), + None, + "Hash collision after disambiguator update!", + ); }); }; } diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 6030c8a86d9f9..d2790335b5abc 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -21,7 +21,6 @@ #![feature(negative_impls)] #![feature(nll)] #![feature(min_specialization)] -#![feature(option_expect_none)] #[macro_use] extern crate rustc_macros; @@ -1996,7 +1995,8 @@ impl HashStable for ExpnId { if cache.len() < new_len { cache.resize(new_len, None); } - cache[index].replace(sub_hash).expect_none("Cache slot was filled"); + let prev = cache[index].replace(sub_hash); + assert_eq!(prev, None, "Cache slot was filled"); }); sub_hash.hash_stable(ctx, hasher); } From 390d1ef6d048563502458ae20d9b7d69d914a680 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 15 Mar 2021 22:59:21 -0400 Subject: [PATCH 18/20] Extend `proc_macro_back_compat` lint to `actix-web` Unlike the other cases of this lint, there's no simple way to detect if an old version of the relevant crate (`syn`) is in use. The `actix-web` crate only depends on `pin-project` v1.0.0, so checking the version of `actix-web` does not guarantee that a new enough version of `pin-project` (and therefore `syn`) is in use. Instead, we rely on the fact that virtually all of the regressed crates are pinned to a pre-1.0 version of `pin-project`. When this is the case, bumping the `actix-web` dependency will pull in the *latest* version of `pin-project`, which has an explicit dependency on a newer v dependency on a newer version of `syn`. The lint message tells users to update `actix-web`, since that's what they're most likely to have control over. We could potentially tell them to run `cargo update -p syn`, but I think it's more straightforward to suggest an explicit change to the `Cargo.toml` The `actori-web` fork had its last commit over a year ago, and appears to just be a renamed fork of `actix-web`. Therefore, I've removed the `actori-web` check entirely - any crates that actually get broken can simply update `syn` themselves. --- .../rustc_expand/src/proc_macro_server.rs | 46 ++++++++----- ...-hack.rs => pin-project-internal-0.4.0.rs} | 4 ++ .../group-compat-hack/group-compat-hack.rs | 8 ++- .../group-compat-hack.stderr | 68 ++++++++++++++++++- .../group-compat-hack.stdout | 6 +- 5 files changed, 108 insertions(+), 24 deletions(-) rename src/test/ui/proc-macro/group-compat-hack/auxiliary/{group-compat-hack.rs => pin-project-internal-0.4.0.rs} (68%) diff --git a/compiler/rustc_expand/src/proc_macro_server.rs b/compiler/rustc_expand/src/proc_macro_server.rs index 67edfe19da383..cb41bc81225ec 100644 --- a/compiler/rustc_expand/src/proc_macro_server.rs +++ b/compiler/rustc_expand/src/proc_macro_server.rs @@ -53,11 +53,11 @@ impl ToInternal for Delimiter { } } -impl FromInternal<(TreeAndSpacing, &'_ ParseSess, &'_ mut Vec)> +impl FromInternal<(TreeAndSpacing, &'_ mut Vec, &mut Rustc<'_>)> for TokenTree { fn from_internal( - ((tree, spacing), sess, stack): (TreeAndSpacing, &ParseSess, &mut Vec), + ((tree, spacing), stack, rustc): (TreeAndSpacing, &mut Vec, &mut Rustc<'_>), ) -> Self { use rustc_ast::token::*; @@ -146,10 +146,10 @@ impl FromInternal<(TreeAndSpacing, &'_ ParseSess, &'_ mut Vec)> SingleQuote => op!('\''), Ident(name, false) if name == kw::DollarCrate => tt!(Ident::dollar_crate()), - Ident(name, is_raw) => tt!(Ident::new(sess, name, is_raw)), + Ident(name, is_raw) => tt!(Ident::new(rustc.sess, name, is_raw)), Lifetime(name) => { let ident = symbol::Ident::new(name, span).without_first_quote(); - stack.push(tt!(Ident::new(sess, ident.name, false))); + stack.push(tt!(Ident::new(rustc.sess, ident.name, false))); tt!(Punct::new('\'', true)) } Literal(lit) => tt!(Literal { lit }), @@ -179,15 +179,15 @@ impl FromInternal<(TreeAndSpacing, &'_ ParseSess, &'_ mut Vec)> } Interpolated(nt) => { - if let Some((name, is_raw)) = ident_name_compatibility_hack(&nt, span, sess) { - TokenTree::Ident(Ident::new(sess, name.name, is_raw, name.span)) + if let Some((name, is_raw)) = ident_name_compatibility_hack(&nt, span, rustc) { + TokenTree::Ident(Ident::new(rustc.sess, name.name, is_raw, name.span)) } else { - let stream = nt_to_tokenstream(&nt, sess, CanSynthesizeMissingTokens::No); + let stream = nt_to_tokenstream(&nt, rustc.sess, CanSynthesizeMissingTokens::No); TokenTree::Group(Group { delimiter: Delimiter::None, stream, span: DelimSpan::from_single(span), - flatten: crate::base::pretty_printing_compatibility_hack(&nt, sess), + flatten: crate::base::pretty_printing_compatibility_hack(&nt, rustc.sess), }) } } @@ -449,7 +449,7 @@ impl server::TokenStreamIter for Rustc<'_> { loop { let tree = iter.stack.pop().or_else(|| { let next = iter.cursor.next_with_spacing()?; - Some(TokenTree::from_internal((next, self.sess, &mut iter.stack))) + Some(TokenTree::from_internal((next, &mut iter.stack, self))) })?; // A hack used to pass AST fragments to attribute and derive macros // as a single nonterminal token instead of a token stream. @@ -719,11 +719,11 @@ impl server::Span for Rustc<'_> { fn ident_name_compatibility_hack( nt: &Nonterminal, orig_span: Span, - sess: &ParseSess, + rustc: &mut Rustc<'_>, ) -> Option<(rustc_span::symbol::Ident, bool)> { if let NtIdent(ident, is_raw) = nt { if let ExpnKind::Macro(_, macro_name) = orig_span.ctxt().outer_expn_data().kind { - let source_map = sess.source_map(); + let source_map = rustc.sess.source_map(); let filename = source_map.span_to_filename(orig_span); if let FileName::Real(RealFileName::Named(path)) = filename { let matches_prefix = |prefix, filename| { @@ -745,7 +745,7 @@ fn ident_name_compatibility_hack( let snippet = source_map.span_to_snippet(orig_span); if snippet.as_deref() == Ok("$name") { if time_macros_impl { - sess.buffer_lint_with_diagnostic( + rustc.sess.buffer_lint_with_diagnostic( &PROC_MACRO_BACK_COMPAT, orig_span, ast::CRATE_NODE_ID, @@ -759,13 +759,25 @@ fn ident_name_compatibility_hack( } } - if macro_name == sym::tuple_from_req - && (matches_prefix("actix-web", "extract.rs") - || matches_prefix("actori-web", "extract.rs")) - { + if macro_name == sym::tuple_from_req && matches_prefix("actix-web", "extract.rs") { let snippet = source_map.span_to_snippet(orig_span); if snippet.as_deref() == Ok("$T") { - return Some((*ident, *is_raw)); + if let FileName::Real(RealFileName::Named(macro_path)) = + source_map.span_to_filename(rustc.def_site) + { + if macro_path.to_string_lossy().contains("pin-project-internal-0.") { + rustc.sess.buffer_lint_with_diagnostic( + &PROC_MACRO_BACK_COMPAT, + orig_span, + ast::CRATE_NODE_ID, + "using an old version of `actix-web`", + BuiltinLintDiagnostics::ProcMacroBackCompat( + "the version of `actix-web` you are using might stop compiling in future versions of Rust; \ + please update to the latest version of the `actix-web` crate to avoid breakage".to_string()) + ); + return Some((*ident, *is_raw)); + } + } } } } diff --git a/src/test/ui/proc-macro/group-compat-hack/auxiliary/group-compat-hack.rs b/src/test/ui/proc-macro/group-compat-hack/auxiliary/pin-project-internal-0.4.0.rs similarity index 68% rename from src/test/ui/proc-macro/group-compat-hack/auxiliary/group-compat-hack.rs rename to src/test/ui/proc-macro/group-compat-hack/auxiliary/pin-project-internal-0.4.0.rs index 5cd3b40a2e42a..baa4fd3a10559 100644 --- a/src/test/ui/proc-macro/group-compat-hack/auxiliary/group-compat-hack.rs +++ b/src/test/ui/proc-macro/group-compat-hack/auxiliary/pin-project-internal-0.4.0.rs @@ -2,6 +2,10 @@ // no-prefer-dynamic #![crate_type = "proc-macro"] +#![crate_name = "group_compat_hack"] + +// This file has an unusual name in order to trigger the back-compat +// code in the compiler extern crate proc_macro; use proc_macro::TokenStream; diff --git a/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.rs b/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.rs index 7f3f5e36f50a9..d9687490cad75 100644 --- a/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.rs +++ b/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.rs @@ -1,5 +1,5 @@ // check-pass -// aux-build:group-compat-hack.rs +// aux-build:pin-project-internal-0.4.0.rs // compile-flags: -Z span-debug #![no_std] // Don't load unnecessary hygiene information from std @@ -51,14 +51,16 @@ mod actix_web_test { include!("actix-web/src/extract.rs"); struct Foo; - tuple_from_req!(Foo); + tuple_from_req!(Foo); //~ WARN using an old version + //~| WARN this was previously } mod actix_web_version_test { include!("actix-web-2.0.0/src/extract.rs"); struct Foo; - tuple_from_req!(Foo); + tuple_from_req!(Foo); //~ WARN using an old version + //~| WARN this was previously } mod actori_web_test { diff --git a/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.stderr b/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.stderr index 9370440a63511..e2b680f8d2760 100644 --- a/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.stderr +++ b/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.stderr @@ -31,7 +31,39 @@ LL | impl_macros!(Foo); = note: the `time-macros-impl` crate will stop compiling in futures version of Rust. Please update to the latest version of the `time` crate to avoid breakage = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -warning: 2 warnings emitted +warning: using an old version of `actix-web` + --> $DIR/actix-web/src/extract.rs:5:34 + | +LL | #[my_macro] struct Three($T); + | ^^ + | + ::: $DIR/group-compat-hack.rs:54:5 + | +LL | tuple_from_req!(Foo); + | --------------------- in this macro invocation + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83125 + = note: the version of `actix-web` you are using might stop compiling in future versions of Rust; please update to the latest version of the `actix-web` crate to avoid breakage + = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +warning: using an old version of `actix-web` + --> $DIR/actix-web-2.0.0/src/extract.rs:5:34 + | +LL | #[my_macro] struct Three($T); + | ^^ + | + ::: $DIR/group-compat-hack.rs:62:5 + | +LL | tuple_from_req!(Foo); + | --------------------- in this macro invocation + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83125 + = note: the version of `actix-web` you are using might stop compiling in future versions of Rust; please update to the latest version of the `actix-web` crate to avoid breakage + = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +warning: 4 warnings emitted Future incompatibility report: Future breakage date: None, diagnostic: warning: using an old version of `time-macros-impl` @@ -68,3 +100,37 @@ LL | impl_macros!(Foo); = note: the `time-macros-impl` crate will stop compiling in futures version of Rust. Please update to the latest version of the `time` crate to avoid breakage = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) +Future breakage date: None, diagnostic: +warning: using an old version of `actix-web` + --> $DIR/actix-web/src/extract.rs:5:34 + | +LL | #[my_macro] struct Three($T); + | ^^ + | + ::: $DIR/group-compat-hack.rs:54:5 + | +LL | tuple_from_req!(Foo); + | --------------------- in this macro invocation + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83125 + = note: the version of `actix-web` you are using might stop compiling in future versions of Rust; please update to the latest version of the `actix-web` crate to avoid breakage + = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +Future breakage date: None, diagnostic: +warning: using an old version of `actix-web` + --> $DIR/actix-web-2.0.0/src/extract.rs:5:34 + | +LL | #[my_macro] struct Three($T); + | ^^ + | + ::: $DIR/group-compat-hack.rs:62:5 + | +LL | tuple_from_req!(Foo); + | --------------------- in this macro invocation + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83125 + = note: the version of `actix-web` you are using might stop compiling in future versions of Rust; please update to the latest version of the `actix-web` crate to avoid breakage + = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + diff --git a/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.stdout b/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.stdout index 468cb51191517..3fe744e12ff04 100644 --- a/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.stdout +++ b/src/test/ui/proc-macro/group-compat-hack/group-compat-hack.stdout @@ -5,6 +5,6 @@ Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/tim Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/js-sys-0.3.17/src/lib.rs:5:21: 5:27 (#24) }, Ident { ident: "Two", span: $DIR/js-sys-0.3.17/src/lib.rs:5:28: 5:31 (#24) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "Foo", span: $DIR/group-compat-hack.rs:46:13: 46:16 (#0) }], span: $DIR/js-sys-0.3.17/src/lib.rs:5:31: 5:38 (#24) }, Punct { ch: ';', spacing: Alone, span: $DIR/js-sys-0.3.17/src/lib.rs:5:38: 5:39 (#24) }] Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/group-compat-hack.rs:39:25: 39:31 (#28) }, Ident { ident: "Three", span: $DIR/group-compat-hack.rs:39:32: 39:37 (#28) }, Group { delimiter: Parenthesis, stream: TokenStream [Group { delimiter: None, stream: TokenStream [Ident { ident: "Foo", span: $DIR/group-compat-hack.rs:47:12: 47:15 (#0) }], span: $DIR/group-compat-hack.rs:39:38: 39:43 (#28) }], span: $DIR/group-compat-hack.rs:39:37: 39:44 (#28) }, Punct { ch: ';', spacing: Alone, span: $DIR/group-compat-hack.rs:39:44: 39:45 (#28) }] Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/actix-web/src/extract.rs:5:21: 5:27 (#33) }, Ident { ident: "Three", span: $DIR/actix-web/src/extract.rs:5:28: 5:33 (#33) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "Foo", span: $DIR/group-compat-hack.rs:54:21: 54:24 (#0) }], span: $DIR/actix-web/src/extract.rs:5:33: 5:37 (#33) }, Punct { ch: ';', spacing: Alone, span: $DIR/actix-web/src/extract.rs:5:37: 5:38 (#33) }] -Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/actix-web-2.0.0/src/extract.rs:5:21: 5:27 (#38) }, Ident { ident: "Three", span: $DIR/actix-web-2.0.0/src/extract.rs:5:28: 5:33 (#38) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "Foo", span: $DIR/group-compat-hack.rs:61:21: 61:24 (#0) }], span: $DIR/actix-web-2.0.0/src/extract.rs:5:33: 5:37 (#38) }, Punct { ch: ';', spacing: Alone, span: $DIR/actix-web-2.0.0/src/extract.rs:5:37: 5:38 (#38) }] -Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/actori-web/src/extract.rs:5:21: 5:27 (#43) }, Ident { ident: "Four", span: $DIR/actori-web/src/extract.rs:5:28: 5:32 (#43) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "Foo", span: $DIR/group-compat-hack.rs:68:21: 68:24 (#0) }], span: $DIR/actori-web/src/extract.rs:5:32: 5:36 (#43) }, Punct { ch: ';', spacing: Alone, span: $DIR/actori-web/src/extract.rs:5:36: 5:37 (#43) }] -Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/actori-web-2.0.0/src/extract.rs:5:21: 5:27 (#48) }, Ident { ident: "Four", span: $DIR/actori-web-2.0.0/src/extract.rs:5:28: 5:32 (#48) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "Foo", span: $DIR/group-compat-hack.rs:75:21: 75:24 (#0) }], span: $DIR/actori-web-2.0.0/src/extract.rs:5:32: 5:36 (#48) }, Punct { ch: ';', spacing: Alone, span: $DIR/actori-web-2.0.0/src/extract.rs:5:36: 5:37 (#48) }] +Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/actix-web-2.0.0/src/extract.rs:5:21: 5:27 (#38) }, Ident { ident: "Three", span: $DIR/actix-web-2.0.0/src/extract.rs:5:28: 5:33 (#38) }, Group { delimiter: Parenthesis, stream: TokenStream [Ident { ident: "Foo", span: $DIR/group-compat-hack.rs:62:21: 62:24 (#0) }], span: $DIR/actix-web-2.0.0/src/extract.rs:5:33: 5:37 (#38) }, Punct { ch: ';', spacing: Alone, span: $DIR/actix-web-2.0.0/src/extract.rs:5:37: 5:38 (#38) }] +Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/actori-web/src/extract.rs:5:21: 5:27 (#43) }, Ident { ident: "Four", span: $DIR/actori-web/src/extract.rs:5:28: 5:32 (#43) }, Group { delimiter: Parenthesis, stream: TokenStream [Group { delimiter: None, stream: TokenStream [Ident { ident: "Foo", span: $DIR/group-compat-hack.rs:70:21: 70:24 (#0) }], span: $DIR/actori-web/src/extract.rs:5:33: 5:35 (#43) }], span: $DIR/actori-web/src/extract.rs:5:32: 5:36 (#43) }, Punct { ch: ';', spacing: Alone, span: $DIR/actori-web/src/extract.rs:5:36: 5:37 (#43) }] +Called proc_macro_hack with TokenStream [Ident { ident: "struct", span: $DIR/actori-web-2.0.0/src/extract.rs:5:21: 5:27 (#48) }, Ident { ident: "Four", span: $DIR/actori-web-2.0.0/src/extract.rs:5:28: 5:32 (#48) }, Group { delimiter: Parenthesis, stream: TokenStream [Group { delimiter: None, stream: TokenStream [Ident { ident: "Foo", span: $DIR/group-compat-hack.rs:77:21: 77:24 (#0) }], span: $DIR/actori-web-2.0.0/src/extract.rs:5:33: 5:35 (#48) }], span: $DIR/actori-web-2.0.0/src/extract.rs:5:32: 5:36 (#48) }, Punct { ch: ';', spacing: Alone, span: $DIR/actori-web-2.0.0/src/extract.rs:5:36: 5:37 (#48) }] From 99b2054fe59669231f595e22641a0f7a1f4ad918 Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Thu, 18 Mar 2021 18:58:22 +0100 Subject: [PATCH 19/20] Fix typo/inaccuracy in the documentation of Iterator::skip_while MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One of the examples used to say “this leads to a possibly confusing situation, where the type of the closure is a double reference” while _actually_ referring to the type of the closure _argument_. --- library/core/src/iter/traits/iterator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index f8504e842ee33..a07750f4ad1fe 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -1012,7 +1012,7 @@ pub trait Iterator { /// /// Because the closure passed to `skip_while()` takes a reference, and many /// iterators iterate over references, this leads to a possibly confusing - /// situation, where the type of the closure is a double reference: + /// situation, where the type of the closure argument is a double reference: /// /// ``` /// let a = [-1, 0, 1]; From 9dfda62763a4462407bf76b916b1808aed57401a Mon Sep 17 00:00:00 2001 From: Jannis Harder Date: Mon, 8 Mar 2021 11:58:05 +0100 Subject: [PATCH 20/20] Clarify docs for Read::read's return value --- library/std/src/io/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 17002e3b8602d..6abb300054af8 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -514,8 +514,8 @@ pub trait Read { /// waiting for data, but if an object needs to block for a read and cannot, /// it will typically signal this via an [`Err`] return value. /// - /// If the return value of this method is [`Ok(n)`], then it must be - /// guaranteed that `0 <= n <= buf.len()`. A nonzero `n` value indicates + /// If the return value of this method is [`Ok(n)`], then implementations must + /// guarantee that `0 <= n <= buf.len()`. A nonzero `n` value indicates /// that the buffer `buf` has been filled in with `n` bytes of data from this /// source. If `n` is `0`, then it can indicate one of two scenarios: /// @@ -529,6 +529,11 @@ pub trait Read { /// This may happen for example because fewer bytes are actually available right now /// (e. g. being close to end-of-file) or because read() was interrupted by a signal. /// + /// As this trait is safe to implement, callers cannot rely on `n <= buf.len()` for safety. + /// Extra care needs to be taken when `unsafe` functions are used to access the read bytes. + /// Callers have to ensure that no unchecked out-of-bounds accesses are possible even if + /// `n > buf.len()`. + /// /// No guarantees are provided about the contents of `buf` when this /// function is called, implementations cannot rely on any property of the /// contents of `buf` being true. It is recommended that *implementations*