From ec20993c4db26c431e700d5f6b59fe26028cab31 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Thu, 19 Nov 2020 18:32:35 +0100 Subject: [PATCH 01/26] Stabilize `unsafe_op_in_unsafe_fn` lint --- compiler/rustc_feature/src/accepted.rs | 2 ++ compiler/rustc_feature/src/active.rs | 3 -- compiler/rustc_lint_defs/src/builtin.rs | 9 +----- compiler/rustc_middle/src/mir/query.rs | 2 -- .../rustc_mir/src/transform/check_unsafety.rs | 5 ++- library/alloc/src/lib.rs | 2 +- library/core/src/lib.rs | 2 +- library/core/tests/lib.rs | 4 +-- library/std/src/lib.rs | 2 +- .../feature-gate-unsafe_block_in_unsafe_fn.rs | 6 ---- ...ture-gate-unsafe_block_in_unsafe_fn.stderr | 30 ----------------- .../unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs | 1 - .../rfc-2585-unsafe_op_in_unsafe_fn.stderr | 32 +++++++++---------- 13 files changed, 26 insertions(+), 74 deletions(-) delete mode 100644 src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs delete mode 100644 src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr diff --git a/compiler/rustc_feature/src/accepted.rs b/compiler/rustc_feature/src/accepted.rs index aa54ffb132dc9..ae2fe79ff1658 100644 --- a/compiler/rustc_feature/src/accepted.rs +++ b/compiler/rustc_feature/src/accepted.rs @@ -275,6 +275,8 @@ declare_features! ( (accepted, move_ref_pattern, "1.48.0", Some(68354), None), /// The smallest useful subset of `const_generics`. (accepted, min_const_generics, "1.51.0", Some(74878), None), + /// The `unsafe_op_in_unsafe_fn` lint (allowed by default): no longer treat an unsafe function as an unsafe block. + (accepted, unsafe_block_in_unsafe_fn, "1.51.0", Some(71668), None), // ------------------------------------------------------------------------- // feature-group-end: accepted features diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 2d0009c225c59..e1274ea44cb9e 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -557,9 +557,6 @@ declare_features! ( /// Allows the use of `#[ffi_const]` on foreign functions. (active, ffi_const, "1.45.0", Some(58328), None), - /// No longer treat an unsafe function as an unsafe block. - (active, unsafe_block_in_unsafe_fn, "1.45.0", Some(71668), None), - /// Allows `extern "avr-interrupt" fn()` and `extern "avr-non-blocking-interrupt" fn()`. (active, abi_avr_interrupt, "1.45.0", Some(69664), None), diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index f0a5ea150b719..67a49be60711a 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -8,7 +8,6 @@ use crate::{declare_lint, declare_lint_pass}; use rustc_span::edition::Edition; -use rustc_span::symbol::sym; declare_lint! { /// The `forbidden_lint_groups` lint detects violations of @@ -2577,16 +2576,11 @@ declare_lint! { declare_lint! { /// The `unsafe_op_in_unsafe_fn` lint detects unsafe operations in unsafe - /// functions without an explicit unsafe block. This lint only works on - /// the [**nightly channel**] with the - /// `#![feature(unsafe_block_in_unsafe_fn)]` feature. - /// - /// [**nightly channel**]: https://doc.rust-lang.org/book/appendix-07-nightly-rust.html + /// functions without an explicit unsafe block. /// /// ### Example /// /// ```rust,compile_fail - /// #![feature(unsafe_block_in_unsafe_fn)] /// #![deny(unsafe_op_in_unsafe_fn)] /// /// unsafe fn foo() {} @@ -2624,7 +2618,6 @@ declare_lint! { pub UNSAFE_OP_IN_UNSAFE_FN, Allow, "unsafe operations in unsafe functions without an explicit unsafe block are deprecated", - @feature_gate = sym::unsafe_block_in_unsafe_fn; } declare_lint! { diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index c293fbe4ef8ca..855a5980e287b 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -28,11 +28,9 @@ pub enum UnsafetyViolationKind { BorrowPacked, /// Unsafe operation in an `unsafe fn` but outside an `unsafe` block. /// Has to be handled as a lint for backwards compatibility. - /// Should stay gated under `#![feature(unsafe_block_in_unsafe_fn)]`. UnsafeFn, /// Borrow of packed field in an `unsafe fn` but outside an `unsafe` block. /// Has to be handled as a lint for backwards compatibility. - /// Should stay gated under `#![feature(unsafe_block_in_unsafe_fn)]`. UnsafeFnBorrowPacked, } diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index f0472758dfb8e..6fd3290b7d03c 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -340,7 +340,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { false } // With the RFC 2585, no longer allow `unsafe` operations in `unsafe fn`s - Safety::FnUnsafe if self.tcx.features().unsafe_block_in_unsafe_fn => { + Safety::FnUnsafe => { for violation in violations { let mut violation = *violation; @@ -355,8 +355,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { } false } - // `unsafe` function bodies allow unsafe without additional unsafe blocks (before RFC 2585) - Safety::BuiltinUnsafe | Safety::FnUnsafe => true, + Safety::BuiltinUnsafe => true, Safety::ExplicitUnsafe(hir_id) => { // mark unsafe block as used if there are any unsafe operations inside if !violations.is_empty() { diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 99c42a4ba4423..1488dc15432e5 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -128,7 +128,7 @@ #![feature(trusted_len)] #![feature(unboxed_closures)] #![feature(unicode_internals)] -#![feature(unsafe_block_in_unsafe_fn)] +#![cfg_attr(bootstrap, feature(unsafe_block_in_unsafe_fn))] #![feature(unsize)] #![feature(unsized_fn_params)] #![feature(allocator_internals)] diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 7c0e5ab8926ef..fbb7178f7b6df 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -162,8 +162,8 @@ #![feature(const_caller_location)] #![feature(slice_ptr_get)] #![feature(no_niche)] // rust-lang/rust#68303 -#![feature(unsafe_block_in_unsafe_fn)] #![feature(int_error_matching)] +#![cfg_attr(bootstrap, feature(unsafe_block_in_unsafe_fn))] #![deny(unsafe_op_in_unsafe_fn)] #[prelude_import] diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 12182accc471a..9a9f27e564e22 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -71,7 +71,6 @@ #![feature(peekable_peek_mut)] #![cfg_attr(not(bootstrap), feature(ptr_metadata))] #![feature(once_cell)] -#![feature(unsafe_block_in_unsafe_fn)] #![feature(unsized_tuple_coercion)] #![feature(int_bits_const)] #![feature(nonzero_leading_trailing_zeros)] @@ -79,8 +78,9 @@ #![feature(integer_atomics)] #![feature(slice_group_by)] #![feature(trusted_random_access)] -#![deny(unsafe_op_in_unsafe_fn)] +#![cfg_attr(bootstrap, feature(unsafe_block_in_unsafe_fn))] #![cfg_attr(not(bootstrap), feature(unsize))] +#![deny(unsafe_op_in_unsafe_fn)] extern crate test; diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 16733b7ccd353..1a5becf2a95ca 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -324,7 +324,7 @@ #![feature(try_blocks)] #![feature(try_reserve)] #![feature(unboxed_closures)] -#![feature(unsafe_block_in_unsafe_fn)] +#![cfg_attr(bootstrap, feature(unsafe_block_in_unsafe_fn))] #![feature(unsafe_cell_raw_get)] #![feature(unwind_attributes)] #![feature(vec_into_raw_parts)] diff --git a/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs b/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs deleted file mode 100644 index 61e512a12a18d..0000000000000 --- a/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs +++ /dev/null @@ -1,6 +0,0 @@ -#![deny(unsafe_op_in_unsafe_fn)] -//~^ ERROR the `unsafe_op_in_unsafe_fn` lint is unstable -//~| ERROR the `unsafe_op_in_unsafe_fn` lint is unstable -//~| ERROR the `unsafe_op_in_unsafe_fn` lint is unstable - -fn main() {} diff --git a/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr b/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr deleted file mode 100644 index c5cad4a98d9ca..0000000000000 --- a/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr +++ /dev/null @@ -1,30 +0,0 @@ -error[E0658]: the `unsafe_op_in_unsafe_fn` lint is unstable - --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:1 - | -LL | #![deny(unsafe_op_in_unsafe_fn)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: see issue #71668 for more information - = help: add `#![feature(unsafe_block_in_unsafe_fn)]` to the crate attributes to enable - -error[E0658]: the `unsafe_op_in_unsafe_fn` lint is unstable - --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:1 - | -LL | #![deny(unsafe_op_in_unsafe_fn)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: see issue #71668 for more information - = help: add `#![feature(unsafe_block_in_unsafe_fn)]` to the crate attributes to enable - -error[E0658]: the `unsafe_op_in_unsafe_fn` lint is unstable - --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:1 - | -LL | #![deny(unsafe_op_in_unsafe_fn)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: see issue #71668 for more information - = help: add `#![feature(unsafe_block_in_unsafe_fn)]` to the crate attributes to enable - -error: aborting due to 3 previous errors - -For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs index 9eec7e0e8fe62..c8400a6fc4d0d 100644 --- a/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs +++ b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs @@ -1,4 +1,3 @@ -#![feature(unsafe_block_in_unsafe_fn)] #![deny(unsafe_op_in_unsafe_fn)] #![deny(unused_unsafe)] diff --git a/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr index 278a036c9f19f..3157783acb6af 100644 --- a/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr +++ b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr @@ -1,18 +1,18 @@ error: call to unsafe function is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:10:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:9:5 | LL | unsf(); | ^^^^^^ call to unsafe function | note: the lint level is defined here - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:2:9 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:1:9 | LL | #![deny(unsafe_op_in_unsafe_fn)] | ^^^^^^^^^^^^^^^^^^^^^^ = note: consult the function's documentation for information on how to avoid undefined behavior error: dereference of raw pointer is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:12:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:11:5 | LL | *PTR; | ^^^^ dereference of raw pointer @@ -20,7 +20,7 @@ LL | *PTR; = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior error: use of mutable static is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:14:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:13:5 | LL | VOID = (); | ^^^^^^^^^ use of mutable static @@ -28,25 +28,25 @@ LL | VOID = (); = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:17:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:16:5 | LL | unsafe {} | ^^^^^^ unnecessary `unsafe` block | note: the lint level is defined here - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:3:9 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:2:9 | LL | #![deny(unused_unsafe)] | ^^^^^^^^^^^^^ error: call to unsafe function is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:25:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:24:5 | LL | unsf(); | ^^^^^^ call to unsafe function | note: the lint level is defined here - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:23:8 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:22:8 | LL | #[deny(warnings)] | ^^^^^^^^ @@ -54,7 +54,7 @@ LL | #[deny(warnings)] = note: consult the function's documentation for information on how to avoid undefined behavior error: dereference of raw pointer is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:27:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:26:5 | LL | *PTR; | ^^^^ dereference of raw pointer @@ -62,7 +62,7 @@ LL | *PTR; = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior error: use of mutable static is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:29:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:28:5 | LL | VOID = (); | ^^^^^^^^^ use of mutable static @@ -70,13 +70,13 @@ LL | VOID = (); = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:31:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:30:5 | LL | unsafe {} | ^^^^^^ unnecessary `unsafe` block error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:45:14 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:44:14 | LL | unsafe { unsafe { unsf() } } | ------ ^^^^^^ unnecessary `unsafe` block @@ -84,7 +84,7 @@ LL | unsafe { unsafe { unsf() } } | because it's nested under this `unsafe` block error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:56:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:55:5 | LL | unsafe fn allow_level() { | ----------------------- because it's nested under this `unsafe` fn @@ -93,7 +93,7 @@ LL | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:68:9 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:67:9 | LL | unsafe fn nested_allow_level() { | ------------------------------ because it's nested under this `unsafe` fn @@ -102,7 +102,7 @@ LL | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block error[E0133]: call to unsafe function is unsafe and requires unsafe block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:74:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:73:5 | LL | unsf(); | ^^^^^^ call to unsafe function @@ -110,7 +110,7 @@ LL | unsf(); = note: consult the function's documentation for information on how to avoid undefined behavior error[E0133]: call to unsafe function is unsafe and requires unsafe function or block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:78:9 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:77:9 | LL | unsf(); | ^^^^^^ call to unsafe function From fbd575aedf1a60ca5528d5be945639e02d44b3e7 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Feb 2021 14:30:03 +0000 Subject: [PATCH 02/26] process::unix: Handle other wait statuses in ExitStatus as Display Currently, on Nightly, this panics: ``` use std::process::ExitStatus; use std::os::unix::process::ExitStatusExt; fn main() { let st = ExitStatus::from_raw(0x007f); println!("st = {}", st); } ``` This is because the impl of Display assumes that if .code() is None, .signal() must be Some. That was a false assumption, although it was true with buggy code before 5b1316f78152a9c066b357ea9addf803d48e114a unix ExitStatus: Do not treat WIFSTOPPED as WIFSIGNALED This is not likely to have affected many people in practice, because `Command` will never produce such a wait status (`ExitStatus`). Signed-off-by: Ian Jackson --- library/std/src/sys/unix/process/process_unix.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 9e82df7755e89..26cbb0a508318 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -527,9 +527,18 @@ impl fmt::Display for ExitStatus { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(code) = self.code() { write!(f, "exit code: {}", code) + } else if let Some(signal) = self.signal() { + if self.core_dumped() { + write!(f, "signal: {} (core dumped)", signal) + } else { + write!(f, "signal: {}", signal) + } + } else if let Some(signal) = self.stopped_signal() { + write!(f, "stopped (not terminated) by signal: {}", signal) + } else if self.continued() { + write!(f, "continued (WIFCONTINUED)") } else { - let signal = self.signal().unwrap(); - write!(f, "signal: {}", signal) + write!(f, "unrecognised wait status: {} {:#x}", self.0, self.0) } } } From d8cfd56985bd8cc32274bfead4b1499da1c38810 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Feb 2021 14:58:52 +0000 Subject: [PATCH 03/26] process::unix: Test wait status formatting Signed-off-by: Ian Jackson --- .../std/src/sys/unix/process/process_unix.rs | 4 ++++ .../sys/unix/process/process_unix/tests.rs | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 library/std/src/sys/unix/process/process_unix/tests.rs diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 26cbb0a508318..d2ef0eb128c5d 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -542,3 +542,7 @@ impl fmt::Display for ExitStatus { } } } + +#[cfg(test)] +#[path = "process_unix/tests.rs"] +mod tests; diff --git a/library/std/src/sys/unix/process/process_unix/tests.rs b/library/std/src/sys/unix/process/process_unix/tests.rs new file mode 100644 index 0000000000000..60cb161aca2ab --- /dev/null +++ b/library/std/src/sys/unix/process/process_unix/tests.rs @@ -0,0 +1,22 @@ +#[test] +fn exitstatus_display_tests() { + // In practice this is the same on every Unix. + // If some weird platform turns out to be different, and this test fails, use #[cfg]. + use crate::os::unix::process::ExitStatusExt; + use crate::process::ExitStatus; + + let t = |v, s| assert_eq!(s, format!("{}", ::from_raw(v))); + + t(0x0000f, "signal: 15"); + t(0x0008b, "signal: 11 (core dumped)"); + t(0x00000, "exit code: 0"); + t(0x0ff00, "exit code: 255"); + t(0x0137f, "stopped (not terminated) by signal: 19"); + t(0x0ffff, "continued (WIFCONTINUED)"); + + // Testing "unrecognised wait status" is hard because the wait.h macros typically + // assume that the value came from wait and isn't mad. With the glibc I have here + // this works: + #[cfg(target_env = "gnu")] + t(0x000ff, "unrecognised wait status: 255 0xff"); +} From 4bb8425af60eb673a932c90ee8d1b5f24c13a34e Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Feb 2021 15:26:19 +0000 Subject: [PATCH 04/26] ExitStatus: Improve documentation re wait status vs exit status The use of `ExitStatus` as the Rust type name for a Unix *wait status*, not an *exit status*, is very confusing, but sadly probably too late to change. This area is confusing enough in Unix already (and many programmers are already confuxed). We can at least document it. I chose *not* to mention the way shells like to exit with signal numbers, thus turning signal numbers into exit statuses. This is only relevant for Rust programs using `std::process` if they run shells. Signed-off-by: Ian Jackson --- library/std/src/process.rs | 25 ++++++++++++++++++------- library/std/src/sys/unix/ext/process.rs | 14 ++++++++++++-- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 6480e654c55f0..15ac9e402c589 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -885,7 +885,7 @@ impl Command { } /// Executes a command as a child process, waiting for it to finish and - /// collecting its exit status. + /// collecting its status. /// /// By default, stdin, stdout and stderr are inherited from the parent. /// @@ -899,7 +899,7 @@ impl Command { /// .status() /// .expect("failed to execute process"); /// - /// println!("process exited with: {}", status); + /// println!("process finished with: {}", status); /// /// assert!(status.success()); /// ``` @@ -1368,11 +1368,17 @@ impl From for Stdio { /// Describes the result of a process after it has terminated. /// -/// This `struct` is used to represent the exit status of a child process. +/// This `struct` is used to represent the exit status or other termination of a child process. /// Child processes are created via the [`Command`] struct and their exit /// status is exposed through the [`status`] method, or the [`wait`] method /// of a [`Child`] process. /// +/// An `ExitStatus` represents every possible disposition of a process. On Unix this +/// is the **wait status**. It is *not* simply an *exit status* (a value passed to `exit`). +/// +/// For proper error reporting of failed processes, print the value of `ExitStatus` using its +/// implementation of [`Display`](crate::fmt::Display). +/// /// [`status`]: Command::status /// [`wait`]: Child::wait #[derive(PartialEq, Eq, Clone, Copy, Debug)] @@ -1400,7 +1406,7 @@ impl ExitStatus { /// if status.success() { /// println!("'projects/' directory created"); /// } else { - /// println!("failed to create 'projects/' directory"); + /// println!("failed to create 'projects/' directory: {}", status); /// } /// ``` #[stable(feature = "process", since = "1.0.0")] @@ -1410,9 +1416,14 @@ impl ExitStatus { /// Returns the exit code of the process, if any. /// - /// On Unix, this will return `None` if the process was terminated - /// by a signal; `std::os::unix` provides an extension trait for - /// extracting the signal and other details from the `ExitStatus`. + /// In Unix terms the return value is the **exit status**: the value passed to `exit`, if the + /// process finished by calling `exit`. Note that on Unix the exit status is truncated to 8 + /// bits, and that values that didn't come from a program's call to `exit` may be invented the + /// runtime system (often, for example, 255, 254, 127 or 126). + /// + /// On Unix, this will return `None` if the process was terminated by a signal. + /// [`ExitStatusExt`](crate::os::unix::process::ExitStatusExt) is an + /// extension trait for extracting any such signal, and other details, from the `ExitStatus`. /// /// # Examples /// diff --git a/library/std/src/sys/unix/ext/process.rs b/library/std/src/sys/unix/ext/process.rs index 7559c1f1d9e29..6dc2033289a4a 100644 --- a/library/std/src/sys/unix/ext/process.rs +++ b/library/std/src/sys/unix/ext/process.rs @@ -186,12 +186,20 @@ impl CommandExt for process::Command { /// Unix-specific extensions to [`process::ExitStatus`]. /// +/// On Unix, `ExitStatus` **does not necessarily represent an exit status**, as passed to the +/// `exit` system call or returned by [`ExitStatus::code()`](crate::process::ExitStatus::code). +/// It represents **any wait status**, as returned by one of the `wait` family of system calls. +/// +/// This is because a Unix wait status (a Rust `ExitStatus`) can represent a Unix exit status, but +/// can also represent other kinds of process event. +/// /// This trait is sealed: it cannot be implemented outside the standard library. /// This is so that future additional methods are not breaking changes. #[stable(feature = "rust1", since = "1.0.0")] pub trait ExitStatusExt: Sealed { - /// Creates a new `ExitStatus` from the raw underlying `i32` return value of - /// a process. + /// Creates a new `ExitStatus` from the raw underlying integer status value from `wait` + /// + /// The value should be a **wait status, not an exit status**. #[stable(feature = "exit_status_from", since = "1.12.0")] fn from_raw(raw: i32) -> Self; @@ -220,6 +228,8 @@ pub trait ExitStatusExt: Sealed { fn continued(&self) -> bool; /// Returns the underlying raw `wait` status. + /// + /// The returned integer is a **wait status, not an exit status**. #[unstable(feature = "unix_process_wait_more", issue = "80695")] fn into_raw(self) -> i32; } From 67cfc22ee228cee1a795ca1f7430165984fe1b04 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 4 Mar 2021 12:18:04 +0000 Subject: [PATCH 05/26] ExitStatus stop signal display test: Make it Linux only MacOS uses a different representation. Signed-off-by: Ian Jackson --- library/std/src/sys/unix/process/process_unix/tests.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix/tests.rs b/library/std/src/sys/unix/process/process_unix/tests.rs index 60cb161aca2ab..3ab568eaa33fe 100644 --- a/library/std/src/sys/unix/process/process_unix/tests.rs +++ b/library/std/src/sys/unix/process/process_unix/tests.rs @@ -11,8 +11,13 @@ fn exitstatus_display_tests() { t(0x0008b, "signal: 11 (core dumped)"); t(0x00000, "exit code: 0"); t(0x0ff00, "exit code: 255"); - t(0x0137f, "stopped (not terminated) by signal: 19"); - t(0x0ffff, "continued (WIFCONTINUED)"); + + // On MacOS, 0x0137f is WIFCONTINUED, not WIFSTOPPED. Probably *BSD is similar. + // https://github.com/rust-lang/rust/pull/82749#issuecomment-790525956 + // The purpose of this test is to test our string formatting, not our understanding of the wait + // status magic numbers. So restrict these to Linux. + #[cfg(target_os = "linux")] t(0x0137f, "stopped (not terminated) by signal: 19"); + #[cfg(target_os = "linux")] t(0x0ffff, "continued (WIFCONTINUED)"); // Testing "unrecognised wait status" is hard because the wait.h macros typically // assume that the value came from wait and isn't mad. With the glibc I have here From a240ff5a7713f744755855c507eab327f90cd824 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 4 Mar 2021 12:26:27 +0000 Subject: [PATCH 06/26] ExitStatus unknown wait status test: Make it Linux only If different unices have different bit patterns for WIFSTOPPED and WIFCONTINUED then simply being glibc is probably not good enough for this rather ad-hoc test to work. Do it on Linux only. Signed-off-by: Ian Jackson --- library/std/src/sys/unix/process/process_unix/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/process/process_unix/tests.rs b/library/std/src/sys/unix/process/process_unix/tests.rs index 3ab568eaa33fe..5d1bf47083513 100644 --- a/library/std/src/sys/unix/process/process_unix/tests.rs +++ b/library/std/src/sys/unix/process/process_unix/tests.rs @@ -22,6 +22,6 @@ fn exitstatus_display_tests() { // Testing "unrecognised wait status" is hard because the wait.h macros typically // assume that the value came from wait and isn't mad. With the glibc I have here // this works: - #[cfg(target_env = "gnu")] + #[cfg(all(target_os = "linux", target_env = "gnu"))] t(0x000ff, "unrecognised wait status: 255 0xff"); } From 8e4433ab3e6a0fe8cc6f83379b30a48f94da4f33 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 4 Mar 2021 12:44:19 +0000 Subject: [PATCH 07/26] ExitStatus tests: Make less legible to satisfy "tidy" I strongly disagree with tidy in this case but AIUI there is no way to override it. Signed-off-by: Ian Jackson --- library/std/src/sys/unix/process/process_unix/tests.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix/tests.rs b/library/std/src/sys/unix/process/process_unix/tests.rs index 5d1bf47083513..915402970f58c 100644 --- a/library/std/src/sys/unix/process/process_unix/tests.rs +++ b/library/std/src/sys/unix/process/process_unix/tests.rs @@ -16,8 +16,10 @@ fn exitstatus_display_tests() { // https://github.com/rust-lang/rust/pull/82749#issuecomment-790525956 // The purpose of this test is to test our string formatting, not our understanding of the wait // status magic numbers. So restrict these to Linux. - #[cfg(target_os = "linux")] t(0x0137f, "stopped (not terminated) by signal: 19"); - #[cfg(target_os = "linux")] t(0x0ffff, "continued (WIFCONTINUED)"); + #[cfg(target_os = "linux")] + t(0x0137f, "stopped (not terminated) by signal: 19"); + #[cfg(target_os = "linux")] + t(0x0ffff, "continued (WIFCONTINUED)"); // Testing "unrecognised wait status" is hard because the wait.h macros typically // assume that the value came from wait and isn't mad. With the glibc I have here From ccca7678345f403eb3db93a285a95b15c13b25d1 Mon Sep 17 00:00:00 2001 From: yn0ga Date: Wed, 3 Mar 2021 21:17:17 +0100 Subject: [PATCH 08/26] Add powerpc-unknown-openbsd target Add powerpc-unknown-openbsd target * Fix missing abi::endian crate * Missing platform-support.md --- compiler/rustc_target/src/spec/mod.rs | 1 + .../src/spec/powerpc_unknown_openbsd.rs | 16 ++++++++++++++++ src/doc/rustc/src/platform-support.md | 1 + 3 files changed, 18 insertions(+) create mode 100644 compiler/rustc_target/src/spec/powerpc_unknown_openbsd.rs diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 039e9a8b2745f..6f7fd16cc6ee9 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -694,6 +694,7 @@ supported_targets! { ("i686-unknown-openbsd", i686_unknown_openbsd), ("sparc64-unknown-openbsd", sparc64_unknown_openbsd), ("x86_64-unknown-openbsd", x86_64_unknown_openbsd), + ("powerpc-unknown-openbsd", powerpc_unknown_openbsd), ("aarch64-unknown-netbsd", aarch64_unknown_netbsd), ("armv6-unknown-netbsd-eabihf", armv6_unknown_netbsd_eabihf), diff --git a/compiler/rustc_target/src/spec/powerpc_unknown_openbsd.rs b/compiler/rustc_target/src/spec/powerpc_unknown_openbsd.rs new file mode 100644 index 0000000000000..c17183faa7ae0 --- /dev/null +++ b/compiler/rustc_target/src/spec/powerpc_unknown_openbsd.rs @@ -0,0 +1,16 @@ +use crate::abi::Endian; +use crate::spec::Target; + +pub fn target() -> Target { + let mut base = super::openbsd_base::opts(); + base.endian = Endian::Big; + base.max_atomic_width = Some(32); + + Target { + llvm_target: "powerpc-unknown-openbsd".to_string(), + pointer_width: 32, + data_layout: "E-m:e-p:32:32-i64:64-n32".to_string(), + arch: "powerpc".to_string(), + options: base, + } +} diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index d3b88c019f019..869428a47d2e4 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -198,6 +198,7 @@ target | std | host | notes `powerpc-unknown-linux-gnuspe` | ✓ | | PowerPC SPE Linux `powerpc-unknown-linux-musl` | ? | | `powerpc-unknown-netbsd` | ✓ | ✓ | +`powerpc-unknown-openbsd` | ? | | `powerpc-wrs-vxworks` | ? | | `powerpc-wrs-vxworks-spe` | ? | | `powerpc64-unknown-freebsd` | ✓ | ✓ | PPC64 FreeBSD (ELFv1 and ELFv2) From a05a890c35150efbfddcd8788e998bbf7d605847 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 5 Mar 2021 10:48:21 -0500 Subject: [PATCH 09/26] Build rustdoc for run-make tests, not just run-make-fulldeps Rustdoc almost never needs a full stage 2 compiler, and requiring rustdoc tests to be in run-make-fulldeps adds a lot of compile time for no reason. --- src/bootstrap/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index c0cd24dd81f01..9b06fcf343bd5 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1076,7 +1076,7 @@ note: if you're sure you want to do this, please open an issue as to why. In the // Avoid depending on rustdoc when we don't need it. if mode == "rustdoc" - || (mode == "run-make" && suite.ends_with("fulldeps")) + || mode == "run-make" || (mode == "ui" && is_rustdoc) || mode == "js-doc-test" || mode == "rustdoc-json" From 7e3ebe76ee6b0b495112f56d16e7067a856c0cea Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Sat, 6 Mar 2021 16:01:34 -0600 Subject: [PATCH 10/26] Add Option::get_or_default --- library/core/src/option.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index e3c812a047c8c..9478e7f06bdf3 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -854,6 +854,34 @@ impl Option { // Entry-like operations to insert if None and return a reference ///////////////////////////////////////////////////////////////////////// + /// Inserts the default value into the option if it is [`None`], then + /// returns a mutable reference to the contained value. + /// + /// # Examples + /// + /// ``` + /// #![feature(option_get_or_default)] + /// + /// let mut x = None; + /// + /// { + /// let y: &mut u32 = x.get_or_default(); + /// assert_eq!(y, &0); + /// + /// *y = 7; + /// } + /// + /// assert_eq!(x, Some(7)); + /// ``` + #[inline] + #[unstable(feature = "option_get_or_default", issue = "82901")] + pub fn get_or_default(&mut self) -> &mut T + where + T: Default, + { + self.get_or_insert_with(Default::default) + } + /// Inserts `value` into the option if it is [`None`], then /// returns a mutable reference to the contained value. /// From 1cc8c4de6aa0549a3d7d1da23ba48d34d0efd7bf Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Sat, 6 Mar 2021 16:40:45 -0600 Subject: [PATCH 11/26] Use Option::get_or_default --- compiler/rustc_mir/src/lib.rs | 1 + compiler/rustc_mir/src/transform/coverage/graph.rs | 5 +---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir/src/lib.rs b/compiler/rustc_mir/src/lib.rs index 508510a81e1fb..bbfcec5a76a43 100644 --- a/compiler/rustc_mir/src/lib.rs +++ b/compiler/rustc_mir/src/lib.rs @@ -25,6 +25,7 @@ Rust MIR: a lowered representation of Rust. #![feature(stmt_expr_attributes)] #![feature(trait_alias)] #![feature(option_expect_none)] +#![feature(option_get_or_default)] #![feature(or_patterns)] #![feature(once_cell)] #![feature(control_flow_enum)] diff --git a/compiler/rustc_mir/src/transform/coverage/graph.rs b/compiler/rustc_mir/src/transform/coverage/graph.rs index e58b915f1264c..8ad0d133b17e1 100644 --- a/compiler/rustc_mir/src/transform/coverage/graph.rs +++ b/compiler/rustc_mir/src/transform/coverage/graph.rs @@ -392,10 +392,7 @@ impl BasicCoverageBlockData { } } let operand = counter_kind.as_operand_id(); - if let Some(replaced) = self - .edge_from_bcbs - .get_or_insert_with(FxHashMap::default) - .insert(from_bcb, counter_kind) + if let Some(replaced) = self.edge_from_bcbs.get_or_default().insert(from_bcb, counter_kind) { Error::from_string(format!( "attempt to set an edge counter more than once; from_bcb: \ From 50bdd51ea8881b27491143de89b8e0bc98839629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauren=C8=9Biu=20Nicola?= Date: Mon, 8 Mar 2021 17:30:42 +0200 Subject: [PATCH 12/26] :arrow_up: rust-analyzer --- src/tools/rust-analyzer | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/rust-analyzer b/src/tools/rust-analyzer index 5df3ee8274fdb..d54e1157b6601 160000 --- a/src/tools/rust-analyzer +++ b/src/tools/rust-analyzer @@ -1 +1 @@ -Subproject commit 5df3ee8274fdb7cdeb2b0871b4efea8cbf4724a1 +Subproject commit d54e1157b66017e4aae38328cd213286e39ca130 From 11ca64401a5d562898e8b5f46bd36d6d1c6dc3ef Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Tue, 9 Mar 2021 10:53:03 +0000 Subject: [PATCH 13/26] Always compile the fragile wait status test cases, just run them conditionally Co-authored-by: David Tolnay --- .../std/src/sys/unix/process/process_unix/tests.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix/tests.rs b/library/std/src/sys/unix/process/process_unix/tests.rs index 915402970f58c..5819d2c2a5a26 100644 --- a/library/std/src/sys/unix/process/process_unix/tests.rs +++ b/library/std/src/sys/unix/process/process_unix/tests.rs @@ -16,14 +16,15 @@ fn exitstatus_display_tests() { // https://github.com/rust-lang/rust/pull/82749#issuecomment-790525956 // The purpose of this test is to test our string formatting, not our understanding of the wait // status magic numbers. So restrict these to Linux. - #[cfg(target_os = "linux")] - t(0x0137f, "stopped (not terminated) by signal: 19"); - #[cfg(target_os = "linux")] - t(0x0ffff, "continued (WIFCONTINUED)"); + if cfg!(target_os = "linux") { + t(0x0137f, "stopped (not terminated) by signal: 19"); + t(0x0ffff, "continued (WIFCONTINUED)"); + } // Testing "unrecognised wait status" is hard because the wait.h macros typically // assume that the value came from wait and isn't mad. With the glibc I have here // this works: - #[cfg(all(target_os = "linux", target_env = "gnu"))] - t(0x000ff, "unrecognised wait status: 255 0xff"); + if cfg!(all(target_os = "linux", target_env = "gnu")) { + t(0x000ff, "unrecognised wait status: 255 0xff"); + } } From 52d9792bc8990d41ced929cff31962f013b5074a Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Tue, 9 Mar 2021 10:59:35 -0500 Subject: [PATCH 14/26] Update README.md to use the correct cmake version number LLVM requires at least cmake 3.13.4 and cmake is only required to build LLVM. Also closes #42555 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 6ab11e7e2be6d..cc073875cde57 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ by running `./x.py --help` or reading the [rustc dev guide][rustcguidebuild]. * `g++` 5.1 or later or `clang++` 3.5 or later * `python` 3 or 2.7 * GNU `make` 3.81 or later - * `cmake` 3.4.3 or later + * `cmake` 3.13.4 or later * `ninja` * `curl` * `git` From 62f2d723300600933be5078c1a42b559aaf03291 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 9 Mar 2021 16:44:51 +0000 Subject: [PATCH 15/26] Bump tracing-tree dependency --- Cargo.lock | 4 ++-- compiler/rustc_driver/Cargo.toml | 2 +- src/librustdoc/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 120403b3bf64c..25039b5cbd92a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5460,9 +5460,9 @@ dependencies = [ [[package]] name = "tracing-tree" -version = "0.1.8" +version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a60657cfbf397c603257a8230b3f427e6a2a4e5911a59331b9bb4dffff5b608" +checksum = "1712b40907f8d9bc2bc66763ab61dec914b7123d7149e59feb0d4e2a95fc4967" dependencies = [ "ansi_term 0.12.1", "atty", diff --git a/compiler/rustc_driver/Cargo.toml b/compiler/rustc_driver/Cargo.toml index 47cff34cd3e8c..93c6ec04e4fd6 100644 --- a/compiler/rustc_driver/Cargo.toml +++ b/compiler/rustc_driver/Cargo.toml @@ -12,7 +12,7 @@ libc = "0.2" atty = "0.2" tracing = { version = "0.1.25" } tracing-subscriber = { version = "0.2.16", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] } -tracing-tree = "0.1.8" +tracing-tree = "0.1.9" rustc_middle = { path = "../rustc_middle" } rustc_ast_pretty = { path = "../rustc_ast_pretty" } rustc_target = { path = "../rustc_target" } diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index 9084a1713cb05..2d0722396fcf3 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -20,7 +20,7 @@ itertools = "0.9" regex = "1" rustdoc-json-types = { path = "../rustdoc-json-types" } tracing = "0.1" -tracing-tree = "0.1.6" +tracing-tree = "0.1.9" [dependencies.tracing-subscriber] version = "0.2.13" From 0fdc07e197e3d0265452e266d038b089b69da6cb Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 3 Oct 2020 20:57:47 +0000 Subject: [PATCH 16/26] Impl StatementKind::CopyNonOverlapping --- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 6 ++++-- .../rustc_codegen_ssa/src/mir/statement.rs | 15 +++++++++++++ compiler/rustc_middle/src/mir/mod.rs | 17 +++++++++++++++ compiler/rustc_middle/src/mir/visit.rs | 21 +++++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 289629d921545..5ad9f46147282 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -293,7 +293,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | MutatingUseContext::AsmOutput | MutatingUseContext::Borrow | MutatingUseContext::AddressOf - | MutatingUseContext::Projection, + | MutatingUseContext::Projection + | MutatingUseContext::CopyNonOverlapping, ) | PlaceContext::NonMutatingUse( NonMutatingUseContext::Inspect @@ -301,7 +302,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | NonMutatingUseContext::UniqueBorrow | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::AddressOf - | NonMutatingUseContext::Projection, + | NonMutatingUseContext::Projection + | NonMutatingUseContext::CopyNonOverlapping, ) => { self.not_ssa(local); } diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 6f74ba77d4c16..2c90054b6c7f5 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -115,6 +115,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { self.codegen_coverage(&mut bx, coverage.clone()); bx } + mir::StatementKind::CopyNonOverlapping(box mir::CopyNonOverlapping { + ref src, + ref dst, + ref size, + }) => { + bx.memcpy( + dst, + todo!(), + src, + todo!(), + size, + todo!(), + ); + bx + } mir::StatementKind::FakeRead(..) | mir::StatementKind::Retag { .. } | mir::StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 42bbc9a0d9552..90111d4080c7e 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1541,6 +1541,11 @@ pub enum StatementKind<'tcx> { /// counter varible at runtime, each time the code region is executed. Coverage(Box), + /// Denotes a call to the intrinsic function copy_overlapping, where `src_dst` denotes the + /// memory being read from and written to(one field to save memory), and size + /// indicates how many bytes are being copied over. + CopyNonOverlapping(Box>), + /// No-op. Useful for deleting instructions without affecting statement indices. Nop, } @@ -1659,6 +1664,11 @@ impl Debug for Statement<'_> { write!(fmt, "Coverage::{:?}", coverage.kind) } } + CopyNonOverlapping(box crate::mir::CopyNonOverlapping { + ref src, + ref dst, + ref size, + }) => write!(fmt, "src {:?} -> dst {:?}, {:?} bytes", src, dst, size), Nop => write!(fmt, "nop"), } } @@ -1670,6 +1680,13 @@ pub struct Coverage { pub code_region: Option, } +#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] +pub struct CopyNonOverlapping<'tcx> { + pub src: Place<'tcx>, + pub dst: Place<'tcx>, + pub size: Operand<'tcx>, +} + /////////////////////////////////////////////////////////////////////////// // Places diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 464220cf77ede..ad773adcd82ac 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -436,6 +436,23 @@ macro_rules! make_mir_visitor { location ) } + StatementKind::CopyNonOverlapping(box crate::mir::CopyNonOverlapping{ + ref $($mutability)? src, + ref $($mutability)? dst, + ref $($mutability)? size, + }) => { + self.visit_place( + src, + PlaceContext::NonMutatingUse(NonMutatingUseContext::CopyNonOverlapping), + location + ); + self.visit_place( + dst, + PlaceContext::MutatingUse(MutatingUseContext::CopyNonOverlapping), + location + ); + self.visit_operand(size, location) + } StatementKind::Nop => {} } } @@ -1151,6 +1168,8 @@ pub enum NonMutatingUseContext { /// f(&x.y); /// Projection, + /// Source from copy_nonoverlapping. + CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -1180,6 +1199,8 @@ pub enum MutatingUseContext { Projection, /// Retagging, a "Stacked Borrows" shadow state operation Retag, + /// Memory written to in copy_nonoverlapping. + CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] From 72c734d001a157e0fb38e1feebf6748189d3e1b9 Mon Sep 17 00:00:00 2001 From: kadmin Date: Mon, 5 Oct 2020 20:13:36 +0000 Subject: [PATCH 17/26] Update fmt and use of memcpy I'm still not totally sure if this is the right way to implement the memcpy, but that portion compiles correctly now. Now to fix the compile errors everywhere else :). --- .../rustc_codegen_ssa/src/mir/statement.rs | 23 +++++++++++-------- compiler/rustc_middle/src/mir/mod.rs | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 2c90054b6c7f5..b507cb0a82312 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -120,15 +120,20 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ref dst, ref size, }) => { - bx.memcpy( - dst, - todo!(), - src, - todo!(), - size, - todo!(), - ); - bx + let dst_val = self.codegen_place(&mut bx, dst.as_ref()); + let src_val = self.codegen_place(&mut bx, src.as_ref()); + let size_val = self.codegen_operand(&mut bx, size); + let size = size_val.immediate_or_packed_pair(&mut bx); + bx.memcpy( + dst_val.llval, + dst_val.align, + src_val.llval, + src_val.align, + size, + // TODO probably want to have this change based on alignment above? + crate::MemFlags::empty(), + ); + bx } mir::StatementKind::FakeRead(..) | mir::StatementKind::Retag { .. } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 90111d4080c7e..2db69c27fe86c 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1668,7 +1668,7 @@ impl Debug for Statement<'_> { ref src, ref dst, ref size, - }) => write!(fmt, "src {:?} -> dst {:?}, {:?} bytes", src, dst, size), + }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?},bytes={:?})", src, dst, size), Nop => write!(fmt, "nop"), } } From 89f45ed9f3ce7366ae7153421d637efcb22a45b8 Mon Sep 17 00:00:00 2001 From: kadmin Date: Mon, 5 Oct 2020 22:53:00 +0000 Subject: [PATCH 18/26] Update match branches This updates all places where match branches check on StatementKind or UseContext. This doesn't properly implement them, but adds TODOs where they are, and also adds some best guesses to what they should be in some cases. --- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 6 +- .../rustc_codegen_ssa/src/mir/statement.rs | 20 +- compiler/rustc_middle/src/mir/mod.rs | 6 +- compiler/rustc_middle/src/mir/visit.rs | 10 +- .../src/borrow_check/invalidation.rs | 15 + compiler/rustc_mir/src/borrow_check/mod.rs | 2 + .../src/borrow_check/type_check/mod.rs | 1 + .../rustc_mir/src/dataflow/impls/borrows.rs | 2 + .../src/dataflow/impls/storage_liveness.rs | 2 + .../src/dataflow/move_paths/builder.rs | 2 + compiler/rustc_mir/src/interpret/step.rs | 17 + .../src/transform/check_consts/validation.rs | 1 + .../rustc_mir/src/transform/check_unsafety.rs | 1 + compiler/rustc_mir/src/transform/dest_prop.rs | 1 + compiler/rustc_mir/src/transform/generator.rs | 1 + .../src/transform/instrument_coverage.rs | 1272 +++++++++++++++++ .../src/transform/remove_noop_landing_pads.rs | 1 + compiler/rustc_mir/src/util/spanview.rs | 1 + .../clippy_utils/src/qualify_min_const_fn.rs | 9 +- 19 files changed, 1346 insertions(+), 24 deletions(-) create mode 100644 compiler/rustc_mir/src/transform/instrument_coverage.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 5ad9f46147282..289629d921545 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -293,8 +293,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | MutatingUseContext::AsmOutput | MutatingUseContext::Borrow | MutatingUseContext::AddressOf - | MutatingUseContext::Projection - | MutatingUseContext::CopyNonOverlapping, + | MutatingUseContext::Projection, ) | PlaceContext::NonMutatingUse( NonMutatingUseContext::Inspect @@ -302,8 +301,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | NonMutatingUseContext::UniqueBorrow | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::AddressOf - | NonMutatingUseContext::Projection - | NonMutatingUseContext::CopyNonOverlapping, + | NonMutatingUseContext::Projection, ) => { self.not_ssa(local); } diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index b507cb0a82312..1dafc8cd2c16d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -120,18 +120,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ref dst, ref size, }) => { - let dst_val = self.codegen_place(&mut bx, dst.as_ref()); - let src_val = self.codegen_place(&mut bx, src.as_ref()); + let dst_val = self.codegen_operand(&mut bx, dst); + let src_val = self.codegen_operand(&mut bx, src); let size_val = self.codegen_operand(&mut bx, size); let size = size_val.immediate_or_packed_pair(&mut bx); + let dst = dst_val.immediate_or_packed_pair(&mut bx); + let src = src_val.immediate_or_packed_pair(&mut bx); + use crate::MemFlags; + let flags = + (!MemFlags::UNALIGNED) & (!MemFlags::VOLATILE) & (!MemFlags::NONTEMPORAL); bx.memcpy( - dst_val.llval, - dst_val.align, - src_val.llval, - src_val.align, + dst, + dst_val.layout.layout.align.pref, + src, + src_val.layout.layout.align.pref, size, - // TODO probably want to have this change based on alignment above? - crate::MemFlags::empty(), + flags, ); bx } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 2db69c27fe86c..dddda0f611229 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1668,7 +1668,7 @@ impl Debug for Statement<'_> { ref src, ref dst, ref size, - }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?},bytes={:?})", src, dst, size), + }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?}, size={:?})", src, dst, size), Nop => write!(fmt, "nop"), } } @@ -1682,8 +1682,8 @@ pub struct Coverage { #[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] pub struct CopyNonOverlapping<'tcx> { - pub src: Place<'tcx>, - pub dst: Place<'tcx>, + pub src: Operand<'tcx>, + pub dst: Operand<'tcx>, pub size: Operand<'tcx>, } diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index ad773adcd82ac..7bd4b72cc1240 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -441,14 +441,12 @@ macro_rules! make_mir_visitor { ref $($mutability)? dst, ref $($mutability)? size, }) => { - self.visit_place( + self.visit_operand( src, - PlaceContext::NonMutatingUse(NonMutatingUseContext::CopyNonOverlapping), location ); - self.visit_place( + self.visit_operand( dst, - PlaceContext::MutatingUse(MutatingUseContext::CopyNonOverlapping), location ); self.visit_operand(size, location) @@ -1168,8 +1166,6 @@ pub enum NonMutatingUseContext { /// f(&x.y); /// Projection, - /// Source from copy_nonoverlapping. - CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -1199,8 +1195,6 @@ pub enum MutatingUseContext { Projection, /// Retagging, a "Stacked Borrows" shadow state operation Retag, - /// Memory written to in copy_nonoverlapping. - CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 1a3ba16585d65..276d9381e32ae 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -92,6 +92,21 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, input); } } + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + ref src, + ref dst, + ref size, + }) => { + self.consume_operand(location, src); + self.consume_operand(location, dst); + self.consume_operand(location, size); + match dst { + Operand::Move(ref place) | Operand::Copy(ref place) => { + self.mutate_place(location, *place, Deep, JustWrite); + } + _ => {} + } + } StatementKind::Nop | StatementKind::Coverage(..) | StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index dcf3093baaf41..539319ab9f25f 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -626,6 +626,8 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc self.consume_operand(location, (input, span), flow_state); } } + + StatementKind::CopyNonOverlapping(..) => todo!(), StatementKind::Nop | StatementKind::Coverage(..) | StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index f6bbd3b6283de..74d7fd84c9e72 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -1520,6 +1520,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ); } } + StatementKind::CopyNonOverlapping(..) => todo!(), StatementKind::FakeRead(..) | StatementKind::StorageLive(..) | StatementKind::StorageDead(..) diff --git a/compiler/rustc_mir/src/dataflow/impls/borrows.rs b/compiler/rustc_mir/src/dataflow/impls/borrows.rs index b149ffa9667a3..ffa02f855c979 100644 --- a/compiler/rustc_mir/src/dataflow/impls/borrows.rs +++ b/compiler/rustc_mir/src/dataflow/impls/borrows.rs @@ -306,6 +306,8 @@ impl<'tcx> dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> { | mir::StatementKind::AscribeUserType(..) | mir::StatementKind::Coverage(..) | mir::StatementKind::Nop => {} + + mir::StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs index 9250cd408479a..e407f394c51ec 100644 --- a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs +++ b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs @@ -150,6 +150,8 @@ impl<'mir, 'tcx> dataflow::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, | StatementKind::Nop | StatementKind::Retag(..) | StatementKind::StorageLive(..) => {} + + StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs index 67c3b043262d5..c14ac74ebadab 100644 --- a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs +++ b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs @@ -319,6 +319,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) | StatementKind::Nop => {} + + StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index c22d91fd82a21..5a3fc9f51611d 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -113,6 +113,23 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { M::retag(self, *kind, &dest)?; } + // Call CopyNonOverlapping + CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, size }) => { + let size = self.eval_operand(size, None)?; + + let dst = { + let dst = self.eval_operand(dst, None)?; + dst.assert_mem_place(self) + }; + let src = { + let src = self.eval_operand(src, None)?; + src.assert_mem_place(self) + }; + // Not sure how to convert an MPlaceTy<'_, >::PointerTag> + // to a pointer, or OpTy to a size + self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + } + // Statements we do not track. AscribeUserType(..) => {} diff --git a/compiler/rustc_mir/src/transform/check_consts/validation.rs b/compiler/rustc_mir/src/transform/check_consts/validation.rs index dd3e28acf96e3..1ad7b8fbbd5ed 100644 --- a/compiler/rustc_mir/src/transform/check_consts/validation.rs +++ b/compiler/rustc_mir/src/transform/check_consts/validation.rs @@ -808,6 +808,7 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} } } diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index f0472758dfb8e..e7fdd5496cb40 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -115,6 +115,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // safe (at least as emitted during MIR construction) } diff --git a/compiler/rustc_mir/src/transform/dest_prop.rs b/compiler/rustc_mir/src/transform/dest_prop.rs index f7568e1d929dd..6656deac967b6 100644 --- a/compiler/rustc_mir/src/transform/dest_prop.rs +++ b/compiler/rustc_mir/src/transform/dest_prop.rs @@ -587,6 +587,7 @@ impl Conflicts<'a> { | StatementKind::FakeRead(..) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} } } diff --git a/compiler/rustc_mir/src/transform/generator.rs b/compiler/rustc_mir/src/transform/generator.rs index 7a1f3d44a5e97..f299b6ecc28dc 100644 --- a/compiler/rustc_mir/src/transform/generator.rs +++ b/compiler/rustc_mir/src/transform/generator.rs @@ -1454,6 +1454,7 @@ impl Visitor<'tcx> for EnsureGeneratorFieldAssignmentsNeverAlias<'_> { | StatementKind::Retag(..) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} } } diff --git a/compiler/rustc_mir/src/transform/instrument_coverage.rs b/compiler/rustc_mir/src/transform/instrument_coverage.rs new file mode 100644 index 0000000000000..7125e1516e9fd --- /dev/null +++ b/compiler/rustc_mir/src/transform/instrument_coverage.rs @@ -0,0 +1,1272 @@ +use crate::transform::MirPass; +use crate::util::pretty; +use crate::util::spanview::{self, SpanViewable}; + +use rustc_data_structures::fingerprint::Fingerprint; +use rustc_data_structures::graph::dominators::Dominators; +use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_data_structures::sync::Lrc; +use rustc_index::bit_set::BitSet; +use rustc_index::vec::IndexVec; +use rustc_middle::hir; +use rustc_middle::hir::map::blocks::FnLikeNode; +use rustc_middle::ich::StableHashingContext; +use rustc_middle::mir; +use rustc_middle::mir::coverage::*; +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::{ + AggregateKind, BasicBlock, BasicBlockData, Coverage, CoverageInfo, FakeReadCause, Location, + Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, +}; +use rustc_middle::ty::query::Providers; +use rustc_middle::ty::TyCtxt; +use rustc_span::def_id::DefId; +use rustc_span::source_map::original_sp; +use rustc_span::{BytePos, CharPos, Pos, SourceFile, Span, Symbol, SyntaxContext}; + +use std::cmp::Ordering; + +const ID_SEPARATOR: &str = ","; + +/// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected +/// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen +/// to construct the coverage map. +pub struct InstrumentCoverage; + +/// The `query` provider for `CoverageInfo`, requested by `codegen_coverage()` (to inject each +/// counter) and `FunctionCoverage::new()` (to extract the coverage map metadata from the MIR). +pub(crate) fn provide(providers: &mut Providers) { + providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id); +} + +/// The `num_counters` argument to `llvm.instrprof.increment` is the max counter_id + 1, or in +/// other words, the number of counter value references injected into the MIR (plus 1 for the +/// reserved `ZERO` counter, which uses counter ID `0` when included in an expression). Injected +/// counters have a counter ID from `1..num_counters-1`. +/// +/// `num_expressions` is the number of counter expressions added to the MIR body. +/// +/// Both `num_counters` and `num_expressions` are used to initialize new vectors, during backend +/// code generate, to lookup counters and expressions by simple u32 indexes. +/// +/// MIR optimization may split and duplicate some BasicBlock sequences, or optimize out some code +/// including injected counters. (It is OK if some counters are optimized out, but those counters +/// are still included in the total `num_counters` or `num_expressions`.) Simply counting the +/// calls may not work; but computing the number of counters or expressions by adding `1` to the +/// highest ID (for a given instrumented function) is valid. +struct CoverageVisitor { + info: CoverageInfo, +} + +impl Visitor<'_> for CoverageVisitor { + fn visit_coverage(&mut self, coverage: &Coverage, _location: Location) { + match coverage.kind { + CoverageKind::Counter { id, .. } => { + let counter_id = u32::from(id); + self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1); + } + CoverageKind::Expression { id, .. } => { + let expression_index = u32::MAX - u32::from(id); + self.info.num_expressions = + std::cmp::max(self.info.num_expressions, expression_index + 1); + } + _ => {} + } + } +} + +fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo { + let mir_body = tcx.optimized_mir(def_id); + + let mut coverage_visitor = + CoverageVisitor { info: CoverageInfo { num_counters: 0, num_expressions: 0 } }; + + coverage_visitor.visit_body(mir_body); + coverage_visitor.info +} + +impl<'tcx> MirPass<'tcx> for InstrumentCoverage { + fn run_pass(&self, tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) { + // If the InstrumentCoverage pass is called on promoted MIRs, skip them. + // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 + if mir_body.source.promoted.is_some() { + trace!( + "InstrumentCoverage skipped for {:?} (already promoted for Miri evaluation)", + mir_body.source.def_id() + ); + return; + } + + let hir_id = tcx.hir().local_def_id_to_hir_id(mir_body.source.def_id().expect_local()); + let is_fn_like = FnLikeNode::from_node(tcx.hir().get(hir_id)).is_some(); + + // Only instrument functions, methods, and closures (not constants since they are evaluated + // at compile time by Miri). + // FIXME(#73156): Handle source code coverage in const eval + if !is_fn_like { + trace!( + "InstrumentCoverage skipped for {:?} (not an FnLikeNode)", + mir_body.source.def_id(), + ); + return; + } + // FIXME(richkadel): By comparison, the MIR pass `ConstProp` includes associated constants, + // with functions, methods, and closures. I assume Miri is used for associated constants as + // well. If not, we may need to include them here too. + + trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); + Instrumentor::new(&self.name(), tcx, mir_body).inject_counters(); + trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); + } +} + +/// A BasicCoverageBlock (BCB) represents the maximal-length sequence of CFG (MIR) BasicBlocks +/// without conditional branches. +/// +/// The BCB allows coverage analysis to be performed on a simplified projection of the underlying +/// MIR CFG, without altering the original CFG. Note that running the MIR `SimplifyCfg` transform, +/// is not sufficient, and therefore not necessary, since the BCB-based CFG projection is a more +/// aggressive simplification. For example: +/// +/// * The BCB CFG projection ignores (trims) branches not relevant to coverage, such as unwind- +/// related code that is injected by the Rust compiler but has no physical source code to +/// count. This also means a BasicBlock with a `Call` terminator can be merged into its +/// primary successor target block, in the same BCB. +/// * Some BasicBlock terminators support Rust-specific concerns--like borrow-checking--that are +/// not relevant to coverage analysis. `FalseUnwind`, for example, can be treated the same as +/// a `Goto`, and merged with its successor into the same BCB. +/// +/// Each BCB with at least one computed `CoverageSpan` will have no more than one `Counter`. +/// In some cases, a BCB's execution count can be computed by `CounterExpression`. Additional +/// disjoint `CoverageSpan`s in a BCB can also be counted by `CounterExpression` (by adding `ZERO` +/// to the BCB's primary counter or expression). +/// +/// Dominator/dominated relationships (which are fundamental to the coverage analysis algorithm) +/// between two BCBs can be computed using the `mir::Body` `dominators()` with any `BasicBlock` +/// member of each BCB. (For consistency, BCB's use the first `BasicBlock`, also referred to as the +/// `bcb_leader_bb`.) +/// +/// The BCB CFG projection is critical to simplifying the coverage analysis by ensuring graph +/// path-based queries (`is_dominated_by()`, `predecessors`, `successors`, etc.) have branch +/// (control flow) significance. +#[derive(Debug, Clone)] +struct BasicCoverageBlock { + pub blocks: Vec, +} + +impl BasicCoverageBlock { + pub fn leader_bb(&self) -> BasicBlock { + self.blocks[0] + } + + pub fn id(&self) -> String { + format!( + "@{}", + self.blocks + .iter() + .map(|bb| bb.index().to_string()) + .collect::>() + .join(ID_SEPARATOR) + ) + } +} + +struct BasicCoverageBlocks { + vec: IndexVec>, +} + +impl BasicCoverageBlocks { + pub fn from_mir(mir_body: &mir::Body<'tcx>) -> Self { + let mut basic_coverage_blocks = + BasicCoverageBlocks { vec: IndexVec::from_elem_n(None, mir_body.basic_blocks().len()) }; + basic_coverage_blocks.extract_from_mir(mir_body); + basic_coverage_blocks + } + + pub fn iter(&self) -> impl Iterator { + self.vec.iter().filter_map(|option| option.as_ref()) + } + + fn extract_from_mir(&mut self, mir_body: &mir::Body<'tcx>) { + // Traverse the CFG but ignore anything following an `unwind` + let cfg_without_unwind = ShortCircuitPreorder::new(mir_body, |term_kind| { + let mut successors = term_kind.successors(); + match &term_kind { + // SwitchInt successors are never unwind, and all of them should be traversed. + + // NOTE: TerminatorKind::FalseEdge targets from SwitchInt don't appear to be + // helpful in identifying unreachable code. I did test the theory, but the following + // changes were not beneficial. (I assumed that replacing some constants with + // non-deterministic variables might effect which blocks were targeted by a + // `FalseEdge` `imaginary_target`. It did not.) + // + // Also note that, if there is a way to identify BasicBlocks that are part of the + // MIR CFG, but not actually reachable, here are some other things to consider: + // + // Injecting unreachable code regions will probably require computing the set + // difference between the basic blocks found without filtering out unreachable + // blocks, and the basic blocks found with the filter; then computing the + // `CoverageSpans` without the filter; and then injecting `Counter`s or + // `CounterExpression`s for blocks that are not unreachable, or injecting + // `Unreachable` code regions otherwise. This seems straightforward, but not + // trivial. + // + // Alternatively, we might instead want to leave the unreachable blocks in + // (bypass the filter here), and inject the counters. This will result in counter + // values of zero (0) for unreachable code (and, notably, the code will be displayed + // with a red background by `llvm-cov show`). + // + // TerminatorKind::SwitchInt { .. } => { + // let some_imaginary_target = successors.clone().find_map(|&successor| { + // let term = mir_body.basic_blocks()[successor].terminator(); + // if let TerminatorKind::FalseEdge { imaginary_target, .. } = term.kind { + // if mir_body.predecessors()[imaginary_target].len() == 1 { + // return Some(imaginary_target); + // } + // } + // None + // }); + // if let Some(imaginary_target) = some_imaginary_target { + // box successors.filter(move |&&successor| successor != imaginary_target) + // } else { + // box successors + // } + // } + // + // Note this also required changing the closure signature for the + // `ShortCurcuitPreorder` to: + // + // F: Fn(&'tcx TerminatorKind<'tcx>) -> Box + 'a>, + TerminatorKind::SwitchInt { .. } => successors, + + // For all other kinds, return only the first successor, if any, and ignore unwinds + _ => successors.next().into_iter().chain(&[]), + } + }); + + // Walk the CFG using a Preorder traversal, which starts from `START_BLOCK` and follows + // each block terminator's `successors()`. Coverage spans must map to actual source code, + // so compiler generated blocks and paths can be ignored. To that end the CFG traversal + // intentionally omits unwind paths. + let mut blocks = Vec::new(); + for (bb, data) in cfg_without_unwind { + if let Some(last) = blocks.last() { + let predecessors = &mir_body.predecessors()[bb]; + if predecessors.len() > 1 || !predecessors.contains(last) { + // The `bb` has more than one _incoming_ edge, and should start its own + // `BasicCoverageBlock`. (Note, the `blocks` vector does not yet include `bb`; + // it contains a sequence of one or more sequential blocks with no intermediate + // branches in or out. Save these as a new `BasicCoverageBlock` before starting + // the new one.) + self.add_basic_coverage_block(blocks.split_off(0)); + debug!( + " because {}", + if predecessors.len() > 1 { + "predecessors.len() > 1".to_owned() + } else { + format!("bb {} is not in precessors: {:?}", bb.index(), predecessors) + } + ); + } + } + blocks.push(bb); + + let term = data.terminator(); + + match term.kind { + TerminatorKind::Return { .. } + | TerminatorKind::Abort + | TerminatorKind::Assert { .. } + | TerminatorKind::Yield { .. } + | TerminatorKind::SwitchInt { .. } => { + // The `bb` has more than one _outgoing_ edge, or exits the function. Save the + // current sequence of `blocks` gathered to this point, as a new + // `BasicCoverageBlock`. + self.add_basic_coverage_block(blocks.split_off(0)); + debug!(" because term.kind = {:?}", term.kind); + // Note that this condition is based on `TerminatorKind`, even though it + // theoretically boils down to `successors().len() != 1`; that is, either zero + // (e.g., `Return`, `Abort`) or multiple successors (e.g., `SwitchInt`), but + // since the Coverage graph (the BCB CFG projection) ignores things like unwind + // branches (which exist in the `Terminator`s `successors()` list) checking the + // number of successors won't work. + } + TerminatorKind::Goto { .. } + | TerminatorKind::Resume + | TerminatorKind::Unreachable + | TerminatorKind::Drop { .. } + | TerminatorKind::DropAndReplace { .. } + | TerminatorKind::Call { .. } + | TerminatorKind::GeneratorDrop + | TerminatorKind::FalseEdge { .. } + | TerminatorKind::FalseUnwind { .. } + | TerminatorKind::InlineAsm { .. } => {} + } + } + + if !blocks.is_empty() { + // process any remaining blocks into a final `BasicCoverageBlock` + self.add_basic_coverage_block(blocks.split_off(0)); + debug!(" because the end of the CFG was reached while traversing"); + } + } + + fn add_basic_coverage_block(&mut self, blocks: Vec) { + let leader_bb = blocks[0]; + let bcb = BasicCoverageBlock { blocks }; + debug!("adding BCB: {:?}", bcb); + self.vec[leader_bb] = Some(bcb); + } +} + +impl std::ops::Index for BasicCoverageBlocks { + type Output = BasicCoverageBlock; + + fn index(&self, index: BasicBlock) -> &Self::Output { + self.vec[index].as_ref().expect("is_some if BasicBlock is a BasicCoverageBlock leader") + } +} + +#[derive(Debug, Copy, Clone)] +enum CoverageStatement { + Statement(BasicBlock, Span, usize), + Terminator(BasicBlock, Span), +} + +impl CoverageStatement { + pub fn format(&self, tcx: TyCtxt<'tcx>, mir_body: &'a mir::Body<'tcx>) -> String { + match *self { + Self::Statement(bb, span, stmt_index) => { + let stmt = &mir_body.basic_blocks()[bb].statements[stmt_index]; + format!( + "{}: @{}[{}]: {:?}", + spanview::source_range_no_file(tcx, &span), + bb.index(), + stmt_index, + stmt + ) + } + Self::Terminator(bb, span) => { + let term = mir_body.basic_blocks()[bb].terminator(); + format!( + "{}: @{}.{}: {:?}", + spanview::source_range_no_file(tcx, &span), + bb.index(), + term_type(&term.kind), + term.kind + ) + } + } + } + + pub fn span(&self) -> &Span { + match self { + Self::Statement(_, span, _) | Self::Terminator(_, span) => span, + } + } +} + +fn term_type(kind: &TerminatorKind<'tcx>) -> &'static str { + match kind { + TerminatorKind::Goto { .. } => "Goto", + TerminatorKind::SwitchInt { .. } => "SwitchInt", + TerminatorKind::Resume => "Resume", + TerminatorKind::Abort => "Abort", + TerminatorKind::Return => "Return", + TerminatorKind::Unreachable => "Unreachable", + TerminatorKind::Drop { .. } => "Drop", + TerminatorKind::DropAndReplace { .. } => "DropAndReplace", + TerminatorKind::Call { .. } => "Call", + TerminatorKind::Assert { .. } => "Assert", + TerminatorKind::Yield { .. } => "Yield", + TerminatorKind::GeneratorDrop => "GeneratorDrop", + TerminatorKind::FalseEdge { .. } => "FalseEdge", + TerminatorKind::FalseUnwind { .. } => "FalseUnwind", + TerminatorKind::InlineAsm { .. } => "InlineAsm", + } +} + +/// A BCB is deconstructed into one or more `Span`s. Each `Span` maps to a `CoverageSpan` that +/// references the originating BCB and one or more MIR `Statement`s and/or `Terminator`s. +/// Initially, the `Span`s come from the `Statement`s and `Terminator`s, but subsequent +/// transforms can combine adjacent `Span`s and `CoverageSpan` from the same BCB, merging the +/// `CoverageStatement` vectors, and the `Span`s to cover the extent of the combined `Span`s. +/// +/// Note: A `CoverageStatement` merged into another CoverageSpan may come from a `BasicBlock` that +/// is not part of the `CoverageSpan` bcb if the statement was included because it's `Span` matches +/// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock` +/// `is_dominated_by()` the `BasicBlock`s in this `CoverageSpan`. +#[derive(Debug, Clone)] +struct CoverageSpan { + span: Span, + bcb_leader_bb: BasicBlock, + coverage_statements: Vec, + is_closure: bool, +} + +impl CoverageSpan { + pub fn for_statement( + statement: &Statement<'tcx>, + span: Span, + bcb: &BasicCoverageBlock, + bb: BasicBlock, + stmt_index: usize, + ) -> Self { + let is_closure = match statement.kind { + StatementKind::Assign(box ( + _, + Rvalue::Aggregate(box AggregateKind::Closure(_, _), _), + )) => true, + _ => false, + }; + + Self { + span, + bcb_leader_bb: bcb.leader_bb(), + coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], + is_closure, + } + } + + pub fn for_terminator(span: Span, bcb: &'a BasicCoverageBlock, bb: BasicBlock) -> Self { + Self { + span, + bcb_leader_bb: bcb.leader_bb(), + coverage_statements: vec![CoverageStatement::Terminator(bb, span)], + is_closure: false, + } + } + + pub fn merge_from(&mut self, mut other: CoverageSpan) { + debug_assert!(self.is_mergeable(&other)); + self.span = self.span.to(other.span); + if other.is_closure { + self.is_closure = true; + } + self.coverage_statements.append(&mut other.coverage_statements); + } + + pub fn cutoff_statements_at(&mut self, cutoff_pos: BytePos) { + self.coverage_statements.retain(|covstmt| covstmt.span().hi() <= cutoff_pos); + if let Some(highest_covstmt) = + self.coverage_statements.iter().max_by_key(|covstmt| covstmt.span().hi()) + { + self.span = self.span.with_hi(highest_covstmt.span().hi()); + } + } + + pub fn is_dominated_by( + &self, + other: &CoverageSpan, + dominators: &Dominators, + ) -> bool { + debug_assert!(!self.is_in_same_bcb(other)); + dominators.is_dominated_by(self.bcb_leader_bb, other.bcb_leader_bb) + } + + pub fn is_mergeable(&self, other: &Self) -> bool { + self.is_in_same_bcb(other) && !(self.is_closure || other.is_closure) + } + + pub fn is_in_same_bcb(&self, other: &Self) -> bool { + self.bcb_leader_bb == other.bcb_leader_bb + } +} + +struct Instrumentor<'a, 'tcx> { + pass_name: &'a str, + tcx: TyCtxt<'tcx>, + mir_body: &'a mut mir::Body<'tcx>, + hir_body: &'tcx rustc_hir::Body<'tcx>, + dominators: Option>, + basic_coverage_blocks: Option, + function_source_hash: Option, + next_counter_id: u32, + num_expressions: u32, +} + +impl<'a, 'tcx> Instrumentor<'a, 'tcx> { + fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self { + let hir_body = hir_body(tcx, mir_body.source.def_id()); + Self { + pass_name, + tcx, + mir_body, + hir_body, + dominators: None, + basic_coverage_blocks: None, + function_source_hash: None, + next_counter_id: CounterValueReference::START.as_u32(), + num_expressions: 0, + } + } + + /// Counter IDs start from one and go up. + fn next_counter(&mut self) -> CounterValueReference { + assert!(self.next_counter_id < u32::MAX - self.num_expressions); + let next = self.next_counter_id; + self.next_counter_id += 1; + CounterValueReference::from(next) + } + + /// Expression IDs start from u32::MAX and go down because a CounterExpression can reference + /// (add or subtract counts) of both Counter regions and CounterExpression regions. The counter + /// expression operand IDs must be unique across both types. + fn next_expression(&mut self) -> InjectedExpressionIndex { + assert!(self.next_counter_id < u32::MAX - self.num_expressions); + let next = u32::MAX - self.num_expressions; + self.num_expressions += 1; + InjectedExpressionIndex::from(next) + } + + fn dominators(&self) -> &Dominators { + self.dominators.as_ref().expect("dominators must be initialized before calling") + } + + fn basic_coverage_blocks(&self) -> &BasicCoverageBlocks { + self.basic_coverage_blocks + .as_ref() + .expect("basic_coverage_blocks must be initialized before calling") + } + + fn function_source_hash(&mut self) -> u64 { + match self.function_source_hash { + Some(hash) => hash, + None => { + let hash = hash_mir_source(self.tcx, self.hir_body); + self.function_source_hash.replace(hash); + hash + } + } + } + + fn inject_counters(&mut self) { + let tcx = self.tcx; + let source_map = tcx.sess.source_map(); + let def_id = self.mir_body.source.def_id(); + let mir_body = &self.mir_body; + let body_span = self.body_span(); + let source_file = source_map.lookup_source_file(body_span.lo()); + let file_name = Symbol::intern(&source_file.name.to_string()); + + debug!("instrumenting {:?}, span: {}", def_id, source_map.span_to_string(body_span)); + + self.dominators.replace(mir_body.dominators()); + self.basic_coverage_blocks.replace(BasicCoverageBlocks::from_mir(mir_body)); + + let coverage_spans = self.coverage_spans(); + + let span_viewables = if pretty::dump_enabled(tcx, self.pass_name, def_id) { + Some(self.span_viewables(&coverage_spans)) + } else { + None + }; + + // Inject a counter for each `CoverageSpan`. There can be multiple `CoverageSpan`s for a + // given BCB, but only one actual counter needs to be incremented per BCB. `bb_counters` + // maps each `bcb_leader_bb` to its `Counter`, when injected. Subsequent `CoverageSpan`s + // for a BCB that already has a `Counter` will inject a `CounterExpression` instead, and + // compute its value by adding `ZERO` to the BCB `Counter` value. + let mut bb_counters = IndexVec::from_elem_n(None, mir_body.basic_blocks().len()); + for CoverageSpan { span, bcb_leader_bb: bb, .. } in coverage_spans { + if let Some(&counter_operand) = bb_counters[bb].as_ref() { + let expression = + self.make_expression(counter_operand, Op::Add, ExpressionOperandId::ZERO); + debug!( + "Injecting counter expression {:?} at: {:?}:\n{}\n==========", + expression, + span, + source_map.span_to_snippet(span).expect("Error getting source for span"), + ); + self.inject_statement(file_name, &source_file, expression, span, bb); + } else { + let counter = self.make_counter(); + debug!( + "Injecting counter {:?} at: {:?}:\n{}\n==========", + counter, + span, + source_map.span_to_snippet(span).expect("Error getting source for span"), + ); + let counter_operand = counter.as_operand_id(); + bb_counters[bb] = Some(counter_operand); + self.inject_statement(file_name, &source_file, counter, span, bb); + } + } + + if let Some(span_viewables) = span_viewables { + let mut file = pretty::create_dump_file( + tcx, + "html", + None, + self.pass_name, + &0, + self.mir_body.source, + ) + .expect("Unexpected error creating MIR spanview HTML file"); + let crate_name = tcx.crate_name(def_id.krate); + let item_name = tcx.def_path(def_id).to_filename_friendly_no_crate(); + let title = format!("{}.{} - Coverage Spans", crate_name, item_name); + spanview::write_document(tcx, def_id, span_viewables, &title, &mut file) + .expect("Unexpected IO error dumping coverage spans as HTML"); + } + } + + fn make_counter(&mut self) -> CoverageKind { + CoverageKind::Counter { + function_source_hash: self.function_source_hash(), + id: self.next_counter(), + } + } + + fn make_expression( + &mut self, + lhs: ExpressionOperandId, + op: Op, + rhs: ExpressionOperandId, + ) -> CoverageKind { + CoverageKind::Expression { id: self.next_expression(), lhs, op, rhs } + } + + fn inject_statement( + &mut self, + file_name: Symbol, + source_file: &Lrc, + coverage_kind: CoverageKind, + span: Span, + block: BasicBlock, + ) { + let code_region = make_code_region(file_name, source_file, span); + debug!(" injecting statement {:?} covering {:?}", coverage_kind, code_region); + + let data = &mut self.mir_body[block]; + let source_info = data.terminator().source_info; + let statement = Statement { + source_info, + kind: StatementKind::Coverage(box Coverage { kind: coverage_kind, code_region }), + }; + data.statements.push(statement); + } + + /// Converts the computed `BasicCoverageBlock`s into `SpanViewable`s. + fn span_viewables(&self, coverage_spans: &Vec) -> Vec { + let tcx = self.tcx; + let mir_body = &self.mir_body; + let mut span_viewables = Vec::new(); + for coverage_span in coverage_spans { + let bcb = self.bcb_from_coverage_span(coverage_span); + let CoverageSpan { span, bcb_leader_bb: bb, coverage_statements, .. } = coverage_span; + let id = bcb.id(); + let mut sorted_coverage_statements = coverage_statements.clone(); + sorted_coverage_statements.sort_unstable_by_key(|covstmt| match *covstmt { + CoverageStatement::Statement(bb, _, index) => (bb, index), + CoverageStatement::Terminator(bb, _) => (bb, usize::MAX), + }); + let tooltip = sorted_coverage_statements + .iter() + .map(|covstmt| covstmt.format(tcx, mir_body)) + .collect::>() + .join("\n"); + span_viewables.push(SpanViewable { bb: *bb, span: *span, id, tooltip }); + } + span_viewables + } + + #[inline(always)] + fn bcb_from_coverage_span(&self, coverage_span: &CoverageSpan) -> &BasicCoverageBlock { + &self.basic_coverage_blocks()[coverage_span.bcb_leader_bb] + } + + #[inline(always)] + fn body_span(&self) -> Span { + self.hir_body.value.span + } + + // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of + // the `BasicBlock`(s) in the given `BasicCoverageBlock`. One `CoverageSpan` is generated for + // each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will + // merge some `CoverageSpan`s, at which point a `CoverageSpan` may represent multiple + // `Statement`s and/or `Terminator`s.) + fn extract_spans(&self, bcb: &'a BasicCoverageBlock) -> Vec { + let body_span = self.body_span(); + let mir_basic_blocks = self.mir_body.basic_blocks(); + bcb.blocks + .iter() + .map(|bbref| { + let bb = *bbref; + let data = &mir_basic_blocks[bb]; + data.statements + .iter() + .enumerate() + .filter_map(move |(index, statement)| { + filtered_statement_span(statement, body_span).map(|span| { + CoverageSpan::for_statement(statement, span, bcb, bb, index) + }) + }) + .chain( + filtered_terminator_span(data.terminator(), body_span) + .map(|span| CoverageSpan::for_terminator(span, bcb, bb)), + ) + }) + .flatten() + .collect() + } + + /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be + /// counted. + /// + /// The basic steps are: + /// + /// 1. Extract an initial set of spans from the `Statement`s and `Terminator`s of each + /// `BasicCoverageBlock`. + /// 2. Sort the spans by span.lo() (starting position). Spans that start at the same position + /// are sorted with longer spans before shorter spans; and equal spans are sorted + /// (deterministically) based on "dominator" relationship (if any). + /// 3. Traverse the spans in sorted order to identify spans that can be dropped (for instance, + /// if another span or spans are already counting the same code region), or should be merged + /// into a broader combined span (because it represents a contiguous, non-branching, and + /// uninterrupted region of source code). + /// + /// Closures are exposed in their enclosing functions as `Assign` `Rvalue`s, and since + /// closures have their own MIR, their `Span` in their enclosing function should be left + /// "uncovered". + /// + /// Note the resulting vector of `CoverageSpan`s does may not be fully sorted (and does not need + /// to be). + fn coverage_spans(&self) -> Vec { + let mut initial_spans = + Vec::::with_capacity(self.mir_body.basic_blocks().len() * 2); + for bcb in self.basic_coverage_blocks().iter() { + for coverage_span in self.extract_spans(bcb) { + initial_spans.push(coverage_span); + } + } + + if initial_spans.is_empty() { + // This can happen if, for example, the function is unreachable (contains only a + // `BasicBlock`(s) with an `Unreachable` terminator). + return initial_spans; + } + + initial_spans.sort_unstable_by(|a, b| { + if a.span.lo() == b.span.lo() { + if a.span.hi() == b.span.hi() { + if a.is_in_same_bcb(b) { + Some(Ordering::Equal) + } else { + // Sort equal spans by dominator relationship, in reverse order (so + // dominators always come after the dominated equal spans). When later + // comparing two spans in order, the first will either dominate the second, + // or they will have no dominator relationship. + self.dominators().rank_partial_cmp(b.bcb_leader_bb, a.bcb_leader_bb) + } + } else { + // Sort hi() in reverse order so shorter spans are attempted after longer spans. + // This guarantees that, if a `prev` span overlaps, and is not equal to, a `curr` + // span, the prev span either extends further left of the curr span, or they + // start at the same position and the prev span extends further right of the end + // of the curr span. + b.span.hi().partial_cmp(&a.span.hi()) + } + } else { + a.span.lo().partial_cmp(&b.span.lo()) + } + .unwrap() + }); + + let refinery = CoverageSpanRefinery::from_sorted_spans(initial_spans, self.dominators()); + refinery.to_refined_spans() + } +} + +struct CoverageSpanRefinery<'a> { + sorted_spans_iter: std::vec::IntoIter, + dominators: &'a Dominators, + some_curr: Option, + curr_original_span: Span, + some_prev: Option, + prev_original_span: Span, + pending_dups: Vec, + refined_spans: Vec, +} + +impl<'a> CoverageSpanRefinery<'a> { + fn from_sorted_spans( + sorted_spans: Vec, + dominators: &'a Dominators, + ) -> Self { + let refined_spans = Vec::with_capacity(sorted_spans.len()); + let mut sorted_spans_iter = sorted_spans.into_iter(); + let prev = sorted_spans_iter.next().expect("at least one span"); + let prev_original_span = prev.span; + Self { + sorted_spans_iter, + dominators, + refined_spans, + some_curr: None, + curr_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), + some_prev: Some(prev), + prev_original_span, + pending_dups: Vec::new(), + } + } + + /// Iterate through the sorted `CoverageSpan`s, and return the refined list of merged and + /// de-duplicated `CoverageSpan`s. + fn to_refined_spans(mut self) -> Vec { + while self.next_coverage_span() { + if self.curr().is_mergeable(self.prev()) { + debug!(" same bcb (and neither is a closure), merge with prev={:?}", self.prev()); + let prev = self.take_prev(); + self.curr_mut().merge_from(prev); + // Note that curr.span may now differ from curr_original_span + } else if self.prev_ends_before_curr() { + debug!( + " different bcbs and disjoint spans, so keep curr for next iter, and add \ + prev={:?}", + self.prev() + ); + let prev = self.take_prev(); + self.add_refined_span(prev); + } else if self.prev().is_closure { + // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the + // next iter + debug!( + " curr overlaps a closure (prev). Drop curr and keep prev for next iter. \ + prev={:?}", + self.prev() + ); + self.discard_curr(); + } else if self.curr().is_closure { + self.carve_out_span_for_closure(); + } else if self.prev_original_span == self.curr().span { + self.hold_pending_dups_unless_dominated(); + } else { + self.cutoff_prev_at_overlapping_curr(); + } + } + debug!(" AT END, adding last prev={:?}", self.prev()); + let pending_dups = self.pending_dups.split_off(0); + for dup in pending_dups.into_iter() { + debug!(" ...adding at least one pending dup={:?}", dup); + self.add_refined_span(dup); + } + let prev = self.take_prev(); + self.add_refined_span(prev); + + // FIXME(richkadel): Replace some counters with expressions if they can be calculated based + // on branching. (For example, one branch of a SwitchInt can be computed from the counter + // for the CoverageSpan just prior to the SwitchInt minus the sum of the counters of all + // other branches). + + self.to_refined_spans_without_closures() + } + + fn add_refined_span(&mut self, coverage_span: CoverageSpan) { + self.refined_spans.push(coverage_span); + } + + /// Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage + /// regions for the current function leave room for the closure's own coverage regions + /// (injected separately, from the closure's own MIR). + fn to_refined_spans_without_closures(mut self) -> Vec { + self.refined_spans.retain(|covspan| !covspan.is_closure); + self.refined_spans + } + + fn curr(&self) -> &CoverageSpan { + self.some_curr + .as_ref() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + } + + fn curr_mut(&mut self) -> &mut CoverageSpan { + self.some_curr + .as_mut() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + } + + fn prev(&self) -> &CoverageSpan { + self.some_prev + .as_ref() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + } + + fn prev_mut(&mut self) -> &mut CoverageSpan { + self.some_prev + .as_mut() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + } + + fn take_prev(&mut self) -> CoverageSpan { + self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + } + + /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the + /// `pending_dups` spans), then one of the following two things happened during the previous + /// iteration: + /// * the `span` of prev was modified (by `curr_mut().merge_from(prev)`); or + /// * the `span` of prev advanced past the end of the span of pending_dups + /// (`prev().span.hi() <= curr().span.lo()`) + /// In either case, no more spans will match the span of `pending_dups`, so + /// add the `pending_dups` if they don't overlap `curr`, and clear the list. + fn check_pending_dups(&mut self) { + if let Some(dup) = self.pending_dups.last() { + if dup.span != self.prev().span { + debug!( + " SAME spans, but pending_dups are NOT THE SAME, so BCBs matched on \ + previous iteration, or prev started a new disjoint span" + ); + if dup.span.hi() <= self.curr().span.lo() { + let pending_dups = self.pending_dups.split_off(0); + for dup in pending_dups.into_iter() { + debug!(" ...adding at least one pending={:?}", dup); + self.add_refined_span(dup); + } + } else { + self.pending_dups.clear(); + } + } + } + } + + /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. + fn next_coverage_span(&mut self) -> bool { + if let Some(curr) = self.some_curr.take() { + self.some_prev = Some(curr); + self.prev_original_span = self.curr_original_span; + } + while let Some(curr) = self.sorted_spans_iter.next() { + debug!("FOR curr={:?}", curr); + if self.prev_starts_after_next(&curr) { + debug!( + " prev.span starts after curr.span, so curr will be dropped (skipping past \ + closure?); prev={:?}", + self.prev() + ); + } else { + // Save a copy of the original span for `curr` in case the `CoverageSpan` is changed + // by `self.curr_mut().merge_from(prev)`. + self.curr_original_span = curr.span; + self.some_curr.replace(curr); + self.check_pending_dups(); + return true; + } + } + false + } + + /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the + /// `curr` coverage span. + fn discard_curr(&mut self) { + self.some_curr = None; + } + + /// Returns true if the curr span should be skipped because prev has already advanced beyond the + /// end of curr. This can only happen if a prior iteration updated `prev` to skip past a region + /// of code, such as skipping past a closure. + fn prev_starts_after_next(&self, next_curr: &CoverageSpan) -> bool { + self.prev().span.lo() > next_curr.span.lo() + } + + /// Returns true if the curr span starts past the end of the prev span, which means they don't + /// overlap, so we now know the prev can be added to the refined coverage spans. + fn prev_ends_before_curr(&self) -> bool { + self.prev().span.hi() <= self.curr().span.lo() + } + + /// If `prev`s span extends left of the closure (`curr`), carve out the closure's + /// span from `prev`'s span. (The closure's coverage counters will be injected when + /// processing the closure's own MIR.) Add the portion of the span to the left of the + /// closure; and if the span extends to the right of the closure, update `prev` to + /// that portion of the span. For any `pending_dups`, repeat the same process. + fn carve_out_span_for_closure(&mut self) { + let curr_span = self.curr().span; + let left_cutoff = curr_span.lo(); + let right_cutoff = curr_span.hi(); + let has_pre_closure_span = self.prev().span.lo() < right_cutoff; + let has_post_closure_span = self.prev().span.hi() > right_cutoff; + let mut pending_dups = self.pending_dups.split_off(0); + if has_pre_closure_span { + let mut pre_closure = self.prev().clone(); + pre_closure.span = pre_closure.span.with_hi(left_cutoff); + debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); + if !pending_dups.is_empty() { + for mut dup in pending_dups.iter().cloned() { + dup.span = dup.span.with_hi(left_cutoff); + debug!(" ...and at least one pre_closure dup={:?}", dup); + self.add_refined_span(dup); + } + } + self.add_refined_span(pre_closure); + } + if has_post_closure_span { + // Update prev.span to start after the closure (and discard curr) + self.prev_mut().span = self.prev().span.with_lo(right_cutoff); + self.prev_original_span = self.prev().span; + for dup in pending_dups.iter_mut() { + dup.span = dup.span.with_lo(right_cutoff); + } + self.pending_dups.append(&mut pending_dups); + self.discard_curr(); // since self.prev() was already updated + } else { + pending_dups.clear(); + } + } + + /// When two `CoverageSpan`s have the same `Span`, dominated spans can be discarded; but if + /// neither `CoverageSpan` dominates the other, both (or possibly more than two) are held, + /// until their disposition is determined. In this latter case, the `prev` dup is moved into + /// `pending_dups` so the new `curr` dup can be moved to `prev` for the next iteration. + fn hold_pending_dups_unless_dominated(&mut self) { + // equal coverage spans are ordered by dominators before dominated (if any) + debug_assert!(!self.prev().is_dominated_by(self.curr(), self.dominators)); + + if self.curr().is_dominated_by(&self.prev(), self.dominators) { + // If one span dominates the other, assocate the span with the dominator only. + // + // For example: + // match somenum { + // x if x < 1 => { ... } + // }... + // The span for the first `x` is referenced by both the pattern block (every + // time it is evaluated) and the arm code (only when matched). The counter + // will be applied only to the dominator block. + // + // The dominator's (`prev`) execution count may be higher than the dominated + // block's execution count, so drop `curr`. + debug!( + " different bcbs but SAME spans, and prev dominates curr. Drop curr and \ + keep prev for next iter. prev={:?}", + self.prev() + ); + self.discard_curr(); + } else { + // Save `prev` in `pending_dups`. (`curr` will become `prev` in the next iteration.) + // If the `curr` CoverageSpan is later discarded, `pending_dups` can be discarded as + // well; but if `curr` is added to refined_spans, the `pending_dups` will also be added. + debug!( + " different bcbs but SAME spans, and neither dominates, so keep curr for \ + next iter, and, pending upcoming spans (unless overlapping) add prev={:?}", + self.prev() + ); + let prev = self.take_prev(); + self.pending_dups.push(prev); + } + } + + /// `curr` overlaps `prev`. If `prev`s span extends left of `curr`s span, keep _only_ + /// statements that end before `curr.lo()` (if any), and add the portion of the + /// combined span for those statements. Any other statements have overlapping spans + /// that can be ignored because `curr` and/or other upcoming statements/spans inside + /// the overlap area will produce their own counters. This disambiguation process + /// avoids injecting multiple counters for overlapping spans, and the potential for + /// double-counting. + fn cutoff_prev_at_overlapping_curr(&mut self) { + debug!( + " different bcbs, overlapping spans, so ignore/drop pending and only add prev \ + if it has statements that end before curr={:?}", + self.prev() + ); + if self.pending_dups.is_empty() { + let curr_span = self.curr().span; + self.prev_mut().cutoff_statements_at(curr_span.lo()); + if self.prev().coverage_statements.is_empty() { + debug!(" ... no non-overlapping statements to add"); + } else { + debug!(" ... adding modified prev={:?}", self.prev()); + let prev = self.take_prev(); + self.add_refined_span(prev); + } + } else { + // with `pending_dups`, `prev` cannot have any statements that don't overlap + self.pending_dups.clear(); + } + } +} + +fn filtered_statement_span(statement: &'a Statement<'tcx>, body_span: Span) -> Option { + match statement.kind { + // These statements have spans that are often outside the scope of the executed source code + // for their parent `BasicBlock`. + StatementKind::StorageLive(_) + | StatementKind::StorageDead(_) + // Coverage should not be encountered, but don't inject coverage coverage + | StatementKind::Coverage(_) + // Ignore `Nop`s + | StatementKind::Nop => None, + + // FIXME(richkadel): Look into a possible issue assigning the span to a + // FakeReadCause::ForGuardBinding, in this example: + // match somenum { + // x if x < 1 => { ... } + // }... + // The BasicBlock within the match arm code included one of these statements, but the span + // for it covered the `1` in this source. The actual statements have nothing to do with that + // source span: + // FakeRead(ForGuardBinding, _4); + // where `_4` is: + // _4 = &_1; (at the span for the first `x`) + // and `_1` is the `Place` for `somenum`. + // + // The arm code BasicBlock already has its own assignment for `x` itself, `_3 = 1`, and I've + // decided it's reasonable for that span (even though outside the arm code) to be part of + // the counted coverage of the arm code execution, but I can't justify including the literal + // `1` in the arm code. I'm pretty sure that, if the `FakeRead(ForGuardBinding)` has a + // purpose in codegen, it's probably in the right BasicBlock, but if so, the `Statement`s + // `source_info.span` can't be right. + // + // Consider correcting the span assignment, assuming there is a better solution, and see if + // the following pattern can be removed here: + StatementKind::FakeRead(cause, _) if cause == FakeReadCause::ForGuardBinding => None, + + // Retain spans from all other statements + StatementKind::FakeRead(_, _) // Not including `ForGuardBinding` + | StatementKind::CopyNonOverlapping(..) + | StatementKind::Assign(_) + | StatementKind::SetDiscriminant { .. } + | StatementKind::LlvmInlineAsm(_) + | StatementKind::Retag(_, _) + | StatementKind::AscribeUserType(_, _) => { + Some(source_info_span(&statement.source_info, body_span)) + } + } +} + +fn filtered_terminator_span(terminator: &'a Terminator<'tcx>, body_span: Span) -> Option { + match terminator.kind { + // These terminators have spans that don't positively contribute to computing a reasonable + // span of actually executed source code. (For example, SwitchInt terminators extracted from + // an `if condition { block }` has a span that includes the executed block, if true, + // but for coverage, the code region executed, up to *and* through the SwitchInt, + // actually stops before the if's block.) + TerminatorKind::Unreachable // Unreachable blocks are not connected to the CFG + | TerminatorKind::Assert { .. } + | TerminatorKind::Drop { .. } + | TerminatorKind::DropAndReplace { .. } + | TerminatorKind::SwitchInt { .. } + | TerminatorKind::Goto { .. } + // For `FalseEdge`, only the `real` branch is taken, so it is similar to a `Goto`. + | TerminatorKind::FalseEdge { .. } => None, + + // Retain spans from all other terminators + TerminatorKind::Resume + | TerminatorKind::Abort + | TerminatorKind::Return + | TerminatorKind::Call { .. } + | TerminatorKind::Yield { .. } + | TerminatorKind::GeneratorDrop + | TerminatorKind::FalseUnwind { .. } + | TerminatorKind::InlineAsm { .. } => { + Some(source_info_span(&terminator.source_info, body_span)) + } + } +} + +#[inline(always)] +fn source_info_span(source_info: &SourceInfo, body_span: Span) -> Span { + let span = original_sp(source_info.span, body_span).with_ctxt(SyntaxContext::root()); + if body_span.contains(span) { span } else { body_span } +} + +/// Convert the Span into its file name, start line and column, and end line and column +fn make_code_region(file_name: Symbol, source_file: &Lrc, span: Span) -> CodeRegion { + let (start_line, mut start_col) = source_file.lookup_file_pos(span.lo()); + let (end_line, end_col) = if span.hi() == span.lo() { + let (end_line, mut end_col) = (start_line, start_col); + // Extend an empty span by one character so the region will be counted. + let CharPos(char_pos) = start_col; + if char_pos > 0 { + start_col = CharPos(char_pos - 1); + } else { + end_col = CharPos(char_pos + 1); + } + (end_line, end_col) + } else { + source_file.lookup_file_pos(span.hi()) + }; + CodeRegion { + file_name, + start_line: start_line as u32, + start_col: start_col.to_u32() + 1, + end_line: end_line as u32, + end_col: end_col.to_u32() + 1, + } +} + +fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> { + let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); + let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); + tcx.hir().body(fn_body_id) +} + +fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { + let mut hcx = tcx.create_no_span_stable_hashing_context(); + hash(&mut hcx, &hir_body.value).to_smaller_hash() +} + +fn hash( + hcx: &mut StableHashingContext<'tcx>, + node: &impl HashStable>, +) -> Fingerprint { + let mut stable_hasher = StableHasher::new(); + node.hash_stable(hcx, &mut stable_hasher); + stable_hasher.finish() +} + +pub struct ShortCircuitPreorder< + 'a, + 'tcx, + F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>, +> { + body: &'a mir::Body<'tcx>, + visited: BitSet, + worklist: Vec, + filtered_successors: F, +} + +impl<'a, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> + ShortCircuitPreorder<'a, 'tcx, F> +{ + pub fn new( + body: &'a mir::Body<'tcx>, + filtered_successors: F, + ) -> ShortCircuitPreorder<'a, 'tcx, F> { + let worklist = vec![mir::START_BLOCK]; + + ShortCircuitPreorder { + body, + visited: BitSet::new_empty(body.basic_blocks().len()), + worklist, + filtered_successors, + } + } +} + +impl<'a: 'tcx, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> Iterator + for ShortCircuitPreorder<'a, 'tcx, F> +{ + type Item = (BasicBlock, &'a BasicBlockData<'tcx>); + + fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { + while let Some(idx) = self.worklist.pop() { + if !self.visited.insert(idx) { + continue; + } + + let data = &self.body[idx]; + + if let Some(ref term) = data.terminator { + self.worklist.extend((self.filtered_successors)(&term.kind)); + } + + return Some((idx, data)); + } + + None + } + + fn size_hint(&self) -> (usize, Option) { + let size = self.body.basic_blocks().len() - self.visited.count(); + (size, Some(size)) + } +} diff --git a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs index 31e201c3a5bbe..a37f5d4f329f3 100644 --- a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs +++ b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs @@ -39,6 +39,7 @@ impl RemoveNoopLandingPads { | StatementKind::StorageDead(_) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // These are all nops in a landing pad } diff --git a/compiler/rustc_mir/src/util/spanview.rs b/compiler/rustc_mir/src/util/spanview.rs index d3ef8c64565c6..a9a30e407b4b0 100644 --- a/compiler/rustc_mir/src/util/spanview.rs +++ b/compiler/rustc_mir/src/util/spanview.rs @@ -245,6 +245,7 @@ pub fn statement_kind_name(statement: &Statement<'_>) -> &'static str { Retag(..) => "Retag", AscribeUserType(..) => "AscribeUserType", Coverage(..) => "Coverage", + CopyNonOverlapping(..) => "CopyNonOverlapping", Nop => "Nop", } } diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 2cb9588e13f98..53935f02a90e4 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -210,7 +210,7 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen StatementKind::Assign(box (place, rval)) => { check_place(tcx, *place, span, body)?; check_rvalue(tcx, body, def_id, rval, span) - }, + } StatementKind::FakeRead(_, place) | // just an assignment @@ -218,6 +218,13 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen StatementKind::LlvmInlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping{ + dst, src, size, + }) => { + check_operand(tcx, dst, span, body)?; + check_operand(tcx, src, span, body)?; + check_operand(tcx, size, span, body) + }, // These are all NOPs StatementKind::StorageLive(_) | StatementKind::StorageDead(_) From 37a6c04718b9e8577b92aff2ab58a5c69a6c822a Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 7 Nov 2020 00:49:55 +0000 Subject: [PATCH 19/26] Update interpret step --- compiler/rustc_mir/src/interpret/step.rs | 14 +++++++++++--- compiler/rustc_mir/src/transform/coverage/spans.rs | 1 + compiler/rustc_mir/src/transform/simplify.rs | 1 + 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 5a3fc9f51611d..cc5e34fbde5a2 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -119,15 +119,23 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dst = { let dst = self.eval_operand(dst, None)?; - dst.assert_mem_place(self) + let mplace = *dst.assert_mem_place(self); + match mplace.ptr { + Scalar::Ptr(ptr) => ptr, + _ => panic!(), + } }; let src = { let src = self.eval_operand(src, None)?; - src.assert_mem_place(self) + let mplace = *src.assert_mem_place(self); + match mplace.ptr { + Scalar::Ptr(ptr) => ptr, + _ => panic!(), + } }; // Not sure how to convert an MPlaceTy<'_, >::PointerTag> // to a pointer, or OpTy to a size - self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + self.memory.copy(src, dst, size.layout.layout.size, /*nonoverlapping*/ true)?; } // Statements we do not track. diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index fd3e782f6df43..e7097ce861902 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -687,6 +687,7 @@ pub(super) fn filtered_statement_span( // Retain spans from all other statements StatementKind::FakeRead(_, _) // Not including `ForGuardBinding` + | StatementKind::CopyNonOverlapping(..) | StatementKind::Assign(_) | StatementKind::SetDiscriminant { .. } | StatementKind::LlvmInlineAsm(_) diff --git a/compiler/rustc_mir/src/transform/simplify.rs b/compiler/rustc_mir/src/transform/simplify.rs index 85f27428bbbf4..a5764d9bf4e3d 100644 --- a/compiler/rustc_mir/src/transform/simplify.rs +++ b/compiler/rustc_mir/src/transform/simplify.rs @@ -428,6 +428,7 @@ impl Visitor<'_> for UsedLocals { fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { match statement.kind { StatementKind::LlvmInlineAsm(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Retag(..) | StatementKind::Coverage(..) | StatementKind::FakeRead(..) From 982382dc0361aa19e2f7e1e2b02003320d34b502 Mon Sep 17 00:00:00 2001 From: kadmin Date: Tue, 29 Dec 2020 02:00:04 +0000 Subject: [PATCH 20/26] Update cranelift --- compiler/rustc_codegen_cranelift/src/base.rs | 10 + compiler/rustc_codegen_cranelift/src/lib.rs | 1 + .../rustc_codegen_ssa/src/mir/statement.rs | 11 +- compiler/rustc_middle/src/mir/mod.rs | 9 +- compiler/rustc_middle/src/mir/visit.rs | 14 +- .../src/borrow_check/invalidation.rs | 4 +- compiler/rustc_mir/src/interpret/step.rs | 36 +- .../src/transform/instrument_coverage.rs | 1272 ----------------- .../clippy_utils/src/qualify_min_const_fn.rs | 4 +- 9 files changed, 52 insertions(+), 1309 deletions(-) delete mode 100644 compiler/rustc_mir/src/transform/instrument_coverage.rs diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index f2c61c95f4ff2..ba7c82d24c517 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -832,6 +832,16 @@ fn codegen_stmt<'tcx>( } } StatementKind::Coverage { .. } => fx.tcx.sess.fatal("-Zcoverage is unimplemented"), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + src, + dst, + count, + }) => { + let dst = codegen_operand(fx, dst).load_scalar(fx); + let src = codegen_operand(fx, src).load_scalar(fx); + let count = codegen_operand(fx, count).load_scalar(fx); + fx.bcx.call_memcpy(fx.cx.module.target_config(), dst, src, count); + } } } diff --git a/compiler/rustc_codegen_cranelift/src/lib.rs b/compiler/rustc_codegen_cranelift/src/lib.rs index 8edb883ccb5f9..fae71fef9e676 100644 --- a/compiler/rustc_codegen_cranelift/src/lib.rs +++ b/compiler/rustc_codegen_cranelift/src/lib.rs @@ -11,6 +11,7 @@ #![warn(rust_2018_idioms)] #![warn(unused_lifetimes)] #![warn(unreachable_pub)] +#![feature(box_patterns)] extern crate snap; #[macro_use] diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 1dafc8cd2c16d..774c920ee9666 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -118,23 +118,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { mir::StatementKind::CopyNonOverlapping(box mir::CopyNonOverlapping { ref src, ref dst, - ref size, + ref count, }) => { let dst_val = self.codegen_operand(&mut bx, dst); let src_val = self.codegen_operand(&mut bx, src); - let size_val = self.codegen_operand(&mut bx, size); - let size = size_val.immediate_or_packed_pair(&mut bx); + let count_val = self.codegen_operand(&mut bx, count); + let count = count_val.immediate_or_packed_pair(&mut bx); let dst = dst_val.immediate_or_packed_pair(&mut bx); let src = src_val.immediate_or_packed_pair(&mut bx); use crate::MemFlags; - let flags = - (!MemFlags::UNALIGNED) & (!MemFlags::VOLATILE) & (!MemFlags::NONTEMPORAL); + let flags = MemFlags::empty(); bx.memcpy( dst, dst_val.layout.layout.align.pref, src, src_val.layout.layout.align.pref, - size, + count, flags, ); bx diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index dddda0f611229..c046333e68d2b 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1667,8 +1667,10 @@ impl Debug for Statement<'_> { CopyNonOverlapping(box crate::mir::CopyNonOverlapping { ref src, ref dst, - ref size, - }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?}, size={:?})", src, dst, size), + ref count, + }) => { + write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?}, count={:?})", src, dst, count) + } Nop => write!(fmt, "nop"), } } @@ -1684,7 +1686,8 @@ pub struct Coverage { pub struct CopyNonOverlapping<'tcx> { pub src: Operand<'tcx>, pub dst: Operand<'tcx>, - pub size: Operand<'tcx>, + /// Number of elements to copy from src to dest, not bytes. + pub count: Operand<'tcx>, } /////////////////////////////////////////////////////////////////////////// diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 7bd4b72cc1240..4e81612c0b9de 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -439,17 +439,11 @@ macro_rules! make_mir_visitor { StatementKind::CopyNonOverlapping(box crate::mir::CopyNonOverlapping{ ref $($mutability)? src, ref $($mutability)? dst, - ref $($mutability)? size, + ref $($mutability)? count, }) => { - self.visit_operand( - src, - location - ); - self.visit_operand( - dst, - location - ); - self.visit_operand(size, location) + self.visit_operand(src, location); + self.visit_operand(dst, location); + self.visit_operand(count, location) } StatementKind::Nop => {} } diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 276d9381e32ae..1f3dfc251e10c 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -95,11 +95,11 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { ref src, ref dst, - ref size, + ref count, }) => { self.consume_operand(location, src); self.consume_operand(location, dst); - self.consume_operand(location, size); + self.consume_operand(location, count); match dst { Operand::Move(ref place) | Operand::Copy(ref place) => { self.mutate_place(location, *place, Deep, JustWrite); diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index cc5e34fbde5a2..89dc93c790090 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -114,28 +114,36 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } // Call CopyNonOverlapping - CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, size }) => { - let size = self.eval_operand(size, None)?; + CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, count }) => { + let (src, size) = { + let src = self.eval_operand(src, None)?; + let size = src.layout.layout.size; + let mplace = *src.assert_mem_place(self); + let ptr = match mplace.ptr { + Scalar::Ptr(ptr) => ptr, + _ => panic!(), + }; + (ptr, size) + }; let dst = { let dst = self.eval_operand(dst, None)?; let mplace = *dst.assert_mem_place(self); match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), + Scalar::Ptr(ptr) => ptr, + _ => panic!(), } }; - let src = { - let src = self.eval_operand(src, None)?; - let mplace = *src.assert_mem_place(self); - match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - } + + let count = self.eval_operand(count, None)?; + let count = self.read_immediate(count)?.to_scalar()?; + let count = if let Scalar::Int(i) = count { + core::convert::TryFrom::try_from(i).unwrap() + } else { + panic!(); }; - // Not sure how to convert an MPlaceTy<'_, >::PointerTag> - // to a pointer, or OpTy to a size - self.memory.copy(src, dst, size.layout.layout.size, /*nonoverlapping*/ true)?; + + self.memory.copy_repeatedly(src, dst, size, count, /*nonoverlapping*/ true)?; } // Statements we do not track. diff --git a/compiler/rustc_mir/src/transform/instrument_coverage.rs b/compiler/rustc_mir/src/transform/instrument_coverage.rs deleted file mode 100644 index 7125e1516e9fd..0000000000000 --- a/compiler/rustc_mir/src/transform/instrument_coverage.rs +++ /dev/null @@ -1,1272 +0,0 @@ -use crate::transform::MirPass; -use crate::util::pretty; -use crate::util::spanview::{self, SpanViewable}; - -use rustc_data_structures::fingerprint::Fingerprint; -use rustc_data_structures::graph::dominators::Dominators; -use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_data_structures::sync::Lrc; -use rustc_index::bit_set::BitSet; -use rustc_index::vec::IndexVec; -use rustc_middle::hir; -use rustc_middle::hir::map::blocks::FnLikeNode; -use rustc_middle::ich::StableHashingContext; -use rustc_middle::mir; -use rustc_middle::mir::coverage::*; -use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{ - AggregateKind, BasicBlock, BasicBlockData, Coverage, CoverageInfo, FakeReadCause, Location, - Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, -}; -use rustc_middle::ty::query::Providers; -use rustc_middle::ty::TyCtxt; -use rustc_span::def_id::DefId; -use rustc_span::source_map::original_sp; -use rustc_span::{BytePos, CharPos, Pos, SourceFile, Span, Symbol, SyntaxContext}; - -use std::cmp::Ordering; - -const ID_SEPARATOR: &str = ","; - -/// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected -/// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen -/// to construct the coverage map. -pub struct InstrumentCoverage; - -/// The `query` provider for `CoverageInfo`, requested by `codegen_coverage()` (to inject each -/// counter) and `FunctionCoverage::new()` (to extract the coverage map metadata from the MIR). -pub(crate) fn provide(providers: &mut Providers) { - providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id); -} - -/// The `num_counters` argument to `llvm.instrprof.increment` is the max counter_id + 1, or in -/// other words, the number of counter value references injected into the MIR (plus 1 for the -/// reserved `ZERO` counter, which uses counter ID `0` when included in an expression). Injected -/// counters have a counter ID from `1..num_counters-1`. -/// -/// `num_expressions` is the number of counter expressions added to the MIR body. -/// -/// Both `num_counters` and `num_expressions` are used to initialize new vectors, during backend -/// code generate, to lookup counters and expressions by simple u32 indexes. -/// -/// MIR optimization may split and duplicate some BasicBlock sequences, or optimize out some code -/// including injected counters. (It is OK if some counters are optimized out, but those counters -/// are still included in the total `num_counters` or `num_expressions`.) Simply counting the -/// calls may not work; but computing the number of counters or expressions by adding `1` to the -/// highest ID (for a given instrumented function) is valid. -struct CoverageVisitor { - info: CoverageInfo, -} - -impl Visitor<'_> for CoverageVisitor { - fn visit_coverage(&mut self, coverage: &Coverage, _location: Location) { - match coverage.kind { - CoverageKind::Counter { id, .. } => { - let counter_id = u32::from(id); - self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1); - } - CoverageKind::Expression { id, .. } => { - let expression_index = u32::MAX - u32::from(id); - self.info.num_expressions = - std::cmp::max(self.info.num_expressions, expression_index + 1); - } - _ => {} - } - } -} - -fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo { - let mir_body = tcx.optimized_mir(def_id); - - let mut coverage_visitor = - CoverageVisitor { info: CoverageInfo { num_counters: 0, num_expressions: 0 } }; - - coverage_visitor.visit_body(mir_body); - coverage_visitor.info -} - -impl<'tcx> MirPass<'tcx> for InstrumentCoverage { - fn run_pass(&self, tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) { - // If the InstrumentCoverage pass is called on promoted MIRs, skip them. - // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 - if mir_body.source.promoted.is_some() { - trace!( - "InstrumentCoverage skipped for {:?} (already promoted for Miri evaluation)", - mir_body.source.def_id() - ); - return; - } - - let hir_id = tcx.hir().local_def_id_to_hir_id(mir_body.source.def_id().expect_local()); - let is_fn_like = FnLikeNode::from_node(tcx.hir().get(hir_id)).is_some(); - - // Only instrument functions, methods, and closures (not constants since they are evaluated - // at compile time by Miri). - // FIXME(#73156): Handle source code coverage in const eval - if !is_fn_like { - trace!( - "InstrumentCoverage skipped for {:?} (not an FnLikeNode)", - mir_body.source.def_id(), - ); - return; - } - // FIXME(richkadel): By comparison, the MIR pass `ConstProp` includes associated constants, - // with functions, methods, and closures. I assume Miri is used for associated constants as - // well. If not, we may need to include them here too. - - trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); - Instrumentor::new(&self.name(), tcx, mir_body).inject_counters(); - trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); - } -} - -/// A BasicCoverageBlock (BCB) represents the maximal-length sequence of CFG (MIR) BasicBlocks -/// without conditional branches. -/// -/// The BCB allows coverage analysis to be performed on a simplified projection of the underlying -/// MIR CFG, without altering the original CFG. Note that running the MIR `SimplifyCfg` transform, -/// is not sufficient, and therefore not necessary, since the BCB-based CFG projection is a more -/// aggressive simplification. For example: -/// -/// * The BCB CFG projection ignores (trims) branches not relevant to coverage, such as unwind- -/// related code that is injected by the Rust compiler but has no physical source code to -/// count. This also means a BasicBlock with a `Call` terminator can be merged into its -/// primary successor target block, in the same BCB. -/// * Some BasicBlock terminators support Rust-specific concerns--like borrow-checking--that are -/// not relevant to coverage analysis. `FalseUnwind`, for example, can be treated the same as -/// a `Goto`, and merged with its successor into the same BCB. -/// -/// Each BCB with at least one computed `CoverageSpan` will have no more than one `Counter`. -/// In some cases, a BCB's execution count can be computed by `CounterExpression`. Additional -/// disjoint `CoverageSpan`s in a BCB can also be counted by `CounterExpression` (by adding `ZERO` -/// to the BCB's primary counter or expression). -/// -/// Dominator/dominated relationships (which are fundamental to the coverage analysis algorithm) -/// between two BCBs can be computed using the `mir::Body` `dominators()` with any `BasicBlock` -/// member of each BCB. (For consistency, BCB's use the first `BasicBlock`, also referred to as the -/// `bcb_leader_bb`.) -/// -/// The BCB CFG projection is critical to simplifying the coverage analysis by ensuring graph -/// path-based queries (`is_dominated_by()`, `predecessors`, `successors`, etc.) have branch -/// (control flow) significance. -#[derive(Debug, Clone)] -struct BasicCoverageBlock { - pub blocks: Vec, -} - -impl BasicCoverageBlock { - pub fn leader_bb(&self) -> BasicBlock { - self.blocks[0] - } - - pub fn id(&self) -> String { - format!( - "@{}", - self.blocks - .iter() - .map(|bb| bb.index().to_string()) - .collect::>() - .join(ID_SEPARATOR) - ) - } -} - -struct BasicCoverageBlocks { - vec: IndexVec>, -} - -impl BasicCoverageBlocks { - pub fn from_mir(mir_body: &mir::Body<'tcx>) -> Self { - let mut basic_coverage_blocks = - BasicCoverageBlocks { vec: IndexVec::from_elem_n(None, mir_body.basic_blocks().len()) }; - basic_coverage_blocks.extract_from_mir(mir_body); - basic_coverage_blocks - } - - pub fn iter(&self) -> impl Iterator { - self.vec.iter().filter_map(|option| option.as_ref()) - } - - fn extract_from_mir(&mut self, mir_body: &mir::Body<'tcx>) { - // Traverse the CFG but ignore anything following an `unwind` - let cfg_without_unwind = ShortCircuitPreorder::new(mir_body, |term_kind| { - let mut successors = term_kind.successors(); - match &term_kind { - // SwitchInt successors are never unwind, and all of them should be traversed. - - // NOTE: TerminatorKind::FalseEdge targets from SwitchInt don't appear to be - // helpful in identifying unreachable code. I did test the theory, but the following - // changes were not beneficial. (I assumed that replacing some constants with - // non-deterministic variables might effect which blocks were targeted by a - // `FalseEdge` `imaginary_target`. It did not.) - // - // Also note that, if there is a way to identify BasicBlocks that are part of the - // MIR CFG, but not actually reachable, here are some other things to consider: - // - // Injecting unreachable code regions will probably require computing the set - // difference between the basic blocks found without filtering out unreachable - // blocks, and the basic blocks found with the filter; then computing the - // `CoverageSpans` without the filter; and then injecting `Counter`s or - // `CounterExpression`s for blocks that are not unreachable, or injecting - // `Unreachable` code regions otherwise. This seems straightforward, but not - // trivial. - // - // Alternatively, we might instead want to leave the unreachable blocks in - // (bypass the filter here), and inject the counters. This will result in counter - // values of zero (0) for unreachable code (and, notably, the code will be displayed - // with a red background by `llvm-cov show`). - // - // TerminatorKind::SwitchInt { .. } => { - // let some_imaginary_target = successors.clone().find_map(|&successor| { - // let term = mir_body.basic_blocks()[successor].terminator(); - // if let TerminatorKind::FalseEdge { imaginary_target, .. } = term.kind { - // if mir_body.predecessors()[imaginary_target].len() == 1 { - // return Some(imaginary_target); - // } - // } - // None - // }); - // if let Some(imaginary_target) = some_imaginary_target { - // box successors.filter(move |&&successor| successor != imaginary_target) - // } else { - // box successors - // } - // } - // - // Note this also required changing the closure signature for the - // `ShortCurcuitPreorder` to: - // - // F: Fn(&'tcx TerminatorKind<'tcx>) -> Box + 'a>, - TerminatorKind::SwitchInt { .. } => successors, - - // For all other kinds, return only the first successor, if any, and ignore unwinds - _ => successors.next().into_iter().chain(&[]), - } - }); - - // Walk the CFG using a Preorder traversal, which starts from `START_BLOCK` and follows - // each block terminator's `successors()`. Coverage spans must map to actual source code, - // so compiler generated blocks and paths can be ignored. To that end the CFG traversal - // intentionally omits unwind paths. - let mut blocks = Vec::new(); - for (bb, data) in cfg_without_unwind { - if let Some(last) = blocks.last() { - let predecessors = &mir_body.predecessors()[bb]; - if predecessors.len() > 1 || !predecessors.contains(last) { - // The `bb` has more than one _incoming_ edge, and should start its own - // `BasicCoverageBlock`. (Note, the `blocks` vector does not yet include `bb`; - // it contains a sequence of one or more sequential blocks with no intermediate - // branches in or out. Save these as a new `BasicCoverageBlock` before starting - // the new one.) - self.add_basic_coverage_block(blocks.split_off(0)); - debug!( - " because {}", - if predecessors.len() > 1 { - "predecessors.len() > 1".to_owned() - } else { - format!("bb {} is not in precessors: {:?}", bb.index(), predecessors) - } - ); - } - } - blocks.push(bb); - - let term = data.terminator(); - - match term.kind { - TerminatorKind::Return { .. } - | TerminatorKind::Abort - | TerminatorKind::Assert { .. } - | TerminatorKind::Yield { .. } - | TerminatorKind::SwitchInt { .. } => { - // The `bb` has more than one _outgoing_ edge, or exits the function. Save the - // current sequence of `blocks` gathered to this point, as a new - // `BasicCoverageBlock`. - self.add_basic_coverage_block(blocks.split_off(0)); - debug!(" because term.kind = {:?}", term.kind); - // Note that this condition is based on `TerminatorKind`, even though it - // theoretically boils down to `successors().len() != 1`; that is, either zero - // (e.g., `Return`, `Abort`) or multiple successors (e.g., `SwitchInt`), but - // since the Coverage graph (the BCB CFG projection) ignores things like unwind - // branches (which exist in the `Terminator`s `successors()` list) checking the - // number of successors won't work. - } - TerminatorKind::Goto { .. } - | TerminatorKind::Resume - | TerminatorKind::Unreachable - | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } - | TerminatorKind::Call { .. } - | TerminatorKind::GeneratorDrop - | TerminatorKind::FalseEdge { .. } - | TerminatorKind::FalseUnwind { .. } - | TerminatorKind::InlineAsm { .. } => {} - } - } - - if !blocks.is_empty() { - // process any remaining blocks into a final `BasicCoverageBlock` - self.add_basic_coverage_block(blocks.split_off(0)); - debug!(" because the end of the CFG was reached while traversing"); - } - } - - fn add_basic_coverage_block(&mut self, blocks: Vec) { - let leader_bb = blocks[0]; - let bcb = BasicCoverageBlock { blocks }; - debug!("adding BCB: {:?}", bcb); - self.vec[leader_bb] = Some(bcb); - } -} - -impl std::ops::Index for BasicCoverageBlocks { - type Output = BasicCoverageBlock; - - fn index(&self, index: BasicBlock) -> &Self::Output { - self.vec[index].as_ref().expect("is_some if BasicBlock is a BasicCoverageBlock leader") - } -} - -#[derive(Debug, Copy, Clone)] -enum CoverageStatement { - Statement(BasicBlock, Span, usize), - Terminator(BasicBlock, Span), -} - -impl CoverageStatement { - pub fn format(&self, tcx: TyCtxt<'tcx>, mir_body: &'a mir::Body<'tcx>) -> String { - match *self { - Self::Statement(bb, span, stmt_index) => { - let stmt = &mir_body.basic_blocks()[bb].statements[stmt_index]; - format!( - "{}: @{}[{}]: {:?}", - spanview::source_range_no_file(tcx, &span), - bb.index(), - stmt_index, - stmt - ) - } - Self::Terminator(bb, span) => { - let term = mir_body.basic_blocks()[bb].terminator(); - format!( - "{}: @{}.{}: {:?}", - spanview::source_range_no_file(tcx, &span), - bb.index(), - term_type(&term.kind), - term.kind - ) - } - } - } - - pub fn span(&self) -> &Span { - match self { - Self::Statement(_, span, _) | Self::Terminator(_, span) => span, - } - } -} - -fn term_type(kind: &TerminatorKind<'tcx>) -> &'static str { - match kind { - TerminatorKind::Goto { .. } => "Goto", - TerminatorKind::SwitchInt { .. } => "SwitchInt", - TerminatorKind::Resume => "Resume", - TerminatorKind::Abort => "Abort", - TerminatorKind::Return => "Return", - TerminatorKind::Unreachable => "Unreachable", - TerminatorKind::Drop { .. } => "Drop", - TerminatorKind::DropAndReplace { .. } => "DropAndReplace", - TerminatorKind::Call { .. } => "Call", - TerminatorKind::Assert { .. } => "Assert", - TerminatorKind::Yield { .. } => "Yield", - TerminatorKind::GeneratorDrop => "GeneratorDrop", - TerminatorKind::FalseEdge { .. } => "FalseEdge", - TerminatorKind::FalseUnwind { .. } => "FalseUnwind", - TerminatorKind::InlineAsm { .. } => "InlineAsm", - } -} - -/// A BCB is deconstructed into one or more `Span`s. Each `Span` maps to a `CoverageSpan` that -/// references the originating BCB and one or more MIR `Statement`s and/or `Terminator`s. -/// Initially, the `Span`s come from the `Statement`s and `Terminator`s, but subsequent -/// transforms can combine adjacent `Span`s and `CoverageSpan` from the same BCB, merging the -/// `CoverageStatement` vectors, and the `Span`s to cover the extent of the combined `Span`s. -/// -/// Note: A `CoverageStatement` merged into another CoverageSpan may come from a `BasicBlock` that -/// is not part of the `CoverageSpan` bcb if the statement was included because it's `Span` matches -/// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock` -/// `is_dominated_by()` the `BasicBlock`s in this `CoverageSpan`. -#[derive(Debug, Clone)] -struct CoverageSpan { - span: Span, - bcb_leader_bb: BasicBlock, - coverage_statements: Vec, - is_closure: bool, -} - -impl CoverageSpan { - pub fn for_statement( - statement: &Statement<'tcx>, - span: Span, - bcb: &BasicCoverageBlock, - bb: BasicBlock, - stmt_index: usize, - ) -> Self { - let is_closure = match statement.kind { - StatementKind::Assign(box ( - _, - Rvalue::Aggregate(box AggregateKind::Closure(_, _), _), - )) => true, - _ => false, - }; - - Self { - span, - bcb_leader_bb: bcb.leader_bb(), - coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], - is_closure, - } - } - - pub fn for_terminator(span: Span, bcb: &'a BasicCoverageBlock, bb: BasicBlock) -> Self { - Self { - span, - bcb_leader_bb: bcb.leader_bb(), - coverage_statements: vec![CoverageStatement::Terminator(bb, span)], - is_closure: false, - } - } - - pub fn merge_from(&mut self, mut other: CoverageSpan) { - debug_assert!(self.is_mergeable(&other)); - self.span = self.span.to(other.span); - if other.is_closure { - self.is_closure = true; - } - self.coverage_statements.append(&mut other.coverage_statements); - } - - pub fn cutoff_statements_at(&mut self, cutoff_pos: BytePos) { - self.coverage_statements.retain(|covstmt| covstmt.span().hi() <= cutoff_pos); - if let Some(highest_covstmt) = - self.coverage_statements.iter().max_by_key(|covstmt| covstmt.span().hi()) - { - self.span = self.span.with_hi(highest_covstmt.span().hi()); - } - } - - pub fn is_dominated_by( - &self, - other: &CoverageSpan, - dominators: &Dominators, - ) -> bool { - debug_assert!(!self.is_in_same_bcb(other)); - dominators.is_dominated_by(self.bcb_leader_bb, other.bcb_leader_bb) - } - - pub fn is_mergeable(&self, other: &Self) -> bool { - self.is_in_same_bcb(other) && !(self.is_closure || other.is_closure) - } - - pub fn is_in_same_bcb(&self, other: &Self) -> bool { - self.bcb_leader_bb == other.bcb_leader_bb - } -} - -struct Instrumentor<'a, 'tcx> { - pass_name: &'a str, - tcx: TyCtxt<'tcx>, - mir_body: &'a mut mir::Body<'tcx>, - hir_body: &'tcx rustc_hir::Body<'tcx>, - dominators: Option>, - basic_coverage_blocks: Option, - function_source_hash: Option, - next_counter_id: u32, - num_expressions: u32, -} - -impl<'a, 'tcx> Instrumentor<'a, 'tcx> { - fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self { - let hir_body = hir_body(tcx, mir_body.source.def_id()); - Self { - pass_name, - tcx, - mir_body, - hir_body, - dominators: None, - basic_coverage_blocks: None, - function_source_hash: None, - next_counter_id: CounterValueReference::START.as_u32(), - num_expressions: 0, - } - } - - /// Counter IDs start from one and go up. - fn next_counter(&mut self) -> CounterValueReference { - assert!(self.next_counter_id < u32::MAX - self.num_expressions); - let next = self.next_counter_id; - self.next_counter_id += 1; - CounterValueReference::from(next) - } - - /// Expression IDs start from u32::MAX and go down because a CounterExpression can reference - /// (add or subtract counts) of both Counter regions and CounterExpression regions. The counter - /// expression operand IDs must be unique across both types. - fn next_expression(&mut self) -> InjectedExpressionIndex { - assert!(self.next_counter_id < u32::MAX - self.num_expressions); - let next = u32::MAX - self.num_expressions; - self.num_expressions += 1; - InjectedExpressionIndex::from(next) - } - - fn dominators(&self) -> &Dominators { - self.dominators.as_ref().expect("dominators must be initialized before calling") - } - - fn basic_coverage_blocks(&self) -> &BasicCoverageBlocks { - self.basic_coverage_blocks - .as_ref() - .expect("basic_coverage_blocks must be initialized before calling") - } - - fn function_source_hash(&mut self) -> u64 { - match self.function_source_hash { - Some(hash) => hash, - None => { - let hash = hash_mir_source(self.tcx, self.hir_body); - self.function_source_hash.replace(hash); - hash - } - } - } - - fn inject_counters(&mut self) { - let tcx = self.tcx; - let source_map = tcx.sess.source_map(); - let def_id = self.mir_body.source.def_id(); - let mir_body = &self.mir_body; - let body_span = self.body_span(); - let source_file = source_map.lookup_source_file(body_span.lo()); - let file_name = Symbol::intern(&source_file.name.to_string()); - - debug!("instrumenting {:?}, span: {}", def_id, source_map.span_to_string(body_span)); - - self.dominators.replace(mir_body.dominators()); - self.basic_coverage_blocks.replace(BasicCoverageBlocks::from_mir(mir_body)); - - let coverage_spans = self.coverage_spans(); - - let span_viewables = if pretty::dump_enabled(tcx, self.pass_name, def_id) { - Some(self.span_viewables(&coverage_spans)) - } else { - None - }; - - // Inject a counter for each `CoverageSpan`. There can be multiple `CoverageSpan`s for a - // given BCB, but only one actual counter needs to be incremented per BCB. `bb_counters` - // maps each `bcb_leader_bb` to its `Counter`, when injected. Subsequent `CoverageSpan`s - // for a BCB that already has a `Counter` will inject a `CounterExpression` instead, and - // compute its value by adding `ZERO` to the BCB `Counter` value. - let mut bb_counters = IndexVec::from_elem_n(None, mir_body.basic_blocks().len()); - for CoverageSpan { span, bcb_leader_bb: bb, .. } in coverage_spans { - if let Some(&counter_operand) = bb_counters[bb].as_ref() { - let expression = - self.make_expression(counter_operand, Op::Add, ExpressionOperandId::ZERO); - debug!( - "Injecting counter expression {:?} at: {:?}:\n{}\n==========", - expression, - span, - source_map.span_to_snippet(span).expect("Error getting source for span"), - ); - self.inject_statement(file_name, &source_file, expression, span, bb); - } else { - let counter = self.make_counter(); - debug!( - "Injecting counter {:?} at: {:?}:\n{}\n==========", - counter, - span, - source_map.span_to_snippet(span).expect("Error getting source for span"), - ); - let counter_operand = counter.as_operand_id(); - bb_counters[bb] = Some(counter_operand); - self.inject_statement(file_name, &source_file, counter, span, bb); - } - } - - if let Some(span_viewables) = span_viewables { - let mut file = pretty::create_dump_file( - tcx, - "html", - None, - self.pass_name, - &0, - self.mir_body.source, - ) - .expect("Unexpected error creating MIR spanview HTML file"); - let crate_name = tcx.crate_name(def_id.krate); - let item_name = tcx.def_path(def_id).to_filename_friendly_no_crate(); - let title = format!("{}.{} - Coverage Spans", crate_name, item_name); - spanview::write_document(tcx, def_id, span_viewables, &title, &mut file) - .expect("Unexpected IO error dumping coverage spans as HTML"); - } - } - - fn make_counter(&mut self) -> CoverageKind { - CoverageKind::Counter { - function_source_hash: self.function_source_hash(), - id: self.next_counter(), - } - } - - fn make_expression( - &mut self, - lhs: ExpressionOperandId, - op: Op, - rhs: ExpressionOperandId, - ) -> CoverageKind { - CoverageKind::Expression { id: self.next_expression(), lhs, op, rhs } - } - - fn inject_statement( - &mut self, - file_name: Symbol, - source_file: &Lrc, - coverage_kind: CoverageKind, - span: Span, - block: BasicBlock, - ) { - let code_region = make_code_region(file_name, source_file, span); - debug!(" injecting statement {:?} covering {:?}", coverage_kind, code_region); - - let data = &mut self.mir_body[block]; - let source_info = data.terminator().source_info; - let statement = Statement { - source_info, - kind: StatementKind::Coverage(box Coverage { kind: coverage_kind, code_region }), - }; - data.statements.push(statement); - } - - /// Converts the computed `BasicCoverageBlock`s into `SpanViewable`s. - fn span_viewables(&self, coverage_spans: &Vec) -> Vec { - let tcx = self.tcx; - let mir_body = &self.mir_body; - let mut span_viewables = Vec::new(); - for coverage_span in coverage_spans { - let bcb = self.bcb_from_coverage_span(coverage_span); - let CoverageSpan { span, bcb_leader_bb: bb, coverage_statements, .. } = coverage_span; - let id = bcb.id(); - let mut sorted_coverage_statements = coverage_statements.clone(); - sorted_coverage_statements.sort_unstable_by_key(|covstmt| match *covstmt { - CoverageStatement::Statement(bb, _, index) => (bb, index), - CoverageStatement::Terminator(bb, _) => (bb, usize::MAX), - }); - let tooltip = sorted_coverage_statements - .iter() - .map(|covstmt| covstmt.format(tcx, mir_body)) - .collect::>() - .join("\n"); - span_viewables.push(SpanViewable { bb: *bb, span: *span, id, tooltip }); - } - span_viewables - } - - #[inline(always)] - fn bcb_from_coverage_span(&self, coverage_span: &CoverageSpan) -> &BasicCoverageBlock { - &self.basic_coverage_blocks()[coverage_span.bcb_leader_bb] - } - - #[inline(always)] - fn body_span(&self) -> Span { - self.hir_body.value.span - } - - // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of - // the `BasicBlock`(s) in the given `BasicCoverageBlock`. One `CoverageSpan` is generated for - // each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will - // merge some `CoverageSpan`s, at which point a `CoverageSpan` may represent multiple - // `Statement`s and/or `Terminator`s.) - fn extract_spans(&self, bcb: &'a BasicCoverageBlock) -> Vec { - let body_span = self.body_span(); - let mir_basic_blocks = self.mir_body.basic_blocks(); - bcb.blocks - .iter() - .map(|bbref| { - let bb = *bbref; - let data = &mir_basic_blocks[bb]; - data.statements - .iter() - .enumerate() - .filter_map(move |(index, statement)| { - filtered_statement_span(statement, body_span).map(|span| { - CoverageSpan::for_statement(statement, span, bcb, bb, index) - }) - }) - .chain( - filtered_terminator_span(data.terminator(), body_span) - .map(|span| CoverageSpan::for_terminator(span, bcb, bb)), - ) - }) - .flatten() - .collect() - } - - /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be - /// counted. - /// - /// The basic steps are: - /// - /// 1. Extract an initial set of spans from the `Statement`s and `Terminator`s of each - /// `BasicCoverageBlock`. - /// 2. Sort the spans by span.lo() (starting position). Spans that start at the same position - /// are sorted with longer spans before shorter spans; and equal spans are sorted - /// (deterministically) based on "dominator" relationship (if any). - /// 3. Traverse the spans in sorted order to identify spans that can be dropped (for instance, - /// if another span or spans are already counting the same code region), or should be merged - /// into a broader combined span (because it represents a contiguous, non-branching, and - /// uninterrupted region of source code). - /// - /// Closures are exposed in their enclosing functions as `Assign` `Rvalue`s, and since - /// closures have their own MIR, their `Span` in their enclosing function should be left - /// "uncovered". - /// - /// Note the resulting vector of `CoverageSpan`s does may not be fully sorted (and does not need - /// to be). - fn coverage_spans(&self) -> Vec { - let mut initial_spans = - Vec::::with_capacity(self.mir_body.basic_blocks().len() * 2); - for bcb in self.basic_coverage_blocks().iter() { - for coverage_span in self.extract_spans(bcb) { - initial_spans.push(coverage_span); - } - } - - if initial_spans.is_empty() { - // This can happen if, for example, the function is unreachable (contains only a - // `BasicBlock`(s) with an `Unreachable` terminator). - return initial_spans; - } - - initial_spans.sort_unstable_by(|a, b| { - if a.span.lo() == b.span.lo() { - if a.span.hi() == b.span.hi() { - if a.is_in_same_bcb(b) { - Some(Ordering::Equal) - } else { - // Sort equal spans by dominator relationship, in reverse order (so - // dominators always come after the dominated equal spans). When later - // comparing two spans in order, the first will either dominate the second, - // or they will have no dominator relationship. - self.dominators().rank_partial_cmp(b.bcb_leader_bb, a.bcb_leader_bb) - } - } else { - // Sort hi() in reverse order so shorter spans are attempted after longer spans. - // This guarantees that, if a `prev` span overlaps, and is not equal to, a `curr` - // span, the prev span either extends further left of the curr span, or they - // start at the same position and the prev span extends further right of the end - // of the curr span. - b.span.hi().partial_cmp(&a.span.hi()) - } - } else { - a.span.lo().partial_cmp(&b.span.lo()) - } - .unwrap() - }); - - let refinery = CoverageSpanRefinery::from_sorted_spans(initial_spans, self.dominators()); - refinery.to_refined_spans() - } -} - -struct CoverageSpanRefinery<'a> { - sorted_spans_iter: std::vec::IntoIter, - dominators: &'a Dominators, - some_curr: Option, - curr_original_span: Span, - some_prev: Option, - prev_original_span: Span, - pending_dups: Vec, - refined_spans: Vec, -} - -impl<'a> CoverageSpanRefinery<'a> { - fn from_sorted_spans( - sorted_spans: Vec, - dominators: &'a Dominators, - ) -> Self { - let refined_spans = Vec::with_capacity(sorted_spans.len()); - let mut sorted_spans_iter = sorted_spans.into_iter(); - let prev = sorted_spans_iter.next().expect("at least one span"); - let prev_original_span = prev.span; - Self { - sorted_spans_iter, - dominators, - refined_spans, - some_curr: None, - curr_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), - some_prev: Some(prev), - prev_original_span, - pending_dups: Vec::new(), - } - } - - /// Iterate through the sorted `CoverageSpan`s, and return the refined list of merged and - /// de-duplicated `CoverageSpan`s. - fn to_refined_spans(mut self) -> Vec { - while self.next_coverage_span() { - if self.curr().is_mergeable(self.prev()) { - debug!(" same bcb (and neither is a closure), merge with prev={:?}", self.prev()); - let prev = self.take_prev(); - self.curr_mut().merge_from(prev); - // Note that curr.span may now differ from curr_original_span - } else if self.prev_ends_before_curr() { - debug!( - " different bcbs and disjoint spans, so keep curr for next iter, and add \ - prev={:?}", - self.prev() - ); - let prev = self.take_prev(); - self.add_refined_span(prev); - } else if self.prev().is_closure { - // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the - // next iter - debug!( - " curr overlaps a closure (prev). Drop curr and keep prev for next iter. \ - prev={:?}", - self.prev() - ); - self.discard_curr(); - } else if self.curr().is_closure { - self.carve_out_span_for_closure(); - } else if self.prev_original_span == self.curr().span { - self.hold_pending_dups_unless_dominated(); - } else { - self.cutoff_prev_at_overlapping_curr(); - } - } - debug!(" AT END, adding last prev={:?}", self.prev()); - let pending_dups = self.pending_dups.split_off(0); - for dup in pending_dups.into_iter() { - debug!(" ...adding at least one pending dup={:?}", dup); - self.add_refined_span(dup); - } - let prev = self.take_prev(); - self.add_refined_span(prev); - - // FIXME(richkadel): Replace some counters with expressions if they can be calculated based - // on branching. (For example, one branch of a SwitchInt can be computed from the counter - // for the CoverageSpan just prior to the SwitchInt minus the sum of the counters of all - // other branches). - - self.to_refined_spans_without_closures() - } - - fn add_refined_span(&mut self, coverage_span: CoverageSpan) { - self.refined_spans.push(coverage_span); - } - - /// Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage - /// regions for the current function leave room for the closure's own coverage regions - /// (injected separately, from the closure's own MIR). - fn to_refined_spans_without_closures(mut self) -> Vec { - self.refined_spans.retain(|covspan| !covspan.is_closure); - self.refined_spans - } - - fn curr(&self) -> &CoverageSpan { - self.some_curr - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) - } - - fn curr_mut(&mut self) -> &mut CoverageSpan { - self.some_curr - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) - } - - fn prev(&self) -> &CoverageSpan { - self.some_prev - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) - } - - fn prev_mut(&mut self) -> &mut CoverageSpan { - self.some_prev - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) - } - - fn take_prev(&mut self) -> CoverageSpan { - self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) - } - - /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the - /// `pending_dups` spans), then one of the following two things happened during the previous - /// iteration: - /// * the `span` of prev was modified (by `curr_mut().merge_from(prev)`); or - /// * the `span` of prev advanced past the end of the span of pending_dups - /// (`prev().span.hi() <= curr().span.lo()`) - /// In either case, no more spans will match the span of `pending_dups`, so - /// add the `pending_dups` if they don't overlap `curr`, and clear the list. - fn check_pending_dups(&mut self) { - if let Some(dup) = self.pending_dups.last() { - if dup.span != self.prev().span { - debug!( - " SAME spans, but pending_dups are NOT THE SAME, so BCBs matched on \ - previous iteration, or prev started a new disjoint span" - ); - if dup.span.hi() <= self.curr().span.lo() { - let pending_dups = self.pending_dups.split_off(0); - for dup in pending_dups.into_iter() { - debug!(" ...adding at least one pending={:?}", dup); - self.add_refined_span(dup); - } - } else { - self.pending_dups.clear(); - } - } - } - } - - /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. - fn next_coverage_span(&mut self) -> bool { - if let Some(curr) = self.some_curr.take() { - self.some_prev = Some(curr); - self.prev_original_span = self.curr_original_span; - } - while let Some(curr) = self.sorted_spans_iter.next() { - debug!("FOR curr={:?}", curr); - if self.prev_starts_after_next(&curr) { - debug!( - " prev.span starts after curr.span, so curr will be dropped (skipping past \ - closure?); prev={:?}", - self.prev() - ); - } else { - // Save a copy of the original span for `curr` in case the `CoverageSpan` is changed - // by `self.curr_mut().merge_from(prev)`. - self.curr_original_span = curr.span; - self.some_curr.replace(curr); - self.check_pending_dups(); - return true; - } - } - false - } - - /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the - /// `curr` coverage span. - fn discard_curr(&mut self) { - self.some_curr = None; - } - - /// Returns true if the curr span should be skipped because prev has already advanced beyond the - /// end of curr. This can only happen if a prior iteration updated `prev` to skip past a region - /// of code, such as skipping past a closure. - fn prev_starts_after_next(&self, next_curr: &CoverageSpan) -> bool { - self.prev().span.lo() > next_curr.span.lo() - } - - /// Returns true if the curr span starts past the end of the prev span, which means they don't - /// overlap, so we now know the prev can be added to the refined coverage spans. - fn prev_ends_before_curr(&self) -> bool { - self.prev().span.hi() <= self.curr().span.lo() - } - - /// If `prev`s span extends left of the closure (`curr`), carve out the closure's - /// span from `prev`'s span. (The closure's coverage counters will be injected when - /// processing the closure's own MIR.) Add the portion of the span to the left of the - /// closure; and if the span extends to the right of the closure, update `prev` to - /// that portion of the span. For any `pending_dups`, repeat the same process. - fn carve_out_span_for_closure(&mut self) { - let curr_span = self.curr().span; - let left_cutoff = curr_span.lo(); - let right_cutoff = curr_span.hi(); - let has_pre_closure_span = self.prev().span.lo() < right_cutoff; - let has_post_closure_span = self.prev().span.hi() > right_cutoff; - let mut pending_dups = self.pending_dups.split_off(0); - if has_pre_closure_span { - let mut pre_closure = self.prev().clone(); - pre_closure.span = pre_closure.span.with_hi(left_cutoff); - debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); - if !pending_dups.is_empty() { - for mut dup in pending_dups.iter().cloned() { - dup.span = dup.span.with_hi(left_cutoff); - debug!(" ...and at least one pre_closure dup={:?}", dup); - self.add_refined_span(dup); - } - } - self.add_refined_span(pre_closure); - } - if has_post_closure_span { - // Update prev.span to start after the closure (and discard curr) - self.prev_mut().span = self.prev().span.with_lo(right_cutoff); - self.prev_original_span = self.prev().span; - for dup in pending_dups.iter_mut() { - dup.span = dup.span.with_lo(right_cutoff); - } - self.pending_dups.append(&mut pending_dups); - self.discard_curr(); // since self.prev() was already updated - } else { - pending_dups.clear(); - } - } - - /// When two `CoverageSpan`s have the same `Span`, dominated spans can be discarded; but if - /// neither `CoverageSpan` dominates the other, both (or possibly more than two) are held, - /// until their disposition is determined. In this latter case, the `prev` dup is moved into - /// `pending_dups` so the new `curr` dup can be moved to `prev` for the next iteration. - fn hold_pending_dups_unless_dominated(&mut self) { - // equal coverage spans are ordered by dominators before dominated (if any) - debug_assert!(!self.prev().is_dominated_by(self.curr(), self.dominators)); - - if self.curr().is_dominated_by(&self.prev(), self.dominators) { - // If one span dominates the other, assocate the span with the dominator only. - // - // For example: - // match somenum { - // x if x < 1 => { ... } - // }... - // The span for the first `x` is referenced by both the pattern block (every - // time it is evaluated) and the arm code (only when matched). The counter - // will be applied only to the dominator block. - // - // The dominator's (`prev`) execution count may be higher than the dominated - // block's execution count, so drop `curr`. - debug!( - " different bcbs but SAME spans, and prev dominates curr. Drop curr and \ - keep prev for next iter. prev={:?}", - self.prev() - ); - self.discard_curr(); - } else { - // Save `prev` in `pending_dups`. (`curr` will become `prev` in the next iteration.) - // If the `curr` CoverageSpan is later discarded, `pending_dups` can be discarded as - // well; but if `curr` is added to refined_spans, the `pending_dups` will also be added. - debug!( - " different bcbs but SAME spans, and neither dominates, so keep curr for \ - next iter, and, pending upcoming spans (unless overlapping) add prev={:?}", - self.prev() - ); - let prev = self.take_prev(); - self.pending_dups.push(prev); - } - } - - /// `curr` overlaps `prev`. If `prev`s span extends left of `curr`s span, keep _only_ - /// statements that end before `curr.lo()` (if any), and add the portion of the - /// combined span for those statements. Any other statements have overlapping spans - /// that can be ignored because `curr` and/or other upcoming statements/spans inside - /// the overlap area will produce their own counters. This disambiguation process - /// avoids injecting multiple counters for overlapping spans, and the potential for - /// double-counting. - fn cutoff_prev_at_overlapping_curr(&mut self) { - debug!( - " different bcbs, overlapping spans, so ignore/drop pending and only add prev \ - if it has statements that end before curr={:?}", - self.prev() - ); - if self.pending_dups.is_empty() { - let curr_span = self.curr().span; - self.prev_mut().cutoff_statements_at(curr_span.lo()); - if self.prev().coverage_statements.is_empty() { - debug!(" ... no non-overlapping statements to add"); - } else { - debug!(" ... adding modified prev={:?}", self.prev()); - let prev = self.take_prev(); - self.add_refined_span(prev); - } - } else { - // with `pending_dups`, `prev` cannot have any statements that don't overlap - self.pending_dups.clear(); - } - } -} - -fn filtered_statement_span(statement: &'a Statement<'tcx>, body_span: Span) -> Option { - match statement.kind { - // These statements have spans that are often outside the scope of the executed source code - // for their parent `BasicBlock`. - StatementKind::StorageLive(_) - | StatementKind::StorageDead(_) - // Coverage should not be encountered, but don't inject coverage coverage - | StatementKind::Coverage(_) - // Ignore `Nop`s - | StatementKind::Nop => None, - - // FIXME(richkadel): Look into a possible issue assigning the span to a - // FakeReadCause::ForGuardBinding, in this example: - // match somenum { - // x if x < 1 => { ... } - // }... - // The BasicBlock within the match arm code included one of these statements, but the span - // for it covered the `1` in this source. The actual statements have nothing to do with that - // source span: - // FakeRead(ForGuardBinding, _4); - // where `_4` is: - // _4 = &_1; (at the span for the first `x`) - // and `_1` is the `Place` for `somenum`. - // - // The arm code BasicBlock already has its own assignment for `x` itself, `_3 = 1`, and I've - // decided it's reasonable for that span (even though outside the arm code) to be part of - // the counted coverage of the arm code execution, but I can't justify including the literal - // `1` in the arm code. I'm pretty sure that, if the `FakeRead(ForGuardBinding)` has a - // purpose in codegen, it's probably in the right BasicBlock, but if so, the `Statement`s - // `source_info.span` can't be right. - // - // Consider correcting the span assignment, assuming there is a better solution, and see if - // the following pattern can be removed here: - StatementKind::FakeRead(cause, _) if cause == FakeReadCause::ForGuardBinding => None, - - // Retain spans from all other statements - StatementKind::FakeRead(_, _) // Not including `ForGuardBinding` - | StatementKind::CopyNonOverlapping(..) - | StatementKind::Assign(_) - | StatementKind::SetDiscriminant { .. } - | StatementKind::LlvmInlineAsm(_) - | StatementKind::Retag(_, _) - | StatementKind::AscribeUserType(_, _) => { - Some(source_info_span(&statement.source_info, body_span)) - } - } -} - -fn filtered_terminator_span(terminator: &'a Terminator<'tcx>, body_span: Span) -> Option { - match terminator.kind { - // These terminators have spans that don't positively contribute to computing a reasonable - // span of actually executed source code. (For example, SwitchInt terminators extracted from - // an `if condition { block }` has a span that includes the executed block, if true, - // but for coverage, the code region executed, up to *and* through the SwitchInt, - // actually stops before the if's block.) - TerminatorKind::Unreachable // Unreachable blocks are not connected to the CFG - | TerminatorKind::Assert { .. } - | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } - | TerminatorKind::SwitchInt { .. } - | TerminatorKind::Goto { .. } - // For `FalseEdge`, only the `real` branch is taken, so it is similar to a `Goto`. - | TerminatorKind::FalseEdge { .. } => None, - - // Retain spans from all other terminators - TerminatorKind::Resume - | TerminatorKind::Abort - | TerminatorKind::Return - | TerminatorKind::Call { .. } - | TerminatorKind::Yield { .. } - | TerminatorKind::GeneratorDrop - | TerminatorKind::FalseUnwind { .. } - | TerminatorKind::InlineAsm { .. } => { - Some(source_info_span(&terminator.source_info, body_span)) - } - } -} - -#[inline(always)] -fn source_info_span(source_info: &SourceInfo, body_span: Span) -> Span { - let span = original_sp(source_info.span, body_span).with_ctxt(SyntaxContext::root()); - if body_span.contains(span) { span } else { body_span } -} - -/// Convert the Span into its file name, start line and column, and end line and column -fn make_code_region(file_name: Symbol, source_file: &Lrc, span: Span) -> CodeRegion { - let (start_line, mut start_col) = source_file.lookup_file_pos(span.lo()); - let (end_line, end_col) = if span.hi() == span.lo() { - let (end_line, mut end_col) = (start_line, start_col); - // Extend an empty span by one character so the region will be counted. - let CharPos(char_pos) = start_col; - if char_pos > 0 { - start_col = CharPos(char_pos - 1); - } else { - end_col = CharPos(char_pos + 1); - } - (end_line, end_col) - } else { - source_file.lookup_file_pos(span.hi()) - }; - CodeRegion { - file_name, - start_line: start_line as u32, - start_col: start_col.to_u32() + 1, - end_line: end_line as u32, - end_col: end_col.to_u32() + 1, - } -} - -fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> { - let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); - let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); - tcx.hir().body(fn_body_id) -} - -fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { - let mut hcx = tcx.create_no_span_stable_hashing_context(); - hash(&mut hcx, &hir_body.value).to_smaller_hash() -} - -fn hash( - hcx: &mut StableHashingContext<'tcx>, - node: &impl HashStable>, -) -> Fingerprint { - let mut stable_hasher = StableHasher::new(); - node.hash_stable(hcx, &mut stable_hasher); - stable_hasher.finish() -} - -pub struct ShortCircuitPreorder< - 'a, - 'tcx, - F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>, -> { - body: &'a mir::Body<'tcx>, - visited: BitSet, - worklist: Vec, - filtered_successors: F, -} - -impl<'a, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> - ShortCircuitPreorder<'a, 'tcx, F> -{ - pub fn new( - body: &'a mir::Body<'tcx>, - filtered_successors: F, - ) -> ShortCircuitPreorder<'a, 'tcx, F> { - let worklist = vec![mir::START_BLOCK]; - - ShortCircuitPreorder { - body, - visited: BitSet::new_empty(body.basic_blocks().len()), - worklist, - filtered_successors, - } - } -} - -impl<'a: 'tcx, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> Iterator - for ShortCircuitPreorder<'a, 'tcx, F> -{ - type Item = (BasicBlock, &'a BasicBlockData<'tcx>); - - fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { - while let Some(idx) = self.worklist.pop() { - if !self.visited.insert(idx) { - continue; - } - - let data = &self.body[idx]; - - if let Some(ref term) = data.terminator { - self.worklist.extend((self.filtered_successors)(&term.kind)); - } - - return Some((idx, data)); - } - - None - } - - fn size_hint(&self) -> (usize, Option) { - let size = self.body.basic_blocks().len() - self.visited.count(); - (size, Some(size)) - } -} diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 53935f02a90e4..0678d8253d590 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -219,11 +219,11 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen StatementKind::LlvmInlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())), StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping{ - dst, src, size, + dst, src, count, }) => { check_operand(tcx, dst, span, body)?; check_operand(tcx, src, span, body)?; - check_operand(tcx, size, span, body) + check_operand(tcx, count, span, body) }, // These are all NOPs StatementKind::StorageLive(_) From 049045b100f2b7f5fbc36ecd36418dec1f6853cb Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 00:23:13 +0000 Subject: [PATCH 21/26] Replace todos with impls Changed to various implementations, copying the style of prior function calls in places I was unsure of. Also one minor style nit. --- .../rustc_codegen_ssa/src/mir/statement.rs | 3 +- .../src/borrow_check/invalidation.rs | 6 ---- compiler/rustc_mir/src/borrow_check/mod.rs | 10 +++++- .../src/borrow_check/type_check/mod.rs | 36 ++++++++++++++++++- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 774c920ee9666..054273262f79c 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -126,8 +126,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let count = count_val.immediate_or_packed_pair(&mut bx); let dst = dst_val.immediate_or_packed_pair(&mut bx); let src = src_val.immediate_or_packed_pair(&mut bx); - use crate::MemFlags; - let flags = MemFlags::empty(); + let flags = crate::MemFlags::empty(); bx.memcpy( dst, dst_val.layout.layout.align.pref, diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 1f3dfc251e10c..17c4f3c649460 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -100,12 +100,6 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, src); self.consume_operand(location, dst); self.consume_operand(location, count); - match dst { - Operand::Move(ref place) | Operand::Copy(ref place) => { - self.mutate_place(location, *place, Deep, JustWrite); - } - _ => {} - } } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 539319ab9f25f..037abb04f56d2 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -627,7 +627,15 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc } } - StatementKind::CopyNonOverlapping(..) => todo!(), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + src, + dst, + count, + }) => { + self.consume_operand(location, (src, span), flow_state); + self.consume_operand(location, (dst, span), flow_state); + self.consume_operand(location, (count, span), flow_state); + } StatementKind::Nop | StatementKind::Coverage(..) | StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index 74d7fd84c9e72..58db2752c9845 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -1520,7 +1520,41 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ); } } - StatementKind::CopyNonOverlapping(..) => todo!(), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + ref src, + ref dst, + ref count, + }) => { + let op_src_ty = self.normalize(src.ty(body, self.tcx()), location); + let op_dst_ty = self.normalize(dst.ty(body, self.tcx()), location); + // since CopyNonOverlapping is parametrized by 1 type, + // we only need to check that they are equal and not keep an extra parameter. + if let Err(terr) = self.eq_types( + op_src_ty, + op_dst_ty, + location.to_locations(), + ConstraintCategory::Internal, + ) { + span_mirbug!( + self, + stmt, + "bad arg ({:?} != {:?}): {:?}", + op_src_ty, + op_dst_ty, + terr + ); + } + + let op_cnt_ty = self.normalize(count.ty(body, self.tcx()), location); + if let Err(terr) = self.eq_types( + op_cnt_ty, + tcx.types.usize, + location.to_locations(), + ConstraintCategory::Internal, + ) { + span_mirbug!(self, stmt, "bad arg ({:?} != usize): {:?}", op_cnt_ty, terr); + } + } StatementKind::FakeRead(..) | StatementKind::StorageLive(..) | StatementKind::StorageDead(..) From 845e4b5962aa84fcfc0b8a6b1e4b9e32725547ef Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 02:28:08 +0000 Subject: [PATCH 22/26] Change CopyNonOverlapping::codegen_ssa Fixes copy_non_overlapping codegen_ssa to properly handle pointees, and use bytes instead of elem count --- .../rustc_codegen_ssa/src/mir/statement.rs | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 054273262f79c..e11539c14760c 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -122,19 +122,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) => { let dst_val = self.codegen_operand(&mut bx, dst); let src_val = self.codegen_operand(&mut bx, src); - let count_val = self.codegen_operand(&mut bx, count); - let count = count_val.immediate_or_packed_pair(&mut bx); - let dst = dst_val.immediate_or_packed_pair(&mut bx); - let src = src_val.immediate_or_packed_pair(&mut bx); - let flags = crate::MemFlags::empty(); - bx.memcpy( - dst, - dst_val.layout.layout.align.pref, - src, - src_val.layout.layout.align.pref, - count, - flags, - ); + let count = self.codegen_operand(&mut bx, count).immediate(); + let get_val_align = |oper_ref: crate::mir::OperandRef<'_, _>| match oper_ref.val { + OperandValue::Ref(val, _, align) => (val, align), + _ => unreachable!(), + }; + let pointee_layout = dst_val + .layout + .pointee_info_at(&mut bx, rustc_target::abi::Size::ZERO) + .expect("Expected pointer"); + let elem_size = bx.const_u64(pointee_layout.size.bytes()); + let byte_count = bx.mul(count, elem_size); + + let (dst, dst_align) = get_val_align(dst_val); + let (src, src_align) = get_val_align(src_val); + bx.memcpy(dst, dst_align, src, src_align, byte_count, crate::MemFlags::empty()); bx } mir::StatementKind::FakeRead(..) From d4ae9ff82664a1d7473e32d59819c208efce48c7 Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 03:55:41 +0000 Subject: [PATCH 23/26] Build StKind::CopyOverlapping This replaces where it was previously being constructed in intrinsics, with direct construction of the Statement. --- compiler/rustc_codegen_cranelift/src/base.rs | 15 +- compiler/rustc_codegen_ssa/src/lib.rs | 1 + compiler/rustc_codegen_ssa/src/mir/block.rs | 130 ++++++++++-------- .../rustc_codegen_ssa/src/mir/intrinsic.rs | 13 +- .../rustc_codegen_ssa/src/mir/statement.rs | 14 +- 5 files changed, 98 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index ba7c82d24c517..8b5ae9e0541ad 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -837,10 +837,21 @@ fn codegen_stmt<'tcx>( dst, count, }) => { - let dst = codegen_operand(fx, dst).load_scalar(fx); + let dst = codegen_operand(fx, dst); + let pointee = dst + .layout() + .pointee_info_at(fx, rustc_target::abi::Size::ZERO) + .expect("Expected pointer"); + let dst = dst.load_scalar(fx); let src = codegen_operand(fx, src).load_scalar(fx); let count = codegen_operand(fx, count).load_scalar(fx); - fx.bcx.call_memcpy(fx.cx.module.target_config(), dst, src, count); + let elem_size: u64 = pointee.size.bytes(); + let bytes = if elem_size != 1 { + fx.bcx.ins().imul_imm(count, elem_size as i64) + } else { + count + }; + fx.bcx.call_memcpy(fx.cx.module.target_config(), dst, src, bytes); } } } diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 0307117e1c8b2..2c2330409fd70 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -9,6 +9,7 @@ #![feature(or_patterns)] #![feature(associated_type_bounds)] #![recursion_limit = "256"] +#![feature(box_syntax)] //! This crate contains codegen code that is used by all codegen backends (LLVM and others). //! The backend-agnostic functions of this crate use functions defined in various traits that diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 9ce9066980066..1150d4d734870 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -641,67 +641,89 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { return; } - if intrinsic.is_some() && intrinsic != Some(sym::drop_in_place) { - let intrinsic = intrinsic.unwrap(); - let dest = match ret_dest { - _ if fn_abi.ret.is_indirect() => llargs[0], - ReturnDest::Nothing => { - bx.const_undef(bx.type_ptr_to(bx.arg_memory_ty(&fn_abi.ret))) - } - ReturnDest::IndirectOperand(dst, _) | ReturnDest::Store(dst) => dst.llval, - ReturnDest::DirectOperand(_) => { - bug!("Cannot use direct operand with an intrinsic call") - } - }; + match intrinsic { + None | Some(sym::drop_in_place) => {} + Some(sym::copy_nonoverlapping) => { + bx = self.codegen_statement( + bx, + &rustc_middle::mir::Statement { + source_info: rustc_middle::mir::SourceInfo::outermost(span), + kind: rustc_middle::mir::StatementKind::CopyNonOverlapping( + box rustc_middle::mir::CopyNonOverlapping { + src: args[0].clone(), + dst: args[1].clone(), + count: args[2].clone(), + }, + ), + }, + ); + helper.funclet_br(self, &mut bx, destination.unwrap().1); + return; + } + Some(intrinsic) => { + let dest = match ret_dest { + _ if fn_abi.ret.is_indirect() => llargs[0], + ReturnDest::Nothing => { + bx.const_undef(bx.type_ptr_to(bx.arg_memory_ty(&fn_abi.ret))) + } + ReturnDest::IndirectOperand(dst, _) | ReturnDest::Store(dst) => dst.llval, + ReturnDest::DirectOperand(_) => { + bug!("Cannot use direct operand with an intrinsic call") + } + }; - let args: Vec<_> = args - .iter() - .enumerate() - .map(|(i, arg)| { - // The indices passed to simd_shuffle* in the - // third argument must be constant. This is - // checked by const-qualification, which also - // promotes any complex rvalues to constants. - if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") { - if let mir::Operand::Constant(constant) = arg { - let c = self.eval_mir_constant(constant); - let (llval, ty) = self.simd_shuffle_indices( - &bx, - constant.span, - constant.literal.ty, - c, - ); - return OperandRef { val: Immediate(llval), layout: bx.layout_of(ty) }; - } else { - span_bug!(span, "shuffle indices must be constant"); + let args: Vec<_> = args + .iter() + .enumerate() + .map(|(i, arg)| { + // The indices passed to simd_shuffle* in the + // third argument must be constant. This is + // checked by const-qualification, which also + // promotes any complex rvalues to constants. + if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") { + if let mir::Operand::Constant(constant) = arg { + let c = self.eval_mir_constant(constant); + let (llval, ty) = self.simd_shuffle_indices( + &bx, + constant.span, + constant.literal.ty, + c, + ); + return OperandRef { + val: Immediate(llval), + layout: bx.layout_of(ty), + }; + } else { + span_bug!(span, "shuffle indices must be constant"); + } } - } - self.codegen_operand(&mut bx, arg) - }) - .collect(); + self.codegen_operand(&mut bx, arg) + }) + .collect(); + + self.codegen_intrinsic_call( + &mut bx, + *instance.as_ref().unwrap(), + &fn_abi, + &args, + dest, + span, + ); - Self::codegen_intrinsic_call( - &mut bx, - *instance.as_ref().unwrap(), - &fn_abi, - &args, - dest, - span, - ); + if let ReturnDest::IndirectOperand(dst, _) = ret_dest { + self.store_return(&mut bx, ret_dest, &fn_abi.ret, dst.llval); + } - if let ReturnDest::IndirectOperand(dst, _) = ret_dest { - self.store_return(&mut bx, ret_dest, &fn_abi.ret, dst.llval); - } + if let Some((_, target)) = *destination { + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); + helper.funclet_br(self, &mut bx, target); + } else { + bx.unreachable(); + } - if let Some((_, target)) = *destination { - helper.maybe_sideeffect(self.mir, &mut bx, &[target]); - helper.funclet_br(self, &mut bx, target); - } else { - bx.unreachable(); + return; } - - return; } // Split the rust-call tupled arguments off. diff --git a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs index 80e3ed75b8585..00fc5b6606151 100644 --- a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs @@ -49,6 +49,7 @@ fn memset_intrinsic<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn codegen_intrinsic_call( + &self, bx: &mut Bx, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, @@ -127,16 +128,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } sym::copy_nonoverlapping => { - copy_intrinsic( - bx, - false, - false, - substs.type_at(0), - args[1].immediate(), - args[0].immediate(), - args[2].immediate(), - ); - return; + // handled explicitly in compiler/rustc_codegen_ssa/src/mir/block.rs + unreachable!(); } sym::copy => { copy_intrinsic( diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index e11539c14760c..5523e5f2e8604 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -123,20 +123,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let dst_val = self.codegen_operand(&mut bx, dst); let src_val = self.codegen_operand(&mut bx, src); let count = self.codegen_operand(&mut bx, count).immediate(); - let get_val_align = |oper_ref: crate::mir::OperandRef<'_, _>| match oper_ref.val { - OperandValue::Ref(val, _, align) => (val, align), - _ => unreachable!(), - }; let pointee_layout = dst_val .layout .pointee_info_at(&mut bx, rustc_target::abi::Size::ZERO) .expect("Expected pointer"); - let elem_size = bx.const_u64(pointee_layout.size.bytes()); - let byte_count = bx.mul(count, elem_size); + let bytes = bx.mul(count, bx.const_usize(pointee_layout.size.bytes())); - let (dst, dst_align) = get_val_align(dst_val); - let (src, src_align) = get_val_align(src_val); - bx.memcpy(dst, dst_align, src, src_align, byte_count, crate::MemFlags::empty()); + let align = pointee_layout.align; + let dst = dst_val.immediate(); + let src = src_val.immediate(); + bx.memcpy(dst, align, src, align, bytes, crate::MemFlags::empty()); bx } mir::StatementKind::FakeRead(..) From 217ff6b7ea5ca80b01ee1436914a061ed190d8a8 Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 08:57:04 +0000 Subject: [PATCH 24/26] Switch to changing cp_non_overlap in tform It was suggested to lower this in MIR instead of ssa, so do that instead. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 20 +------ .../rustc_codegen_ssa/src/mir/intrinsic.rs | 6 -- compiler/rustc_middle/src/mir/mod.rs | 2 +- compiler/rustc_mir/src/borrow_check/mod.rs | 6 ++ .../rustc_mir/src/interpret/intrinsics.rs | 12 ++-- compiler/rustc_mir/src/interpret/step.rs | 56 ++++++++++--------- .../rustc_mir/src/transform/check_unsafety.rs | 2 +- .../src/transform/lower_intrinsics.rs | 21 +++++++ .../src/transform/remove_noop_landing_pads.rs | 2 +- .../clippy_utils/src/qualify_min_const_fn.rs | 2 +- 10 files changed, 67 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 1150d4d734870..e148ed7ad3bce 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -643,23 +643,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { match intrinsic { None | Some(sym::drop_in_place) => {} - Some(sym::copy_nonoverlapping) => { - bx = self.codegen_statement( - bx, - &rustc_middle::mir::Statement { - source_info: rustc_middle::mir::SourceInfo::outermost(span), - kind: rustc_middle::mir::StatementKind::CopyNonOverlapping( - box rustc_middle::mir::CopyNonOverlapping { - src: args[0].clone(), - dst: args[1].clone(), - count: args[2].clone(), - }, - ), - }, - ); - helper.funclet_br(self, &mut bx, destination.unwrap().1); - return; - } + Some(sym::copy_nonoverlapping) => unreachable!(), Some(intrinsic) => { let dest = match ret_dest { _ if fn_abi.ret.is_indirect() => llargs[0], @@ -702,7 +686,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) .collect(); - self.codegen_intrinsic_call( + Self::codegen_intrinsic_call( &mut bx, *instance.as_ref().unwrap(), &fn_abi, diff --git a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs index 00fc5b6606151..8502309b90e5a 100644 --- a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs @@ -49,7 +49,6 @@ fn memset_intrinsic<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn codegen_intrinsic_call( - &self, bx: &mut Bx, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, @@ -126,11 +125,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let offset = args[1].immediate(); bx.gep(ptr, &[offset]) } - - sym::copy_nonoverlapping => { - // handled explicitly in compiler/rustc_codegen_ssa/src/mir/block.rs - unreachable!(); - } sym::copy => { copy_intrinsic( bx, diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index c046333e68d2b..f6952667494db 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1682,7 +1682,7 @@ pub struct Coverage { pub code_region: Option, } -#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] +#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable)] pub struct CopyNonOverlapping<'tcx> { pub src: Operand<'tcx>, pub dst: Operand<'tcx>, diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 037abb04f56d2..8683318e4a7b5 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -628,13 +628,19 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc } StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + .. + /* src, dst, count, + */ }) => { + unreachable!() + /* self.consume_operand(location, (src, span), flow_state); self.consume_operand(location, (dst, span), flow_state); self.consume_operand(location, (count, span), flow_state); + */ } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index d36b3a7d9b56e..b0a2d906519ac 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -323,7 +323,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let result = Scalar::from_uint(truncated_bits, layout.size); self.write_scalar(result, dest)?; } - sym::copy | sym::copy_nonoverlapping => { + sym::copy_nonoverlapping => { + self.copy_nonoverlapping(args[0], args[1], args[2])?; + } + sym::copy => { let elem_ty = instance.substs.type_at(0); let elem_layout = self.layout_of(elem_ty)?; let count = self.read_scalar(&args[2])?.to_machine_usize(self)?; @@ -338,12 +341,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dest = self.memory.check_ptr_access(dest, size, elem_align)?; if let (Some(src), Some(dest)) = (src, dest) { - self.memory.copy( - src, - dest, - size, - intrinsic_name == sym::copy_nonoverlapping, - )?; + self.memory.copy(src, dest, size, false)?; } } sym::offset => { diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 89dc93c790090..1c2bc57c99a83 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -2,6 +2,7 @@ //! //! The main entry point is the `step` method. +use crate::interpret::OpTy; use rustc_middle::mir; use rustc_middle::mir::interpret::{InterpResult, Scalar}; use rustc_target::abi::LayoutOf; @@ -115,35 +116,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Call CopyNonOverlapping CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, count }) => { - let (src, size) = { - let src = self.eval_operand(src, None)?; - let size = src.layout.layout.size; - let mplace = *src.assert_mem_place(self); - let ptr = match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - }; - (ptr, size) - }; - - let dst = { - let dst = self.eval_operand(dst, None)?; - let mplace = *dst.assert_mem_place(self); - match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - } - }; - let count = self.eval_operand(count, None)?; - let count = self.read_immediate(count)?.to_scalar()?; - let count = if let Scalar::Int(i) = count { - core::convert::TryFrom::try_from(i).unwrap() - } else { - panic!(); - }; - self.memory.copy_repeatedly(src, dst, size, count, /*nonoverlapping*/ true)?; + let src = self.eval_operand(src, None)?; + let dst = self.eval_operand(dst, None)?; + self.copy_nonoverlapping(src, dst, count)?; } // Statements we do not track. @@ -173,6 +150,31 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } + pub(crate) fn copy_nonoverlapping( + &mut self, + src: OpTy<'tcx, >::PointerTag>, + dst: OpTy<'tcx, >::PointerTag>, + count: OpTy<'tcx, >::PointerTag>, + ) -> InterpResult<'tcx> { + let count = self.read_scalar(&count)?.to_machine_usize(self)?; + let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?; + let (size, align) = (layout.size, layout.align.abi); + let src = + self.memory.check_ptr_access(self.read_scalar(&src)?.check_init()?, size, align)?; + + let dst = + self.memory.check_ptr_access(self.read_scalar(&dst)?.check_init()?, size, align)?; + + let size = size.checked_mul(count, self).ok_or_else(|| { + err_ub_format!("overflow computing total size of `copy_nonoverlapping`") + })?; + + if let (Some(src), Some(dst)) = (src, dst) { + self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + } + Ok(()) + } + /// Evaluate an assignment statement. /// /// There is no separate `eval_rvalue` function. Instead, the code for handling each rvalue diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index e7fdd5496cb40..33848bc130581 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -115,7 +115,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) - | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // safe (at least as emitted during MIR construction) } @@ -124,6 +123,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { UnsafetyViolationKind::General, UnsafetyViolationDetails::UseOfInlineAssembly, ), + StatementKind::CopyNonOverlapping(..) => unreachable!(), } self.super_statement(statement, location); } diff --git a/compiler/rustc_mir/src/transform/lower_intrinsics.rs b/compiler/rustc_mir/src/transform/lower_intrinsics.rs index 177b00b00da36..d6a7336061608 100644 --- a/compiler/rustc_mir/src/transform/lower_intrinsics.rs +++ b/compiler/rustc_mir/src/transform/lower_intrinsics.rs @@ -40,6 +40,27 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics { terminator.kind = TerminatorKind::Goto { target }; } } + sym::copy_nonoverlapping => { + let target = destination.unwrap().1; + let mut args = args.drain(..); + block.statements.push(Statement { + source_info: terminator.source_info, + kind: StatementKind::CopyNonOverlapping( + box rustc_middle::mir::CopyNonOverlapping { + src: args.next().unwrap(), + dst: args.next().unwrap(), + count: args.next().unwrap(), + }, + ), + }); + assert_eq!( + args.next(), + None, + "Extra argument for copy_non_overlapping intrinsic" + ); + drop(args); + terminator.kind = TerminatorKind::Goto { target }; + } sym::wrapping_add | sym::wrapping_sub | sym::wrapping_mul => { if let Some((destination, target)) = *destination { let lhs; diff --git a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs index a37f5d4f329f3..5347846a4b334 100644 --- a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs +++ b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs @@ -39,7 +39,6 @@ impl RemoveNoopLandingPads { | StatementKind::StorageDead(_) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) - | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // These are all nops in a landing pad } @@ -56,6 +55,7 @@ impl RemoveNoopLandingPads { StatementKind::Assign { .. } | StatementKind::SetDiscriminant { .. } | StatementKind::LlvmInlineAsm { .. } + | StatementKind::CopyNonOverlapping(..) | StatementKind::Retag { .. } => { return false; } diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 0678d8253d590..1391f7505e27c 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -224,7 +224,7 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen check_operand(tcx, dst, span, body)?; check_operand(tcx, src, span, body)?; check_operand(tcx, count, span, body) - }, + } // These are all NOPs StatementKind::StorageLive(_) | StatementKind::StorageDead(_) From 1e4d8042fc98dd07546a0eb201517983e82f4235 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 9 Mar 2021 19:40:01 +0100 Subject: [PATCH 25/26] Don't hardcode the `v1` prelude in diagnostics. Instead of looking for `std::prelude::v1`, this changes it to look for `std::prelude::`. --- compiler/rustc_resolve/src/late/diagnostics.rs | 2 +- compiler/rustc_typeck/src/check/demand.rs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 87bf79d722bde..8954eefef7a77 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -324,7 +324,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { .lookup_import_candidates(ident, ns, &self.parent_scope, is_enum_variant) .into_iter() .map(|suggestion| import_candidate_to_enum_paths(&suggestion)) - .filter(|(_, enum_ty_path)| enum_ty_path != "std::prelude::v1") + .filter(|(_, enum_ty_path)| !enum_ty_path.starts_with("std::prelude::")) .collect(); if !enum_candidates.is_empty() { if let (PathSource::Type, Some(span)) = diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 39b973ed371ae..f9f67769e96a4 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -200,7 +200,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if self.can_coerce(expr_ty, sole_field_ty) { let variant_path = self.tcx.def_path_str(variant.def_id); // FIXME #56861: DRYer prelude filtering - Some(variant_path.trim_start_matches("std::prelude::v1::").to_string()) + if let Some(path) = variant_path.strip_prefix("std::prelude::") { + if let Some((_, path)) = path.split_once("::") { + return Some(path.to_string()); + } + } + Some(variant_path) } else { None } From 4bceb294f419066e98cab9a953a43ddeaea5494a Mon Sep 17 00:00:00 2001 From: kadmin Date: Fri, 26 Feb 2021 16:42:51 +0000 Subject: [PATCH 26/26] Clean up todos Also add some span_bugs where it is unreachable --- compiler/rustc_codegen_cranelift/src/lib.rs | 1 - compiler/rustc_mir/src/borrow_check/mod.rs | 15 ++----- .../src/borrow_check/type_check/mod.rs | 39 +++------------- .../rustc_mir/src/dataflow/impls/borrows.rs | 3 +- .../src/dataflow/impls/storage_liveness.rs | 3 +- .../src/dataflow/move_paths/builder.rs | 3 +- .../rustc_mir/src/interpret/intrinsics.rs | 20 +-------- compiler/rustc_mir/src/interpret/step.rs | 13 +++--- compiler/rustc_mir/src/transform/validate.rs | 44 ++++++++++++++++++- 9 files changed, 63 insertions(+), 78 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/lib.rs b/compiler/rustc_codegen_cranelift/src/lib.rs index fae71fef9e676..8edb883ccb5f9 100644 --- a/compiler/rustc_codegen_cranelift/src/lib.rs +++ b/compiler/rustc_codegen_cranelift/src/lib.rs @@ -11,7 +11,6 @@ #![warn(rust_2018_idioms)] #![warn(unused_lifetimes)] #![warn(unreachable_pub)] -#![feature(box_patterns)] extern crate snap; #[macro_use] diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 8683318e4a7b5..5b8bb7257e230 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -629,18 +629,11 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { .. - /* - src, - dst, - count, - */ }) => { - unreachable!() - /* - self.consume_operand(location, (src, span), flow_state); - self.consume_operand(location, (dst, span), flow_state); - self.consume_operand(location, (count, span), flow_state); - */ + span_bug!( + span, + "Unexpected CopyNonOverlapping, should only appear after lower_intrinsics", + ) } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index 58db2752c9845..ab7e75bf4f10c 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -1521,40 +1521,11 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } } StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { - ref src, - ref dst, - ref count, - }) => { - let op_src_ty = self.normalize(src.ty(body, self.tcx()), location); - let op_dst_ty = self.normalize(dst.ty(body, self.tcx()), location); - // since CopyNonOverlapping is parametrized by 1 type, - // we only need to check that they are equal and not keep an extra parameter. - if let Err(terr) = self.eq_types( - op_src_ty, - op_dst_ty, - location.to_locations(), - ConstraintCategory::Internal, - ) { - span_mirbug!( - self, - stmt, - "bad arg ({:?} != {:?}): {:?}", - op_src_ty, - op_dst_ty, - terr - ); - } - - let op_cnt_ty = self.normalize(count.ty(body, self.tcx()), location); - if let Err(terr) = self.eq_types( - op_cnt_ty, - tcx.types.usize, - location.to_locations(), - ConstraintCategory::Internal, - ) { - span_mirbug!(self, stmt, "bad arg ({:?} != usize): {:?}", op_cnt_ty, terr); - } - } + .. + }) => span_bug!( + stmt.source_info.span, + "Unexpected StatementKind::CopyNonOverlapping, should only appear after lowering_intrinsics", + ), StatementKind::FakeRead(..) | StatementKind::StorageLive(..) | StatementKind::StorageDead(..) diff --git a/compiler/rustc_mir/src/dataflow/impls/borrows.rs b/compiler/rustc_mir/src/dataflow/impls/borrows.rs index ffa02f855c979..f24d0f0266d9f 100644 --- a/compiler/rustc_mir/src/dataflow/impls/borrows.rs +++ b/compiler/rustc_mir/src/dataflow/impls/borrows.rs @@ -305,9 +305,8 @@ impl<'tcx> dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> { | mir::StatementKind::Retag { .. } | mir::StatementKind::AscribeUserType(..) | mir::StatementKind::Coverage(..) + | mir::StatementKind::CopyNonOverlapping(..) | mir::StatementKind::Nop => {} - - mir::StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs index e407f394c51ec..792664597fd9a 100644 --- a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs +++ b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs @@ -149,9 +149,8 @@ impl<'mir, 'tcx> dataflow::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, | StatementKind::FakeRead(..) | StatementKind::Nop | StatementKind::Retag(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::StorageLive(..) => {} - - StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs index c14ac74ebadab..1ddd81e779b15 100644 --- a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs +++ b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs @@ -318,9 +318,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} - - StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index b0a2d906519ac..25c3c2c632d81 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -323,26 +323,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let result = Scalar::from_uint(truncated_bits, layout.size); self.write_scalar(result, dest)?; } - sym::copy_nonoverlapping => { - self.copy_nonoverlapping(args[0], args[1], args[2])?; - } sym::copy => { - let elem_ty = instance.substs.type_at(0); - let elem_layout = self.layout_of(elem_ty)?; - let count = self.read_scalar(&args[2])?.to_machine_usize(self)?; - let elem_align = elem_layout.align.abi; - - let size = elem_layout.size.checked_mul(count, self).ok_or_else(|| { - err_ub_format!("overflow computing total size of `{}`", intrinsic_name) - })?; - let src = self.read_scalar(&args[0])?.check_init()?; - let src = self.memory.check_ptr_access(src, size, elem_align)?; - let dest = self.read_scalar(&args[1])?.check_init()?; - let dest = self.memory.check_ptr_access(dest, size, elem_align)?; - - if let (Some(src), Some(dest)) = (src, dest) { - self.memory.copy(src, dest, size, false)?; - } + self.copy(&args[0], &args[1], &args[2], /*nonoverlapping*/ false)?; } sym::offset => { let ptr = self.read_scalar(&args[0])?.check_init()?; diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 1c2bc57c99a83..0f365eaa41dde 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -120,7 +120,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let src = self.eval_operand(src, None)?; let dst = self.eval_operand(dst, None)?; - self.copy_nonoverlapping(src, dst, count)?; + self.copy(&src, &dst, &count, /* nonoverlapping */ true)?; } // Statements we do not track. @@ -150,11 +150,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } - pub(crate) fn copy_nonoverlapping( + pub(crate) fn copy( &mut self, - src: OpTy<'tcx, >::PointerTag>, - dst: OpTy<'tcx, >::PointerTag>, - count: OpTy<'tcx, >::PointerTag>, + src: &OpTy<'tcx, >::PointerTag>, + dst: &OpTy<'tcx, >::PointerTag>, + count: &OpTy<'tcx, >::PointerTag>, + nonoverlapping: bool, ) -> InterpResult<'tcx> { let count = self.read_scalar(&count)?.to_machine_usize(self)?; let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?; @@ -170,7 +171,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { })?; if let (Some(src), Some(dst)) = (src, dst) { - self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + self.memory.copy(src, dst, size, nonoverlapping)?; } Ok(()) } diff --git a/compiler/rustc_mir/src/transform/validate.rs b/compiler/rustc_mir/src/transform/validate.rs index 29b90bff210bd..d009b0b1b2384 100644 --- a/compiler/rustc_mir/src/transform/validate.rs +++ b/compiler/rustc_mir/src/transform/validate.rs @@ -294,7 +294,49 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { ); } } - _ => {} + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + ref src, + ref dst, + ref count, + }) => { + let src_ty = src.ty(&self.body.local_decls, self.tcx); + let op_src_ty = if let Some(src_deref) = src_ty.builtin_deref(true) { + src_deref.ty + } else { + self.fail( + location, + format!("Expected src to be ptr in copy_nonoverlapping, got: {}", src_ty), + ); + return; + }; + let dst_ty = dst.ty(&self.body.local_decls, self.tcx); + let op_dst_ty = if let Some(dst_deref) = dst_ty.builtin_deref(true) { + dst_deref.ty + } else { + self.fail( + location, + format!("Expected dst to be ptr in copy_nonoverlapping, got: {}", dst_ty), + ); + return; + }; + // since CopyNonOverlapping is parametrized by 1 type, + // we only need to check that they are equal and not keep an extra parameter. + if op_src_ty != op_dst_ty { + self.fail(location, format!("bad arg ({:?} != {:?})", op_src_ty, op_dst_ty)); + } + + let op_cnt_ty = count.ty(&self.body.local_decls, self.tcx); + if op_cnt_ty != self.tcx.types.usize { + self.fail(location, format!("bad arg ({:?} != usize)", op_cnt_ty)) + } + } + StatementKind::SetDiscriminant { .. } + | StatementKind::StorageLive(..) + | StatementKind::StorageDead(..) + | StatementKind::LlvmInlineAsm(..) + | StatementKind::Retag(_, _) + | StatementKind::Coverage(_) + | StatementKind::Nop => {} } self.super_statement(statement, location);