From 59be3e856f218f55be0f73bcfed6265772437ec7 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Fri, 17 Jun 2022 11:39:59 -0500 Subject: [PATCH 01/13] Stabilized Option::unzip() --- library/core/src/option.rs | 11 +++++++---- library/core/tests/lib.rs | 1 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 28ea45ed235..1d6784af2a5 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -1711,8 +1711,6 @@ impl Option<(T, U)> { /// # Examples /// /// ``` - /// #![feature(unzip_option)] - /// /// let x = Some((1, "hi")); /// let y = None::<(u8, u32)>; /// @@ -1720,8 +1718,13 @@ impl Option<(T, U)> { /// assert_eq!(y.unzip(), (None, None)); /// ``` #[inline] - #[unstable(feature = "unzip_option", issue = "87800", reason = "recently added")] - pub const fn unzip(self) -> (Option, Option) { + #[stable(feature = "unzip_option", since = "1.63.0")] + #[rustc_const_unstable(feature = "const_option", issue = "67441")] + pub const fn unzip(self) -> (Option, Option) + where + T: ~const Destruct, + U: ~const Destruct, + { match self { Some((a, b)) => (Some(a), Some(b)), None => (None, None), diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 63c9602abe7..30b9cb4dadc 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -91,7 +91,6 @@ #![feature(strict_provenance)] #![feature(trusted_random_access)] #![feature(unsize)] -#![feature(unzip_option)] #![feature(const_array_from_ref)] #![feature(const_slice_from_ref)] #![feature(waker_getters)] From df8a62d4f380d656444a6fc4e704bcd3693f4b9a Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Wed, 7 Sep 2022 10:27:42 -0500 Subject: [PATCH 02/13] Use `CURRENT_RUSTC_VERSION` --- library/core/src/option.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 1d6784af2a5..df5a20869a3 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -1718,7 +1718,7 @@ impl Option<(T, U)> { /// assert_eq!(y.unzip(), (None, None)); /// ``` #[inline] - #[stable(feature = "unzip_option", since = "1.63.0")] + #[stable(feature = "unzip_option", since = "CURRENT_RUSTC_VERSION")] #[rustc_const_unstable(feature = "const_option", issue = "67441")] pub const fn unzip(self) -> (Option, Option) where From 093b075d327d1c7c7e0730e46f1d5c2d4aa47f5a Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Sun, 2 Oct 2022 17:31:43 +0200 Subject: [PATCH 03/13] rustc: Use `unix_sigpipe` instead of `rustc_driver::set_sigpipe_handler` This is the first (known) step towards starting to use `unix_sigpipe` in the wild. Eventually, `rustc_driver::set_sigpipe_handler` can be removed and all clients can use `unix_sigpipe` instead. For now we just start using `unix_sigpipe` in once place: `rustc` itself. It is easy to manually verify this change. If you remove `#[unix_sigpipe = "sig_dfl"]` and run `./x.py build` you will get an ICE when you do `./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --help | false`. Add back `#[unix_sigpipe = "sig_dfl"]` and the ICE disappears again. --- compiler/rustc/src/main.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc/src/main.rs b/compiler/rustc/src/main.rs index 0de1a781913..e21c9b66044 100644 --- a/compiler/rustc/src/main.rs +++ b/compiler/rustc/src/main.rs @@ -1,3 +1,5 @@ +#![feature(unix_sigpipe)] + // A note about jemalloc: rustc uses jemalloc when built for CI and // distribution. The obvious way to do this is with the `#[global_allocator]` // mechanism. However, for complicated reasons (see @@ -23,6 +25,7 @@ // libraries. So we must reference jemalloc symbols one way or another, because // this file is the only object code in the rustc executable. +#[unix_sigpipe = "sig_dfl"] fn main() { // See the comment at the top of this file for an explanation of this. #[cfg(feature = "jemalloc-sys")] @@ -58,6 +61,5 @@ fn main() { } } - rustc_driver::set_sigpipe_handler(); rustc_driver::main() } From 7280f3d28aa139cec0c75072a3e66294b7f99b59 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 21 Oct 2022 17:44:35 -0700 Subject: [PATCH 04/13] Truncate thread names on Linux and Apple targets These targets have system limits on the thread names, 16 and 64 bytes respectively, and `pthread_setname_np` returns an error if the name is longer. However, we're not in a context that can propagate errors when we call this, and we used to implicitly truncate on Linux with `prctl`, so now we manually truncate these names ahead of time. --- library/std/src/sys/unix/thread.rs | 43 ++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index 42ac6fcd8bf..6a533854fad 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -132,8 +132,11 @@ impl Thread { #[cfg(target_os = "linux")] pub fn set_name(name: &CStr) { + const TASK_COMM_LEN: usize = 16; + unsafe { // Available since glibc 2.12, musl 1.1.16, and uClibc 1.0.20. + let name = truncate_cstr(name, TASK_COMM_LEN); libc::pthread_setname_np(libc::pthread_self(), name.as_ptr()); } } @@ -148,6 +151,7 @@ impl Thread { #[cfg(any(target_os = "macos", target_os = "ios", target_os = "watchos"))] pub fn set_name(name: &CStr) { unsafe { + let name = truncate_cstr(name, libc::MAXTHREADNAMESIZE); libc::pthread_setname_np(name.as_ptr()); } } @@ -276,6 +280,20 @@ impl Drop for Thread { } } +#[cfg(any(target_os = "linux", target_os = "macos", target_os = "ios", target_os = "watchos"))] +fn truncate_cstr(cstr: &CStr, max_with_nul: usize) -> crate::borrow::Cow<'_, CStr> { + use crate::{borrow::Cow, ffi::CString}; + + if cstr.to_bytes_with_nul().len() > max_with_nul { + let bytes = cstr.to_bytes()[..max_with_nul - 1].to_vec(); + // SAFETY: the non-nul bytes came straight from a CStr. + // (CString will add the terminating nul.) + Cow::Owned(unsafe { CString::from_vec_unchecked(bytes) }) + } else { + Cow::Borrowed(cstr) + } +} + pub fn available_parallelism() -> io::Result { cfg_if::cfg_if! { if #[cfg(any( @@ -902,3 +920,28 @@ fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { 2048 // just a guess } + +#[test] +#[cfg(any(target_os = "linux", target_os = "macos", target_os = "ios", target_os = "watchos"))] +fn test_named_thread_truncation() { + use crate::thread::{self, Builder}; + + let long_name = crate::iter::once("test_named_thread_truncation") + .chain(crate::iter::repeat(" yada").take(100)) + .collect::(); + + let result = Builder::new().name(long_name.clone()).spawn(move || { + // Rust remembers the full thread name itself. + assert_eq!(thread::current().name(), Some(long_name.as_str())); + + // But the kernel is limited -- make sure we successfully set a truncation. + let mut buf = vec![0u8; long_name.len() + 1]; + unsafe { + libc::pthread_getname_np(libc::pthread_self(), buf.as_mut_ptr().cast(), buf.len()); + } + let cstr = CStr::from_bytes_until_nul(&buf).unwrap(); + assert!(cstr.to_bytes().len() > 0); + assert!(long_name.as_bytes().starts_with(cstr.to_bytes())); + }); + result.unwrap().join().unwrap(); +} From 12e45846ebbc32bd6a56f2de2658d3d9ad459032 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 21 Oct 2022 18:13:22 -0700 Subject: [PATCH 05/13] Move truncation next to other thread tests for tidy --- library/std/src/sys/unix/thread.rs | 25 ------------------------- library/std/src/thread/tests.rs | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index 6a533854fad..69cd2b500a1 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -920,28 +920,3 @@ fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { 2048 // just a guess } - -#[test] -#[cfg(any(target_os = "linux", target_os = "macos", target_os = "ios", target_os = "watchos"))] -fn test_named_thread_truncation() { - use crate::thread::{self, Builder}; - - let long_name = crate::iter::once("test_named_thread_truncation") - .chain(crate::iter::repeat(" yada").take(100)) - .collect::(); - - let result = Builder::new().name(long_name.clone()).spawn(move || { - // Rust remembers the full thread name itself. - assert_eq!(thread::current().name(), Some(long_name.as_str())); - - // But the kernel is limited -- make sure we successfully set a truncation. - let mut buf = vec![0u8; long_name.len() + 1]; - unsafe { - libc::pthread_getname_np(libc::pthread_self(), buf.as_mut_ptr().cast(), buf.len()); - } - let cstr = CStr::from_bytes_until_nul(&buf).unwrap(); - assert!(cstr.to_bytes().len() > 0); - assert!(long_name.as_bytes().starts_with(cstr.to_bytes())); - }); - result.unwrap().join().unwrap(); -} diff --git a/library/std/src/thread/tests.rs b/library/std/src/thread/tests.rs index dfb8765ab4e..71eb41cd564 100644 --- a/library/std/src/thread/tests.rs +++ b/library/std/src/thread/tests.rs @@ -37,6 +37,31 @@ fn test_named_thread() { .unwrap(); } +#[cfg(any(target_os = "linux", target_os = "macos", target_os = "ios", target_os = "watchos"))] +#[test] +fn test_named_thread_truncation() { + use crate::ffi::CStr; + + let long_name = crate::iter::once("test_named_thread_truncation") + .chain(crate::iter::repeat(" yada").take(100)) + .collect::(); + + let result = Builder::new().name(long_name.clone()).spawn(move || { + // Rust remembers the full thread name itself. + assert_eq!(thread::current().name(), Some(long_name.as_str())); + + // But the system is limited -- make sure we successfully set a truncation. + let mut buf = vec![0u8; long_name.len() + 1]; + unsafe { + libc::pthread_getname_np(libc::pthread_self(), buf.as_mut_ptr().cast(), buf.len()); + } + let cstr = CStr::from_bytes_until_nul(&buf).unwrap(); + assert!(cstr.to_bytes().len() > 0); + assert!(long_name.as_bytes().starts_with(cstr.to_bytes())); + }); + result.unwrap().join().unwrap(); +} + #[test] #[should_panic] fn test_invalid_named_thread() { From 4bd98443ed368a781812545915cf758e2091128d Mon Sep 17 00:00:00 2001 From: ouz-a Date: Sun, 16 Oct 2022 21:30:32 +0300 Subject: [PATCH 06/13] remove misc_cast and validate types --- .../rustc_const_eval/src/interpret/cast.rs | 63 +++++++++++++------ .../src/transform/validate.rs | 25 ++++++-- src/tools/miri/src/shims/intrinsics/simd.rs | 6 +- 3 files changed, 66 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/cast.rs b/compiler/rustc_const_eval/src/interpret/cast.rs index 764224fd007..f980e606b93 100644 --- a/compiler/rustc_const_eval/src/interpret/cast.rs +++ b/compiler/rustc_const_eval/src/interpret/cast.rs @@ -42,10 +42,22 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let res = self.pointer_from_exposed_address_cast(&src, cast_ty)?; self.write_immediate(res, dest)?; } - // FIXME: We shouldn't use `misc_cast` for these but handle them separately. - IntToInt | FloatToInt | FloatToFloat | IntToFloat | FnPtrToPtr | PtrToPtr => { + + IntToInt | IntToFloat => { let src = self.read_immediate(src)?; - let res = self.misc_cast(&src, cast_ty)?; + let res = self.int_to_int_or_float(&src, cast_ty)?; + self.write_immediate(res, dest)?; + } + + FloatToFloat | FloatToInt => { + let src = self.read_immediate(src)?; + let res = self.float_to_float_or_int(&src, cast_ty)?; + self.write_immediate(res, dest)?; + } + + FnPtrToPtr | PtrToPtr => { + let src = self.read_immediate(&src)?; + let res = self.ptr_to_ptr(&src, cast_ty)?; self.write_immediate(res, dest)?; } @@ -126,13 +138,27 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } - pub fn misc_cast( + pub fn int_to_int_or_float( + &mut self, + src: &ImmTy<'tcx, M::Provenance>, + cast_ty: Ty<'tcx>, + ) -> InterpResult<'tcx, Immediate> { + if (src.layout.ty.is_integral() || src.layout.ty.is_char() || src.layout.ty.is_bool()) + && (cast_ty.is_floating_point() || cast_ty.is_integral() || cast_ty.is_char()) + { + let scalar = src.to_scalar(); + Ok(self.cast_from_int_like(scalar, src.layout, cast_ty)?.into()) + } else { + bug!("Unexpected cast from type {:?}", src.layout.ty) + } + } + + pub fn float_to_float_or_int( &mut self, src: &ImmTy<'tcx, M::Provenance>, cast_ty: Ty<'tcx>, ) -> InterpResult<'tcx, Immediate> { use rustc_type_ir::sty::TyKind::*; - trace!("Casting {:?}: {:?} to {:?}", *src, src.layout.ty, cast_ty); match src.layout.ty.kind() { // Floating point @@ -142,19 +168,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Float(FloatTy::F64) => { return Ok(self.cast_from_float(src.to_scalar().to_f64()?, cast_ty).into()); } - // The rest is integer/pointer-"like", including fn ptr casts - _ => assert!( - src.layout.ty.is_bool() - || src.layout.ty.is_char() - || src.layout.ty.is_integral() - || src.layout.ty.is_any_ptr(), - "Unexpected cast from type {:?}", - src.layout.ty - ), + _ => { + bug!("Can't cast 'Float' type into {:?}", cast_ty); + } } + } - // # First handle non-scalar source values. - + /// Handles 'FnPtrToPtr' and 'PtrToPtr' casts. + pub fn ptr_to_ptr( + &mut self, + src: &ImmTy<'tcx, M::Provenance>, + cast_ty: Ty<'tcx>, + ) -> InterpResult<'tcx, Immediate> { // Handle casting any ptr to raw ptr (might be a fat ptr). if src.layout.ty.is_any_ptr() && cast_ty.is_unsafe_ptr() { let dest_layout = self.layout_of(cast_ty)?; @@ -178,11 +203,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Immediate::Uninit => throw_ub!(InvalidUninitBytes(None)), }; } + } else { + bug!("Can't cast 'Ptr' or 'FnPtr' into {:?}", cast_ty); } - - // # The remaining source values are scalar and "int-like". - let scalar = src.to_scalar(); - Ok(self.cast_from_int_like(scalar, src.layout, cast_ty)?.into()) } pub fn pointer_expose_address_cast( diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 8abf767f89f..40946344a35 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -556,21 +556,36 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { check_kinds!(a, "Cannot shallow init type {:?}", ty::RawPtr(..)); } Rvalue::Cast(kind, operand, target_type) => { + let op_ty = operand.ty(self.body, self.tcx); match kind { CastKind::DynStar => { // FIXME(dyn-star): make sure nothing needs to be done here. } - // Nothing to check here + // FIXME: Add Checks for these CastKind::PointerFromExposedAddress | CastKind::PointerExposeAddress | CastKind::Pointer(_) => {} - _ => { - let op_ty = operand.ty(self.body, self.tcx); - if op_ty.is_enum() { + CastKind::IntToInt | CastKind::IntToFloat => { + let input_valid = op_ty.is_integral() || op_ty.is_char() || op_ty.is_bool(); + let target_valid = target_type.is_numeric() || target_type.is_char(); + if !input_valid || !target_valid { + self.fail( + location, + format!("Wrong cast kind {kind:?} for the type {op_ty}",), + ); + } + } + CastKind::FnPtrToPtr | CastKind::PtrToPtr => { + if !(op_ty.is_any_ptr() && target_type.is_unsafe_ptr()) { + self.fail(location, "Can't cast {op_ty} into 'Ptr'"); + } + } + CastKind::FloatToFloat | CastKind::FloatToInt => { + if !op_ty.is_floating_point() || !target_type.is_numeric() { self.fail( location, format!( - "enum -> int casts should go through `Rvalue::Discriminant`: {operand:?}:{op_ty} as {target_type}", + "Trying to cast non 'Float' as {kind:?} into {target_type:?}" ), ); } diff --git a/src/tools/miri/src/shims/intrinsics/simd.rs b/src/tools/miri/src/shims/intrinsics/simd.rs index 163d185f66f..c1b949b1f79 100644 --- a/src/tools/miri/src/shims/intrinsics/simd.rs +++ b/src/tools/miri/src/shims/intrinsics/simd.rs @@ -437,13 +437,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let val = match (op.layout.ty.kind(), dest.layout.ty.kind()) { // Int-to-(int|float): always safe (ty::Int(_) | ty::Uint(_), ty::Int(_) | ty::Uint(_) | ty::Float(_)) => - this.misc_cast(&op, dest.layout.ty)?, + this.int_to_int_or_float(&op, dest.layout.ty)?, // Float-to-float: always safe (ty::Float(_), ty::Float(_)) => - this.misc_cast(&op, dest.layout.ty)?, + this.float_to_float_or_int(&op, dest.layout.ty)?, // Float-to-int in safe mode (ty::Float(_), ty::Int(_) | ty::Uint(_)) if safe_cast => - this.misc_cast(&op, dest.layout.ty)?, + this.float_to_float_or_int(&op, dest.layout.ty)?, // Float-to-int in unchecked mode (ty::Float(FloatTy::F32), ty::Int(_) | ty::Uint(_)) if !safe_cast => this.float_to_int_unchecked(op.to_scalar().to_f32()?, dest.layout.ty)?.into(), From 15cfeb33b08c91090d52685651aa9762b8f3c43f Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Sun, 23 Oct 2022 11:53:39 -0700 Subject: [PATCH 07/13] Only test pthread_getname_np on linux-gnu --- library/std/src/thread/tests.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/library/std/src/thread/tests.rs b/library/std/src/thread/tests.rs index 71eb41cd564..6c9ce6fa0dd 100644 --- a/library/std/src/thread/tests.rs +++ b/library/std/src/thread/tests.rs @@ -37,7 +37,13 @@ fn test_named_thread() { .unwrap(); } -#[cfg(any(target_os = "linux", target_os = "macos", target_os = "ios", target_os = "watchos"))] +#[cfg(any( + // Note: musl didn't add pthread_getname_np until 1.2.3 + all(target_os = "linux", target_env = "gnu"), + target_os = "macos", + target_os = "ios", + target_os = "watchos" +))] #[test] fn test_named_thread_truncation() { use crate::ffi::CStr; From 674cd6125da1277af2d864b7d4e637c0a73f142c Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Mon, 24 Oct 2022 15:01:58 +0100 Subject: [PATCH 08/13] Clairify Vec::capacity docs Fixes #103326 --- library/alloc/src/vec/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 6a71f08330c..bbbdc3aa2a2 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -868,13 +868,14 @@ impl Vec { (ptr, len, capacity, alloc) } - /// Returns the number of elements the vector can hold without + /// Returns the total number of elements the vector can hold without /// reallocating. /// /// # Examples /// /// ``` - /// let vec: Vec = Vec::with_capacity(10); + /// let mut vec: Vec = Vec::with_capacity(10); + /// vec.push(42); /// assert_eq!(vec.capacity(), 10); /// ``` #[inline] From 4e4092f8ccb4b7f8b3c4a5cdcbfcdfafa8f5f1e1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Oct 2022 10:34:45 +1100 Subject: [PATCH 09/13] rustc_codegen_ssa: use more consistent naming. Ensure: - builders always have a `bx` suffix; - backend basic blocks always have an `llbb` suffix, - paired builders and basic blocks have consistent prefixes. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 41 +++++++++++---------- compiler/rustc_codegen_ssa/src/mir/mod.rs | 20 +++++----- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index bd4f0cac7eb..e7abae665e3 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -95,10 +95,10 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { debug!("llblock: creating cleanup trampoline for {:?}", target); let name = &format!("{:?}_cleanup_trampoline_{:?}", self.bb, target); - let trampoline = Bx::append_block(fx.cx, fx.llfn, name); - let mut trampoline_bx = Bx::build(fx.cx, trampoline); + let trampoline_llbb = Bx::append_block(fx.cx, fx.llfn, name); + let mut trampoline_bx = Bx::build(fx.cx, trampoline_llbb); trampoline_bx.cleanup_ret(self.funclet(fx).unwrap(), Some(lltarget)); - trampoline + trampoline_llbb } else { lltarget } @@ -1459,20 +1459,20 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // bar(); // } Some(&mir::TerminatorKind::Abort) => { - let cs_bb = + let cs_llbb = Bx::append_block(self.cx, self.llfn, &format!("cs_funclet{:?}", bb)); - let cp_bb = + let cp_llbb = Bx::append_block(self.cx, self.llfn, &format!("cp_funclet{:?}", bb)); - ret_llbb = cs_bb; + ret_llbb = cs_llbb; - let mut cs_bx = Bx::build(self.cx, cs_bb); - let cs = cs_bx.catch_switch(None, None, &[cp_bb]); + let mut cs_bx = Bx::build(self.cx, cs_llbb); + let cs = cs_bx.catch_switch(None, None, &[cp_llbb]); // The "null" here is actually a RTTI type descriptor for the // C++ personality function, but `catch (...)` has no type so // it's null. The 64 here is actually a bitfield which // represents that this is a catch-all block. - let mut cp_bx = Bx::build(self.cx, cp_bb); + let mut cp_bx = Bx::build(self.cx, cp_llbb); let null = cp_bx.const_null( cp_bx.type_i8p_ext(cp_bx.cx().data_layout().instruction_address_space), ); @@ -1481,10 +1481,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { cp_bx.br(llbb); } _ => { - let cleanup_bb = + let cleanup_llbb = Bx::append_block(self.cx, self.llfn, &format!("funclet_{:?}", bb)); - ret_llbb = cleanup_bb; - let mut cleanup_bx = Bx::build(self.cx, cleanup_bb); + ret_llbb = cleanup_llbb; + let mut cleanup_bx = Bx::build(self.cx, cleanup_llbb); funclet = cleanup_bx.cleanup_pad(None, &[]); cleanup_bx.br(llbb); } @@ -1492,19 +1492,20 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { self.funclets[bb] = Some(funclet); ret_llbb } else { - let bb = Bx::append_block(self.cx, self.llfn, "cleanup"); - let mut bx = Bx::build(self.cx, bb); + let cleanup_llbb = Bx::append_block(self.cx, self.llfn, "cleanup"); + let mut cleanup_bx = Bx::build(self.cx, cleanup_llbb); let llpersonality = self.cx.eh_personality(); let llretty = self.landing_pad_type(); - let lp = bx.cleanup_landing_pad(llretty, llpersonality); + let lp = cleanup_bx.cleanup_landing_pad(llretty, llpersonality); - let slot = self.get_personality_slot(&mut bx); - slot.storage_live(&mut bx); - Pair(bx.extract_value(lp, 0), bx.extract_value(lp, 1)).store(&mut bx, slot); + let slot = self.get_personality_slot(&mut cleanup_bx); + slot.storage_live(&mut cleanup_bx); + Pair(cleanup_bx.extract_value(lp, 0), cleanup_bx.extract_value(lp, 1)) + .store(&mut cleanup_bx, slot); - bx.br(llbb); - bx.llbb() + cleanup_bx.br(llbb); + cleanup_llbb } } diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 2b931bfc91d..da9aaf00ecf 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -148,10 +148,10 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( let debug_context = cx.create_function_debug_context(instance, &fn_abi, llfn, &mir); let start_llbb = Bx::append_block(cx, llfn, "start"); - let mut bx = Bx::build(cx, start_llbb); + let mut start_bx = Bx::build(cx, start_llbb); if mir.basic_blocks.iter().any(|bb| bb.is_cleanup) { - bx.set_personality_fn(cx.eh_personality()); + start_bx.set_personality_fn(cx.eh_personality()); } let cleanup_kinds = analyze::cleanup_kinds(&mir); @@ -180,7 +180,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( caller_location: None, }; - fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut bx); + fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut start_bx); // Evaluate all required consts; codegen later assumes that CTFE will never fail. let mut all_consts_ok = true; @@ -206,29 +206,29 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( // Allocate variable and temp allocas fx.locals = { - let args = arg_local_refs(&mut bx, &mut fx, &memory_locals); + let args = arg_local_refs(&mut start_bx, &mut fx, &memory_locals); let mut allocate_local = |local| { let decl = &mir.local_decls[local]; - let layout = bx.layout_of(fx.monomorphize(decl.ty)); + let layout = start_bx.layout_of(fx.monomorphize(decl.ty)); assert!(!layout.ty.has_erasable_regions()); if local == mir::RETURN_PLACE && fx.fn_abi.ret.is_indirect() { debug!("alloc: {:?} (return place) -> place", local); - let llretptr = bx.get_param(0); + let llretptr = start_bx.get_param(0); return LocalRef::Place(PlaceRef::new_sized(llretptr, layout)); } if memory_locals.contains(local) { debug!("alloc: {:?} -> place", local); if layout.is_unsized() { - LocalRef::UnsizedPlace(PlaceRef::alloca_unsized_indirect(&mut bx, layout)) + LocalRef::UnsizedPlace(PlaceRef::alloca_unsized_indirect(&mut start_bx, layout)) } else { - LocalRef::Place(PlaceRef::alloca(&mut bx, layout)) + LocalRef::Place(PlaceRef::alloca(&mut start_bx, layout)) } } else { debug!("alloc: {:?} -> operand", local); - LocalRef::new_operand(&mut bx, layout) + LocalRef::new_operand(&mut start_bx, layout) } }; @@ -240,7 +240,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( }; // Apply debuginfo to the newly allocated locals. - fx.debug_introduce_locals(&mut bx); + fx.debug_introduce_locals(&mut start_bx); // Codegen the body of each block using reverse postorder for (bb, _) in traversal::reverse_postorder(&mir) { From a5bd5da59479e1581b9cb76d3d05a9b85c9b02fc Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Oct 2022 10:53:38 +1100 Subject: [PATCH 10/13] Rename two `TerminatorCodegenHelper` methods. `TerminatorCodegenHelper` has three methods `llblock`, `llbb`, and `lltarget`. They're all similar, but the names given no indication of the differences. This commit renames `lltarget` as `llbb_with_landing_pad`, and `llblock` as `llbb_with_cleanup`. These aren't fantastic names, but at least it's now clear that `llbb` is the lowest-level of the three and the other two wrap it. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 29 ++++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index e7abae665e3..63fafeeefda 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -63,7 +63,9 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { } } - fn lltarget>( + /// Get a basic block (creating it if necessary), possibly with a landing + /// pad next to it. + fn llbb_with_landing_pad>( &self, fx: &mut FunctionCx<'a, 'tcx, Bx>, target: mir::BasicBlock, @@ -83,17 +85,18 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { } } - /// Create a basic block. - fn llblock>( + /// Get a basic block (creating it if necessary), possibly with cleanup + /// stuff in it or next to it. + fn llbb_with_cleanup>( &self, fx: &mut FunctionCx<'a, 'tcx, Bx>, target: mir::BasicBlock, ) -> Bx::BasicBlock { - let (lltarget, is_cleanupret) = self.lltarget(fx, target); + let (lltarget, is_cleanupret) = self.llbb_with_landing_pad(fx, target); if is_cleanupret { // MSVC cross-funclet jump - need a trampoline - debug!("llblock: creating cleanup trampoline for {:?}", target); + debug!("llbb_with_cleanup: creating cleanup trampoline for {:?}", target); let name = &format!("{:?}_cleanup_trampoline_{:?}", self.bb, target); let trampoline_llbb = Bx::append_block(fx.cx, fx.llfn, name); let mut trampoline_bx = Bx::build(fx.cx, trampoline_llbb); @@ -110,7 +113,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { bx: &mut Bx, target: mir::BasicBlock, ) { - let (lltarget, is_cleanupret) = self.lltarget(fx, target); + let (lltarget, is_cleanupret) = self.llbb_with_landing_pad(fx, target); if is_cleanupret { // micro-optimization: generate a `ret` rather than a jump // to a trampoline. @@ -138,7 +141,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { let fn_ty = bx.fn_decl_backend_type(&fn_abi); let unwind_block = if let Some(cleanup) = cleanup.filter(|_| fn_abi.can_unwind) { - Some(self.llblock(fx, cleanup)) + Some(self.llbb_with_cleanup(fx, cleanup)) } else if fx.mir[self.bb].is_cleanup && fn_abi.can_unwind && !base::wants_msvc_seh(fx.cx.tcx().sess) @@ -231,7 +234,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { options, line_spans, instance, - Some((ret_llbb, self.llblock(fx, cleanup), self.funclet(fx))), + Some((ret_llbb, self.llbb_with_cleanup(fx, cleanup), self.funclet(fx))), ); } else { bx.codegen_inline_asm(template, &operands, options, line_spans, instance, None); @@ -281,8 +284,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { if target_iter.len() == 1 { // If there are two targets (one conditional, one fallback), emit br instead of switch let (test_value, target) = target_iter.next().unwrap(); - let lltrue = helper.llblock(self, target); - let llfalse = helper.llblock(self, targets.otherwise()); + let lltrue = helper.llbb_with_cleanup(self, target); + let llfalse = helper.llbb_with_cleanup(self, targets.otherwise()); if switch_ty == bx.tcx().types.bool { // Don't generate trivial icmps when switching on bool match test_value { @@ -299,8 +302,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } else { bx.switch( discr.immediate(), - helper.llblock(self, targets.otherwise()), - target_iter.map(|(value, target)| (value, helper.llblock(self, target))), + helper.llbb_with_cleanup(self, targets.otherwise()), + target_iter.map(|(value, target)| (value, helper.llbb_with_cleanup(self, target))), ); } } @@ -530,7 +533,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let cond = bx.expect(cond, expected); // Create the failure block and the conditional branch to it. - let lltarget = helper.llblock(self, target); + let lltarget = helper.llbb_with_cleanup(self, target); let panic_block = bx.append_sibling_block("panic"); if expected { bx.cond_br(cond, lltarget, panic_block); From 03f350f5a5464e19484ab17f6a745c48d98d3415 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Oct 2022 10:59:02 +1100 Subject: [PATCH 11/13] Clarify some cleanup stuff. - Rearrange the match in `llbb_with_landing_pad` so the `(Some,Some)` cases are together. - Add assertions to indicate two MSVC-only paths. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 63fafeeefda..29b7c9b0a88 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -75,13 +75,16 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { let target_funclet = fx.cleanup_kinds[target].funclet_bb(target); match (self.funclet_bb, target_funclet) { (None, None) => (lltarget, false), - (Some(f), Some(t_f)) if f == t_f || !base::wants_msvc_seh(fx.cx.tcx().sess) => { - (lltarget, false) - } // jump *into* cleanup - need a landing pad if GNU, cleanup pad if MSVC (None, Some(_)) => (fx.landing_pad_for(target), false), (Some(_), None) => span_bug!(span, "{:?} - jump out of cleanup?", self.terminator), - (Some(_), Some(_)) => (fx.landing_pad_for(target), true), + (Some(f), Some(t_f)) => { + if f == t_f || !base::wants_msvc_seh(fx.cx.tcx().sess) { + (lltarget, false) + } else { + (fx.landing_pad_for(target), true) + } + } } } @@ -95,7 +98,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { let (lltarget, is_cleanupret) = self.llbb_with_landing_pad(fx, target); if is_cleanupret { // MSVC cross-funclet jump - need a trampoline - + debug_assert!(base::wants_msvc_seh(fx.cx.tcx().sess)); debug!("llbb_with_cleanup: creating cleanup trampoline for {:?}", target); let name = &format!("{:?}_cleanup_trampoline_{:?}", self.bb, target); let trampoline_llbb = Bx::append_block(fx.cx, fx.llfn, name); @@ -115,8 +118,9 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { ) { let (lltarget, is_cleanupret) = self.llbb_with_landing_pad(fx, target); if is_cleanupret { - // micro-optimization: generate a `ret` rather than a jump + // MSVC micro-optimization: generate a `ret` rather than a jump // to a trampoline. + debug_assert!(base::wants_msvc_seh(fx.cx.tcx().sess)); bx.cleanup_ret(self.funclet(fx).unwrap(), Some(lltarget)); } else { bx.br(lltarget); From 8c02f4d05d8840e387a97faad577187e80d3da88 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Oct 2022 13:51:52 +1100 Subject: [PATCH 12/13] Inline and remove `cast_shift_rhs`. It has a single call site. --- compiler/rustc_codegen_ssa/src/base.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index ff1eee37ad9..09f803ad88f 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -340,15 +340,6 @@ pub fn cast_shift_expr_rhs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( op: hir::BinOpKind, lhs: Bx::Value, rhs: Bx::Value, -) -> Bx::Value { - cast_shift_rhs(bx, op, lhs, rhs) -} - -fn cast_shift_rhs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( - bx: &mut Bx, - op: hir::BinOpKind, - lhs: Bx::Value, - rhs: Bx::Value, ) -> Bx::Value { // Shifts may have any size int on the rhs if op.is_shift() { From 6cd35ac2035327339d2428ff0be695ed5d5681dc Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Oct 2022 14:39:20 +1100 Subject: [PATCH 13/13] Simplify `cast_shift_expr_rhs`. It's only ever used with shift operators. --- compiler/rustc_codegen_ssa/src/base.rs | 37 ++++++++++-------------- compiler/rustc_codegen_ssa/src/common.rs | 5 ++-- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index 09f803ad88f..84b89cd71a6 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -337,31 +337,26 @@ pub fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( pub fn cast_shift_expr_rhs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( bx: &mut Bx, - op: hir::BinOpKind, lhs: Bx::Value, rhs: Bx::Value, ) -> Bx::Value { // Shifts may have any size int on the rhs - if op.is_shift() { - let mut rhs_llty = bx.cx().val_ty(rhs); - let mut lhs_llty = bx.cx().val_ty(lhs); - if bx.cx().type_kind(rhs_llty) == TypeKind::Vector { - rhs_llty = bx.cx().element_type(rhs_llty) - } - if bx.cx().type_kind(lhs_llty) == TypeKind::Vector { - lhs_llty = bx.cx().element_type(lhs_llty) - } - let rhs_sz = bx.cx().int_width(rhs_llty); - let lhs_sz = bx.cx().int_width(lhs_llty); - if lhs_sz < rhs_sz { - bx.trunc(rhs, lhs_llty) - } else if lhs_sz > rhs_sz { - // FIXME (#1877: If in the future shifting by negative - // values is no longer undefined then this is wrong. - bx.zext(rhs, lhs_llty) - } else { - rhs - } + let mut rhs_llty = bx.cx().val_ty(rhs); + let mut lhs_llty = bx.cx().val_ty(lhs); + if bx.cx().type_kind(rhs_llty) == TypeKind::Vector { + rhs_llty = bx.cx().element_type(rhs_llty) + } + if bx.cx().type_kind(lhs_llty) == TypeKind::Vector { + lhs_llty = bx.cx().element_type(lhs_llty) + } + let rhs_sz = bx.cx().int_width(rhs_llty); + let lhs_sz = bx.cx().int_width(lhs_llty); + if lhs_sz < rhs_sz { + bx.trunc(rhs, lhs_llty) + } else if lhs_sz > rhs_sz { + // FIXME (#1877: If in the future shifting by negative + // values is no longer undefined then this is wrong. + bx.zext(rhs, lhs_llty) } else { rhs } diff --git a/compiler/rustc_codegen_ssa/src/common.rs b/compiler/rustc_codegen_ssa/src/common.rs index 8ca1a6084cf..71f9179d02c 100644 --- a/compiler/rustc_codegen_ssa/src/common.rs +++ b/compiler/rustc_codegen_ssa/src/common.rs @@ -1,7 +1,6 @@ #![allow(non_camel_case_types)] use rustc_errors::struct_span_err; -use rustc_hir as hir; use rustc_hir::LangItem; use rustc_middle::mir::interpret::ConstValue; use rustc_middle::ty::{self, layout::TyAndLayout, Ty, TyCtxt}; @@ -140,7 +139,7 @@ pub fn build_unchecked_lshift<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( lhs: Bx::Value, rhs: Bx::Value, ) -> Bx::Value { - let rhs = base::cast_shift_expr_rhs(bx, hir::BinOpKind::Shl, lhs, rhs); + let rhs = base::cast_shift_expr_rhs(bx, lhs, rhs); // #1877, #10183: Ensure that input is always valid let rhs = shift_mask_rhs(bx, rhs); bx.shl(lhs, rhs) @@ -152,7 +151,7 @@ pub fn build_unchecked_rshift<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( lhs: Bx::Value, rhs: Bx::Value, ) -> Bx::Value { - let rhs = base::cast_shift_expr_rhs(bx, hir::BinOpKind::Shr, lhs, rhs); + let rhs = base::cast_shift_expr_rhs(bx, lhs, rhs); // #1877, #10183: Ensure that input is always valid let rhs = shift_mask_rhs(bx, rhs); let is_signed = lhs_t.is_signed();