From 207ee73b7afa40a6e8adb683daef454a523bd143 Mon Sep 17 00:00:00 2001 From: joboet Date: Sun, 4 Aug 2024 18:34:04 +0200 Subject: [PATCH 01/19] std: refactor UNIX random data generation This PR makes a number of changes to the UNIX randomness implementation: * Use `io::Error` for centralized error handling * Move the file-fallback logic out of the `getrandom`-specific module * Stop redefining the syscalls on macOS and DragonFly, they have appeared in `libc` * Add a `OnceLock` to cache the random device file descriptor --- library/std/src/sys/pal/unix/rand.rs | 310 ++++++++++++--------------- 1 file changed, 137 insertions(+), 173 deletions(-) diff --git a/library/std/src/sys/pal/unix/rand.rs b/library/std/src/sys/pal/unix/rand.rs index 8a78ea8e7ccc7..9cb96e0888899 100644 --- a/library/std/src/sys/pal/unix/rand.rs +++ b/library/std/src/sys/pal/unix/rand.rs @@ -2,7 +2,9 @@ pub fn hashmap_random_keys() -> (u64, u64) { const KEY_LEN: usize = core::mem::size_of::(); let mut v = [0u8; KEY_LEN * 2]; - imp::fill_bytes(&mut v); + if let Err(err) = read(&mut v) { + panic!("failed to retrieve random hash map seed: {err}"); + } let key1 = v[0..KEY_LEN].try_into().unwrap(); let key2 = v[KEY_LEN..].try_into().unwrap(); @@ -10,27 +12,78 @@ pub fn hashmap_random_keys() -> (u64, u64) { (u64::from_ne_bytes(key1), u64::from_ne_bytes(key2)) } -#[cfg(all( - unix, - not(target_os = "openbsd"), - not(target_os = "netbsd"), - not(target_os = "fuchsia"), - not(target_os = "redox"), - not(target_os = "vxworks"), - not(target_os = "emscripten"), - not(target_os = "vita"), - not(target_vendor = "apple"), +cfg_if::cfg_if! { + if #[cfg(any( + target_vendor = "apple", + target_os = "openbsd", + target_os = "emscripten", + target_os = "vita", + all(target_os = "netbsd", not(netbsd10)), + target_os = "fuchsia", + target_os = "vxworks", + ))] { + // Some systems have a syscall that directly retrieves random data. + // If that is guaranteed to be available, use it. + use imp::syscall as read; + } else { + // Otherwise, try the syscall to see if it exists only on some systems + // and fall back to reading from the random device otherwise. + fn read(bytes: &mut [u8]) -> crate::io::Result<()> { + use crate::fs::File; + use crate::io::Read; + use crate::sync::OnceLock; + + #[cfg(any( + target_os = "linux", + target_os = "android", + target_os = "espidf", + target_os = "horizon", + target_os = "freebsd", + target_os = "dragonfly", + target_os = "solaris", + target_os = "illumos", + netbsd10, + ))] + if let Some(res) = imp::syscall(bytes) { + return res; + } + + const PATH: &'static str = if cfg!(target_os = "redox") { + "rand:" + } else { + "/dev/urandom" + }; + + static FILE: OnceLock = OnceLock::new(); + + FILE.get_or_try_init(|| File::open(PATH))?.read_exact(bytes) + } + } +} + +// All these systems a `getrandom` syscall. +// +// It is not guaranteed to be available, so return None to fallback to the file +// implementation. +#[cfg(any( + target_os = "linux", + target_os = "android", + target_os = "espidf", + target_os = "horizon", + target_os = "freebsd", + target_os = "dragonfly", + target_os = "solaris", + target_os = "illumos", + netbsd10, ))] mod imp { - use crate::fs::File; - use crate::io::Read; - #[cfg(any(target_os = "linux", target_os = "android"))] - use crate::sys::weak::syscall; + use crate::io::{Error, Result}; + use crate::sync::atomic::{AtomicBool, Ordering}; + use crate::sys::os::errno; #[cfg(any(target_os = "linux", target_os = "android"))] fn getrandom(buf: &mut [u8]) -> libc::ssize_t { - use crate::sync::atomic::{AtomicBool, Ordering}; - use crate::sys::os::errno; + use crate::sys::weak::syscall; // A weak symbol allows interposition, e.g. for perf measurements that want to // disable randomness for consistency. Otherwise, we'll try a raw syscall. @@ -59,6 +112,7 @@ mod imp { } #[cfg(any( + target_os = "dragonfly", target_os = "espidf", target_os = "horizon", target_os = "freebsd", @@ -70,51 +124,11 @@ mod imp { unsafe { libc::getrandom(buf.as_mut_ptr().cast(), buf.len(), 0) } } - #[cfg(target_os = "dragonfly")] - fn getrandom(buf: &mut [u8]) -> libc::ssize_t { - extern "C" { - fn getrandom( - buf: *mut libc::c_void, - buflen: libc::size_t, - flags: libc::c_uint, - ) -> libc::ssize_t; - } - unsafe { getrandom(buf.as_mut_ptr().cast(), buf.len(), 0) } - } - - #[cfg(not(any( - target_os = "linux", - target_os = "android", - target_os = "espidf", - target_os = "horizon", - target_os = "freebsd", - target_os = "dragonfly", - target_os = "solaris", - target_os = "illumos", - netbsd10 - )))] - fn getrandom_fill_bytes(_buf: &mut [u8]) -> bool { - false - } - - #[cfg(any( - target_os = "linux", - target_os = "android", - target_os = "espidf", - target_os = "horizon", - target_os = "freebsd", - target_os = "dragonfly", - target_os = "solaris", - target_os = "illumos", - netbsd10 - ))] - fn getrandom_fill_bytes(v: &mut [u8]) -> bool { - use crate::sync::atomic::{AtomicBool, Ordering}; - use crate::sys::os::errno; - + pub fn syscall(v: &mut [u8]) -> Option> { static GETRANDOM_UNAVAILABLE: AtomicBool = AtomicBool::new(false); + if GETRANDOM_UNAVAILABLE.load(Ordering::Relaxed) { - return false; + return None; } let mut read = 0; @@ -125,8 +139,7 @@ mod imp { if err == libc::EINTR { continue; } else if err == libc::ENOSYS || err == libc::EPERM { - // Fall back to reading /dev/urandom if `getrandom` is not - // supported on the current kernel. + // `getrandom` is not supported on the current system. // // Also fall back in case it is disabled by something like // seccomp or inside of docker. @@ -142,123 +155,83 @@ mod imp { // https://github.com/moby/moby/issues/42680 // GETRANDOM_UNAVAILABLE.store(true, Ordering::Relaxed); - return false; + return None; } else if err == libc::EAGAIN { - return false; + // getrandom has failed because it would have blocked as the + // non-blocking pool (urandom) has not been initialized in + // the kernel yet due to a lack of entropy. Fallback to + // reading from `/dev/urandom` which will return potentially + // insecure random data to avoid blocking applications which + // could depend on this call without ever knowing they do and + // don't have a work around. + return None; } else { - panic!("unexpected getrandom error: {err}"); + return Some(Err(Error::from_raw_os_error(err))); } } else { read += result as usize; } } - true - } - - pub fn fill_bytes(v: &mut [u8]) { - // getrandom_fill_bytes here can fail if getrandom() returns EAGAIN, - // meaning it would have blocked because the non-blocking pool (urandom) - // has not initialized in the kernel yet due to a lack of entropy. The - // fallback we do here is to avoid blocking applications which could - // depend on this call without ever knowing they do and don't have a - // work around. The PRNG of /dev/urandom will still be used but over a - // possibly predictable entropy pool. - if getrandom_fill_bytes(v) { - return; - } - // getrandom failed because it is permanently or temporarily (because - // of missing entropy) unavailable. Open /dev/urandom, read from it, - // and close it again. - let mut file = File::open("/dev/urandom").expect("failed to open /dev/urandom"); - file.read_exact(v).expect("failed to read /dev/urandom") + Some(Ok(())) } } -#[cfg(target_vendor = "apple")] +#[cfg(any( + target_os = "macos", // Supported since macOS 10.12+. + target_os = "openbsd", + target_os = "emscripten", + target_os = "vita", +))] mod imp { - use libc::{c_int, c_void, size_t}; - - use crate::io; - - #[inline(always)] - fn random_failure() -> ! { - panic!("unexpected random generation error: {}", io::Error::last_os_error()); - } - - #[cfg(target_os = "macos")] - fn getentropy_fill_bytes(v: &mut [u8]) { - extern "C" { - fn getentropy(bytes: *mut c_void, count: size_t) -> c_int; - } + use crate::io::{Error, Result}; + pub fn syscall(v: &mut [u8]) -> Result<()> { // getentropy(2) permits a maximum buffer size of 256 bytes for s in v.chunks_mut(256) { - let ret = unsafe { getentropy(s.as_mut_ptr().cast(), s.len()) }; + let ret = unsafe { libc::getentropy(s.as_mut_ptr().cast(), s.len()) }; if ret == -1 { - random_failure() + return Err(Error::last_os_error()); } } - } - #[cfg(not(target_os = "macos"))] - fn ccrandom_fill_bytes(v: &mut [u8]) { - extern "C" { - fn CCRandomGenerateBytes(bytes: *mut c_void, count: size_t) -> c_int; - } - - let ret = unsafe { CCRandomGenerateBytes(v.as_mut_ptr().cast(), v.len()) }; - if ret == -1 { - random_failure() - } - } - - pub fn fill_bytes(v: &mut [u8]) { - // All supported versions of macOS (10.12+) support getentropy. - // - // `getentropy` is measurably faster (via Divan) then the other alternatives so its preferred - // when usable. - #[cfg(target_os = "macos")] - getentropy_fill_bytes(v); - - // On Apple platforms, `CCRandomGenerateBytes` and `SecRandomCopyBytes` simply - // call into `CCRandomCopyBytes` with `kCCRandomDefault`. `CCRandomCopyBytes` - // manages a CSPRNG which is seeded from the kernel's CSPRNG and which runs on - // its own thread accessed via GCD. This seems needlessly heavyweight for our purposes - // so we only use it on non-Mac OSes where the better entrypoints are blocked. - // - // `CCRandomGenerateBytes` is used instead of `SecRandomCopyBytes` because the former is accessible - // via `libSystem` (libc) while the other needs to link to `Security.framework`. - // - // Note that while `getentropy` has a available attribute in the macOS headers, the lack - // of a header in the iOS (and others) SDK means that its can cause app store rejections. - // Just use `CCRandomGenerateBytes` instead. - #[cfg(not(target_os = "macos"))] - ccrandom_fill_bytes(v); + Ok(()) } } -#[cfg(any(target_os = "openbsd", target_os = "emscripten", target_os = "vita"))] +// On Apple platforms, `CCRandomGenerateBytes` and `SecRandomCopyBytes` simply +// call into `CCRandomCopyBytes` with `kCCRandomDefault`. `CCRandomCopyBytes` +// manages a CSPRNG which is seeded from the kernel's CSPRNG and which runs on +// its own thread accessed via GCD. This seems needlessly heavyweight for our purposes +// so we only use it when `getentropy` is blocked, which appears to be the case +// on all platforms except macOS (see #102643). +// +// `CCRandomGenerateBytes` is used instead of `SecRandomCopyBytes` because the former is accessible +// via `libSystem` (libc) while the other needs to link to `Security.framework`. +#[cfg(all(target_vendor = "apple", not(target_os = "macos")))] mod imp { - use crate::sys::os::errno; + use libc::size_t; - pub fn fill_bytes(v: &mut [u8]) { - // getentropy(2) permits a maximum buffer size of 256 bytes - for s in v.chunks_mut(256) { - let ret = unsafe { libc::getentropy(s.as_mut_ptr() as *mut libc::c_void, s.len()) }; - if ret == -1 { - panic!("unexpected getentropy error: {}", errno()); - } + use crate::ffi::{c_int, c_void}; + use crate::io::{Error, Result}; + + pub fn syscall(v: &mut [u8]) -> Result<()> { + extern "C" { + fn CCRandomGenerateBytes(bytes: *mut c_void, count: size_t) -> c_int; } + + let ret = unsafe { CCRandomGenerateBytes(v.as_mut_ptr().cast(), v.len()) }; + if ret != -1 { Ok(()) } else { Err(Error::last_os_error()) } } } // FIXME: once the 10.x release becomes the minimum, this can be dropped for simplification. #[cfg(all(target_os = "netbsd", not(netbsd10)))] mod imp { + use crate::io::{Error, Result}; use crate::ptr; - pub fn fill_bytes(v: &mut [u8]) { + pub fn syscall(v: &mut [u8]) -> Result<()> { let mib = [libc::CTL_KERN, libc::KERN_ARND]; // kern.arandom permits a maximum buffer size of 256 bytes for s in v.chunks_mut(256) { @@ -273,39 +246,30 @@ mod imp { 0, ) }; - if ret == -1 || s_len != s.len() { - panic!( - "kern.arandom sysctl failed! (returned {}, s.len() {}, oldlenp {})", - ret, - s.len(), - s_len - ); + if ret == -1 { + return Err(Error::last_os_error()); + } else if s_len != s.len() { + // FIXME(joboet): this can't actually happen, can it? + panic!("read less bytes than requested from kern.arandom"); } } + + Ok(()) } } #[cfg(target_os = "fuchsia")] mod imp { + use crate::io::Result; + #[link(name = "zircon")] extern "C" { fn zx_cprng_draw(buffer: *mut u8, len: usize); } - pub fn fill_bytes(v: &mut [u8]) { - unsafe { zx_cprng_draw(v.as_mut_ptr(), v.len()) } - } -} - -#[cfg(target_os = "redox")] -mod imp { - use crate::fs::File; - use crate::io::Read; - - pub fn fill_bytes(v: &mut [u8]) { - // Open rand:, read from it, and close it again. - let mut file = File::open("rand:").expect("failed to open rand:"); - file.read_exact(v).expect("failed to read rand:") + pub fn syscall(v: &mut [u8]) -> Result<()> { + unsafe { zx_cprng_draw(v.as_mut_ptr(), v.len()) }; + Ok(()) } } @@ -314,25 +278,25 @@ mod imp { use core::sync::atomic::AtomicBool; use core::sync::atomic::Ordering::Relaxed; - use crate::io; + use crate::io::{Error, Result}; - pub fn fill_bytes(v: &mut [u8]) { + pub fn syscall(v: &mut [u8]) -> Result<()> { static RNG_INIT: AtomicBool = AtomicBool::new(false); while !RNG_INIT.load(Relaxed) { let ret = unsafe { libc::randSecure() }; if ret < 0 { - panic!("couldn't generate random bytes: {}", io::Error::last_os_error()); + return Err(Error::last_os_error()); } else if ret > 0 { RNG_INIT.store(true, Relaxed); break; } + unsafe { libc::usleep(10) }; } + let ret = unsafe { libc::randABytes(v.as_mut_ptr() as *mut libc::c_uchar, v.len() as libc::c_int) }; - if ret < 0 { - panic!("couldn't generate random bytes: {}", io::Error::last_os_error()); - } + if ret >= 0 { Ok(()) } else { Err(Error::last_os_error()) } } } From acb024110f337e77c515ebcafffd6c4c09c29ee7 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Fri, 9 Aug 2024 01:32:13 +0000 Subject: [PATCH 02/19] Add windows-targets crate to std's sysroot --- library/Cargo.lock | 7 ++++++- library/std/Cargo.toml | 5 ++++- library/std/src/sys/pal/windows/alloc.rs | 2 +- library/std/src/sys/pal/windows/c.rs | 2 -- library/std/src/sys/pal/windows/c/windows_sys.rs | 1 - library/windows_targets/Cargo.toml | 10 ++++++++++ .../windows_targets.rs => windows_targets/src/lib.rs} | 4 ++++ src/tools/generate-windows-sys/src/main.rs | 1 - src/tools/tidy/src/pal.rs | 1 + 9 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 library/windows_targets/Cargo.toml rename library/{std/src/sys/pal/windows/c/windows_targets.rs => windows_targets/src/lib.rs} (95%) diff --git a/library/Cargo.lock b/library/Cargo.lock index c5182516f7d14..8430b30ae9b79 100644 --- a/library/Cargo.lock +++ b/library/Cargo.lock @@ -339,6 +339,7 @@ dependencies = [ "std_detect", "unwind", "wasi", + "windows-targets 0.0.0", ] [[package]] @@ -421,9 +422,13 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets", + "windows-targets 0.52.5", ] +[[package]] +name = "windows-targets" +version = "0.0.0" + [[package]] name = "windows-targets" version = "0.52.5" diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 06e818fb7c09d..cea74f651acd8 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -57,6 +57,9 @@ object = { version = "0.36.0", default-features = false, optional = true, featur 'archive', ] } +[target.'cfg(windows)'.dependencies.windows-targets] +path = "../windows_targets" + [dev-dependencies] rand = { version = "0.8.5", default-features = false, features = ["alloc"] } rand_xorshift = "0.3.0" @@ -116,7 +119,7 @@ std_detect_env_override = ["std_detect/std_detect_env_override"] # Enable using raw-dylib for Windows imports. # This will eventually be the default. -windows_raw_dylib = [] +windows_raw_dylib = ["windows-targets/windows_raw_dylib"] [package.metadata.fortanix-sgx] # Maximum possible number of threads when testing diff --git a/library/std/src/sys/pal/windows/alloc.rs b/library/std/src/sys/pal/windows/alloc.rs index 92b68b26032c6..2205885687dea 100644 --- a/library/std/src/sys/pal/windows/alloc.rs +++ b/library/std/src/sys/pal/windows/alloc.rs @@ -4,7 +4,7 @@ use crate::alloc::{GlobalAlloc, Layout, System}; use crate::ffi::c_void; use crate::ptr; use crate::sync::atomic::{AtomicPtr, Ordering}; -use crate::sys::c::{self, windows_targets}; +use crate::sys::c; use crate::sys::common::alloc::{realloc_fallback, MIN_ALIGN}; #[cfg(test)] diff --git a/library/std/src/sys/pal/windows/c.rs b/library/std/src/sys/pal/windows/c.rs index 08b75186aef90..2f5d75dc4bc23 100644 --- a/library/std/src/sys/pal/windows/c.rs +++ b/library/std/src/sys/pal/windows/c.rs @@ -8,8 +8,6 @@ use core::ffi::{c_uint, c_ulong, c_ushort, c_void, CStr}; use core::{mem, ptr}; -pub(super) mod windows_targets; - mod windows_sys; pub use windows_sys::*; diff --git a/library/std/src/sys/pal/windows/c/windows_sys.rs b/library/std/src/sys/pal/windows/c/windows_sys.rs index 9f22f54819509..529c96a0e1e6b 100644 --- a/library/std/src/sys/pal/windows/c/windows_sys.rs +++ b/library/std/src/sys/pal/windows/c/windows_sys.rs @@ -3317,4 +3317,3 @@ pub struct WSADATA { #[cfg(target_arch = "arm")] pub enum CONTEXT {} // ignore-tidy-filelength -use super::windows_targets; diff --git a/library/windows_targets/Cargo.toml b/library/windows_targets/Cargo.toml new file mode 100644 index 0000000000000..94d7c8210647c --- /dev/null +++ b/library/windows_targets/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "windows-targets" +description = "A drop-in replacement for the real windows-targets crate for use in std only." +version = "0.0.0" +edition = "2021" + +[features] +# Enable using raw-dylib for Windows imports. +# This will eventually be the default. +windows_raw_dylib = [] diff --git a/library/std/src/sys/pal/windows/c/windows_targets.rs b/library/windows_targets/src/lib.rs similarity index 95% rename from library/std/src/sys/pal/windows/c/windows_targets.rs rename to library/windows_targets/src/lib.rs index 252bceb70942b..1965b6cf4ce8f 100644 --- a/library/std/src/sys/pal/windows/c/windows_targets.rs +++ b/library/windows_targets/src/lib.rs @@ -2,6 +2,10 @@ //! //! This is a simple wrapper around an `extern` block with a `#[link]` attribute. //! It's very roughly equivalent to the windows-targets crate. +#![no_std] +#![no_core] +#![feature(decl_macro)] +#![feature(no_core)] #[cfg(feature = "windows_raw_dylib")] pub macro link { diff --git a/src/tools/generate-windows-sys/src/main.rs b/src/tools/generate-windows-sys/src/main.rs index fe1b1bd5ceb14..6dbf29d957f12 100644 --- a/src/tools/generate-windows-sys/src/main.rs +++ b/src/tools/generate-windows-sys/src/main.rs @@ -35,7 +35,6 @@ fn main() -> Result<(), Box> { let mut f = std::fs::File::options().append(true).open("windows_sys.rs")?; f.write_all(ARM32_SHIM.as_bytes())?; writeln!(&mut f, "// ignore-tidy-filelength")?; - writeln!(&mut f, "use super::windows_targets;")?; Ok(()) } diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index b7ddf47186d99..c650fd0eec6d8 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -36,6 +36,7 @@ use crate::walk::{filter_dirs, walk}; // Paths that may contain platform-specific code. const EXCEPTION_PATHS: &[&str] = &[ + "library/windows_targets", "library/panic_abort", "library/panic_unwind", "library/unwind", From a1b2b7ff78b9dbaeffe8341b3616a8c8c4fd4351 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Fri, 9 Aug 2024 11:04:25 +0000 Subject: [PATCH 03/19] Exclude windows-targets from the workspace --- library/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/library/Cargo.toml b/library/Cargo.toml index c4513b4c127d8..d8ece6b0ebd3e 100644 --- a/library/Cargo.toml +++ b/library/Cargo.toml @@ -8,6 +8,7 @@ members = [ exclude = [ # stdarch has its own Cargo workspace "stdarch", + "windows_targets" ] [profile.release.package.compiler_builtins] From 0e4136b9160fdea8cfb2887c994b9070e74bbd73 Mon Sep 17 00:00:00 2001 From: Kyle Huey Date: Fri, 9 Aug 2024 05:41:28 -0700 Subject: [PATCH 04/19] Rework MIR inlining debuginfo so function parameters show up in debuggers. Line numbers of multiply-inlined functions were fixed in #114643 by using a single DISubprogram. That, however, triggered assertions because parameters weren't deduplicated. The "solution" to that in #115417 was to insert a DILexicalScope below the DISubprogram and parent all of the parameters to that scope. That fixed the assertion, but debuggers (including gdb and lldb) don't recognize variables that are not parented to the subprogram itself as parameters, even if they are emitted with DW_TAG_formal_parameter. Consider the program: use std::env; fn square(n: i32) -> i32 { n * n } fn square_no_inline(n: i32) -> i32 { n * n } fn main() { let x = square(env::vars().count() as i32); let y = square_no_inline(env::vars().count() as i32); println!("{x} == {y}"); } When making a release build with debug=2 and rustc 1.82.0-nightly (8b3870784 2024-08-07) (gdb) r Starting program: /ephemeral/tmp/target/release/tmp [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Breakpoint 1, tmp::square () at src/main.rs:5 5 n * n (gdb) info args No arguments. (gdb) info locals n = 31 (gdb) c Continuing. Breakpoint 2, tmp::square_no_inline (n=31) at src/main.rs:10 10 n * n (gdb) info args n = 31 (gdb) info locals No locals. This issue is particularly annoying because it removes arguments from stack traces. The DWARF for the inlined function looks like this: < 2><0x00002132 GOFF=0x00002132> DW_TAG_subprogram DW_AT_linkage_name _ZN3tmp6square17hc507052ff3d2a488E DW_AT_name square DW_AT_decl_file 0x0000000f /ephemeral/tmp/src/main.rs DW_AT_decl_line 0x00000004 DW_AT_type 0x00001a56<.debug_info+0x00001a56> DW_AT_inline DW_INL_inlined < 3><0x00002142 GOFF=0x00002142> DW_TAG_lexical_block < 4><0x00002143 GOFF=0x00002143> DW_TAG_formal_parameter DW_AT_name n DW_AT_decl_file 0x0000000f /ephemeral/tmp/src/main.rs DW_AT_decl_line 0x00000004 DW_AT_type 0x00001a56<.debug_info+0x00001a56> < 4><0x0000214e GOFF=0x0000214e> DW_TAG_null < 3><0x0000214f GOFF=0x0000214f> DW_TAG_null That DW_TAG_lexical_block inhibits every debugger I've tested from recognizing 'n' as a parameter. This patch removes the additional lexical scope. Parameters can be easily deduplicated by a tuple of their scope and the argument index, at the trivial cost of taking a Hash + Eq bound on DIScope. --- .../src/debuginfo/create_scope_map.rs | 22 +++++++++---------- .../rustc_codegen_ssa/src/mir/debuginfo.rs | 15 ++++++++++++- compiler/rustc_codegen_ssa/src/mir/mod.rs | 2 +- .../rustc_codegen_ssa/src/traits/backend.rs | 3 ++- .../debuginfo-inline-callsite-location.rs | 9 ++++---- .../inline-function-args-debug-info.rs | 1 + 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs index efe616838bfe4..0a02c230cfcf8 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs @@ -88,7 +88,7 @@ fn make_mir_scope<'ll, 'tcx>( let loc = cx.lookup_debug_loc(scope_data.span.lo()); let file_metadata = file_metadata(cx, &loc.file); - let parent_dbg_scope = match scope_data.inlined { + let dbg_scope = match scope_data.inlined { Some((callee, _)) => { // FIXME(eddyb) this would be `self.monomorphize(&callee)` // if this is moved to `rustc_codegen_ssa::mir::debuginfo`. @@ -102,17 +102,15 @@ fn make_mir_scope<'ll, 'tcx>( cx.dbg_scope_fn(callee, callee_fn_abi, None) }) } - None => parent_scope.dbg_scope, - }; - - let dbg_scope = unsafe { - llvm::LLVMRustDIBuilderCreateLexicalBlock( - DIB(cx), - parent_dbg_scope, - file_metadata, - loc.line, - loc.col, - ) + None => unsafe { + llvm::LLVMRustDIBuilderCreateLexicalBlock( + DIB(cx), + parent_scope.dbg_scope, + file_metadata, + loc.line, + loc.col, + ) + }, }; let inlined_at = scope_data.inlined.map(|(_, callsite_span)| { diff --git a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs index 0e495973a01d9..75692540c0345 100644 --- a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs +++ b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs @@ -1,3 +1,4 @@ +use std::collections::hash_map::Entry; use std::ops::Range; use rustc_data_structures::fx::FxHashMap; @@ -447,6 +448,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } let mut per_local = IndexVec::from_elem(vec![], &self.mir.local_decls); + let mut params_seen: FxHashMap<_, Bx::DIVariable> = Default::default(); for var in &self.mir.var_debug_info { let dbg_scope_and_span = if full_debug_info { self.adjusted_span_and_dbg_scope(var.source_info) @@ -491,7 +493,18 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { VariableKind::LocalVariable }; - self.cx.create_dbg_var(var.name, var_ty, dbg_scope, var_kind, span) + if let VariableKind::ArgumentVariable(arg_index) = var_kind { + match params_seen.entry((dbg_scope, arg_index)) { + Entry::Occupied(o) => o.get().clone(), + Entry::Vacant(v) => v + .insert( + self.cx.create_dbg_var(var.name, var_ty, dbg_scope, var_kind, span), + ) + .clone(), + } + } else { + self.cx.create_dbg_var(var.name, var_ty, dbg_scope, var_kind, span) + } }); let fragment = if let Some(ref fragment) = var.composite { diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 4ce07269cd2c1..de94d87bcea7a 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -106,7 +106,7 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> { locals: locals::Locals<'tcx, Bx::Value>, /// All `VarDebugInfo` from the MIR body, partitioned by `Local`. - /// This is `None` if no var`#[non_exhaustive]`iable debuginfo/names are needed. + /// This is `None` if no variable debuginfo/names are needed. per_local_var_debug_info: Option>>>, diff --git a/compiler/rustc_codegen_ssa/src/traits/backend.rs b/compiler/rustc_codegen_ssa/src/traits/backend.rs index 81e96413a9f34..c5e2d55be833d 100644 --- a/compiler/rustc_codegen_ssa/src/traits/backend.rs +++ b/compiler/rustc_codegen_ssa/src/traits/backend.rs @@ -1,4 +1,5 @@ use std::any::Any; +use std::hash::Hash; use rustc_ast::expand::allocator::AllocatorKind; use rustc_data_structures::fx::FxIndexMap; @@ -30,7 +31,7 @@ pub trait BackendTypes { // FIXME(eddyb) find a common convention for all of the debuginfo-related // names (choose between `Dbg`, `Debug`, `DebugInfo`, `DI` etc.). - type DIScope: Copy; + type DIScope: Copy + Hash + PartialEq + Eq; type DILocation: Copy; type DIVariable: Copy; } diff --git a/tests/codegen/debuginfo-inline-callsite-location.rs b/tests/codegen/debuginfo-inline-callsite-location.rs index aee07b4eb8c68..56fba614a5cdc 100644 --- a/tests/codegen/debuginfo-inline-callsite-location.rs +++ b/tests/codegen/debuginfo-inline-callsite-location.rs @@ -9,13 +9,12 @@ // CHECK: tail call void @{{[A-Za-z0-9_]+4core6option13unwrap_failed}} // CHECK-SAME: !dbg ![[#second_dbg:]] -// CHECK-DAG: ![[#func_dbg:]] = distinct !DISubprogram(name: "unwrap" -// CHECK-DAG: ![[#first_scope:]] = distinct !DILexicalBlock(scope: ![[#func_dbg]], -// CHECK: ![[#second_scope:]] = distinct !DILexicalBlock(scope: ![[#func_dbg]], +// CHECK-DAG: ![[#func_scope:]] = distinct !DISubprogram(name: "unwrap" +// CHECK-DAG: ![[#]] = !DILocalVariable(name: "self", arg: 1, scope: ![[#func_scope]] // CHECK: ![[#first_dbg]] = !DILocation(line: [[#]] -// CHECK-SAME: scope: ![[#first_scope]], inlinedAt: ![[#]]) +// CHECK-SAME: scope: ![[#func_scope]], inlinedAt: ![[#]]) // CHECK: ![[#second_dbg]] = !DILocation(line: [[#]] -// CHECK-SAME: scope: ![[#second_scope]], inlinedAt: ![[#]]) +// CHECK-SAME: scope: ![[#func_scope]], inlinedAt: ![[#]]) #![crate_type = "lib"] diff --git a/tests/codegen/inline-function-args-debug-info.rs b/tests/codegen/inline-function-args-debug-info.rs index 7263374b22e11..53a179160dc38 100644 --- a/tests/codegen/inline-function-args-debug-info.rs +++ b/tests/codegen/inline-function-args-debug-info.rs @@ -15,6 +15,7 @@ pub fn outer_function(x: usize, y: usize) -> usize { fn inner_function(aaaa: usize, bbbb: usize) -> usize { // CHECK: !DILocalVariable(name: "aaaa", arg: 1 // CHECK-SAME: line: 15 + // CHECK-NOT: !DILexicalBlock( // CHECK: !DILocalVariable(name: "bbbb", arg: 2 // CHECK-SAME: line: 15 aaaa + bbbb From ed7bdbb17b9c03fe3530e5e3f21b7c6c7879dbca Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 9 Aug 2024 22:02:20 -0400 Subject: [PATCH 05/19] Store do_not_recommend-ness in impl header --- compiler/rustc_hir_analysis/src/collect.rs | 2 ++ compiler/rustc_middle/src/ty/context.rs | 6 ++++++ compiler/rustc_middle/src/ty/mod.rs | 1 + .../traits/fulfillment_errors.rs | 18 +++--------------- .../rustc_trait_selection/src/solve/fulfill.rs | 6 +----- 5 files changed, 13 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 47ff748547a90..07bf5d90b55fa 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -1699,6 +1699,8 @@ fn impl_trait_header(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option TyCtxt<'tcx> { pub fn impl_polarity(self, def_id: impl IntoQueryParam) -> ty::ImplPolarity { self.impl_trait_header(def_id).map_or(ty::ImplPolarity::Positive, |h| h.polarity) } + + /// Whether this is a trait implementation that has `#[diagnostic::do_not_recommend]` + pub fn do_not_recommend_impl(self, def_id: DefId) -> bool { + matches!(self.def_kind(def_id), DefKind::Impl { of_trait: true }) + && self.impl_trait_header(def_id).is_some_and(|header| header.do_not_recommend) + } } /// Parameter attributes that can only be determined by examining the body of a function instead diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 9736428e6f7c7..69b194045ad08 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -262,6 +262,7 @@ pub struct ImplTraitHeader<'tcx> { pub trait_ref: ty::EarlyBinder<'tcx, ty::TraitRef<'tcx>>, pub polarity: ImplPolarity, pub safety: hir::Safety, + pub do_not_recommend: bool, } #[derive(Copy, Clone, PartialEq, Eq, Debug, TypeFoldable, TypeVisitable)] diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 1cee82f04ea0e..7f7de4a963b68 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -687,10 +687,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { let mut applied_do_not_recommend = false; loop { if let ObligationCauseCode::ImplDerived(ref c) = base_cause { - if self.tcx.has_attrs_with_path( - c.impl_or_alias_def_id, - &[sym::diagnostic, sym::do_not_recommend], - ) { + if self.tcx.do_not_recommend_impl(c.impl_or_alias_def_id) { let code = (*c.derived.parent_code).clone(); obligation.cause.map_code(|_| code); obligation.predicate = c.derived.parent_trait_pred.upcast(self.tcx); @@ -1630,11 +1627,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { .tcx .all_impls(def_id) // ignore `do_not_recommend` items - .filter(|def_id| { - !self - .tcx - .has_attrs_with_path(*def_id, &[sym::diagnostic, sym::do_not_recommend]) - }) + .filter(|def_id| !self.tcx.do_not_recommend_impl(*def_id)) // Ignore automatically derived impls and `!Trait` impls. .filter_map(|def_id| self.tcx.impl_trait_header(def_id)) .filter_map(|header| { @@ -1904,12 +1897,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { let impl_candidates = impl_candidates .into_iter() .cloned() - .filter(|cand| { - !self.tcx.has_attrs_with_path( - cand.impl_def_id, - &[sym::diagnostic, sym::do_not_recommend], - ) - }) + .filter(|cand| !self.tcx.do_not_recommend_impl(cand.impl_def_id)) .collect::>(); let def_id = trait_ref.def_id(); diff --git a/compiler/rustc_trait_selection/src/solve/fulfill.rs b/compiler/rustc_trait_selection/src/solve/fulfill.rs index 49fa775a0a191..de8951ef72046 100644 --- a/compiler/rustc_trait_selection/src/solve/fulfill.rs +++ b/compiler/rustc_trait_selection/src/solve/fulfill.rs @@ -13,7 +13,6 @@ use rustc_middle::bug; use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::{self, TyCtxt}; use rustc_next_trait_solver::solve::{GenerateProofTree, SolverDelegateEvalExt as _}; -use rustc_span::symbol::sym; use super::delegate::SolverDelegate; use super::inspect::{self, ProofTreeInferCtxtExt, ProofTreeVisitor}; @@ -440,10 +439,7 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> { source: CandidateSource::Impl(impl_def_id), result: _, } = candidate.kind() - && goal - .infcx() - .tcx - .has_attrs_with_path(impl_def_id, &[sym::diagnostic, sym::do_not_recommend]) + && goal.infcx().tcx.do_not_recommend_impl(impl_def_id) { return ControlFlow::Break(self.obligation.clone()); } From 20a16bb3c54ea7523ebd0c933fe5674aea50942e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 9 Aug 2024 21:14:07 -0400 Subject: [PATCH 06/19] Add test Co-authored-by: Georg Semmler --- ...ot_apply_attribute_without_feature_flag.rs | 21 +++++++++++++++++++ ...pply_attribute_without_feature_flag.stderr | 21 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.rs create mode 100644 tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.stderr diff --git a/tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.rs b/tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.rs new file mode 100644 index 0000000000000..5548fa2f52e17 --- /dev/null +++ b/tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.rs @@ -0,0 +1,21 @@ +#![allow(unknown_or_malformed_diagnostic_attributes)] + +trait Foo {} + +#[diagnostic::do_not_recommend] +impl Foo for (A,) {} + +#[diagnostic::do_not_recommend] +impl Foo for (A, B) {} + +#[diagnostic::do_not_recommend] +impl Foo for (A, B, C) {} + +impl Foo for i32 {} + +fn check(a: impl Foo) {} + +fn main() { + check(()); + //~^ ERROR the trait bound `(): Foo` is not satisfied +} diff --git a/tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.stderr b/tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.stderr new file mode 100644 index 0000000000000..e56af28f3fb5d --- /dev/null +++ b/tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.stderr @@ -0,0 +1,21 @@ +error[E0277]: the trait bound `(): Foo` is not satisfied + --> $DIR/do_not_apply_attribute_without_feature_flag.rs:19:11 + | +LL | check(()); + | ----- ^^ the trait `Foo` is not implemented for `()` + | | + | required by a bound introduced by this call + | + = help: the following other types implement trait `Foo`: + (A, B) + (A, B, C) + (A,) +note: required by a bound in `check` + --> $DIR/do_not_apply_attribute_without_feature_flag.rs:16:18 + | +LL | fn check(a: impl Foo) {} + | ^^^ required by this bound in `check` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. From ce9de3b9732cf787b776fc8e919d04138316466b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 15:02:13 +0000 Subject: [PATCH 07/19] compiletest: implement `needs-lvm-zstd` directive --- src/tools/compiletest/src/command-list.rs | 1 + src/tools/compiletest/src/header.rs | 101 ++++++++++++++++++++++ src/tools/compiletest/src/header/needs.rs | 10 ++- 3 files changed, 111 insertions(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/command-list.rs b/src/tools/compiletest/src/command-list.rs index 50c909793f5e1..c95dd2fcf9f94 100644 --- a/src/tools/compiletest/src/command-list.rs +++ b/src/tools/compiletest/src/command-list.rs @@ -138,6 +138,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "needs-force-clang-based-tests", "needs-git-hash", "needs-llvm-components", + "needs-llvm-zstd", "needs-profiler-support", "needs-relocation-model-pic", "needs-run-enabled", diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 1fc24301c85e6..277a88584ba60 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -1203,6 +1203,107 @@ pub fn extract_llvm_version_from_binary(binary_path: &str) -> Option { None } +/// For tests using the `needs-llvm-zstd` directive: +/// - for local LLVM builds, try to find the static zstd library in the llvm-config system libs. +/// - for `download-ci-llvm`, see if `lld` was built with zstd support. +pub fn llvm_has_libzstd(config: &Config) -> bool { + // Strategy 1: works for local builds but not with `download-ci-llvm`. + // + // We check whether `llvm-config` returns the zstd library. Bootstrap's `llvm.libzstd` will only + // ask to statically link it when building LLVM, so we only check if the list of system libs + // contains a path to that static lib, and that it exists. + // + // See compiler/rustc_llvm/build.rs for more details and similar expectations. + fn is_zstd_in_config(llvm_bin_dir: &Path) -> Option<()> { + let llvm_config_path = llvm_bin_dir.join("llvm-config"); + let output = Command::new(llvm_config_path).arg("--system-libs").output().ok()?; + assert!(output.status.success(), "running llvm-config --system-libs failed"); + + let libs = String::from_utf8(output.stdout).ok()?; + for lib in libs.split_whitespace() { + if lib.ends_with("libzstd.a") && Path::new(lib).exists() { + return Some(()); + } + } + + None + } + + // Strategy 2: `download-ci-llvm`'s `llvm-config --system-libs` will not return any libs to + // use. + // + // The CI artifacts also don't contain the bootstrap config used to build them: otherwise we + // could have looked at the `llvm.libzstd` config. + // + // We infer whether `LLVM_ENABLE_ZSTD` was used to build LLVM as a byproduct of testing whether + // `lld` supports it. If not, an error will be emitted: "LLVM was not built with + // LLVM_ENABLE_ZSTD or did not find zstd at build time". + #[cfg(unix)] + fn is_lld_built_with_zstd(llvm_bin_dir: &Path) -> Option<()> { + let lld_path = llvm_bin_dir.join("lld"); + if lld_path.exists() { + // We can't call `lld` as-is, it expects to be invoked by a compiler driver using a + // different name. Prepare a temporary symlink to do that. + let lld_symlink_path = llvm_bin_dir.join("ld.lld"); + if !lld_symlink_path.exists() { + std::os::unix::fs::symlink(lld_path, &lld_symlink_path).ok()?; + } + + // Run `lld` with a zstd flag. We expect this command to always error here, we don't + // want to link actual files and don't pass any. + let output = Command::new(&lld_symlink_path) + .arg("--compress-debug-sections=zstd") + .output() + .ok()?; + assert!(!output.status.success()); + + // Look for a specific error caused by LLVM not being built with zstd support. We could + // also look for the "no input files" message, indicating the zstd flag was accepted. + let stderr = String::from_utf8(output.stderr).ok()?; + let zstd_available = !stderr.contains("LLVM was not built with LLVM_ENABLE_ZSTD"); + + // We don't particularly need to clean the link up (so the previous commands could fail + // in theory but won't in practice), but we can try. + std::fs::remove_file(lld_symlink_path).ok()?; + + if zstd_available { + return Some(()); + } + } + + None + } + + #[cfg(not(unix))] + fn is_lld_built_with_zstd(llvm_bin_dir: &Path) -> Option<()> { + None + } + + if let Some(llvm_bin_dir) = &config.llvm_bin_dir { + // Strategy 1: for local LLVM builds. + if is_zstd_in_config(llvm_bin_dir).is_some() { + return true; + } + + // Strategy 2: for LLVM artifacts built on CI via `download-ci-llvm`. + // + // It doesn't work for cases where the artifacts don't contain the linker, but it's + // best-effort: CI has `llvm.libzstd` and `lld` enabled on the x64 linux artifacts, so it + // will at least work there. + // + // If this can be improved and expanded to less common cases in the future, it should. + if config.target == "x86_64-unknown-linux-gnu" + && config.host == config.target + && is_lld_built_with_zstd(llvm_bin_dir).is_some() + { + return true; + } + } + + // Otherwise, all hope is lost. + false +} + /// Takes a directive of the form " [- ]", /// returns the numeric representation of and as /// tuple: ( as u32, as u32) diff --git a/src/tools/compiletest/src/header/needs.rs b/src/tools/compiletest/src/header/needs.rs index 5b2665f7d0ba7..3a24adaa2a3b7 100644 --- a/src/tools/compiletest/src/header/needs.rs +++ b/src/tools/compiletest/src/header/needs.rs @@ -1,5 +1,5 @@ use crate::common::{Config, Debugger, Sanitizer}; -use crate::header::IgnoreDecision; +use crate::header::{llvm_has_libzstd, IgnoreDecision}; pub(super) fn handle_needs( cache: &CachedNeedsConditions, @@ -149,6 +149,11 @@ pub(super) fn handle_needs( condition: cache.symlinks, ignore_reason: "ignored if symlinks are unavailable", }, + Need { + name: "needs-llvm-zstd", + condition: cache.llvm_zstd, + ignore_reason: "ignored if LLVM wasn't build with zstd for ELF section compression", + }, ]; let (name, comment) = match ln.split_once([':', ' ']) { @@ -215,6 +220,8 @@ pub(super) struct CachedNeedsConditions { rust_lld: bool, dlltool: bool, symlinks: bool, + /// Whether LLVM built with zstd, for the `needs-llvm-zstd` directive. + llvm_zstd: bool, } impl CachedNeedsConditions { @@ -258,6 +265,7 @@ impl CachedNeedsConditions { .join(if config.host.contains("windows") { "rust-lld.exe" } else { "rust-lld" }) .exists(), + llvm_zstd: llvm_has_libzstd(&config), dlltool: find_dlltool(&config), symlinks: has_symlinks(), } From 002878cd3b6b37b539ad753568ea905e3a2bd36c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 15:09:20 +0000 Subject: [PATCH 08/19] mark `rust-lld-compress-debug-sections` test as needing zstd also make it fail if there's a compression issue --- .../rust-lld-compress-debug-sections/rmake.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/tests/run-make/rust-lld-compress-debug-sections/rmake.rs b/tests/run-make/rust-lld-compress-debug-sections/rmake.rs index df9691ccbcf39..9889659046ea7 100644 --- a/tests/run-make/rust-lld-compress-debug-sections/rmake.rs +++ b/tests/run-make/rust-lld-compress-debug-sections/rmake.rs @@ -1,12 +1,13 @@ // Checks the `compress-debug-sections` option on rust-lld. //@ needs-rust-lld +//@ needs-llvm-zstd //@ only-linux //@ ignore-cross-compile // FIXME: This test isn't comprehensive and isn't covering all possible combinations. -use run_make_support::{assert_contains, cmd, llvm_readobj, run_in_tmpdir, rustc}; +use run_make_support::{llvm_readobj, run_in_tmpdir, rustc}; fn check_compression(compression: &str, to_find: &str) { run_in_tmpdir(|| { @@ -17,19 +18,8 @@ fn check_compression(compression: &str, to_find: &str) { .arg("-Cdebuginfo=full") .link_arg(&format!("-Wl,--compress-debug-sections={compression}")) .input("main.rs") - .run_unchecked(); - let stderr = out.stderr_utf8(); - if stderr.is_empty() { - llvm_readobj().arg("-t").arg("main").run().assert_stdout_contains(to_find); - } else { - assert_contains( - stderr, - format!( - "LLVM was not built with LLVM_ENABLE_{to_find} \ - or did not find {compression} at build time" - ), - ); - } + .run(); + llvm_readobj().arg("-t").arg("main").run().assert_stdout_contains(to_find); }); } From 435405aeee6914dfa3bf0fcac9cad79b77bb6cd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 15:52:58 +0000 Subject: [PATCH 09/19] make `compressed-debuginfo` test about zlib only zlib is seemingly always enabled, so we can test it unconditionally --- tests/run-make/compressed-debuginfo/rmake.rs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/tests/run-make/compressed-debuginfo/rmake.rs b/tests/run-make/compressed-debuginfo/rmake.rs index 3a656f28c2241..362b2e2e1e449 100644 --- a/tests/run-make/compressed-debuginfo/rmake.rs +++ b/tests/run-make/compressed-debuginfo/rmake.rs @@ -1,11 +1,9 @@ -// Checks the `debuginfo-compression` option. +// Checks the always enabled `debuginfo-compression` option: zlib. //@ only-linux //@ ignore-cross-compile -// FIXME: This test isn't comprehensive and isn't covering all possible combinations. - -use run_make_support::{assert_contains, cmd, llvm_readobj, run_in_tmpdir, rustc}; +use run_make_support::{llvm_readobj, run_in_tmpdir, rustc}; fn check_compression(compression: &str, to_find: &str) { run_in_tmpdir(|| { @@ -17,19 +15,10 @@ fn check_compression(compression: &str, to_find: &str) { .arg(&format!("-Zdebuginfo-compression={compression}")) .input("foo.rs") .run(); - let stderr = out.stderr_utf8(); - if stderr.is_empty() { - llvm_readobj().arg("-t").arg("foo.o").run().assert_stdout_contains(to_find); - } else { - assert_contains( - stderr, - format!("unknown debuginfo compression algorithm {compression}"), - ); - } + llvm_readobj().arg("-t").arg("foo.o").run().assert_stdout_contains(to_find); }); } fn main() { check_compression("zlib", "ZLIB"); - check_compression("zstd", "ZSTD"); } From 57b7c0a364e2b8f0f75db411c5952dd11da521a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 16:22:04 +0000 Subject: [PATCH 10/19] prepare test for expanding scope --- .../main.rs | 0 .../rmake.rs | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/run-make/{rust-lld-compress-debug-sections => compressed-debuginfo-zstd}/main.rs (100%) rename tests/run-make/{rust-lld-compress-debug-sections => compressed-debuginfo-zstd}/rmake.rs (100%) diff --git a/tests/run-make/rust-lld-compress-debug-sections/main.rs b/tests/run-make/compressed-debuginfo-zstd/main.rs similarity index 100% rename from tests/run-make/rust-lld-compress-debug-sections/main.rs rename to tests/run-make/compressed-debuginfo-zstd/main.rs diff --git a/tests/run-make/rust-lld-compress-debug-sections/rmake.rs b/tests/run-make/compressed-debuginfo-zstd/rmake.rs similarity index 100% rename from tests/run-make/rust-lld-compress-debug-sections/rmake.rs rename to tests/run-make/compressed-debuginfo-zstd/rmake.rs From f656c5c6a71b629422a0b1a6bdd5ff186b168bd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 16:59:00 +0000 Subject: [PATCH 11/19] expand zstd debuginfo compression test it now checks zlib and zstd, via rustc and rust-lld --- .../compressed-debuginfo-zstd/rmake.rs | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/tests/run-make/compressed-debuginfo-zstd/rmake.rs b/tests/run-make/compressed-debuginfo-zstd/rmake.rs index 9889659046ea7..8356373e949aa 100644 --- a/tests/run-make/compressed-debuginfo-zstd/rmake.rs +++ b/tests/run-make/compressed-debuginfo-zstd/rmake.rs @@ -1,24 +1,37 @@ -// Checks the `compress-debug-sections` option on rust-lld. +// Checks debuginfo compression both for the always-enabled zlib, and when the optional zstd is +// enabled: +// - via rustc's `debuginfo-compression`, +// - and via rust-lld's `compress-debug-sections` -//@ needs-rust-lld -//@ needs-llvm-zstd +//@ needs-llvm-zstd: we want LLVM/LLD to be built with zstd support +//@ needs-rust-lld: the system linker will most likely not support zstd //@ only-linux //@ ignore-cross-compile -// FIXME: This test isn't comprehensive and isn't covering all possible combinations. - -use run_make_support::{llvm_readobj, run_in_tmpdir, rustc}; +use run_make_support::{llvm_readobj, run_in_tmpdir, Rustc}; fn check_compression(compression: &str, to_find: &str) { + // check compressed debug sections via rustc flag + prepare_and_check(to_find, |rustc| { + rustc.arg(&format!("-Zdebuginfo-compression={compression}")) + }); + + // check compressed debug sections via rust-lld flag + prepare_and_check(to_find, |rustc| { + rustc.link_arg(&format!("-Wl,--compress-debug-sections={compression}")) + }); +} + +fn prepare_and_check &mut Rustc>(to_find: &str, prepare_rustc: F) { run_in_tmpdir(|| { - let out = rustc() + let mut rustc = Rustc::new(); + rustc .arg("-Zlinker-features=+lld") .arg("-Clink-self-contained=+linker") .arg("-Zunstable-options") .arg("-Cdebuginfo=full") - .link_arg(&format!("-Wl,--compress-debug-sections={compression}")) - .input("main.rs") - .run(); + .input("main.rs"); + prepare_rustc(&mut rustc).run(); llvm_readobj().arg("-t").arg("main").run().assert_stdout_contains(to_find); }); } From 4d6ac1ee330c91f1d020c38f8bf9a23d860e8e4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 22:25:22 +0000 Subject: [PATCH 12/19] move and rename zstd script move it where it's used, and name it like the other scripts --- src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile | 6 +++--- .../zstd.sh => host-x86_64/dist-x86_64-linux/build-zstd.sh} | 0 2 files changed, 3 insertions(+), 3 deletions(-) rename src/ci/docker/{scripts/zstd.sh => host-x86_64/dist-x86_64-linux/build-zstd.sh} (100%) diff --git a/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile b/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile index 61e9694f1e2ae..e857f38e68a85 100644 --- a/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile @@ -62,9 +62,9 @@ COPY host-x86_64/dist-x86_64-linux/build-clang.sh /tmp/ RUN ./build-clang.sh ENV CC=clang CXX=clang++ -# rustc's LLVM needs zstd. -COPY scripts/zstd.sh /tmp/ -RUN ./zstd.sh +# Build zstd to enable `llvm.libzstd`. +COPY host-x86_64/dist-x86_64-linux/build-zstd.sh /tmp/ +RUN ./build-zstd.sh COPY scripts/sccache.sh /scripts/ RUN sh /scripts/sccache.sh diff --git a/src/ci/docker/scripts/zstd.sh b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-zstd.sh similarity index 100% rename from src/ci/docker/scripts/zstd.sh rename to src/ci/docker/host-x86_64/dist-x86_64-linux/build-zstd.sh From d39d46594cb339d027a5696434910f6c3b175771 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sun, 11 Aug 2024 15:02:13 +0000 Subject: [PATCH 13/19] strip whitespace for ignored tests reason comments --- src/tools/compiletest/src/header/needs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/header/needs.rs b/src/tools/compiletest/src/header/needs.rs index 3a24adaa2a3b7..3c5a391007bed 100644 --- a/src/tools/compiletest/src/header/needs.rs +++ b/src/tools/compiletest/src/header/needs.rs @@ -179,7 +179,7 @@ pub(super) fn handle_needs( } else { return IgnoreDecision::Ignore { reason: if let Some(comment) = comment { - format!("{} ({comment})", need.ignore_reason) + format!("{} ({})", need.ignore_reason, comment.trim()) } else { need.ignore_reason.into() }, From a566252ac92184a5443245390da794eaddcbef1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sun, 11 Aug 2024 19:05:30 +0000 Subject: [PATCH 14/19] enable `llvm.libzstd` on test x64 linux builder --- src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile index 19683317126ab..83c2aa8cfb3b7 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile @@ -28,5 +28,6 @@ ENV RUST_CONFIGURE_ARGS \ --build=x86_64-unknown-linux-gnu \ --enable-sanitizers \ --enable-profiler \ - --enable-compiler-docs + --enable-compiler-docs \ + --set llvm.libzstd=true ENV SCRIPT python3 ../x.py --stage 2 test From c5205e9d56d3c632e83e1e5f0853d3bd82f16c0e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 11 Aug 2024 12:28:15 -0400 Subject: [PATCH 15/19] Normalize struct tail properly in borrowck and hir typeck --- .../src/type_check/canonical.rs | 47 +++++++++++++++++++ compiler/rustc_borrowck/src/type_check/mod.rs | 13 +---- compiler/rustc_hir_typeck/src/cast.rs | 2 + ...different-regions-id-trait.current.stderr} | 2 +- ...obj-different-regions-id-trait.next.stderr | 15 ++++++ ...to-trait-obj-different-regions-id-trait.rs | 3 ++ 6 files changed, 70 insertions(+), 12 deletions(-) rename tests/ui/cast/{ptr-to-trait-obj-different-regions-id-trait.stderr => ptr-to-trait-obj-different-regions-id-trait.current.stderr} (91%) create mode 100644 tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.next.stderr diff --git a/compiler/rustc_borrowck/src/type_check/canonical.rs b/compiler/rustc_borrowck/src/type_check/canonical.rs index 86cd8b918fc6e..2558cf6347d7c 100644 --- a/compiler/rustc_borrowck/src/type_check/canonical.rs +++ b/compiler/rustc_borrowck/src/type_check/canonical.rs @@ -7,6 +7,7 @@ use rustc_middle::mir::ConstraintCategory; use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, Upcast}; use rustc_span::def_id::DefId; use rustc_span::Span; +use rustc_trait_selection::traits::query::type_op::custom::CustomTypeOp; use rustc_trait_selection::traits::query::type_op::{self, TypeOpOutput}; use rustc_trait_selection::traits::ObligationCause; @@ -165,6 +166,52 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { result.unwrap_or(value) } + #[instrument(skip(self), level = "debug")] + pub(super) fn struct_tail( + &mut self, + ty: Ty<'tcx>, + location: impl NormalizeLocation, + ) -> Ty<'tcx> { + let tcx = self.tcx(); + if self.infcx.next_trait_solver() { + let body = self.body; + let param_env = self.param_env; + self.fully_perform_op( + location.to_locations(), + ConstraintCategory::Boring, + CustomTypeOp::new( + |ocx| { + let structurally_normalize = |ty| { + ocx.structurally_normalize( + &ObligationCause::misc( + location.to_locations().span(body), + body.source.def_id().expect_local(), + ), + param_env, + ty, + ) + .unwrap_or_else(|_| bug!("struct tail should have been computable, since we computed it in HIR")) + }; + + let tail = tcx.struct_tail_with_normalize( + ty, + structurally_normalize, + || {}, + ); + + Ok(tail) + }, + "normalizing struct tail", + ), + ) + .unwrap_or_else(|guar| Ty::new_error(tcx, guar)) + } else { + let mut normalize = |ty| self.normalize(ty, location); + let tail = tcx.struct_tail_with_normalize(ty, &mut normalize, || {}); + normalize(tail) + } + } + #[instrument(skip(self), level = "debug")] pub(super) fn ascribe_user_type( &mut self, diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index b13773ffe1460..6bab0f33c198c 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -2329,17 +2329,8 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { let cast_ty_to = CastTy::from_ty(*ty); match (cast_ty_from, cast_ty_to) { (Some(CastTy::Ptr(src)), Some(CastTy::Ptr(dst))) => { - let mut normalize = |t| self.normalize(t, location); - - // N.B. `struct_tail_with_normalize` only "structurally resolves" - // the type. It is not fully normalized, so we have to normalize it - // afterwards. - let src_tail = - tcx.struct_tail_with_normalize(src.ty, &mut normalize, || ()); - let src_tail = normalize(src_tail); - let dst_tail = - tcx.struct_tail_with_normalize(dst.ty, &mut normalize, || ()); - let dst_tail = normalize(dst_tail); + let src_tail = self.struct_tail(src.ty, location); + let dst_tail = self.struct_tail(dst.ty, location); // This checks (lifetime part of) vtable validity for pointer casts, // which is irrelevant when there are aren't principal traits on both sides (aka only auto traits). diff --git a/compiler/rustc_hir_typeck/src/cast.rs b/compiler/rustc_hir_typeck/src/cast.rs index de70273390466..7cd97166ed1e9 100644 --- a/compiler/rustc_hir_typeck/src/cast.rs +++ b/compiler/rustc_hir_typeck/src/cast.rs @@ -97,6 +97,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return Ok(Some(PointerKind::Thin)); } + let t = self.try_structurally_resolve_type(span, t); + Ok(match *t.kind() { ty::Slice(_) | ty::Str => Some(PointerKind::Length), ty::Dynamic(tty, _, ty::Dyn) => Some(PointerKind::VTable(tty)), diff --git a/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.stderr b/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.current.stderr similarity index 91% rename from tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.stderr rename to tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.current.stderr index d1d598e603f18..5a5b4bfcacf43 100644 --- a/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.stderr +++ b/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.current.stderr @@ -1,5 +1,5 @@ error: lifetime may not live long enough - --> $DIR/ptr-to-trait-obj-different-regions-id-trait.rs:21:17 + --> $DIR/ptr-to-trait-obj-different-regions-id-trait.rs:24:17 | LL | fn m<'a>() { | -- lifetime `'a` defined here diff --git a/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.next.stderr b/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.next.stderr new file mode 100644 index 0000000000000..5a5b4bfcacf43 --- /dev/null +++ b/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.next.stderr @@ -0,0 +1,15 @@ +error: lifetime may not live long enough + --> $DIR/ptr-to-trait-obj-different-regions-id-trait.rs:24:17 + | +LL | fn m<'a>() { + | -- lifetime `'a` defined here +LL | let unsend: *const dyn Cat<'a> = &(); +LL | let _send = unsend as *const S>; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static` + | + = note: requirement occurs because of the type `S>`, which makes the generic argument `dyn Cat<'_>` invariant + = note: the struct `S` is invariant over the parameter `T` + = help: see for more information about variance + +error: aborting due to 1 previous error + diff --git a/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.rs b/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.rs index cdd55e243927c..f968dca4fd310 100644 --- a/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.rs +++ b/tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.rs @@ -1,3 +1,6 @@ +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver //@ check-fail // // Make sure we can't trick the compiler by using a projection. From b5d2079fb9c9ac9f0fe594f65452b4097e71c2de Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 11 Aug 2024 12:30:38 -0400 Subject: [PATCH 16/19] Rename normalization functions to raw --- .../rustc_borrowck/src/type_check/canonical.rs | 4 ++-- .../rustc_const_eval/src/const_eval/valtrees.rs | 2 +- compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs | 2 +- compiler/rustc_middle/src/ty/layout.rs | 2 +- compiler/rustc_middle/src/ty/sty.rs | 6 +++--- compiler/rustc_middle/src/ty/util.rs | 16 +++++++++------- .../rustc_trait_selection/src/traits/project.rs | 2 +- 7 files changed, 18 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/canonical.rs b/compiler/rustc_borrowck/src/type_check/canonical.rs index 2558cf6347d7c..b58691fbeae3a 100644 --- a/compiler/rustc_borrowck/src/type_check/canonical.rs +++ b/compiler/rustc_borrowck/src/type_check/canonical.rs @@ -193,7 +193,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { .unwrap_or_else(|_| bug!("struct tail should have been computable, since we computed it in HIR")) }; - let tail = tcx.struct_tail_with_normalize( + let tail = tcx.struct_tail_raw( ty, structurally_normalize, || {}, @@ -207,7 +207,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { .unwrap_or_else(|guar| Ty::new_error(tcx, guar)) } else { let mut normalize = |ty| self.normalize(ty, location); - let tail = tcx.struct_tail_with_normalize(ty, &mut normalize, || {}); + let tail = tcx.struct_tail_raw(ty, &mut normalize, || {}); normalize(tail) } } diff --git a/compiler/rustc_const_eval/src/const_eval/valtrees.rs b/compiler/rustc_const_eval/src/const_eval/valtrees.rs index 8227c04594883..460c9797f3663 100644 --- a/compiler/rustc_const_eval/src/const_eval/valtrees.rs +++ b/compiler/rustc_const_eval/src/const_eval/valtrees.rs @@ -195,7 +195,7 @@ fn reconstruct_place_meta<'tcx>( let mut last_valtree = valtree; // Traverse the type, and update `last_valtree` as we go. - let tail = tcx.struct_tail_with_normalize( + let tail = tcx.struct_tail_raw( layout.ty, |ty| ty, || { diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 841d25b54cc88..b169f75796b3a 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -404,7 +404,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { code: traits::ObligationCauseCode<'tcx>, ) { if !ty.references_error() { - let tail = self.tcx.struct_tail_with_normalize( + let tail = self.tcx.struct_tail_raw( ty, |ty| { if self.next_trait_solver() { diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 9204405d58f11..684574825e345 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -362,7 +362,7 @@ impl<'tcx> SizeSkeleton<'tcx> { ty::Ref(_, pointee, _) | ty::RawPtr(pointee, _) => { let non_zero = !ty.is_unsafe_ptr(); - let tail = tcx.struct_tail_with_normalize( + let tail = tcx.struct_tail_raw( pointee, |ty| match tcx.try_normalize_erasing_regions(param_env, ty) { Ok(ty) => ty, diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 8c97de1c59b26..8781a670acb30 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1590,7 +1590,7 @@ impl<'tcx> Ty<'tcx> { tcx: TyCtxt<'tcx>, normalize: impl FnMut(Ty<'tcx>) -> Ty<'tcx>, ) -> Result, Ty<'tcx>> { - let tail = tcx.struct_tail_with_normalize(self, normalize, || {}); + let tail = tcx.struct_tail_raw(self, normalize, || {}); match tail.kind() { // Sized types ty::Infer(ty::IntVar(_) | ty::FloatVar(_)) @@ -1614,10 +1614,10 @@ impl<'tcx> Ty<'tcx> { | ty::Foreign(..) // `dyn*` has metadata = (). | ty::Dynamic(_, _, ty::DynStar) - // If returned by `struct_tail_with_normalize` this is a unit struct + // If returned by `struct_tail_raw` this is a unit struct // without any fields, or not a struct, and therefore is Sized. | ty::Adt(..) - // If returned by `struct_tail_with_normalize` this is the empty tuple, + // If returned by `struct_tail_raw` this is the empty tuple, // a.k.a. unit type, which is Sized | ty::Tuple(..) => Ok(tcx.types.unit), diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 365f434a264e2..fc5e0c44d96fc 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -176,7 +176,7 @@ impl<'tcx> TyCtxt<'tcx> { /// if input `ty` is not a structure at all. pub fn struct_tail_without_normalization(self, ty: Ty<'tcx>) -> Ty<'tcx> { let tcx = self; - tcx.struct_tail_with_normalize(ty, |ty| ty, || {}) + tcx.struct_tail_raw(ty, |ty| ty, || {}) } /// Returns the deeply last field of nested structures, or the same type if @@ -188,7 +188,7 @@ impl<'tcx> TyCtxt<'tcx> { /// normalization attempt may cause compiler bugs. pub fn struct_tail_for_codegen(self, ty: Ty<'tcx>, param_env: ty::ParamEnv<'tcx>) -> Ty<'tcx> { let tcx = self; - tcx.struct_tail_with_normalize(ty, |ty| tcx.normalize_erasing_regions(param_env, ty), || {}) + tcx.struct_tail_raw(ty, |ty| tcx.normalize_erasing_regions(param_env, ty), || {}) } /// Returns the deeply last field of nested structures, or the same type if @@ -196,12 +196,14 @@ impl<'tcx> TyCtxt<'tcx> { /// and its type can be used to determine unsizing strategy. /// /// This is parameterized over the normalization strategy (i.e. how to - /// handle `::Assoc` and `impl Trait`); pass the identity - /// function to indicate no normalization should take place. + /// handle `::Assoc` and `impl Trait`). You almost certainly do + /// **NOT** want to pass the identity function here, unless you know what + /// you're doing, or you're within normalization code itself and will handle + /// an unnormalized tail recursively. /// /// See also `struct_tail_for_codegen`, which is suitable for use /// during codegen. - pub fn struct_tail_with_normalize( + pub fn struct_tail_raw( self, mut ty: Ty<'tcx>, mut normalize: impl FnMut(Ty<'tcx>) -> Ty<'tcx>, @@ -281,7 +283,7 @@ impl<'tcx> TyCtxt<'tcx> { param_env: ty::ParamEnv<'tcx>, ) -> (Ty<'tcx>, Ty<'tcx>) { let tcx = self; - tcx.struct_lockstep_tails_with_normalize(source, target, |ty| { + tcx.struct_lockstep_tails_raw(source, target, |ty| { tcx.normalize_erasing_regions(param_env, ty) }) } @@ -294,7 +296,7 @@ impl<'tcx> TyCtxt<'tcx> { /// /// See also `struct_lockstep_tails_for_codegen`, which is suitable for use /// during codegen. - pub fn struct_lockstep_tails_with_normalize( + pub fn struct_lockstep_tails_raw( self, source: Ty<'tcx>, target: Ty<'tcx>, diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 4b62a5c59b2f0..0e4233ba7bc56 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -1110,7 +1110,7 @@ fn assemble_candidates_from_impls<'cx, 'tcx>( | ty::Error(_) => false, } } else if tcx.is_lang_item(trait_ref.def_id, LangItem::PointeeTrait) { - let tail = selcx.tcx().struct_tail_with_normalize( + let tail = selcx.tcx().struct_tail_raw( self_ty, |ty| { // We throw away any obligations we get from this, since we normalize From f15997ffeca4c7da66e7de9e348ccb8d3cccc946 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 11 Aug 2024 19:21:33 -0400 Subject: [PATCH 17/19] Remove struct_tail_no_normalization --- compiler/rustc_const_eval/src/const_eval/eval_queries.rs | 2 +- compiler/rustc_hir_typeck/src/expectation.rs | 3 ++- compiler/rustc_middle/src/ty/util.rs | 8 -------- compiler/rustc_trait_selection/src/traits/project.rs | 4 ++-- compiler/rustc_ty_utils/src/layout.rs | 6 +++++- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index ff27e4000163a..96b3ec6f18728 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -226,7 +226,7 @@ pub(super) fn op_to_const<'tcx>( let pointee_ty = imm.layout.ty.builtin_deref(false).unwrap(); // `false` = no raw ptrs debug_assert!( matches!( - ecx.tcx.struct_tail_without_normalization(pointee_ty).kind(), + ecx.tcx.struct_tail_for_codegen(pointee_ty, ecx.param_env).kind(), ty::Str | ty::Slice(..), ), "`ConstValue::Slice` is for slice-tailed types only, but got {}", diff --git a/compiler/rustc_hir_typeck/src/expectation.rs b/compiler/rustc_hir_typeck/src/expectation.rs index 91deae4174b00..76ae41db5c51c 100644 --- a/compiler/rustc_hir_typeck/src/expectation.rs +++ b/compiler/rustc_hir_typeck/src/expectation.rs @@ -70,7 +70,8 @@ impl<'a, 'tcx> Expectation<'tcx> { /// See the test case `test/ui/coerce-expect-unsized.rs` and #20169 /// for examples of where this comes up,. pub(super) fn rvalue_hint(fcx: &FnCtxt<'a, 'tcx>, ty: Ty<'tcx>) -> Expectation<'tcx> { - match fcx.tcx.struct_tail_without_normalization(ty).kind() { + // FIXME: This is not right, even in the old solver... + match fcx.tcx.struct_tail_raw(ty, |ty| ty, || {}).kind() { ty::Slice(_) | ty::Str | ty::Dynamic(..) => ExpectRvalueLikeUnsized(ty), _ => ExpectHasType(ty), } diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index fc5e0c44d96fc..6be3dc423deb3 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -171,14 +171,6 @@ impl<'tcx> TyCtxt<'tcx> { } } - /// Attempts to returns the deeply last field of nested structures, but - /// does not apply any normalization in its search. Returns the same type - /// if input `ty` is not a structure at all. - pub fn struct_tail_without_normalization(self, ty: Ty<'tcx>) -> Ty<'tcx> { - let tcx = self; - tcx.struct_tail_raw(ty, |ty| ty, || {}) - } - /// Returns the deeply last field of nested structures, or the same type if /// not a structure at all. Corresponds to the only possible unsized field, /// and its type can be used to determine unsizing strategy. diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 0e4233ba7bc56..8a17d7ed64184 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -1149,10 +1149,10 @@ fn assemble_candidates_from_impls<'cx, 'tcx>( | ty::Never // Extern types have unit metadata, according to RFC 2850 | ty::Foreign(_) - // If returned by `struct_tail_without_normalization` this is a unit struct + // If returned by `struct_tail` this is a unit struct // without any fields, or not a struct, and therefore is Sized. | ty::Adt(..) - // If returned by `struct_tail_without_normalization` this is the empty tuple. + // If returned by `struct_tail` this is the empty tuple. | ty::Tuple(..) // Integers and floats are always Sized, and so have unit type metadata. | ty::Infer(ty::InferTy::IntVar(_) | ty::InferTy::FloatVar(..)) => true, diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 1eb03fc3bd6a1..244a6afcf979a 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -219,9 +219,13 @@ fn layout_of_uncached<'tcx>( // its struct tail cannot be normalized either, so try to get a // more descriptive layout error here, which will lead to less confusing // diagnostics. + // + // We use the raw struct tail function here to get the first tail + // that is an alias, which is likely the cause of the normalization + // error. match tcx.try_normalize_erasing_regions( param_env, - tcx.struct_tail_without_normalization(pointee), + tcx.struct_tail_raw(pointee, |ty| ty, || {}), ) { Ok(_) => {} Err(better_err) => { From 99c0d768b0399b8f4f95dc285bd21e2e7ca1b10a Mon Sep 17 00:00:00 2001 From: joboet Date: Mon, 12 Aug 2024 10:23:26 +0200 Subject: [PATCH 18/19] std: use `/scheme/rand` on Redox --- library/std/src/sys/pal/unix/rand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/pal/unix/rand.rs b/library/std/src/sys/pal/unix/rand.rs index 9cb96e0888899..cc0852aab4396 100644 --- a/library/std/src/sys/pal/unix/rand.rs +++ b/library/std/src/sys/pal/unix/rand.rs @@ -49,7 +49,7 @@ cfg_if::cfg_if! { } const PATH: &'static str = if cfg!(target_os = "redox") { - "rand:" + "/scheme/rand" } else { "/dev/urandom" }; From 6839a8f6d55ebb00b1904cda37d03caf15144440 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 12 Aug 2024 11:32:21 +0200 Subject: [PATCH 19/19] bootstrap: clear miri ui-test deps when miri sysroot gets rebuilt --- src/bootstrap/src/core/build_steps/test.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 597d7733abe75..6ed001d8fd5f7 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -434,7 +434,7 @@ impl Miri { builder: &Builder<'_>, compiler: Compiler, target: TargetSelection, - ) -> String { + ) -> PathBuf { let miri_sysroot = builder.out.join(compiler.host.triple).join("miri-sysroot"); let mut cargo = builder::Cargo::new( builder, @@ -467,7 +467,7 @@ impl Miri { // Output is "\n". let sysroot = stdout.trim_end(); builder.verbose(|| println!("`cargo miri setup --print-sysroot` said: {sysroot:?}")); - sysroot.to_owned() + PathBuf::from(sysroot) } } @@ -520,12 +520,14 @@ impl Step for Miri { builder.ensure(compile::Std::new(target_compiler, host)); let host_sysroot = builder.sysroot(target_compiler); - // Miri has its own "target dir" for ui test dependencies. Make sure it gets cleared - // properly when rustc changes. Similar to `Builder::cargo`, we skip this in dry runs to - // make sure the relevant compiler has been set up properly. + // Miri has its own "target dir" for ui test dependencies. Make sure it gets cleared when + // the sysroot gets rebuilt, to avoid "found possibly newer version of crate `std`" errors. if !builder.config.dry_run() { let ui_test_dep_dir = builder.stage_out(host_compiler, Mode::ToolStd).join("miri_ui"); - builder.clear_if_dirty(&ui_test_dep_dir, &builder.rustc(host_compiler)); + // The mtime of `miri_sysroot` changes when the sysroot gets rebuilt (also see + // ). + // We can hence use that directly as a signal to clear the ui test dir. + builder.clear_if_dirty(&ui_test_dep_dir, &miri_sysroot); } // Run `cargo test`.