From 9678cece6de806693fc585225c0d6a4a5adaacd3 Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 22 Jun 2022 16:42:49 +0200 Subject: [PATCH 01/19] std: rewrite SGX thread parker --- library/std/src/sys/sgx/mod.rs | 1 + library/std/src/sys/sgx/thread_parker.rs | 93 +++++++++++++++++++ .../std/src/sys_common/thread_parker/mod.rs | 2 + 3 files changed, 96 insertions(+) create mode 100644 library/std/src/sys/sgx/thread_parker.rs diff --git a/library/std/src/sys/sgx/mod.rs b/library/std/src/sys/sgx/mod.rs index 696400670e04d..65c1d0afe4608 100644 --- a/library/std/src/sys/sgx/mod.rs +++ b/library/std/src/sys/sgx/mod.rs @@ -33,6 +33,7 @@ pub mod process; pub mod stdio; pub mod thread; pub mod thread_local_key; +pub mod thread_parker; pub mod time; mod condvar; diff --git a/library/std/src/sys/sgx/thread_parker.rs b/library/std/src/sys/sgx/thread_parker.rs new file mode 100644 index 0000000000000..f768abddd44dc --- /dev/null +++ b/library/std/src/sys/sgx/thread_parker.rs @@ -0,0 +1,93 @@ +//! Thread parking based on SGX events. + +use super::abi::{thread, usercalls}; +use crate::io::ErrorKind; +use crate::pin::Pin; +use crate::ptr::{self, NonNull}; +use crate::sync::atomic::AtomicPtr; +use crate::sync::atomic::Ordering::{Acquire, Relaxed, Release}; +use crate::time::Duration; +use fortanix_sgx_abi::{EV_UNPARK, WAIT_INDEFINITE}; + +const EMPTY: *mut u8 = ptr::invalid_mut(0); +/// The TCS structure must be page-aligned, so this cannot be a valid pointer +const NOTIFIED: *mut u8 = ptr::invalid_mut(1); + +pub struct Parker { + state: AtomicPtr, +} + +impl Parker { + /// Construct the thread parker. The UNIX parker implementation + /// requires this to happen in-place. + pub unsafe fn new(parker: *mut Parker) { + unsafe { parker.write(Parker::new_internal()) } + } + + pub(super) fn new_internal() -> Parker { + Parker { state: AtomicPtr::new(EMPTY) } + } + + // This implementation doesn't require `unsafe` and `Pin`, but other implementations do. + pub unsafe fn park(self: Pin<&Self>) { + let tcs = thread::current().as_ptr(); + + if self.state.load(Acquire) != NOTIFIED { + if self.state.compare_exchange(EMPTY, tcs, Acquire, Acquire).is_ok() { + // Loop to guard against spurious wakeups. + loop { + let event = usercalls::wait(EV_UNPARK, WAIT_INDEFINITE).unwrap(); + assert!(event & EV_UNPARK == EV_UNPARK); + if self.state.load(Acquire) == NOTIFIED { + break; + } + } + } + } + + // At this point, the token was definately read with acquire ordering, + // so this can be a store. + self.state.store(EMPTY, Relaxed); + } + + // This implementation doesn't require `unsafe` and `Pin`, but other implementations do. + pub unsafe fn park_timeout(self: Pin<&Self>, dur: Duration) { + let timeout = u128::min(dur.as_nanos(), WAIT_INDEFINITE as u128 - 1) as u64; + let tcs = thread::current().as_ptr(); + + if self.state.load(Acquire) != NOTIFIED { + if self.state.compare_exchange(EMPTY, tcs, Acquire, Acquire).is_ok() { + match usercalls::wait(EV_UNPARK, timeout) { + Ok(event) => assert!(event & EV_UNPARK == EV_UNPARK), + Err(e) => { + assert!(matches!(e.kind(), ErrorKind::TimedOut | ErrorKind::WouldBlock)) + } + } + + // Swap to provide acquire ordering even if the timeout occurred + // before the token was set. This situation can result in spurious + // wakeups on the next call to `park_timeout`, but it is better to let + // those be handled by the user than do some perhaps unnecessary, but + // always expensive guarding. + self.state.swap(EMPTY, Acquire); + return; + } + } + + // The token was already read with `acquire` ordering, this can be a store. + self.state.store(EMPTY, Relaxed); + } + + // This implementation doesn't require `Pin`, but other implementations do. + pub fn unpark(self: Pin<&Self>) { + let state = self.state.swap(NOTIFIED, Release); + + if !matches!(state, EMPTY | NOTIFIED) { + // There is a thread waiting, wake it up. + let tcs = NonNull::new(state).unwrap(); + // This will fail if the thread has already terminated by the time the signal is send, + // but that is OK. + let _ = usercalls::send(EV_UNPARK, Some(tcs)); + } + } +} diff --git a/library/std/src/sys_common/thread_parker/mod.rs b/library/std/src/sys_common/thread_parker/mod.rs index c789a388e05ad..505f26a4001ae 100644 --- a/library/std/src/sys_common/thread_parker/mod.rs +++ b/library/std/src/sys_common/thread_parker/mod.rs @@ -13,6 +13,8 @@ cfg_if::cfg_if! { pub use crate::sys::thread_parker::Parker; } else if #[cfg(target_family = "unix")] { pub use crate::sys::thread_parker::Parker; + } else if #[cfg(all(target_vendor = "fortanix", target_env = "sgx"))] { + pub use crate::sys::thread_parker::Parker; } else { mod generic; pub use generic::Parker; From 633d46d0245bf7f60b341c8df8da230c0b689396 Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 22 Jun 2022 16:44:43 +0200 Subject: [PATCH 02/19] std: reimplement SGX thread joining to use `Parker` --- library/std/src/sys/sgx/thread.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/library/std/src/sys/sgx/thread.rs b/library/std/src/sys/sgx/thread.rs index d745a61961404..579f758c6cc33 100644 --- a/library/std/src/sys/sgx/thread.rs +++ b/library/std/src/sys/sgx/thread.rs @@ -65,39 +65,36 @@ mod task_queue { /// execution. The signal is sent once all TLS destructors have finished at /// which point no new thread locals should be created. pub mod wait_notify { - use super::super::waitqueue::{SpinMutex, WaitQueue, WaitVariable}; + use super::super::thread_parker::Parker; + use crate::pin::Pin; use crate::sync::Arc; - pub struct Notifier(Arc>>); + pub struct Notifier(Arc); impl Notifier { /// Notify the waiter. The waiter is either notified right away (if /// currently blocked in `Waiter::wait()`) or later when it calls the /// `Waiter::wait()` method. pub fn notify(self) { - let mut guard = self.0.lock(); - *guard.lock_var_mut() = true; - let _ = WaitQueue::notify_one(guard); + Pin::new(&*self.0).unpark() } } - pub struct Waiter(Arc>>); + pub struct Waiter(Arc); impl Waiter { /// Wait for a notification. If `Notifier::notify()` has already been /// called, this will return immediately, otherwise the current thread /// is blocked until notified. pub fn wait(self) { - let guard = self.0.lock(); - if *guard.lock_var() { - return; - } - WaitQueue::wait(guard, || {}); + // This is not actually `unsafe`, but it uses the `Parker` API, + // which needs `unsafe` on some platforms. + unsafe { Pin::new(&*self.0).park() } } } pub fn new() -> (Notifier, Waiter) { - let inner = Arc::new(SpinMutex::new(WaitVariable::new(false))); + let inner = Arc::new(Parker::new_internal()); (Notifier(inner.clone()), Waiter(inner)) } } From a40d300100a5e48cb66f5261738496dbacf11f99 Mon Sep 17 00:00:00 2001 From: joboet Date: Mon, 5 Sep 2022 10:19:12 +0200 Subject: [PATCH 03/19] std: clarify semantics of SGX parker --- library/std/src/sys/sgx/thread_parker.rs | 44 ++++++++++++++++-------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/library/std/src/sys/sgx/thread_parker.rs b/library/std/src/sys/sgx/thread_parker.rs index f768abddd44dc..1c55bcffb1e8c 100644 --- a/library/std/src/sys/sgx/thread_parker.rs +++ b/library/std/src/sys/sgx/thread_parker.rs @@ -9,11 +9,17 @@ use crate::sync::atomic::Ordering::{Acquire, Relaxed, Release}; use crate::time::Duration; use fortanix_sgx_abi::{EV_UNPARK, WAIT_INDEFINITE}; -const EMPTY: *mut u8 = ptr::invalid_mut(0); -/// The TCS structure must be page-aligned, so this cannot be a valid pointer -const NOTIFIED: *mut u8 = ptr::invalid_mut(1); +// The TCS structure must be page-aligned (this is checked by EENTER), so these cannot +// be valid pointers +const EMPTY: *mut u8 = ptr::invalid_mut(1); +const NOTIFIED: *mut u8 = ptr::invalid_mut(2); pub struct Parker { + /// The park state. One of EMPTY, NOTIFIED or a TCS address. + /// A state change to NOTIFIED must be done with release ordering + /// and be observed with acquire ordering so that operations after + /// `thread::park` returns will not occur before the unpark message + /// was sent. state: AtomicPtr, } @@ -30,23 +36,28 @@ impl Parker { // This implementation doesn't require `unsafe` and `Pin`, but other implementations do. pub unsafe fn park(self: Pin<&Self>) { - let tcs = thread::current().as_ptr(); - if self.state.load(Acquire) != NOTIFIED { - if self.state.compare_exchange(EMPTY, tcs, Acquire, Acquire).is_ok() { - // Loop to guard against spurious wakeups. - loop { + let mut prev = EMPTY; + loop { + // Guard against changing TCS addresses by always setting the state to + // the current value. + let tcs = thread::current().as_ptr(); + if self.state.compare_exchange(prev, tcs, Relaxed, Acquire).is_ok() { let event = usercalls::wait(EV_UNPARK, WAIT_INDEFINITE).unwrap(); assert!(event & EV_UNPARK == EV_UNPARK); - if self.state.load(Acquire) == NOTIFIED { - break; - } + prev = tcs; + } else { + // The state was definitely changed by another thread at this point. + // The only time this occurs is when the state is changed to NOTIFIED. + // We observed this change with acquire ordering, so we can simply + // change the state to EMPTY with a relaxed store. + break; } } } // At this point, the token was definately read with acquire ordering, - // so this can be a store. + // so this can be a relaxed store. self.state.store(EMPTY, Relaxed); } @@ -56,7 +67,7 @@ impl Parker { let tcs = thread::current().as_ptr(); if self.state.load(Acquire) != NOTIFIED { - if self.state.compare_exchange(EMPTY, tcs, Acquire, Acquire).is_ok() { + if self.state.compare_exchange(EMPTY, tcs, Relaxed, Acquire).is_ok() { match usercalls::wait(EV_UNPARK, timeout) { Ok(event) => assert!(event & EV_UNPARK == EV_UNPARK), Err(e) => { @@ -85,8 +96,11 @@ impl Parker { if !matches!(state, EMPTY | NOTIFIED) { // There is a thread waiting, wake it up. let tcs = NonNull::new(state).unwrap(); - // This will fail if the thread has already terminated by the time the signal is send, - // but that is OK. + // This will fail if the thread has already terminated or its TCS is destroyed + // by the time the signal is sent, but that is fine. If another thread receives + // the same TCS, it will receive this notification as a spurious wakeup, but + // all users of `wait` should and (internally) do guard against those where + // necessary. let _ = usercalls::send(EV_UNPARK, Some(tcs)); } } From ac672621c0d9f10c0f5026cadba4b326eb213856 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 16 Nov 2022 20:50:39 -0500 Subject: [PATCH 04/19] Set `download-ci-llvm = "if-available"` by default when `channel = "dev"` See https://github.com/rust-lang/compiler-team/issues/566. The motivation for changing the default is to avoid downloading and building LLVM when someone runs `x build` before running `x setup`. The motivation for only doing it on `channel = "dev"` is to avoid breaking distros or users installing from source. It works because `dev` is also the default channel. The diff looks larger than it is; most of it is moving the `llvm` branch below the `rust` so `config.channel` is set. --- config.toml.example | 7 +- src/bootstrap/config.rs | 202 +++++++++++++----------- src/bootstrap/config/tests.rs | 24 +++ src/bootstrap/defaults/config.user.toml | 4 + src/bootstrap/lib.rs | 4 +- 5 files changed, 141 insertions(+), 100 deletions(-) create mode 100644 src/bootstrap/config/tests.rs diff --git a/config.toml.example b/config.toml.example index c94a27b12a3a7..4ace5bd9bdf34 100644 --- a/config.toml.example +++ b/config.toml.example @@ -35,9 +35,6 @@ changelog-seen = 2 # Unless you're developing for a target where Rust CI doesn't build a compiler # toolchain or changing LLVM locally, you probably want to set this to true. # -# This is false by default so that distributions don't unexpectedly download -# LLVM from the internet. -# # All tier 1 targets are currently supported; set this to `"if-available"` if # you are not sure whether you're on a tier 1 target. # @@ -45,7 +42,9 @@ changelog-seen = 2 # # Note that many of the LLVM options are not currently supported for # downloading. Currently only the "assertions" option can be toggled. -#download-ci-llvm = false +# +# Defaults to "if-available" when `channel = "dev"` and "false" otherwise. +#download-ci-llvm = "if-available" # Indicates whether LLVM rebuild should be skipped when running bootstrap. If # this is `false` then the compiler's LLVM will be rebuilt whenever the built diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index af004aa509854..e1603430fe773 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -3,6 +3,9 @@ //! This module implements parsing `config.toml` configuration files to tweak //! how the build runs. +#[cfg(test)] +mod tests; + use std::cell::{Cell, RefCell}; use std::cmp; use std::collections::{HashMap, HashSet}; @@ -693,7 +696,7 @@ define_config! { } } -#[derive(Deserialize)] +#[derive(Debug, Deserialize)] #[serde(untagged)] enum StringOrBool { String(String), @@ -819,6 +822,29 @@ impl Config { } pub fn parse(args: &[String]) -> Config { + #[cfg(test)] + let get_toml = |_: &_| TomlConfig::default(); + #[cfg(not(test))] + let get_toml = |file: &Path| { + let contents = + t!(fs::read_to_string(file), format!("config file {} not found", file.display())); + // Deserialize to Value and then TomlConfig to prevent the Deserialize impl of + // TomlConfig and sub types to be monomorphized 5x by toml. + match toml::from_str(&contents) + .and_then(|table: toml::Value| TomlConfig::deserialize(table)) + { + Ok(table) => table, + Err(err) => { + eprintln!("failed to parse TOML configuration '{}': {}", file.display(), err); + crate::detail_exit(2); + } + } + }; + + Self::parse_inner(args, get_toml) + } + + fn parse_inner<'a>(args: &[String], get_toml: impl 'a + Fn(&Path) -> TomlConfig) -> Config { let flags = Flags::parse(&args); let mut config = Config::default_opts(); @@ -904,25 +930,6 @@ impl Config { config.stage0_metadata = t!(serde_json::from_slice::(&stage0_json)); - #[cfg(test)] - let get_toml = |_| TomlConfig::default(); - #[cfg(not(test))] - let get_toml = |file: &Path| { - let contents = - t!(fs::read_to_string(file), format!("config file {} not found", file.display())); - // Deserialize to Value and then TomlConfig to prevent the Deserialize impl of - // TomlConfig and sub types to be monomorphized 5x by toml. - match toml::from_str(&contents) - .and_then(|table: toml::Value| TomlConfig::deserialize(table)) - { - Ok(table) => table, - Err(err) => { - eprintln!("failed to parse TOML configuration '{}': {}", file.display(), err); - crate::detail_exit(2); - } - } - }; - // Read from `--config`, then `RUST_BOOTSTRAP_CONFIG`, then `./config.toml`, then `config.toml` in the root directory. let toml_path = flags .config @@ -1059,6 +1066,79 @@ impl Config { let mut optimize = None; let mut ignore_git = None; + if let Some(rust) = toml.rust { + debug = rust.debug; + debug_assertions = rust.debug_assertions; + debug_assertions_std = rust.debug_assertions_std; + overflow_checks = rust.overflow_checks; + overflow_checks_std = rust.overflow_checks_std; + debug_logging = rust.debug_logging; + debuginfo_level = rust.debuginfo_level; + debuginfo_level_rustc = rust.debuginfo_level_rustc; + debuginfo_level_std = rust.debuginfo_level_std; + debuginfo_level_tools = rust.debuginfo_level_tools; + debuginfo_level_tests = rust.debuginfo_level_tests; + config.rust_split_debuginfo = rust + .split_debuginfo + .as_deref() + .map(SplitDebuginfo::from_str) + .map(|v| v.expect("invalid value for rust.split_debuginfo")) + .unwrap_or(SplitDebuginfo::default_for_platform(&config.build.triple)); + optimize = rust.optimize; + ignore_git = rust.ignore_git; + config.rust_new_symbol_mangling = rust.new_symbol_mangling; + set(&mut config.rust_optimize_tests, rust.optimize_tests); + set(&mut config.codegen_tests, rust.codegen_tests); + set(&mut config.rust_rpath, rust.rpath); + set(&mut config.jemalloc, rust.jemalloc); + set(&mut config.test_compare_mode, rust.test_compare_mode); + set(&mut config.backtrace, rust.backtrace); + set(&mut config.channel, rust.channel); + config.description = rust.description; + set(&mut config.rust_dist_src, rust.dist_src); + set(&mut config.verbose_tests, rust.verbose_tests); + // in the case "false" is set explicitly, do not overwrite the command line args + if let Some(true) = rust.incremental { + config.incremental = true; + } + set(&mut config.use_lld, rust.use_lld); + set(&mut config.lld_enabled, rust.lld); + set(&mut config.llvm_tools_enabled, rust.llvm_tools); + config.rustc_parallel = rust.parallel_compiler.unwrap_or(false); + config.rustc_default_linker = rust.default_linker; + config.musl_root = rust.musl_root.map(PathBuf::from); + config.save_toolstates = rust.save_toolstates.map(PathBuf::from); + set(&mut config.deny_warnings, flags.deny_warnings.or(rust.deny_warnings)); + set(&mut config.backtrace_on_ice, rust.backtrace_on_ice); + set(&mut config.rust_verify_llvm_ir, rust.verify_llvm_ir); + config.rust_thin_lto_import_instr_limit = rust.thin_lto_import_instr_limit; + set(&mut config.rust_remap_debuginfo, rust.remap_debuginfo); + set(&mut config.control_flow_guard, rust.control_flow_guard); + config.llvm_libunwind_default = rust + .llvm_libunwind + .map(|v| v.parse().expect("failed to parse rust.llvm-libunwind")); + + if let Some(ref backends) = rust.codegen_backends { + config.rust_codegen_backends = + backends.iter().map(|s| INTERNER.intern_str(s)).collect(); + } + + config.rust_codegen_units = rust.codegen_units.map(threads_from_config); + config.rust_codegen_units_std = rust.codegen_units_std.map(threads_from_config); + config.rust_profile_use = flags.rust_profile_use.or(rust.profile_use); + config.rust_profile_generate = flags.rust_profile_generate.or(rust.profile_generate); + config.download_rustc_commit = config.download_ci_rustc_commit(rust.download_rustc); + + config.rust_lto = rust + .lto + .as_deref() + .map(|value| RustcLto::from_str(value).unwrap()) + .unwrap_or_default(); + } else { + config.rust_profile_use = flags.rust_profile_use; + config.rust_profile_generate = flags.rust_profile_generate; + } + if let Some(llvm) = toml.llvm { match llvm.ccache { Some(StringOrBool::String(ref s)) => config.ccache = Some(s.to_string()), @@ -1095,13 +1175,17 @@ impl Config { config.llvm_polly = llvm.polly.unwrap_or(false); config.llvm_clang = llvm.clang.unwrap_or(false); config.llvm_build_config = llvm.build_config.clone().unwrap_or(Default::default()); + + let asserts = llvm_assertions.unwrap_or(false); config.llvm_from_ci = match llvm.download_ci_llvm { Some(StringOrBool::String(s)) => { assert!(s == "if-available", "unknown option `{}` for download-ci-llvm", s); - crate::native::is_ci_llvm_available(&config, llvm_assertions.unwrap_or(false)) + crate::native::is_ci_llvm_available(&config, asserts) } Some(StringOrBool::Bool(b)) => b, - None => false, + None => { + config.channel == "dev" && crate::native::is_ci_llvm_available(&config, asserts) + } }; if config.llvm_from_ci { @@ -1141,79 +1225,9 @@ impl Config { // the link step) with each stage. config.llvm_link_shared.set(Some(true)); } - } - - if let Some(rust) = toml.rust { - debug = rust.debug; - debug_assertions = rust.debug_assertions; - debug_assertions_std = rust.debug_assertions_std; - overflow_checks = rust.overflow_checks; - overflow_checks_std = rust.overflow_checks_std; - debug_logging = rust.debug_logging; - debuginfo_level = rust.debuginfo_level; - debuginfo_level_rustc = rust.debuginfo_level_rustc; - debuginfo_level_std = rust.debuginfo_level_std; - debuginfo_level_tools = rust.debuginfo_level_tools; - debuginfo_level_tests = rust.debuginfo_level_tests; - config.rust_split_debuginfo = rust - .split_debuginfo - .as_deref() - .map(SplitDebuginfo::from_str) - .map(|v| v.expect("invalid value for rust.split_debuginfo")) - .unwrap_or(SplitDebuginfo::default_for_platform(&config.build.triple)); - optimize = rust.optimize; - ignore_git = rust.ignore_git; - config.rust_new_symbol_mangling = rust.new_symbol_mangling; - set(&mut config.rust_optimize_tests, rust.optimize_tests); - set(&mut config.codegen_tests, rust.codegen_tests); - set(&mut config.rust_rpath, rust.rpath); - set(&mut config.jemalloc, rust.jemalloc); - set(&mut config.test_compare_mode, rust.test_compare_mode); - set(&mut config.backtrace, rust.backtrace); - set(&mut config.channel, rust.channel); - config.description = rust.description; - set(&mut config.rust_dist_src, rust.dist_src); - set(&mut config.verbose_tests, rust.verbose_tests); - // in the case "false" is set explicitly, do not overwrite the command line args - if let Some(true) = rust.incremental { - config.incremental = true; - } - set(&mut config.use_lld, rust.use_lld); - set(&mut config.lld_enabled, rust.lld); - set(&mut config.llvm_tools_enabled, rust.llvm_tools); - config.rustc_parallel = rust.parallel_compiler.unwrap_or(false); - config.rustc_default_linker = rust.default_linker; - config.musl_root = rust.musl_root.map(PathBuf::from); - config.save_toolstates = rust.save_toolstates.map(PathBuf::from); - set(&mut config.deny_warnings, flags.deny_warnings.or(rust.deny_warnings)); - set(&mut config.backtrace_on_ice, rust.backtrace_on_ice); - set(&mut config.rust_verify_llvm_ir, rust.verify_llvm_ir); - config.rust_thin_lto_import_instr_limit = rust.thin_lto_import_instr_limit; - set(&mut config.rust_remap_debuginfo, rust.remap_debuginfo); - set(&mut config.control_flow_guard, rust.control_flow_guard); - config.llvm_libunwind_default = rust - .llvm_libunwind - .map(|v| v.parse().expect("failed to parse rust.llvm-libunwind")); - - if let Some(ref backends) = rust.codegen_backends { - config.rust_codegen_backends = - backends.iter().map(|s| INTERNER.intern_str(s)).collect(); - } - - config.rust_codegen_units = rust.codegen_units.map(threads_from_config); - config.rust_codegen_units_std = rust.codegen_units_std.map(threads_from_config); - config.rust_profile_use = flags.rust_profile_use.or(rust.profile_use); - config.rust_profile_generate = flags.rust_profile_generate.or(rust.profile_generate); - config.download_rustc_commit = config.download_ci_rustc_commit(rust.download_rustc); - - config.rust_lto = rust - .lto - .as_deref() - .map(|value| RustcLto::from_str(value).unwrap()) - .unwrap_or_default(); } else { - config.rust_profile_use = flags.rust_profile_use; - config.rust_profile_generate = flags.rust_profile_generate; + config.llvm_from_ci = + config.channel == "dev" && crate::native::is_ci_llvm_available(&config, false); } if let Some(t) = toml.target { diff --git a/src/bootstrap/config/tests.rs b/src/bootstrap/config/tests.rs new file mode 100644 index 0000000000000..c30c9131745c8 --- /dev/null +++ b/src/bootstrap/config/tests.rs @@ -0,0 +1,24 @@ +use super::{Config, TomlConfig}; +use std::path::Path; + +fn toml(config: &str) -> impl '_ + Fn(&Path) -> TomlConfig { + |&_| toml::from_str(config).unwrap() +} + +fn parse(config: &str) -> Config { + Config::parse_inner(&["check".to_owned(), "--config=/does/not/exist".to_owned()], toml(config)) +} + +#[test] +fn download_ci_llvm() { + let parse_llvm = |s| parse(s).llvm_from_ci; + let if_available = parse_llvm("llvm.download-ci-llvm = \"if-available\""); + + assert!(parse_llvm("llvm.download-ci-llvm = true")); + assert!(!parse_llvm("llvm.download-ci-llvm = false")); + assert_eq!(parse_llvm(""), if_available); + assert_eq!(parse_llvm("rust.channel = \"dev\""), if_available); + assert!(!parse_llvm("rust.channel = \"stable\"")); +} + +// FIXME: add test for detecting `src` and `out` diff --git a/src/bootstrap/defaults/config.user.toml b/src/bootstrap/defaults/config.user.toml index 6647061d88fcb..48ae2fe448de2 100644 --- a/src/bootstrap/defaults/config.user.toml +++ b/src/bootstrap/defaults/config.user.toml @@ -7,3 +7,7 @@ test-stage = 2 doc-stage = 2 # When compiling from source, you usually want all tools. extended = true + +[llvm] +# Most users installing from source want to build all parts of the project from source, not just rustc itself. +download-ci-llvm = false diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index f4fa556b97450..fc43c093ff1cf 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -1619,10 +1619,10 @@ fn chmod(_path: &Path, _perms: u32) {} /// If the test is running and code is an error code, it will cause a panic. fn detail_exit(code: i32) -> ! { // if in test and code is an error code, panic with status code provided - if cfg!(test) && code != 0 { + if cfg!(test) { panic!("status code: {}", code); } else { - //otherwise,exit with provided status code + // otherwise,exit with provided status code std::process::exit(code); } } From 77005950f09d2f9ba54962bf5adc3f2bc3a7213f Mon Sep 17 00:00:00 2001 From: Arthur Carcano Date: Fri, 25 Nov 2022 17:47:00 +0100 Subject: [PATCH 05/19] Implement masking in FileType comparison on Unix Fixes: https://github.com/rust-lang/rust/issues/104900 --- library/std/src/sys/unix/fs.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 37a49f2d78acd..382204d689318 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -332,11 +332,16 @@ pub struct FileTimes { modified: Option, } -#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] +#[derive(Copy, Clone, Eq, Hash, Debug)] pub struct FileType { mode: mode_t, } +impl PartialEq for FileType { + fn eq(&self, other: &Self) -> bool { + self.masked() == other.masked() + } +} #[derive(Debug)] pub struct DirBuilder { mode: mode_t, @@ -550,6 +555,10 @@ impl FileType { pub fn is(&self, mode: mode_t) -> bool { self.mode & libc::S_IFMT == mode } + + fn masked(&self) -> mode_t { + self.mode & libc::S_IFMT + } } impl FromInner for FilePermissions { From 62590288621fe9c1fd9b379ed613538f7225f3eb Mon Sep 17 00:00:00 2001 From: Arthur Carcano Date: Wed, 30 Nov 2022 23:31:42 +0100 Subject: [PATCH 06/19] Add test for regression for FileType equality Cf: https://github.com/rust-lang/rust/issues/104900 --- library/std/src/fs/tests.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index b8959316de170..4748ac9d97ef8 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -1551,3 +1551,19 @@ fn hiberfil_sys() { fs::metadata(hiberfil).unwrap(); assert_eq!(true, hiberfil.exists()); } + +/// Test that two different ways of obtaining the FileType give the same result. +/// Cf. https://github.com/rust-lang/rust/issues/104900 +#[test] +fn test_eq_direntry_metadata() { + let tmpdir = tmpdir(); + let file_path = tmpdir.join("file"); + File::create(file_path).unwrap(); + for e in fs::read_dir(tmpdir.path()).unwrap() { + let e = e.unwrap(); + let p = e.path(); + let ft1 = e.file_type().unwrap(); + let ft2 = p.metadata().unwrap().file_type(); + assert_eq!(ft1, ft2); + } +} From 4198d2975dd24322515f8894a3ac06e342aad183 Mon Sep 17 00:00:00 2001 From: Arthur Carcano Date: Tue, 6 Dec 2022 10:35:34 +0100 Subject: [PATCH 07/19] Implement masking in FileType hashing on Unix Commit 77005950f09d2f9ba54962bf5adc3f2bc3a7213f implemented masking of FileType to fix an issue[^1] in the semantic of FileType comparison. This commit introduces masking to Hash to maintain the invariant that x == y => hash(x) == hash(y). [^1]: https://github.com/rust-lang/rust/issues/104900 --- library/std/src/sys/unix/fs.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 382204d689318..e464388069059 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -332,7 +332,7 @@ pub struct FileTimes { modified: Option, } -#[derive(Copy, Clone, Eq, Hash, Debug)] +#[derive(Copy, Clone, Eq, Debug)] pub struct FileType { mode: mode_t, } @@ -342,6 +342,13 @@ impl PartialEq for FileType { self.masked() == other.masked() } } + +impl core::hash::Hash for FileType { + fn hash(&self, state: &mut H) { + self.masked().hash(state); + } +} + #[derive(Debug)] pub struct DirBuilder { mode: mode_t, From b45b9489bb3fb918fbe267154f8dcf4fee61854d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 5 Nov 2022 18:25:41 +0000 Subject: [PATCH 08/19] Compute generator sizes with -Zprint_type_sizes --- compiler/rustc_session/src/code_stats.rs | 3 ++- compiler/rustc_ty_utils/src/layout.rs | 6 +++++ src/test/ui/print_type_sizes/async.rs | 19 ++++++++++++++ src/test/ui/print_type_sizes/async.stdout | 25 +++++++++++++++++++ src/test/ui/print_type_sizes/generator.rs | 20 +++++++++++++++ src/test/ui/print_type_sizes/generator.stdout | 2 ++ 6 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/print_type_sizes/async.rs create mode 100644 src/test/ui/print_type_sizes/async.stdout create mode 100644 src/test/ui/print_type_sizes/generator.rs create mode 100644 src/test/ui/print_type_sizes/generator.stdout diff --git a/compiler/rustc_session/src/code_stats.rs b/compiler/rustc_session/src/code_stats.rs index eede4d16ea378..7a6da1b7350d2 100644 --- a/compiler/rustc_session/src/code_stats.rs +++ b/compiler/rustc_session/src/code_stats.rs @@ -33,6 +33,7 @@ pub enum DataTypeKind { Union, Enum, Closure, + Generator, } #[derive(PartialEq, Eq, Hash, Debug)] @@ -113,7 +114,7 @@ impl CodeStats { let mut max_variant_size = discr_size; let struct_like = match kind { - DataTypeKind::Struct | DataTypeKind::Closure => true, + DataTypeKind::Struct | DataTypeKind::Closure | DataTypeKind::Generator => true, DataTypeKind::Enum | DataTypeKind::Union => false, }; for (i, variant_info) in variants.into_iter().enumerate() { diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index fbc055b5d238f..1c65bec6964c9 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -826,6 +826,12 @@ fn record_layout_for_printing_outlined<'tcx>( return; } + ty::Generator(..) => { + debug!("print-type-size t: `{:?}` record generator", layout.ty); + record(DataTypeKind::Generator, false, None, vec![]); + return; + } + _ => { debug!("print-type-size t: `{:?}` skip non-nominal", layout.ty); return; diff --git a/src/test/ui/print_type_sizes/async.rs b/src/test/ui/print_type_sizes/async.rs new file mode 100644 index 0000000000000..3491ad5afbc15 --- /dev/null +++ b/src/test/ui/print_type_sizes/async.rs @@ -0,0 +1,19 @@ +// compile-flags: -Z print-type-sizes +// edition:2021 +// build-pass +// ignore-pass + +#![feature(start)] + +async fn wait() {} + +async fn test(arg: [u8; 8192]) { + wait().await; + drop(arg); +} + +#[start] +fn start(_: isize, _: *const *const u8) -> isize { + let _ = test([0; 8192]); + 0 +} diff --git a/src/test/ui/print_type_sizes/async.stdout b/src/test/ui/print_type_sizes/async.stdout new file mode 100644 index 0000000000000..3ea0ff65f610d --- /dev/null +++ b/src/test/ui/print_type_sizes/async.stdout @@ -0,0 +1,25 @@ +print-type-size type: `[static generator@$DIR/async.rs:10:32: 13:2]`: 16386 bytes, alignment: 1 bytes +print-type-size end padding: 16386 bytes +print-type-size type: `std::future::from_generator::GenFuture<[static generator@$DIR/async.rs:10:32: 13:2]>`: 16386 bytes, alignment: 1 bytes +print-type-size field `.0`: 16386 bytes +print-type-size type: `std::mem::ManuallyDrop<[u8; 8192]>`: 8192 bytes, alignment: 1 bytes +print-type-size field `.value`: 8192 bytes +print-type-size type: `std::mem::MaybeUninit<[u8; 8192]>`: 8192 bytes, alignment: 1 bytes +print-type-size variant `MaybeUninit`: 8192 bytes +print-type-size field `.uninit`: 0 bytes +print-type-size field `.value`: 8192 bytes +print-type-size type: `[static generator@$DIR/async.rs:8:17: 8:19]`: 1 bytes, alignment: 1 bytes +print-type-size end padding: 1 bytes +print-type-size type: `std::future::from_generator::GenFuture<[static generator@$DIR/async.rs:8:17: 8:19]>`: 1 bytes, alignment: 1 bytes +print-type-size field `.0`: 1 bytes +print-type-size type: `std::mem::ManuallyDrop>`: 1 bytes, alignment: 1 bytes +print-type-size field `.value`: 1 bytes +print-type-size type: `std::mem::MaybeUninit>`: 1 bytes, alignment: 1 bytes +print-type-size variant `MaybeUninit`: 1 bytes +print-type-size field `.uninit`: 0 bytes +print-type-size field `.value`: 1 bytes +print-type-size type: `std::task::Poll<()>`: 1 bytes, alignment: 1 bytes +print-type-size discriminant: 1 bytes +print-type-size variant `Ready`: 0 bytes +print-type-size field `.0`: 0 bytes +print-type-size variant `Pending`: 0 bytes diff --git a/src/test/ui/print_type_sizes/generator.rs b/src/test/ui/print_type_sizes/generator.rs new file mode 100644 index 0000000000000..a46db6121046b --- /dev/null +++ b/src/test/ui/print_type_sizes/generator.rs @@ -0,0 +1,20 @@ +// compile-flags: -Z print-type-sizes +// build-pass +// ignore-pass + +#![feature(start, generators, generator_trait)] + +use std::ops::Generator; + +fn generator(array: [u8; C]) -> impl Generator { + move |()| { + yield (); + let _ = array; + } +} + +#[start] +fn start(_: isize, _: *const *const u8) -> isize { + let _ = generator([0; 8192]); + 0 +} diff --git a/src/test/ui/print_type_sizes/generator.stdout b/src/test/ui/print_type_sizes/generator.stdout new file mode 100644 index 0000000000000..8270991829767 --- /dev/null +++ b/src/test/ui/print_type_sizes/generator.stdout @@ -0,0 +1,2 @@ +print-type-size type: `[generator@$DIR/generator.rs:10:5: 10:14]`: 8193 bytes, alignment: 1 bytes +print-type-size end padding: 8193 bytes From b0dcadfc45bb04be3ba56d8bd62f1331a98949dc Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 7 Dec 2022 17:00:33 +0000 Subject: [PATCH 09/19] Move closure/generator type info methods to TyCtxt --- .../src/debuginfo/metadata.rs | 68 +---------------- .../src/debuginfo/metadata/enums/cpp_like.rs | 8 +- .../src/debuginfo/metadata/enums/native.rs | 7 +- compiler/rustc_middle/src/ty/util.rs | 76 +++++++++++++++++++ 4 files changed, 84 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index d87117dffdc60..a9e3dcf4cb39a 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -27,9 +27,7 @@ use rustc_codegen_ssa::traits::*; use rustc_fs_util::path_to_c_string; use rustc_hir::def::CtorKind; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; -use rustc_index::vec::{Idx, IndexVec}; use rustc_middle::bug; -use rustc_middle::mir::{self, GeneratorLayout}; use rustc_middle::ty::layout::{LayoutOf, TyAndLayout}; use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::{ @@ -1026,33 +1024,6 @@ fn build_struct_type_di_node<'ll, 'tcx>( // Tuples //=----------------------------------------------------------------------------- -/// Returns names of captured upvars for closures and generators. -/// -/// Here are some examples: -/// - `name__field1__field2` when the upvar is captured by value. -/// - `_ref__name__field` when the upvar is captured by reference. -/// -/// For generators this only contains upvars that are shared by all states. -fn closure_saved_names_of_captured_variables(tcx: TyCtxt<'_>, def_id: DefId) -> SmallVec { - let body = tcx.optimized_mir(def_id); - - body.var_debug_info - .iter() - .filter_map(|var| { - let is_ref = match var.value { - mir::VarDebugInfoContents::Place(place) if place.local == mir::Local::new(1) => { - // The projection is either `[.., Field, Deref]` or `[.., Field]`. It - // implies whether the variable is captured by value or by reference. - matches!(place.projection.last().unwrap(), mir::ProjectionElem::Deref) - } - _ => return None, - }; - let prefix = if is_ref { "_ref__" } else { "" }; - Some(prefix.to_owned() + var.name.as_str()) - }) - .collect() -} - /// Builds the DW_TAG_member debuginfo nodes for the upvars of a closure or generator. /// For a generator, this will handle upvars shared by all states. fn build_upvar_field_di_nodes<'ll, 'tcx>( @@ -1083,7 +1054,7 @@ fn build_upvar_field_di_nodes<'ll, 'tcx>( .all(|&t| t == cx.tcx.normalize_erasing_regions(ParamEnv::reveal_all(), t)) ); - let capture_names = closure_saved_names_of_captured_variables(cx.tcx, def_id); + let capture_names = cx.tcx.closure_saved_names_of_captured_variables(def_id); let layout = cx.layout_of(closure_or_generator_ty); up_var_tys @@ -1229,43 +1200,6 @@ fn build_union_type_di_node<'ll, 'tcx>( ) } -// FIXME(eddyb) maybe precompute this? Right now it's computed once -// per generator monomorphization, but it doesn't depend on substs. -fn generator_layout_and_saved_local_names<'tcx>( - tcx: TyCtxt<'tcx>, - def_id: DefId, -) -> (&'tcx GeneratorLayout<'tcx>, IndexVec>) { - let body = tcx.optimized_mir(def_id); - let generator_layout = body.generator_layout().unwrap(); - let mut generator_saved_local_names = IndexVec::from_elem(None, &generator_layout.field_tys); - - let state_arg = mir::Local::new(1); - for var in &body.var_debug_info { - let mir::VarDebugInfoContents::Place(place) = &var.value else { continue }; - if place.local != state_arg { - continue; - } - match place.projection[..] { - [ - // Deref of the `Pin<&mut Self>` state argument. - mir::ProjectionElem::Field(..), - mir::ProjectionElem::Deref, - // Field of a variant of the state. - mir::ProjectionElem::Downcast(_, variant), - mir::ProjectionElem::Field(field, _), - ] => { - let name = &mut generator_saved_local_names - [generator_layout.variant_fields[variant][field]]; - if name.is_none() { - name.replace(var.name); - } - } - _ => {} - } - } - (generator_layout, generator_saved_local_names) -} - /// Computes the type parameters for a type, if any, for the given metadata. fn build_generic_type_param_di_nodes<'ll, 'tcx>( cx: &CodegenCx<'ll, 'tcx>, diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs index 53e8a291d1e8a..69443b9b828e2 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs @@ -22,9 +22,9 @@ use crate::{ common::CodegenCx, debuginfo::{ metadata::{ - build_field_di_node, closure_saved_names_of_captured_variables, + build_field_di_node, enums::{tag_base_type, DiscrResult}, - file_metadata, generator_layout_and_saved_local_names, size_and_align_of, type_di_node, + file_metadata, size_and_align_of, type_di_node, type_map::{self, Stub, UniqueTypeId}, unknown_file_metadata, DINodeCreationResult, SmallVec, NO_GENERICS, NO_SCOPE_METADATA, UNKNOWN_LINE_NUMBER, @@ -677,9 +677,9 @@ fn build_union_fields_for_direct_tag_generator<'ll, 'tcx>( }; let (generator_layout, state_specific_upvar_names) = - generator_layout_and_saved_local_names(cx.tcx, generator_def_id); + cx.tcx.generator_layout_and_saved_local_names(generator_def_id); - let common_upvar_names = closure_saved_names_of_captured_variables(cx.tcx, generator_def_id); + let common_upvar_names = cx.tcx.closure_saved_names_of_captured_variables(generator_def_id); let variant_range = generator_substs.variant_range(generator_def_id, cx.tcx); let variant_count = (variant_range.start.as_u32()..variant_range.end.as_u32()).len(); diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs index becbccc434d9a..93419d27a6236 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs @@ -4,9 +4,8 @@ use crate::{ common::CodegenCx, debuginfo::{ metadata::{ - closure_saved_names_of_captured_variables, enums::tag_base_type, - file_metadata, generator_layout_and_saved_local_names, size_and_align_of, type_di_node, + file_metadata, size_and_align_of, type_di_node, type_map::{self, Stub, StubInfo, UniqueTypeId}, unknown_file_metadata, DINodeCreationResult, SmallVec, NO_GENERICS, UNKNOWN_LINE_NUMBER, @@ -157,7 +156,7 @@ pub(super) fn build_generator_di_node<'ll, 'tcx>( ), |cx, generator_type_di_node| { let (generator_layout, state_specific_upvar_names) = - generator_layout_and_saved_local_names(cx.tcx, generator_def_id); + cx.tcx.generator_layout_and_saved_local_names(generator_def_id); let Variants::Multiple { tag_encoding: TagEncoding::Direct, ref variants, .. } = generator_type_and_layout.variants else { bug!( @@ -167,7 +166,7 @@ pub(super) fn build_generator_di_node<'ll, 'tcx>( }; let common_upvar_names = - closure_saved_names_of_captured_variables(cx.tcx, generator_def_id); + cx.tcx.closure_saved_names_of_captured_variables(generator_def_id); // Build variant struct types let variant_struct_type_di_nodes: SmallVec<_> = variants diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 47c1ce8075674..9ea8dc6e69fdd 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -1,6 +1,7 @@ //! Miscellaneous type-system utilities that are too small to deserve their own modules. use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags; +use crate::mir; use crate::ty::layout::IntegerExt; use crate::ty::{ self, DefIdTree, FallibleTypeFolder, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, @@ -15,6 +16,7 @@ use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_index::bit_set::GrowableBitSet; +use rustc_index::vec::{Idx, IndexVec}; use rustc_macros::HashStable; use rustc_span::{sym, DUMMY_SP}; use rustc_target::abi::{Integer, IntegerType, Size, TargetDataLayout}; @@ -692,6 +694,80 @@ impl<'tcx> TyCtxt<'tcx> { pub fn bound_impl_subject(self, def_id: DefId) -> ty::EarlyBinder> { ty::EarlyBinder(self.impl_subject(def_id)) } + + /// Returns names of captured upvars for closures and generators. + /// + /// Here are some examples: + /// - `name__field1__field2` when the upvar is captured by value. + /// - `_ref__name__field` when the upvar is captured by reference. + /// + /// For generators this only contains upvars that are shared by all states. + pub fn closure_saved_names_of_captured_variables( + self, + def_id: DefId, + ) -> SmallVec<[String; 16]> { + let body = self.optimized_mir(def_id); + + body.var_debug_info + .iter() + .filter_map(|var| { + let is_ref = match var.value { + mir::VarDebugInfoContents::Place(place) + if place.local == mir::Local::new(1) => + { + // The projection is either `[.., Field, Deref]` or `[.., Field]`. It + // implies whether the variable is captured by value or by reference. + matches!(place.projection.last().unwrap(), mir::ProjectionElem::Deref) + } + _ => return None, + }; + let prefix = if is_ref { "_ref__" } else { "" }; + Some(prefix.to_owned() + var.name.as_str()) + }) + .collect() + } + + // FIXME(eddyb) maybe precompute this? Right now it's computed once + // per generator monomorphization, but it doesn't depend on substs. + pub fn generator_layout_and_saved_local_names( + self, + def_id: DefId, + ) -> ( + &'tcx ty::GeneratorLayout<'tcx>, + IndexVec>, + ) { + let tcx = self; + let body = tcx.optimized_mir(def_id); + let generator_layout = body.generator_layout().unwrap(); + let mut generator_saved_local_names = + IndexVec::from_elem(None, &generator_layout.field_tys); + + let state_arg = mir::Local::new(1); + for var in &body.var_debug_info { + let mir::VarDebugInfoContents::Place(place) = &var.value else { continue }; + if place.local != state_arg { + continue; + } + match place.projection[..] { + [ + // Deref of the `Pin<&mut Self>` state argument. + mir::ProjectionElem::Field(..), + mir::ProjectionElem::Deref, + // Field of a variant of the state. + mir::ProjectionElem::Downcast(_, variant), + mir::ProjectionElem::Field(field, _), + ] => { + let name = &mut generator_saved_local_names + [generator_layout.variant_fields[variant][field]]; + if name.is_none() { + name.replace(var.name); + } + } + _ => {} + } + } + (generator_layout, generator_saved_local_names) + } } struct OpaqueTypeExpander<'tcx> { From 57b722688d03fc8115987375eee1d076cc89bc68 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 7 Dec 2022 18:33:26 +0000 Subject: [PATCH 10/19] Properly print generator interior type sizes --- compiler/rustc_session/src/code_stats.rs | 6 +- compiler/rustc_ty_utils/src/layout.rs | 274 ++++++++++++------ src/test/ui/print_type_sizes/async.stdout | 29 +- src/test/ui/print_type_sizes/generator.stdout | 10 +- 4 files changed, 219 insertions(+), 100 deletions(-) diff --git a/compiler/rustc_session/src/code_stats.rs b/compiler/rustc_session/src/code_stats.rs index 7a6da1b7350d2..1085bce44758f 100644 --- a/compiler/rustc_session/src/code_stats.rs +++ b/compiler/rustc_session/src/code_stats.rs @@ -19,7 +19,7 @@ pub enum SizeKind { Min, } -#[derive(Clone, PartialEq, Eq, Hash, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] pub struct FieldInfo { pub name: Symbol, pub offset: u64, @@ -114,8 +114,8 @@ impl CodeStats { let mut max_variant_size = discr_size; let struct_like = match kind { - DataTypeKind::Struct | DataTypeKind::Closure | DataTypeKind::Generator => true, - DataTypeKind::Enum | DataTypeKind::Union => false, + DataTypeKind::Struct | DataTypeKind::Closure => true, + DataTypeKind::Enum | DataTypeKind::Union | DataTypeKind::Generator => false, }; for (i, variant_info) in variants.into_iter().enumerate() { let VariantInfo { ref name, kind: _, align: _, size, ref fields } = *variant_info; diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 1c65bec6964c9..8bbbf26f47069 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -814,10 +814,196 @@ fn record_layout_for_printing_outlined<'tcx>( ); }; - let adt_def = match *layout.ty.kind() { + match *layout.ty.kind() { ty::Adt(ref adt_def, _) => { debug!("print-type-size t: `{:?}` process adt", layout.ty); - adt_def + let adt_kind = adt_def.adt_kind(); + let adt_packed = adt_def.repr().pack.is_some(); + + let build_variant_info = + |n: Option, flds: &[Symbol], layout: TyAndLayout<'tcx>| { + let mut min_size = Size::ZERO; + let field_info: Vec<_> = flds + .iter() + .enumerate() + .map(|(i, &name)| { + let field_layout = layout.field(cx, i); + let offset = layout.fields.offset(i); + min_size = min_size.max(offset + field_layout.size); + FieldInfo { + name, + offset: offset.bytes(), + size: field_layout.size.bytes(), + align: field_layout.align.abi.bytes(), + } + }) + .collect(); + + VariantInfo { + name: n, + kind: if layout.is_unsized() { SizeKind::Min } else { SizeKind::Exact }, + align: layout.align.abi.bytes(), + size: if min_size.bytes() == 0 { + layout.size.bytes() + } else { + min_size.bytes() + }, + fields: field_info, + } + }; + + match layout.variants { + Variants::Single { index } => { + if !adt_def.variants().is_empty() && layout.fields != FieldsShape::Primitive { + debug!( + "print-type-size `{:#?}` variant {}", + layout, + adt_def.variant(index).name + ); + let variant_def = &adt_def.variant(index); + let fields: Vec<_> = variant_def.fields.iter().map(|f| f.name).collect(); + record( + adt_kind.into(), + adt_packed, + None, + vec![build_variant_info(Some(variant_def.name), &fields, layout)], + ); + } else { + // (This case arises for *empty* enums; so give it + // zero variants.) + record(adt_kind.into(), adt_packed, None, vec![]); + } + } + + Variants::Multiple { tag, ref tag_encoding, .. } => { + debug!( + "print-type-size `{:#?}` adt general variants def {}", + layout.ty, + adt_def.variants().len() + ); + let variant_infos: Vec<_> = adt_def + .variants() + .iter_enumerated() + .map(|(i, variant_def)| { + let fields: Vec<_> = + variant_def.fields.iter().map(|f| f.name).collect(); + build_variant_info( + Some(variant_def.name), + &fields, + layout.for_variant(cx, i), + ) + }) + .collect(); + record( + adt_kind.into(), + adt_packed, + match tag_encoding { + TagEncoding::Direct => Some(tag.size(cx)), + _ => None, + }, + variant_infos, + ); + } + } + } + + ty::Generator(def_id, substs, _) => { + debug!("print-type-size t: `{:?}` record generator", layout.ty); + // Generators always have a begin/poisoned/end state with additional suspend points + match layout.variants { + Variants::Multiple { tag, ref tag_encoding, .. } => { + let (generator, state_specific_names) = + cx.tcx.generator_layout_and_saved_local_names(def_id); + let upvar_names = cx.tcx.closure_saved_names_of_captured_variables(def_id); + + let mut upvars_size = Size::ZERO; + let upvar_fields: Vec<_> = substs + .as_generator() + .upvar_tys() + .zip(upvar_names) + .enumerate() + .map(|(field_idx, (_, name))| { + let field_layout = layout.field(cx, field_idx); + let offset = layout.fields.offset(field_idx); + upvars_size = upvars_size.max(offset + field_layout.size); + FieldInfo { + name: Symbol::intern(&name), + offset: offset.bytes(), + size: field_layout.size.bytes(), + align: field_layout.align.abi.bytes(), + } + }) + .collect(); + + let variant_infos: Vec<_> = generator + .variant_fields + .iter_enumerated() + .map(|(variant_idx, variant_def)| { + let variant_layout = layout.for_variant(cx, variant_idx); + let mut variant_size = Size::ZERO; + let fields = variant_def + .iter() + .enumerate() + .map(|(field_idx, local)| { + let field_layout = variant_layout.field(cx, field_idx); + let offset = variant_layout.fields.offset(field_idx); + // The struct is as large as the last field's end + variant_size = variant_size.max(offset + field_layout.size); + FieldInfo { + name: state_specific_names + .get(*local) + .copied() + .flatten() + .unwrap_or(Symbol::intern(&format!( + ".generator_field{}", + local.as_usize() + ))), + offset: offset.bytes(), + size: field_layout.size.bytes(), + align: field_layout.align.abi.bytes(), + } + }) + .chain(upvar_fields.iter().copied()) + .collect(); + + // If the variant has no state-specific fields, then it's the size of the upvars. + if variant_size == Size::ZERO { + variant_size = upvars_size; + } + // We need to add the discriminant size back into min_size, since it is subtracted + // later during printing. + variant_size += match tag_encoding { + TagEncoding::Direct => tag.size(cx), + _ => Size::ZERO, + }; + + VariantInfo { + name: Some(Symbol::intern(&ty::GeneratorSubsts::variant_name( + variant_idx, + ))), + kind: SizeKind::Exact, + size: variant_size.bytes(), + align: variant_layout.align.abi.bytes(), + fields, + } + }) + .collect(); + record( + DataTypeKind::Generator, + false, + match tag_encoding { + TagEncoding::Direct => Some(tag.size(cx)), + _ => None, + }, + variant_infos, + ); + } + _ => { + // This should never happen, but I would rather not panic. + record(DataTypeKind::Generator, false, None, vec![]); + return; + } + } } ty::Closure(..) => { @@ -826,93 +1012,9 @@ fn record_layout_for_printing_outlined<'tcx>( return; } - ty::Generator(..) => { - debug!("print-type-size t: `{:?}` record generator", layout.ty); - record(DataTypeKind::Generator, false, None, vec![]); - return; - } - _ => { debug!("print-type-size t: `{:?}` skip non-nominal", layout.ty); return; } }; - - let adt_kind = adt_def.adt_kind(); - let adt_packed = adt_def.repr().pack.is_some(); - - let build_variant_info = |n: Option, flds: &[Symbol], layout: TyAndLayout<'tcx>| { - let mut min_size = Size::ZERO; - let field_info: Vec<_> = flds - .iter() - .enumerate() - .map(|(i, &name)| { - let field_layout = layout.field(cx, i); - let offset = layout.fields.offset(i); - let field_end = offset + field_layout.size; - if min_size < field_end { - min_size = field_end; - } - FieldInfo { - name, - offset: offset.bytes(), - size: field_layout.size.bytes(), - align: field_layout.align.abi.bytes(), - } - }) - .collect(); - - VariantInfo { - name: n, - kind: if layout.is_unsized() { SizeKind::Min } else { SizeKind::Exact }, - align: layout.align.abi.bytes(), - size: if min_size.bytes() == 0 { layout.size.bytes() } else { min_size.bytes() }, - fields: field_info, - } - }; - - match layout.variants { - Variants::Single { index } => { - if !adt_def.variants().is_empty() && layout.fields != FieldsShape::Primitive { - debug!("print-type-size `{:#?}` variant {}", layout, adt_def.variant(index).name); - let variant_def = &adt_def.variant(index); - let fields: Vec<_> = variant_def.fields.iter().map(|f| f.name).collect(); - record( - adt_kind.into(), - adt_packed, - None, - vec![build_variant_info(Some(variant_def.name), &fields, layout)], - ); - } else { - // (This case arises for *empty* enums; so give it - // zero variants.) - record(adt_kind.into(), adt_packed, None, vec![]); - } - } - - Variants::Multiple { tag, ref tag_encoding, .. } => { - debug!( - "print-type-size `{:#?}` adt general variants def {}", - layout.ty, - adt_def.variants().len() - ); - let variant_infos: Vec<_> = adt_def - .variants() - .iter_enumerated() - .map(|(i, variant_def)| { - let fields: Vec<_> = variant_def.fields.iter().map(|f| f.name).collect(); - build_variant_info(Some(variant_def.name), &fields, layout.for_variant(cx, i)) - }) - .collect(); - record( - adt_kind.into(), - adt_packed, - match tag_encoding { - TagEncoding::Direct => Some(tag.size(cx)), - _ => None, - }, - variant_infos, - ); - } - } } diff --git a/src/test/ui/print_type_sizes/async.stdout b/src/test/ui/print_type_sizes/async.stdout index 3ea0ff65f610d..94ad09ef296d3 100644 --- a/src/test/ui/print_type_sizes/async.stdout +++ b/src/test/ui/print_type_sizes/async.stdout @@ -1,20 +1,29 @@ -print-type-size type: `[static generator@$DIR/async.rs:10:32: 13:2]`: 16386 bytes, alignment: 1 bytes -print-type-size end padding: 16386 bytes -print-type-size type: `std::future::from_generator::GenFuture<[static generator@$DIR/async.rs:10:32: 13:2]>`: 16386 bytes, alignment: 1 bytes -print-type-size field `.0`: 16386 bytes +print-type-size type: `[async fn body@$DIR/async.rs:10:32: 13:2]`: 16386 bytes, alignment: 1 bytes +print-type-size discriminant: 1 bytes +print-type-size variant `Suspend0`: 16385 bytes +print-type-size field `.arg`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size field `.arg`: 8192 bytes +print-type-size field `.__awaitee`: 1 bytes +print-type-size variant `Unresumed`: 8192 bytes +print-type-size field `.arg`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size variant `Returned`: 8192 bytes +print-type-size field `.arg`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size variant `Panicked`: 8192 bytes +print-type-size field `.arg`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes print-type-size type: `std::mem::ManuallyDrop<[u8; 8192]>`: 8192 bytes, alignment: 1 bytes print-type-size field `.value`: 8192 bytes print-type-size type: `std::mem::MaybeUninit<[u8; 8192]>`: 8192 bytes, alignment: 1 bytes print-type-size variant `MaybeUninit`: 8192 bytes print-type-size field `.uninit`: 0 bytes print-type-size field `.value`: 8192 bytes -print-type-size type: `[static generator@$DIR/async.rs:8:17: 8:19]`: 1 bytes, alignment: 1 bytes -print-type-size end padding: 1 bytes -print-type-size type: `std::future::from_generator::GenFuture<[static generator@$DIR/async.rs:8:17: 8:19]>`: 1 bytes, alignment: 1 bytes -print-type-size field `.0`: 1 bytes -print-type-size type: `std::mem::ManuallyDrop>`: 1 bytes, alignment: 1 bytes +print-type-size type: `[async fn body@$DIR/async.rs:8:17: 8:19]`: 1 bytes, alignment: 1 bytes +print-type-size discriminant: 1 bytes +print-type-size variant `Unresumed`: 0 bytes +print-type-size variant `Returned`: 0 bytes +print-type-size variant `Panicked`: 0 bytes +print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/async.rs:8:17: 8:19]>`: 1 bytes, alignment: 1 bytes print-type-size field `.value`: 1 bytes -print-type-size type: `std::mem::MaybeUninit>`: 1 bytes, alignment: 1 bytes +print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/async.rs:8:17: 8:19]>`: 1 bytes, alignment: 1 bytes print-type-size variant `MaybeUninit`: 1 bytes print-type-size field `.uninit`: 0 bytes print-type-size field `.value`: 1 bytes diff --git a/src/test/ui/print_type_sizes/generator.stdout b/src/test/ui/print_type_sizes/generator.stdout index 8270991829767..28d4a6e6cff40 100644 --- a/src/test/ui/print_type_sizes/generator.stdout +++ b/src/test/ui/print_type_sizes/generator.stdout @@ -1,2 +1,10 @@ print-type-size type: `[generator@$DIR/generator.rs:10:5: 10:14]`: 8193 bytes, alignment: 1 bytes -print-type-size end padding: 8193 bytes +print-type-size discriminant: 1 bytes +print-type-size variant `Unresumed`: 8192 bytes +print-type-size field `.array`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size variant `Returned`: 8192 bytes +print-type-size field `.array`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size variant `Panicked`: 8192 bytes +print-type-size field `.array`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes +print-type-size variant `Suspend0`: 8192 bytes +print-type-size field `.array`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes From 7d23e29f9fb61985aca0227acbabb62f95208c01 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 7 Dec 2022 20:30:42 +0000 Subject: [PATCH 11/19] Pull out logic into distinct functions --- compiler/rustc_ty_utils/src/layout.rs | 349 ++++++++++++-------------- 1 file changed, 165 insertions(+), 184 deletions(-) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 8bbbf26f47069..f4672a70072b2 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -1,3 +1,4 @@ +use hir::def_id::DefId; use rustc_hir as hir; use rustc_index::bit_set::BitSet; use rustc_index::vec::{Idx, IndexVec}; @@ -6,7 +7,7 @@ use rustc_middle::ty::layout::{ IntegerExt, LayoutCx, LayoutError, LayoutOf, TyAndLayout, MAX_SIMD_LANES, }; use rustc_middle::ty::{ - self, subst::SubstsRef, EarlyBinder, ReprOptions, Ty, TyCtxt, TypeVisitable, + self, subst::SubstsRef, AdtDef, EarlyBinder, ReprOptions, Ty, TyCtxt, TypeVisitable, }; use rustc_session::{DataTypeKind, FieldInfo, SizeKind, VariantInfo}; use rustc_span::symbol::Symbol; @@ -815,206 +816,186 @@ fn record_layout_for_printing_outlined<'tcx>( }; match *layout.ty.kind() { - ty::Adt(ref adt_def, _) => { + ty::Adt(adt_def, _) => { debug!("print-type-size t: `{:?}` process adt", layout.ty); let adt_kind = adt_def.adt_kind(); let adt_packed = adt_def.repr().pack.is_some(); - - let build_variant_info = - |n: Option, flds: &[Symbol], layout: TyAndLayout<'tcx>| { - let mut min_size = Size::ZERO; - let field_info: Vec<_> = flds - .iter() - .enumerate() - .map(|(i, &name)| { - let field_layout = layout.field(cx, i); - let offset = layout.fields.offset(i); - min_size = min_size.max(offset + field_layout.size); - FieldInfo { - name, - offset: offset.bytes(), - size: field_layout.size.bytes(), - align: field_layout.align.abi.bytes(), - } - }) - .collect(); - - VariantInfo { - name: n, - kind: if layout.is_unsized() { SizeKind::Min } else { SizeKind::Exact }, - align: layout.align.abi.bytes(), - size: if min_size.bytes() == 0 { - layout.size.bytes() - } else { - min_size.bytes() - }, - fields: field_info, - } - }; - - match layout.variants { - Variants::Single { index } => { - if !adt_def.variants().is_empty() && layout.fields != FieldsShape::Primitive { - debug!( - "print-type-size `{:#?}` variant {}", - layout, - adt_def.variant(index).name - ); - let variant_def = &adt_def.variant(index); - let fields: Vec<_> = variant_def.fields.iter().map(|f| f.name).collect(); - record( - adt_kind.into(), - adt_packed, - None, - vec![build_variant_info(Some(variant_def.name), &fields, layout)], - ); - } else { - // (This case arises for *empty* enums; so give it - // zero variants.) - record(adt_kind.into(), adt_packed, None, vec![]); - } - } - - Variants::Multiple { tag, ref tag_encoding, .. } => { - debug!( - "print-type-size `{:#?}` adt general variants def {}", - layout.ty, - adt_def.variants().len() - ); - let variant_infos: Vec<_> = adt_def - .variants() - .iter_enumerated() - .map(|(i, variant_def)| { - let fields: Vec<_> = - variant_def.fields.iter().map(|f| f.name).collect(); - build_variant_info( - Some(variant_def.name), - &fields, - layout.for_variant(cx, i), - ) - }) - .collect(); - record( - adt_kind.into(), - adt_packed, - match tag_encoding { - TagEncoding::Direct => Some(tag.size(cx)), - _ => None, - }, - variant_infos, - ); - } - } + let (variant_infos, opt_discr_size) = variant_info_for_adt(cx, layout, adt_def); + record(adt_kind.into(), adt_packed, opt_discr_size, variant_infos); } ty::Generator(def_id, substs, _) => { debug!("print-type-size t: `{:?}` record generator", layout.ty); // Generators always have a begin/poisoned/end state with additional suspend points - match layout.variants { - Variants::Multiple { tag, ref tag_encoding, .. } => { - let (generator, state_specific_names) = - cx.tcx.generator_layout_and_saved_local_names(def_id); - let upvar_names = cx.tcx.closure_saved_names_of_captured_variables(def_id); - - let mut upvars_size = Size::ZERO; - let upvar_fields: Vec<_> = substs - .as_generator() - .upvar_tys() - .zip(upvar_names) - .enumerate() - .map(|(field_idx, (_, name))| { - let field_layout = layout.field(cx, field_idx); - let offset = layout.fields.offset(field_idx); - upvars_size = upvars_size.max(offset + field_layout.size); - FieldInfo { - name: Symbol::intern(&name), - offset: offset.bytes(), - size: field_layout.size.bytes(), - align: field_layout.align.abi.bytes(), - } - }) - .collect(); - - let variant_infos: Vec<_> = generator - .variant_fields - .iter_enumerated() - .map(|(variant_idx, variant_def)| { - let variant_layout = layout.for_variant(cx, variant_idx); - let mut variant_size = Size::ZERO; - let fields = variant_def - .iter() - .enumerate() - .map(|(field_idx, local)| { - let field_layout = variant_layout.field(cx, field_idx); - let offset = variant_layout.fields.offset(field_idx); - // The struct is as large as the last field's end - variant_size = variant_size.max(offset + field_layout.size); - FieldInfo { - name: state_specific_names - .get(*local) - .copied() - .flatten() - .unwrap_or(Symbol::intern(&format!( - ".generator_field{}", - local.as_usize() - ))), - offset: offset.bytes(), - size: field_layout.size.bytes(), - align: field_layout.align.abi.bytes(), - } - }) - .chain(upvar_fields.iter().copied()) - .collect(); - - // If the variant has no state-specific fields, then it's the size of the upvars. - if variant_size == Size::ZERO { - variant_size = upvars_size; - } - // We need to add the discriminant size back into min_size, since it is subtracted - // later during printing. - variant_size += match tag_encoding { - TagEncoding::Direct => tag.size(cx), - _ => Size::ZERO, - }; - - VariantInfo { - name: Some(Symbol::intern(&ty::GeneratorSubsts::variant_name( - variant_idx, - ))), - kind: SizeKind::Exact, - size: variant_size.bytes(), - align: variant_layout.align.abi.bytes(), - fields, - } - }) - .collect(); - record( - DataTypeKind::Generator, - false, - match tag_encoding { - TagEncoding::Direct => Some(tag.size(cx)), - _ => None, - }, - variant_infos, - ); - } - _ => { - // This should never happen, but I would rather not panic. - record(DataTypeKind::Generator, false, None, vec![]); - return; - } - } + let (variant_infos, opt_discr_size) = + variant_info_for_generator(cx, layout, def_id, substs); + record(DataTypeKind::Generator, false, opt_discr_size, variant_infos); } ty::Closure(..) => { debug!("print-type-size t: `{:?}` record closure", layout.ty); record(DataTypeKind::Closure, false, None, vec![]); - return; } _ => { debug!("print-type-size t: `{:?}` skip non-nominal", layout.ty); - return; } }; } + +fn variant_info_for_adt<'tcx>( + cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, + layout: TyAndLayout<'tcx>, + adt_def: AdtDef<'tcx>, +) -> (Vec, Option) { + let build_variant_info = |n: Option, flds: &[Symbol], layout: TyAndLayout<'tcx>| { + let mut min_size = Size::ZERO; + let field_info: Vec<_> = flds + .iter() + .enumerate() + .map(|(i, &name)| { + let field_layout = layout.field(cx, i); + let offset = layout.fields.offset(i); + min_size = min_size.max(offset + field_layout.size); + FieldInfo { + name, + offset: offset.bytes(), + size: field_layout.size.bytes(), + align: field_layout.align.abi.bytes(), + } + }) + .collect(); + + VariantInfo { + name: n, + kind: if layout.is_unsized() { SizeKind::Min } else { SizeKind::Exact }, + align: layout.align.abi.bytes(), + size: if min_size.bytes() == 0 { layout.size.bytes() } else { min_size.bytes() }, + fields: field_info, + } + }; + + match layout.variants { + Variants::Single { index } => { + if !adt_def.variants().is_empty() && layout.fields != FieldsShape::Primitive { + debug!("print-type-size `{:#?}` variant {}", layout, adt_def.variant(index).name); + let variant_def = &adt_def.variant(index); + let fields: Vec<_> = variant_def.fields.iter().map(|f| f.name).collect(); + (vec![build_variant_info(Some(variant_def.name), &fields, layout)], None) + } else { + (vec![], None) + } + } + + Variants::Multiple { tag, ref tag_encoding, .. } => { + debug!( + "print-type-size `{:#?}` adt general variants def {}", + layout.ty, + adt_def.variants().len() + ); + let variant_infos: Vec<_> = adt_def + .variants() + .iter_enumerated() + .map(|(i, variant_def)| { + let fields: Vec<_> = variant_def.fields.iter().map(|f| f.name).collect(); + build_variant_info(Some(variant_def.name), &fields, layout.for_variant(cx, i)) + }) + .collect(); + + ( + variant_infos, + match tag_encoding { + TagEncoding::Direct => Some(tag.size(cx)), + _ => None, + }, + ) + } + } +} + +fn variant_info_for_generator<'tcx>( + cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, + layout: TyAndLayout<'tcx>, + def_id: DefId, + substs: ty::SubstsRef<'tcx>, +) -> (Vec, Option) { + let Variants::Multiple { tag, ref tag_encoding, .. } = layout.variants else { + return (vec![], None); + }; + + let (generator, state_specific_names) = cx.tcx.generator_layout_and_saved_local_names(def_id); + let upvar_names = cx.tcx.closure_saved_names_of_captured_variables(def_id); + + let mut upvars_size = Size::ZERO; + let upvar_fields: Vec<_> = substs + .as_generator() + .upvar_tys() + .zip(upvar_names) + .enumerate() + .map(|(field_idx, (_, name))| { + let field_layout = layout.field(cx, field_idx); + let offset = layout.fields.offset(field_idx); + upvars_size = upvars_size.max(offset + field_layout.size); + FieldInfo { + name: Symbol::intern(&name), + offset: offset.bytes(), + size: field_layout.size.bytes(), + align: field_layout.align.abi.bytes(), + } + }) + .collect(); + + let variant_infos: Vec<_> = generator + .variant_fields + .iter_enumerated() + .map(|(variant_idx, variant_def)| { + let variant_layout = layout.for_variant(cx, variant_idx); + let mut variant_size = Size::ZERO; + let fields = variant_def + .iter() + .enumerate() + .map(|(field_idx, local)| { + let field_layout = variant_layout.field(cx, field_idx); + let offset = variant_layout.fields.offset(field_idx); + // The struct is as large as the last field's end + variant_size = variant_size.max(offset + field_layout.size); + FieldInfo { + name: state_specific_names.get(*local).copied().flatten().unwrap_or( + Symbol::intern(&format!(".generator_field{}", local.as_usize())), + ), + offset: offset.bytes(), + size: field_layout.size.bytes(), + align: field_layout.align.abi.bytes(), + } + }) + .chain(upvar_fields.iter().copied()) + .collect(); + + // If the variant has no state-specific fields, then it's the size of the upvars. + if variant_size == Size::ZERO { + variant_size = upvars_size; + } + // We need to add the discriminant size back into min_size, since it is subtracted + // later during printing. + variant_size += match tag_encoding { + TagEncoding::Direct => tag.size(cx), + _ => Size::ZERO, + }; + + VariantInfo { + name: Some(Symbol::intern(&ty::GeneratorSubsts::variant_name(variant_idx))), + kind: SizeKind::Exact, + size: variant_size.bytes(), + align: variant_layout.align.abi.bytes(), + fields, + } + }) + .collect(); + ( + variant_infos, + match tag_encoding { + TagEncoding::Direct => Some(tag.size(cx)), + _ => None, + }, + ) +} From ecf812777a260e35ec9cd0c7d9dbd17a3f5cf5f9 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Tue, 29 Nov 2022 23:17:08 +0100 Subject: [PATCH 12/19] Fix Async Generator ABI This change was missed when making async generators implement `Future` directly. It did not cause any problems in codegen so far, as `GeneratorState<(), Output>` happens to have the same ABI as `Poll`. --- compiler/rustc_ty_utils/src/abi.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index 73c7eb6992f07..d644cbccea11b 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -85,7 +85,7 @@ fn fn_sig_for_fn_abi<'tcx>( bound_vars, ) } - ty::Generator(_, substs, _) => { + ty::Generator(did, substs, _) => { let sig = substs.as_generator().poly_sig(); let bound_vars = tcx.mk_bound_variable_kinds( @@ -104,10 +104,22 @@ fn fn_sig_for_fn_abi<'tcx>( let env_ty = tcx.mk_adt(pin_adt_ref, pin_substs); let sig = sig.skip_binder(); - let state_did = tcx.require_lang_item(LangItem::GeneratorState, None); - let state_adt_ref = tcx.adt_def(state_did); - let state_substs = tcx.intern_substs(&[sig.yield_ty.into(), sig.return_ty.into()]); - let ret_ty = tcx.mk_adt(state_adt_ref, state_substs); + // The `FnSig` and the `ret_ty` here is for a generators main + // `Generator::resume(...) -> GeneratorState` function in case we + // have an ordinary generator, or the `Future::poll(...) -> Poll` + // function in case this is a special generator backing an async construct. + let ret_ty = if tcx.generator_is_async(did) { + let state_did = tcx.require_lang_item(LangItem::Poll, None); + let state_adt_ref = tcx.adt_def(state_did); + let state_substs = tcx.intern_substs(&[sig.return_ty.into()]); + tcx.mk_adt(state_adt_ref, state_substs) + } else { + let state_did = tcx.require_lang_item(LangItem::GeneratorState, None); + let state_adt_ref = tcx.adt_def(state_did); + let state_substs = tcx.intern_substs(&[sig.yield_ty.into(), sig.return_ty.into()]); + tcx.mk_adt(state_adt_ref, state_substs) + }; + ty::Binder::bind_with_vars( tcx.mk_fn_sig( [env_ty, sig.resume_ty].iter(), From 65698ae9f30f5ad72224edd1884fb4ddd1279366 Mon Sep 17 00:00:00 2001 From: Ramon de C Valle Date: Mon, 21 Nov 2022 21:29:00 -0800 Subject: [PATCH 13/19] Add LLVM KCFI support to the Rust compiler This commit adds LLVM Kernel Control Flow Integrity (KCFI) support to the Rust compiler. It initially provides forward-edge control flow protection for operating systems kernels for Rust-compiled code only by aggregating function pointers in groups identified by their return and parameter types. (See llvm/llvm-project@cff5bef.) Forward-edge control flow protection for C or C++ and Rust -compiled code "mixed binaries" (i.e., for when C or C++ and Rust -compiled code share the same virtual address space) will be provided in later work as part of this project by identifying C char and integer type uses at the time types are encoded (see Type metadata in the design document in the tracking issue #89653). LLVM KCFI can be enabled with -Zsanitizer=kcfi. Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com> --- Cargo.lock | 12 +++++ compiler/rustc_codegen_gcc/src/type_.rs | 4 ++ compiler/rustc_codegen_llvm/src/allocator.rs | 14 ++++-- compiler/rustc_codegen_llvm/src/builder.rs | 50 ++++++++++++++++--- compiler/rustc_codegen_llvm/src/context.rs | 5 ++ compiler/rustc_codegen_llvm/src/declare.rs | 7 ++- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 8 ++- compiler/rustc_codegen_llvm/src/type_.rs | 15 ++++++ .../rustc_codegen_ssa/src/traits/type_.rs | 1 + compiler/rustc_feature/src/builtin_attrs.rs | 2 +- compiler/rustc_hir_analysis/src/collect.rs | 4 +- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 16 +++--- compiler/rustc_session/src/options.rs | 3 +- compiler/rustc_session/src/session.rs | 12 +++++ compiler/rustc_span/src/symbol.rs | 1 + compiler/rustc_symbol_mangling/Cargo.toml | 1 + compiler/rustc_symbol_mangling/src/typeid.rs | 24 +++++++++ .../src/spec/aarch64_unknown_none.rs | 5 +- compiler/rustc_target/src/spec/mod.rs | 6 ++- .../src/spec/x86_64_unknown_none.rs | 3 +- .../codegen/sanitizer-kcfi-add-kcfi-flag.rs | 11 ++++ ...mit-kcfi-operand-bundle-itanium-cxx-abi.rs | 47 +++++++++++++++++ .../ui/invalid/invalid-no-sanitize.stderr | 2 +- src/tools/compiletest/src/header.rs | 2 + src/tools/compiletest/src/util.rs | 2 + src/tools/tidy/src/deps.rs | 2 + 26 files changed, 231 insertions(+), 28 deletions(-) create mode 100644 src/test/codegen/sanitizer-kcfi-add-kcfi-flag.rs create mode 100644 src/test/codegen/sanitizer-kcfi-emit-kcfi-operand-bundle-itanium-cxx-abi.rs diff --git a/Cargo.lock b/Cargo.lock index fb6140e74fea4..51108df4406d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4269,6 +4269,7 @@ dependencies = [ "rustc_span", "rustc_target", "tracing", + "twox-hash", ] [[package]] @@ -5197,6 +5198,17 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "twox-hash" +version = "1.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97fee6b57c6a41524a810daee9286c02d7752c4253064d0b05472833a438f675" +dependencies = [ + "cfg-if 1.0.0", + "rand 0.8.5", + "static_assertions", +] + [[package]] name = "type-map" version = "0.4.0" diff --git a/compiler/rustc_codegen_gcc/src/type_.rs b/compiler/rustc_codegen_gcc/src/type_.rs index bdf7318ce48c9..89a415cdb36cd 100644 --- a/compiler/rustc_codegen_gcc/src/type_.rs +++ b/compiler/rustc_codegen_gcc/src/type_.rs @@ -300,4 +300,8 @@ impl<'gcc, 'tcx> TypeMembershipMethods<'tcx> for CodegenCx<'gcc, 'tcx> { // Unsupported. self.context.new_rvalue_from_int(self.int_type, 0) } + + fn set_kcfi_type_metadata(&self, _function: RValue<'gcc>, _kcfi_typeid: u32) { + // Unsupported. + } } diff --git a/compiler/rustc_codegen_llvm/src/allocator.rs b/compiler/rustc_codegen_llvm/src/allocator.rs index fed56cdd43821..668d929270530 100644 --- a/compiler/rustc_codegen_llvm/src/allocator.rs +++ b/compiler/rustc_codegen_llvm/src/allocator.rs @@ -88,7 +88,8 @@ pub(crate) unsafe fn codegen( callee, args.as_ptr(), args.len() as c_uint, - None, + [].as_ptr(), + 0 as c_uint, ); llvm::LLVMSetTailCall(ret, True); if output.is_some() { @@ -132,8 +133,15 @@ pub(crate) unsafe fn codegen( .enumerate() .map(|(i, _)| llvm::LLVMGetParam(llfn, i as c_uint)) .collect::>(); - let ret = - llvm::LLVMRustBuildCall(llbuilder, ty, callee, args.as_ptr(), args.len() as c_uint, None); + let ret = llvm::LLVMRustBuildCall( + llbuilder, + ty, + callee, + args.as_ptr(), + args.len() as c_uint, + [].as_ptr(), + 0 as c_uint, + ); llvm::LLVMSetTailCall(ret, True); llvm::LLVMBuildRetVoid(llbuilder); llvm::LLVMDisposeBuilder(llbuilder); diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 77dd15ef4d807..83bffb20e0ce0 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -20,6 +20,7 @@ use rustc_middle::ty::layout::{ }; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::Span; +use rustc_symbol_mangling::typeid::kcfi_typeid_for_fnabi; use rustc_target::abi::{self, call::FnAbi, Align, Size, WrappingRange}; use rustc_target::spec::{HasTargetSpec, Target}; use std::borrow::Cow; @@ -225,9 +226,25 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { debug!("invoke {:?} with args ({:?})", llfn, args); let args = self.check_call("invoke", llty, llfn, args); - let bundle = funclet.map(|funclet| funclet.bundle()); - let bundle = bundle.as_ref().map(|b| &*b.raw); + let funclet_bundle = funclet.map(|funclet| funclet.bundle()); + let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw); + let mut bundles = vec![funclet_bundle]; + + // Set KCFI operand bundle + let is_indirect_call = unsafe { llvm::LLVMIsAFunction(llfn).is_none() }; + let kcfi_bundle = + if self.tcx.sess.is_sanitizer_kcfi_enabled() && fn_abi.is_some() && is_indirect_call { + let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi.unwrap()); + Some(llvm::OperandBundleDef::new("kcfi", &[self.const_u32(kcfi_typeid)])) + } else { + None + }; + if kcfi_bundle.is_some() { + let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw); + bundles.push(kcfi_bundle); + } + bundles.retain(|bundle| bundle.is_some()); let invoke = unsafe { llvm::LLVMRustBuildInvoke( self.llbuilder, @@ -237,7 +254,8 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { args.len() as c_uint, then, catch, - bundle, + bundles.as_ptr(), + bundles.len() as c_uint, UNNAMED, ) }; @@ -1143,7 +1161,8 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { llfn, args.as_ptr() as *const &llvm::Value, args.len() as c_uint, - None, + [].as_ptr(), + 0 as c_uint, ); } } @@ -1159,9 +1178,25 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { debug!("call {:?} with args ({:?})", llfn, args); let args = self.check_call("call", llty, llfn, args); - let bundle = funclet.map(|funclet| funclet.bundle()); - let bundle = bundle.as_ref().map(|b| &*b.raw); + let funclet_bundle = funclet.map(|funclet| funclet.bundle()); + let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw); + let mut bundles = vec![funclet_bundle]; + + // Set KCFI operand bundle + let is_indirect_call = unsafe { llvm::LLVMIsAFunction(llfn).is_none() }; + let kcfi_bundle = + if self.tcx.sess.is_sanitizer_kcfi_enabled() && fn_abi.is_some() && is_indirect_call { + let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi.unwrap()); + Some(llvm::OperandBundleDef::new("kcfi", &[self.const_u32(kcfi_typeid)])) + } else { + None + }; + if kcfi_bundle.is_some() { + let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw); + bundles.push(kcfi_bundle); + } + bundles.retain(|bundle| bundle.is_some()); let call = unsafe { llvm::LLVMRustBuildCall( self.llbuilder, @@ -1169,7 +1204,8 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { llfn, args.as_ptr() as *const &llvm::Value, args.len() as c_uint, - bundle, + bundles.as_ptr(), + bundles.len() as c_uint, ) }; if let Some(fn_abi) = fn_abi { diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 4dcc7cd54477d..aa1735f38acfd 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -250,6 +250,11 @@ pub unsafe fn create_module<'ll>( ); } + if sess.is_sanitizer_kcfi_enabled() { + let kcfi = "kcfi\0".as_ptr().cast(); + llvm::LLVMRustAddModuleFlag(llmod, llvm::LLVMModFlagBehavior::Override, kcfi, 1); + } + // Control Flow Guard is currently only supported by the MSVC linker on Windows. if sess.target.is_like_msvc { match sess.opts.cg.control_flow_guard { diff --git a/compiler/rustc_codegen_llvm/src/declare.rs b/compiler/rustc_codegen_llvm/src/declare.rs index dc21a02cec44a..6a575095f7e45 100644 --- a/compiler/rustc_codegen_llvm/src/declare.rs +++ b/compiler/rustc_codegen_llvm/src/declare.rs @@ -20,7 +20,7 @@ use crate::type_::Type; use crate::value::Value; use rustc_codegen_ssa::traits::TypeMembershipMethods; use rustc_middle::ty::Ty; -use rustc_symbol_mangling::typeid::typeid_for_fnabi; +use rustc_symbol_mangling::typeid::{kcfi_typeid_for_fnabi, typeid_for_fnabi}; use smallvec::SmallVec; /// Declare a function. @@ -136,6 +136,11 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { self.set_type_metadata(llfn, typeid); } + if self.tcx.sess.is_sanitizer_kcfi_enabled() { + let kcfi_typeid = kcfi_typeid_for_fnabi(self.tcx, fn_abi); + self.set_kcfi_type_metadata(llfn, kcfi_typeid); + } + llfn } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index f451984973048..9bcc39cc7601d 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -427,6 +427,7 @@ pub enum MetadataType { MD_type = 19, MD_vcall_visibility = 28, MD_noundef = 29, + MD_kcfi_type = 36, } /// LLVMRustAsmDialect @@ -1060,6 +1061,7 @@ extern "C" { pub fn LLVMGlobalSetMetadata<'a>(Val: &'a Value, KindID: c_uint, Metadata: &'a Metadata); pub fn LLVMRustGlobalAddMetadata<'a>(Val: &'a Value, KindID: c_uint, Metadata: &'a Metadata); pub fn LLVMValueAsMetadata(Node: &Value) -> &Metadata; + pub fn LLVMIsAFunction(Val: &Value) -> Option<&Value>; // Operations on constants of any type pub fn LLVMConstNull(Ty: &Type) -> &Value; @@ -1270,7 +1272,8 @@ extern "C" { NumArgs: c_uint, Then: &'a BasicBlock, Catch: &'a BasicBlock, - Bundle: Option<&OperandBundleDef<'a>>, + OpBundles: *const Option<&OperandBundleDef<'a>>, + NumOpBundles: c_uint, Name: *const c_char, ) -> &'a Value; pub fn LLVMBuildLandingPad<'a>( @@ -1640,7 +1643,8 @@ extern "C" { Fn: &'a Value, Args: *const &'a Value, NumArgs: c_uint, - Bundle: Option<&OperandBundleDef<'a>>, + OpBundles: *const Option<&OperandBundleDef<'a>>, + NumOpBundles: c_uint, ) -> &'a Value; pub fn LLVMRustBuildMemCpy<'a>( B: &Builder<'a>, diff --git a/compiler/rustc_codegen_llvm/src/type_.rs b/compiler/rustc_codegen_llvm/src/type_.rs index 5eec7dc613028..a5fab3479e71e 100644 --- a/compiler/rustc_codegen_llvm/src/type_.rs +++ b/compiler/rustc_codegen_llvm/src/type_.rs @@ -316,4 +316,19 @@ impl<'ll, 'tcx> TypeMembershipMethods<'tcx> for CodegenCx<'ll, 'tcx> { ) } } + + fn set_kcfi_type_metadata(&self, function: &'ll Value, kcfi_typeid: u32) { + let kcfi_type_metadata = self.const_u32(kcfi_typeid); + unsafe { + llvm::LLVMGlobalSetMetadata( + function, + llvm::MD_kcfi_type as c_uint, + llvm::LLVMMDNodeInContext2( + self.llcx, + &llvm::LLVMValueAsMetadata(kcfi_type_metadata), + 1, + ), + ) + } + } } diff --git a/compiler/rustc_codegen_ssa/src/traits/type_.rs b/compiler/rustc_codegen_ssa/src/traits/type_.rs index 86481d5d758d6..109161ccc8368 100644 --- a/compiler/rustc_codegen_ssa/src/traits/type_.rs +++ b/compiler/rustc_codegen_ssa/src/traits/type_.rs @@ -122,6 +122,7 @@ pub trait LayoutTypeMethods<'tcx>: Backend<'tcx> { pub trait TypeMembershipMethods<'tcx>: Backend<'tcx> { fn set_type_metadata(&self, function: Self::Function, typeid: String); fn typeid_metadata(&self, typeid: String) -> Self::Value; + fn set_kcfi_type_metadata(&self, function: Self::Function, typeid: u32); } pub trait ArgAbiMethods<'tcx>: HasCodegen<'tcx> { diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 4b6e068db4312..6d8e78a0f185d 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -394,7 +394,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ungated!(instruction_set, Normal, template!(List: "set"), ErrorPreceding), gated!( no_sanitize, Normal, - template!(List: "address, memory, thread"), DuplicatesOk, + template!(List: "address, kcfi, memory, thread"), DuplicatesOk, experimental!(no_sanitize) ), gated!(no_coverage, Normal, template!(Word), WarnFollowing, experimental!(no_coverage)), diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index a738ee4a14887..7dce80de561c8 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -1859,6 +1859,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { codegen_fn_attrs.no_sanitize |= SanitizerSet::ADDRESS; } else if item.has_name(sym::cfi) { codegen_fn_attrs.no_sanitize |= SanitizerSet::CFI; + } else if item.has_name(sym::kcfi) { + codegen_fn_attrs.no_sanitize |= SanitizerSet::KCFI; } else if item.has_name(sym::memory) { codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMORY; } else if item.has_name(sym::memtag) { @@ -1872,7 +1874,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { } else { tcx.sess .struct_span_err(item.span(), "invalid argument for `no_sanitize`") - .note("expected one of: `address`, `cfi`, `hwaddress`, `memory`, `memtag`, `shadow-call-stack`, or `thread`") + .note("expected one of: `address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow-call-stack`, or `thread`") .emit(); } } diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 216c35d6da078..49d028009fa5c 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1460,13 +1460,13 @@ extern "C" void LLVMRustFreeOperandBundleDef(OperandBundleDef *Bundle) { extern "C" LLVMValueRef LLVMRustBuildCall(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn, LLVMValueRef *Args, unsigned NumArgs, - OperandBundleDef *Bundle) { + OperandBundleDef **OpBundles, + unsigned NumOpBundles) { Value *Callee = unwrap(Fn); FunctionType *FTy = unwrap(Ty); - unsigned Len = Bundle ? 1 : 0; - ArrayRef Bundles = makeArrayRef(Bundle, Len); return wrap(unwrap(B)->CreateCall( - FTy, Callee, makeArrayRef(unwrap(Args), NumArgs), Bundles)); + FTy, Callee, makeArrayRef(unwrap(Args), NumArgs), + makeArrayRef(*OpBundles, NumOpBundles))); } extern "C" LLVMValueRef LLVMRustGetInstrProfIncrementIntrinsic(LLVMModuleRef M) { @@ -1506,14 +1506,14 @@ extern "C" LLVMValueRef LLVMRustBuildInvoke(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn, LLVMValueRef *Args, unsigned NumArgs, LLVMBasicBlockRef Then, LLVMBasicBlockRef Catch, - OperandBundleDef *Bundle, const char *Name) { + OperandBundleDef **OpBundles, unsigned NumOpBundles, + const char *Name) { Value *Callee = unwrap(Fn); FunctionType *FTy = unwrap(Ty); - unsigned Len = Bundle ? 1 : 0; - ArrayRef Bundles = makeArrayRef(Bundle, Len); return wrap(unwrap(B)->CreateInvoke(FTy, Callee, unwrap(Then), unwrap(Catch), makeArrayRef(unwrap(Args), NumArgs), - Bundles, Name)); + makeArrayRef(*OpBundles, NumOpBundles), + Name)); } extern "C" void LLVMRustPositionBuilderAtStart(LLVMBuilderRef B, diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index f9ee202466f67..088e9be28d3ee 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -368,7 +368,7 @@ mod desc { pub const parse_opt_panic_strategy: &str = parse_panic_strategy; pub const parse_oom_strategy: &str = "either `panic` or `abort`"; pub const parse_relro_level: &str = "one of: `full`, `partial`, or `off`"; - pub const parse_sanitizers: &str = "comma separated list of sanitizers: `address`, `cfi`, `hwaddress`, `leak`, `memory`, `memtag`, `shadow-call-stack`, or `thread`"; + pub const parse_sanitizers: &str = "comma separated list of sanitizers: `address`, `cfi`, `hwaddress`, `kcfi`, `leak`, `memory`, `memtag`, `shadow-call-stack`, or `thread`"; pub const parse_sanitizer_memory_track_origins: &str = "0, 1, or 2"; pub const parse_cfguard: &str = "either a boolean (`yes`, `no`, `on`, `off`, etc), `checks`, or `nochecks`"; @@ -675,6 +675,7 @@ mod parse { *slot |= match s { "address" => SanitizerSet::ADDRESS, "cfi" => SanitizerSet::CFI, + "kcfi" => SanitizerSet::KCFI, "leak" => SanitizerSet::LEAK, "memory" => SanitizerSet::MEMORY, "memtag" => SanitizerSet::MEMTAG, diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index d602acec53e32..f13f42cdf752d 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -683,6 +683,10 @@ impl Session { self.opts.unstable_opts.sanitizer.contains(SanitizerSet::CFI) } + pub fn is_sanitizer_kcfi_enabled(&self) -> bool { + self.opts.unstable_opts.sanitizer.contains(SanitizerSet::KCFI) + } + /// Check whether this compile session and crate type use static crt. pub fn crt_static(&self, crate_type: Option) -> bool { if !self.target.crt_static_respected { @@ -1530,6 +1534,14 @@ fn validate_commandline_args_with_session_available(sess: &Session) { } } + // LLVM CFI and KCFI are mutually exclusive + if sess.is_sanitizer_cfi_enabled() && sess.is_sanitizer_kcfi_enabled() { + sess.emit_err(CannotMixAndMatchSanitizers { + first: "cfi".to_string(), + second: "kcfi".to_string(), + }); + } + if sess.opts.unstable_opts.stack_protector != StackProtector::None { if !sess.target.options.supports_stack_protector { sess.emit_warning(StackProtectorNotSupportedForTarget { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 29312a21b4d46..55eae88caf412 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -828,6 +828,7 @@ symbols! { item_like_imports, iter, iter_repeat, + kcfi, keyword, kind, kreg, diff --git a/compiler/rustc_symbol_mangling/Cargo.toml b/compiler/rustc_symbol_mangling/Cargo.toml index 2a29ad6a9e56b..4e447eab02e7c 100644 --- a/compiler/rustc_symbol_mangling/Cargo.toml +++ b/compiler/rustc_symbol_mangling/Cargo.toml @@ -10,6 +10,7 @@ bitflags = "1.2.1" tracing = "0.1" punycode = "0.4.0" rustc-demangle = "0.1.21" +twox-hash = "1.6.3" rustc_span = { path = "../rustc_span" } rustc_middle = { path = "../rustc_middle" } diff --git a/compiler/rustc_symbol_mangling/src/typeid.rs b/compiler/rustc_symbol_mangling/src/typeid.rs index 9228bea43f932..53983bed71892 100644 --- a/compiler/rustc_symbol_mangling/src/typeid.rs +++ b/compiler/rustc_symbol_mangling/src/typeid.rs @@ -3,6 +3,8 @@ use rustc_middle::ty::{FnSig, Ty, TyCtxt}; use rustc_target::abi::call::FnAbi; +use std::hash::Hasher; +use twox_hash::XxHash64; mod typeid_itanium_cxx_abi; use typeid_itanium_cxx_abi::TypeIdOptions; @@ -16,3 +18,25 @@ pub fn typeid_for_fnabi<'tcx>(tcx: TyCtxt<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) pub fn typeid_for_fnsig<'tcx>(tcx: TyCtxt<'tcx>, fn_sig: &FnSig<'tcx>) -> String { typeid_itanium_cxx_abi::typeid_for_fnsig(tcx, fn_sig, TypeIdOptions::NO_OPTIONS) } + +/// Returns an LLVM KCFI type metadata identifier for the specified FnAbi. +pub fn kcfi_typeid_for_fnabi<'tcx>(tcx: TyCtxt<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> u32 { + // An LLVM KCFI type metadata identifier is a 32-bit constant produced by taking the lower half + // of the xxHash64 of the type metadata identifier. (See llvm/llvm-project@cff5bef.) + let mut hash: XxHash64 = Default::default(); + hash.write( + typeid_itanium_cxx_abi::typeid_for_fnabi(tcx, fn_abi, TypeIdOptions::NO_OPTIONS).as_bytes(), + ); + hash.finish() as u32 +} + +/// Returns an LLVM KCFI type metadata identifier for the specified FnSig. +pub fn kcfi_typeid_for_fnsig<'tcx>(tcx: TyCtxt<'tcx>, fn_sig: &FnSig<'tcx>) -> u32 { + // An LLVM KCFI type metadata identifier is a 32-bit constant produced by taking the lower half + // of the xxHash64 of the type metadata identifier. (See llvm/llvm-project@cff5bef.) + let mut hash: XxHash64 = Default::default(); + hash.write( + typeid_itanium_cxx_abi::typeid_for_fnsig(tcx, fn_sig, TypeIdOptions::NO_OPTIONS).as_bytes(), + ); + hash.finish() as u32 +} diff --git a/compiler/rustc_target/src/spec/aarch64_unknown_none.rs b/compiler/rustc_target/src/spec/aarch64_unknown_none.rs index 4ae6d4120c995..aca52e1478eb8 100644 --- a/compiler/rustc_target/src/spec/aarch64_unknown_none.rs +++ b/compiler/rustc_target/src/spec/aarch64_unknown_none.rs @@ -6,13 +6,16 @@ // // For example, `-C target-cpu=cortex-a53`. -use super::{Cc, LinkerFlavor, Lld, PanicStrategy, RelocModel, Target, TargetOptions}; +use super::{ + Cc, LinkerFlavor, Lld, PanicStrategy, RelocModel, SanitizerSet, Target, TargetOptions, +}; pub fn target() -> Target { let opts = TargetOptions { linker_flavor: LinkerFlavor::Gnu(Cc::No, Lld::Yes), linker: Some("rust-lld".into()), features: "+strict-align,+neon,+fp-armv8".into(), + supported_sanitizers: SanitizerSet::KCFI, relocation_model: RelocModel::Static, disable_redzone: true, max_atomic_width: Some(128), diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 6d936d2cb9ff0..65ab7b953a17b 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -803,7 +803,7 @@ impl ToJson for StackProbeType { bitflags::bitflags! { #[derive(Default, Encodable, Decodable)] - pub struct SanitizerSet: u8 { + pub struct SanitizerSet: u16 { const ADDRESS = 1 << 0; const LEAK = 1 << 1; const MEMORY = 1 << 2; @@ -812,6 +812,7 @@ bitflags::bitflags! { const CFI = 1 << 5; const MEMTAG = 1 << 6; const SHADOWCALLSTACK = 1 << 7; + const KCFI = 1 << 8; } } @@ -823,6 +824,7 @@ impl SanitizerSet { Some(match self { SanitizerSet::ADDRESS => "address", SanitizerSet::CFI => "cfi", + SanitizerSet::KCFI => "kcfi", SanitizerSet::LEAK => "leak", SanitizerSet::MEMORY => "memory", SanitizerSet::MEMTAG => "memtag", @@ -858,6 +860,7 @@ impl IntoIterator for SanitizerSet { [ SanitizerSet::ADDRESS, SanitizerSet::CFI, + SanitizerSet::KCFI, SanitizerSet::LEAK, SanitizerSet::MEMORY, SanitizerSet::MEMTAG, @@ -2292,6 +2295,7 @@ impl Target { base.$key_name |= match s.as_str() { Some("address") => SanitizerSet::ADDRESS, Some("cfi") => SanitizerSet::CFI, + Some("kcfi") => SanitizerSet::KCFI, Some("leak") => SanitizerSet::LEAK, Some("memory") => SanitizerSet::MEMORY, Some("memtag") => SanitizerSet::MEMTAG, diff --git a/compiler/rustc_target/src/spec/x86_64_unknown_none.rs b/compiler/rustc_target/src/spec/x86_64_unknown_none.rs index e4d33c2b8c62d..32060c35c11b8 100644 --- a/compiler/rustc_target/src/spec/x86_64_unknown_none.rs +++ b/compiler/rustc_target/src/spec/x86_64_unknown_none.rs @@ -5,7 +5,7 @@ // features. use super::{Cc, CodeModel, LinkerFlavor, Lld, PanicStrategy}; -use super::{RelroLevel, StackProbeType, Target, TargetOptions}; +use super::{RelroLevel, SanitizerSet, StackProbeType, Target, TargetOptions}; pub fn target() -> Target { let opts = TargetOptions { @@ -20,6 +20,7 @@ pub fn target() -> Target { features: "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-3dnow,-3dnowa,-avx,-avx2,+soft-float" .into(), + supported_sanitizers: SanitizerSet::KCFI, disable_redzone: true, panic_strategy: PanicStrategy::Abort, code_model: Some(CodeModel::Kernel), diff --git a/src/test/codegen/sanitizer-kcfi-add-kcfi-flag.rs b/src/test/codegen/sanitizer-kcfi-add-kcfi-flag.rs new file mode 100644 index 0000000000000..c2eb852aec3c7 --- /dev/null +++ b/src/test/codegen/sanitizer-kcfi-add-kcfi-flag.rs @@ -0,0 +1,11 @@ +// Verifies that "kcfi" module flag is added. +// +// needs-sanitizer-kcfi +// compile-flags: -Ctarget-feature=-crt-static -Zsanitizer=kcfi + +#![crate_type="lib"] + +pub fn foo() { +} + +// CHECK: !{{[0-9]+}} = !{i32 4, !"kcfi", i32 1} diff --git a/src/test/codegen/sanitizer-kcfi-emit-kcfi-operand-bundle-itanium-cxx-abi.rs b/src/test/codegen/sanitizer-kcfi-emit-kcfi-operand-bundle-itanium-cxx-abi.rs new file mode 100644 index 0000000000000..0afd9727517ed --- /dev/null +++ b/src/test/codegen/sanitizer-kcfi-emit-kcfi-operand-bundle-itanium-cxx-abi.rs @@ -0,0 +1,47 @@ +// Verifies that KCFI type metadata for functions are emitted. +// +// revisions: aarch64 x86_64 +// [aarch64] compile-flags: --target aarch64-unknown-none +// [aarch64] needs-llvm-components: aarch64 +// [x86_64] compile-flags: --target x86_64-unknown-none +// [x86_64] needs-llvm-components: +// compile-flags: -Cno-prepopulate-passes -Zsanitizer=kcfi + +#![crate_type="lib"] +#![feature(no_core, lang_items)] +#![no_core] + +#[lang="sized"] +trait Sized { } +#[lang="copy"] +trait Copy { } + +impl Copy for i32 {} + +pub fn foo(f: fn(i32) -> i32, arg: i32) -> i32 { + // CHECK-LABEL: define{{.*}}foo + // FIXME(rcvalle): Change to !kcfi_type when Rust is updated to LLVM 16 + // CHECK-SAME: {{.*}}! ![[TYPE1:[0-9]+]] + // CHECK: call i32 %f(i32 %arg){{.*}}[ "kcfi"(i32 -1666898348) ] + f(arg) +} + +pub fn bar(f: fn(i32, i32) -> i32, arg1: i32, arg2: i32) -> i32 { + // CHECK-LABEL: define{{.*}}bar + // FIXME(rcvalle): Change to !kcfi_type when Rust is updated to LLVM 16 + // CHECK-SAME: {{.*}}! ![[TYPE2:[0-9]+]] + // CHECK: call i32 %f(i32 %arg1, i32 %arg2){{.*}}[ "kcfi"(i32 -1789026986) ] + f(arg1, arg2) +} + +pub fn baz(f: fn(i32, i32, i32) -> i32, arg1: i32, arg2: i32, arg3: i32) -> i32 { + // CHECK-LABEL: define{{.*}}baz + // FIXME(rcvalle): Change to !kcfi_type when Rust is updated to LLVM 16 + // CHECK-SAME: {{.*}}! ![[TYPE3:[0-9]+]] + // CHECK: call i32 %f(i32 %arg1, i32 %arg2, i32 %arg3){{.*}}[ "kcfi"(i32 1248878270) ] + f(arg1, arg2, arg3) +} + +// CHECK: ![[TYPE1]] = !{i32 653723426} +// CHECK: ![[TYPE2]] = !{i32 412174924} +// CHECK: ![[TYPE3]] = !{i32 -636668840} diff --git a/src/test/ui/invalid/invalid-no-sanitize.stderr b/src/test/ui/invalid/invalid-no-sanitize.stderr index d328cafa00b93..4600034952b39 100644 --- a/src/test/ui/invalid/invalid-no-sanitize.stderr +++ b/src/test/ui/invalid/invalid-no-sanitize.stderr @@ -4,7 +4,7 @@ error: invalid argument for `no_sanitize` LL | #[no_sanitize(brontosaurus)] | ^^^^^^^^^^^^ | - = note: expected one of: `address`, `cfi`, `hwaddress`, `memory`, `memtag`, `shadow-call-stack`, or `thread` + = note: expected one of: `address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow-call-stack`, or `thread` error: aborting due to previous error diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 0d9a629e179b8..eae9e80a397c4 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -906,6 +906,7 @@ pub fn make_test_description( let has_asm_support = config.has_asm_support(); let has_asan = util::ASAN_SUPPORTED_TARGETS.contains(&&*config.target); let has_cfi = util::CFI_SUPPORTED_TARGETS.contains(&&*config.target); + let has_kcfi = util::KCFI_SUPPORTED_TARGETS.contains(&&*config.target); let has_lsan = util::LSAN_SUPPORTED_TARGETS.contains(&&*config.target); let has_msan = util::MSAN_SUPPORTED_TARGETS.contains(&&*config.target); let has_tsan = util::TSAN_SUPPORTED_TARGETS.contains(&&*config.target); @@ -957,6 +958,7 @@ pub fn make_test_description( && config.parse_name_directive(ln, "needs-sanitizer-support"); ignore |= !has_asan && config.parse_name_directive(ln, "needs-sanitizer-address"); ignore |= !has_cfi && config.parse_name_directive(ln, "needs-sanitizer-cfi"); + ignore |= !has_kcfi && config.parse_name_directive(ln, "needs-sanitizer-kcfi"); ignore |= !has_lsan && config.parse_name_directive(ln, "needs-sanitizer-leak"); ignore |= !has_msan && config.parse_name_directive(ln, "needs-sanitizer-memory"); ignore |= !has_tsan && config.parse_name_directive(ln, "needs-sanitizer-thread"); diff --git a/src/tools/compiletest/src/util.rs b/src/tools/compiletest/src/util.rs index ec36f1e4fb72e..ccba313ee357b 100644 --- a/src/tools/compiletest/src/util.rs +++ b/src/tools/compiletest/src/util.rs @@ -42,6 +42,8 @@ pub const CFI_SUPPORTED_TARGETS: &[&str] = &[ "x86_64-unknown-netbsd", ]; +pub const KCFI_SUPPORTED_TARGETS: &[&str] = &["aarch64-linux-none", "x86_64-linux-none"]; + pub const LSAN_SUPPORTED_TARGETS: &[&str] = &[ // FIXME: currently broken, see #88132 // "aarch64-apple-darwin", diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 8155ec9dd27e2..36dec1d415f30 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -214,6 +214,7 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ "snap", "stable_deref_trait", "stacker", + "static_assertions", "syn", "synstructure", "tempfile", @@ -234,6 +235,7 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ "tracing-log", "tracing-subscriber", "tracing-tree", + "twox-hash", "type-map", "typenum", "unic-char-property", From e1741baeede88f0341c7fac9c9d0e452521a01b7 Mon Sep 17 00:00:00 2001 From: Ramon de C Valle Date: Wed, 30 Nov 2022 16:41:35 -0800 Subject: [PATCH 14/19] Add documentation for LLVM KCFI support This commit adds initial documentation for LLVM Kernel Control Flow Integrity (KCFI) support to the Rust compiler (see #105109 and #89653). Co-authored-by: Miguel Ojeda --- .../src/compiler-flags/sanitizer.md | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/doc/unstable-book/src/compiler-flags/sanitizer.md b/src/doc/unstable-book/src/compiler-flags/sanitizer.md index b33405f18e90b..9bbf9e28fffe2 100644 --- a/src/doc/unstable-book/src/compiler-flags/sanitizer.md +++ b/src/doc/unstable-book/src/compiler-flags/sanitizer.md @@ -14,6 +14,9 @@ This feature allows for use of one of following sanitizers: forward-edge control flow protection. * [HWAddressSanitizer](#hwaddresssanitizer) a memory error detector similar to AddressSanitizer, but based on partial hardware assistance. +* [KernelControlFlowIntegrity](#kernelcontrolflowintegrity) LLVM Kernel Control + Flow Integrity (KCFI) provides forward-edge control flow protection for + operating systems kernels. * [LeakSanitizer](#leaksanitizer) a run-time memory leak detector. * [MemorySanitizer](#memorysanitizer) a detector of uninitialized reads. * [MemTagSanitizer](#memtagsanitizer) fast memory error detector based on @@ -502,6 +505,32 @@ Registers where the failure occurred (pc 0xaaaae0ae4a98): SUMMARY: HWAddressSanitizer: tag-mismatch (/.../main+0x54a94) ``` +# KernelControlFlowIntegrity + +The LLVM Kernel Control Flow Integrity (CFI) support to the Rust compiler +initially provides forward-edge control flow protection for operating systems +kernels for Rust-compiled code only by aggregating function pointers in groups +identified by their return and parameter types. (See [LLVM commit cff5bef "KCFI +sanitizer"](https://github.com/llvm/llvm-project/commit/cff5bef948c91e4919de8a5fb9765e0edc13f3de).) + +Forward-edge control flow protection for C or C++ and Rust -compiled code "mixed +binaries" (i.e., for when C or C++ and Rust -compiled code share the same +virtual address space) will be provided in later work by defining and using +compatible type identifiers (see Type metadata in the design document in the +tracking issue [#89653](https://github.com/rust-lang/rust/issues/89653)). + +LLVM KCFI can be enabled with `-Zsanitizer=kcfi`. + +LLVM KCFI is supported on the following targets: + +* `aarch64-linux-android` +* `aarch64-unknown-linux-gnu` +* `x86_64-linux-android` +* `x86_64-unknown-linux-gnu` + +See the [Clang KernelControlFlowIntegrity documentation][clang-kcfi] for more +details. + # LeakSanitizer LeakSanitizer is run-time memory leak detector. @@ -693,6 +722,7 @@ Sanitizers produce symbolized stacktraces when llvm-symbolizer binary is in `PAT [clang-asan]: https://clang.llvm.org/docs/AddressSanitizer.html [clang-cfi]: https://clang.llvm.org/docs/ControlFlowIntegrity.html [clang-hwasan]: https://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html +[clang-kcfi]: https://clang.llvm.org/docs/ControlFlowIntegrity.html#fsanitize-kcfi [clang-lsan]: https://clang.llvm.org/docs/LeakSanitizer.html [clang-msan]: https://clang.llvm.org/docs/MemorySanitizer.html [clang-scs]: https://clang.llvm.org/docs/ShadowCallStack.html From 24cd863a3815f7ae7828c9f161a0b3f863218106 Mon Sep 17 00:00:00 2001 From: Arthur Carcano Date: Fri, 9 Dec 2022 15:04:36 +0100 Subject: [PATCH 15/19] Replace hand-made masking by call to masked() method in FileType --- library/std/src/sys/unix/fs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index e464388069059..fb8d06c66820c 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -560,7 +560,7 @@ impl FileType { } pub fn is(&self, mode: mode_t) -> bool { - self.mode & libc::S_IFMT == mode + self.masked() == mode } fn masked(&self) -> mode_t { From 84a46352ac541d93efaf142c163ebcf4c9a8b1fd Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 9 Dec 2022 18:32:06 +0000 Subject: [PATCH 16/19] Don't warn about unused parens when they are used by yeet expr --- compiler/rustc_lint/src/unused.rs | 5 ++- .../issue-54538-unused-parens-lint.fixed | 9 ++++- .../unused/issue-54538-unused-parens-lint.rs | 9 ++++- .../issue-54538-unused-parens-lint.stderr | 36 +++++++++---------- 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index a7836ea8e7a4b..13b3ef43a5920 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -617,7 +617,10 @@ trait UnusedDelimLint { lhs_needs_parens || (followed_by_block && match &inner.kind { - ExprKind::Ret(_) | ExprKind::Break(..) | ExprKind::Yield(..) => true, + ExprKind::Ret(_) + | ExprKind::Break(..) + | ExprKind::Yield(..) + | ExprKind::Yeet(..) => true, ExprKind::Range(_lhs, Some(rhs), _limits) => { matches!(rhs.kind, ExprKind::Block(..)) } diff --git a/src/test/ui/lint/unused/issue-54538-unused-parens-lint.fixed b/src/test/ui/lint/unused/issue-54538-unused-parens-lint.fixed index 0b3fe9371f775..71ebaea8ed2c1 100644 --- a/src/test/ui/lint/unused/issue-54538-unused-parens-lint.fixed +++ b/src/test/ui/lint/unused/issue-54538-unused-parens-lint.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![feature(box_patterns, stmt_expr_attributes)] +#![feature(box_patterns, stmt_expr_attributes, yeet_expr)] #![allow( dead_code, @@ -25,6 +25,13 @@ fn _no_lint_attr() { let _x = #[allow(dead_code)] (1 + 2); } +fn _no_lint_yeet() -> Result<(), ()> { + #[allow(unreachable_code)] + if (do yeet) {} + + Ok(()) +} + // Don't lint in these cases (#64106). fn or_patterns_no_lint() { match Box::new(0) { diff --git a/src/test/ui/lint/unused/issue-54538-unused-parens-lint.rs b/src/test/ui/lint/unused/issue-54538-unused-parens-lint.rs index 1e78ec5f7d95d..28b662dd0242c 100644 --- a/src/test/ui/lint/unused/issue-54538-unused-parens-lint.rs +++ b/src/test/ui/lint/unused/issue-54538-unused-parens-lint.rs @@ -1,6 +1,6 @@ // run-rustfix -#![feature(box_patterns, stmt_expr_attributes)] +#![feature(box_patterns, stmt_expr_attributes, yeet_expr)] #![allow( dead_code, @@ -25,6 +25,13 @@ fn _no_lint_attr() { let _x = #[allow(dead_code)] (1 + 2); } +fn _no_lint_yeet() -> Result<(), ()> { + #[allow(unreachable_code)] + if (do yeet) {} + + Ok(()) +} + // Don't lint in these cases (#64106). fn or_patterns_no_lint() { match Box::new(0) { diff --git a/src/test/ui/lint/unused/issue-54538-unused-parens-lint.stderr b/src/test/ui/lint/unused/issue-54538-unused-parens-lint.stderr index c73884663c8f5..a5e69e6d9385c 100644 --- a/src/test/ui/lint/unused/issue-54538-unused-parens-lint.stderr +++ b/src/test/ui/lint/unused/issue-54538-unused-parens-lint.stderr @@ -76,7 +76,7 @@ LL + let _ = |a: u8| 0; | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:49:12 + --> $DIR/issue-54538-unused-parens-lint.rs:56:12 | LL | if let (0 | 1) = 0 {} | ^ ^ @@ -88,7 +88,7 @@ LL + if let 0 | 1 = 0 {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:50:13 + --> $DIR/issue-54538-unused-parens-lint.rs:57:13 | LL | if let ((0 | 1),) = (0,) {} | ^ ^ @@ -100,7 +100,7 @@ LL + if let (0 | 1,) = (0,) {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:51:13 + --> $DIR/issue-54538-unused-parens-lint.rs:58:13 | LL | if let [(0 | 1)] = [0] {} | ^ ^ @@ -112,7 +112,7 @@ LL + if let [0 | 1] = [0] {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:52:16 + --> $DIR/issue-54538-unused-parens-lint.rs:59:16 | LL | if let 0 | (1 | 2) = 0 {} | ^ ^ @@ -124,7 +124,7 @@ LL + if let 0 | 1 | 2 = 0 {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:54:15 + --> $DIR/issue-54538-unused-parens-lint.rs:61:15 | LL | if let TS((0 | 1)) = TS(0) {} | ^ ^ @@ -136,7 +136,7 @@ LL + if let TS(0 | 1) = TS(0) {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:56:20 + --> $DIR/issue-54538-unused-parens-lint.rs:63:20 | LL | if let NS { f: (0 | 1) } = (NS { f: 0 }) {} | ^ ^ @@ -148,7 +148,7 @@ LL + if let NS { f: 0 | 1 } = (NS { f: 0 }) {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:66:9 + --> $DIR/issue-54538-unused-parens-lint.rs:73:9 | LL | (_) => {} | ^ ^ @@ -160,7 +160,7 @@ LL + _ => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:67:9 + --> $DIR/issue-54538-unused-parens-lint.rs:74:9 | LL | (y) => {} | ^ ^ @@ -172,7 +172,7 @@ LL + y => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:68:9 + --> $DIR/issue-54538-unused-parens-lint.rs:75:9 | LL | (ref r) => {} | ^ ^ @@ -184,7 +184,7 @@ LL + ref r => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:69:9 + --> $DIR/issue-54538-unused-parens-lint.rs:76:9 | LL | (e @ 1...2) => {} | ^ ^ @@ -196,7 +196,7 @@ LL + e @ 1...2 => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:75:9 + --> $DIR/issue-54538-unused-parens-lint.rs:82:9 | LL | (e @ &(1...2)) => {} | ^ ^ @@ -208,7 +208,7 @@ LL + e @ &(1...2) => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:76:10 + --> $DIR/issue-54538-unused-parens-lint.rs:83:10 | LL | &(_) => {} | ^ ^ @@ -220,7 +220,7 @@ LL + &_ => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:87:9 + --> $DIR/issue-54538-unused-parens-lint.rs:94:9 | LL | (_) => {} | ^ ^ @@ -232,7 +232,7 @@ LL + _ => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:88:9 + --> $DIR/issue-54538-unused-parens-lint.rs:95:9 | LL | (y) => {} | ^ ^ @@ -244,7 +244,7 @@ LL + y => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:89:9 + --> $DIR/issue-54538-unused-parens-lint.rs:96:9 | LL | (ref r) => {} | ^ ^ @@ -256,7 +256,7 @@ LL + ref r => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:90:9 + --> $DIR/issue-54538-unused-parens-lint.rs:97:9 | LL | (e @ 1..=2) => {} | ^ ^ @@ -268,7 +268,7 @@ LL + e @ 1..=2 => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:96:9 + --> $DIR/issue-54538-unused-parens-lint.rs:103:9 | LL | (e @ &(1..=2)) => {} | ^ ^ @@ -280,7 +280,7 @@ LL + e @ &(1..=2) => {} | error: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:97:10 + --> $DIR/issue-54538-unused-parens-lint.rs:104:10 | LL | &(_) => {} | ^ ^ From b9da55afb5c2773b6fb50141aa71305891583526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 9 Dec 2022 14:35:55 -0800 Subject: [PATCH 17/19] Introduce `Span::is_visible` --- compiler/rustc_span/src/lib.rs | 9 +++++++++ .../src/traits/error_reporting/suggestions.rs | 6 +++--- src/test/ui/span/issue-71363.stderr | 4 ---- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index cef4c6f79cefd..335bfc3302f27 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -491,6 +491,10 @@ impl SpanData { pub fn is_dummy(self) -> bool { self.lo.0 == 0 && self.hi.0 == 0 } + #[inline] + pub fn is_visible(self, sm: &SourceMap) -> bool { + !self.is_dummy() && sm.is_span_accessible(self.span()) + } /// Returns `true` if `self` fully encloses `other`. pub fn contains(self, other: Self) -> bool { self.lo <= other.lo && other.hi <= self.hi @@ -556,6 +560,11 @@ impl Span { self.data_untracked().is_dummy() } + #[inline] + pub fn is_visible(self, sm: &SourceMap) -> bool { + self.data_untracked().is_visible(sm) + } + /// Returns `true` if this span comes from any kind of macro, desugaring or inlining. #[inline] pub fn from_expansion(self) -> bool { diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 6ea54b625bbc0..443d57aaf3dce 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -2413,19 +2413,19 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { | ObligationCauseCode::ExprBindingObligation(item_def_id, span, ..) => { let item_name = tcx.def_path_str(item_def_id); let mut multispan = MultiSpan::from(span); + let sm = tcx.sess.source_map(); if let Some(ident) = tcx.opt_item_ident(item_def_id) { - let sm = tcx.sess.source_map(); let same_line = match (sm.lookup_line(ident.span.hi()), sm.lookup_line(span.lo())) { (Ok(l), Ok(r)) => l.line == r.line, _ => true, }; - if !ident.span.is_dummy() && !ident.span.overlaps(span) && !same_line { + if ident.span.is_visible(sm) && !ident.span.overlaps(span) && !same_line { multispan.push_span_label(ident.span, "required by a bound in this"); } } let descr = format!("required by a bound in `{}`", item_name); - if !span.is_dummy() { + if span.is_visible(sm) { let msg = format!("required by this bound in `{}`", item_name); multispan.push_span_label(span, msg); err.span_note(multispan, &descr); diff --git a/src/test/ui/span/issue-71363.stderr b/src/test/ui/span/issue-71363.stderr index 0370e46e6ceb7..6c7ea007ee03e 100644 --- a/src/test/ui/span/issue-71363.stderr +++ b/src/test/ui/span/issue-71363.stderr @@ -8,8 +8,6 @@ error[E0277]: `MyError` doesn't implement `std::fmt::Display` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead note: required by a bound in `std::error::Error` --> $SRC_DIR/core/src/error.rs:LL:COL - | - = note: required by this bound in `std::error::Error` error[E0277]: `MyError` doesn't implement `Debug` --> $DIR/issue-71363.rs:4:6 @@ -21,8 +19,6 @@ error[E0277]: `MyError` doesn't implement `Debug` = note: add `#[derive(Debug)]` to `MyError` or manually `impl Debug for MyError` note: required by a bound in `std::error::Error` --> $SRC_DIR/core/src/error.rs:LL:COL - | - = note: required by this bound in `std::error::Error` help: consider annotating `MyError` with `#[derive(Debug)]` | 3 | #[derive(Debug)] From ac90c9be6d1da5f5d902a355552915e89c82662d Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 10 Dec 2022 01:00:27 +0000 Subject: [PATCH 18/19] Update cargo 2 commits in f6e737b1e3386adb89333bf06a01f68a91ac5306..70898e522116f6c23971e2a554b2dc85fd4c84cd 2022-12-02 20:21:24 +0000 to 2022-12-05 19:43:44 +0000 - Rename `generate_units` -> `generate_root_units` (rust-lang/cargo#11458) - Implements cargo file locking using fcntl on Solaris. (rust-lang/cargo#11439) --- src/tools/cargo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/cargo b/src/tools/cargo index f6e737b1e3386..70898e522116f 160000 --- a/src/tools/cargo +++ b/src/tools/cargo @@ -1 +1 @@ -Subproject commit f6e737b1e3386adb89333bf06a01f68a91ac5306 +Subproject commit 70898e522116f6c23971e2a554b2dc85fd4c84cd From f069e7159f1c6532a4c011cc192ff45cc96e5cdc Mon Sep 17 00:00:00 2001 From: est31 Date: Sat, 10 Dec 2022 04:36:23 +0100 Subject: [PATCH 19/19] Correct wrong note for short circuiting operators They *are* representable by traits, even if the short-circuiting behaviour requires a different approach than the non-short-circuiting operators. For an example proposal, see the postponed RFC 2722. As it is not accurate, reword the note. --- library/core/src/ops/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/ops/mod.rs b/library/core/src/ops/mod.rs index a5e5b13b33674..eb2a92f4644d1 100644 --- a/library/core/src/ops/mod.rs +++ b/library/core/src/ops/mod.rs @@ -17,10 +17,10 @@ //! should have some resemblance to multiplication (and share expected //! properties like associativity). //! -//! Note that the `&&` and `||` operators short-circuit, i.e., they only -//! evaluate their second operand if it contributes to the result. Since this -//! behavior is not enforceable by traits, `&&` and `||` are not supported as -//! overloadable operators. +//! Note that the `&&` and `||` operators are currently not supported for +//! overloading. Due to their short circuiting nature, they require a different +//! design from traits for other operators like [`BitAnd`]. Designs for them are +//! under discussion. //! //! Many of the operators take their operands by value. In non-generic //! contexts involving built-in types, this is usually not a problem.