Auto merge of rust-lang#123910 - matthiaskrgr:rollup-8imjhnf, r=matth…

…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#123651 (Thread local updates for idiomatic examples)
 - rust-lang#123699 (run-make-support: tidy up support library)
 - rust-lang#123779 (OpenBSD fix long socket addresses)
 - rust-lang#123803 (Fix `VecDeque::shrink_to` UB when `handle_alloc_error` unwinds.)
 - rust-lang#123875 (Doc: replace x with y for hexa-decimal fmt)
 - rust-lang#123879 (Add missing `unsafe` to some internal `std` functions)
 - rust-lang#123889 (reduce tidy overheads in run-make checks)
 - rust-lang#123898 (Generic associated consts: Check regions earlier when comparing impl with trait item def)
 - rust-lang#123902 (compiletest: Update rustfix to 0.8.1)

r? `@ghost`
`@rustbot` modify labels: rollup

Cargo.lock

@@ -766,7 +766,7 @@ dependencies = [
  "miropt-test-tools",
  "once_cell",
  "regex",
- "rustfix",
+ "rustfix 0.8.1",
  "serde",
  "serde_json",
  "tracing",
@@ -4855,6 +4855,18 @@ dependencies = [
  "serde_json",
 ]
 
+[[package]]
+name = "rustfix"
+version = "0.8.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "81864b097046da5df3758fdc6e4822bbb70afa06317e8ca45ea1b51cb8c5e5a4"
+dependencies = [
+ "serde",
+ "serde_json",
+ "thiserror",
+ "tracing",
+]
+
 [[package]]
 name = "rustfmt-config_proc_macro"
 version = "0.3.0"
@@ -5896,7 +5908,7 @@ dependencies = [
  "prettydiff",
  "regex",
  "rustc_version",
- "rustfix",
+ "rustfix 0.6.1",
  "serde",
  "serde_json",
  "tempfile",
@@ -5923,7 +5935,7 @@ dependencies = [
  "prettydiff",
  "regex",
  "rustc_version",
- "rustfix",
+ "rustfix 0.6.1",
  "serde",
  "serde_json",
  "spanned",

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -1723,6 +1723,7 @@ pub(super) fn compare_impl_const_raw(
 
     compare_number_of_generics(tcx, impl_const_item, trait_const_item, false)?;
     compare_generic_param_kinds(tcx, impl_const_item, trait_const_item, false)?;
+    check_region_bounds_on_impl_item(tcx, impl_const_item, trait_const_item, false)?;
     compare_const_predicate_entailment(tcx, impl_const_item, trait_const_item, impl_trait_ref)
 }
 
@@ -1763,8 +1764,6 @@ fn compare_const_predicate_entailment<'tcx>(
     let impl_ct_predicates = tcx.predicates_of(impl_ct.def_id);
     let trait_ct_predicates = tcx.predicates_of(trait_ct.def_id);
 
-    check_region_bounds_on_impl_item(tcx, impl_ct, trait_ct, false)?;
-
     // The predicates declared by the impl definition, the trait and the
     // associated const in the trait are assumed.
     let impl_predicates = tcx.predicates_of(impl_ct_predicates.parent.unwrap());
@@ -1866,6 +1865,7 @@ pub(super) fn compare_impl_ty<'tcx>(
     let _: Result<(), ErrorGuaranteed> = try {
         compare_number_of_generics(tcx, impl_ty, trait_ty, false)?;
         compare_generic_param_kinds(tcx, impl_ty, trait_ty, false)?;
+        check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?;
         compare_type_predicate_entailment(tcx, impl_ty, trait_ty, impl_trait_ref)?;
         check_type_bounds(tcx, trait_ty, impl_ty, impl_trait_ref)?;
     };
@@ -1886,8 +1886,6 @@ fn compare_type_predicate_entailment<'tcx>(
     let impl_ty_predicates = tcx.predicates_of(impl_ty.def_id);
     let trait_ty_predicates = tcx.predicates_of(trait_ty.def_id);
 
-    check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?;
-
     let impl_ty_own_bounds = impl_ty_predicates.instantiate_own(tcx, impl_args);
     if impl_ty_own_bounds.len() == 0 {
         // Nothing to check.

library/alloc/src/collections/vec_deque/mod.rs

@@ -978,6 +978,8 @@ impl<T, A: Allocator> VecDeque<T, A> {
         // `head` and `len` are at most `isize::MAX` and `target_cap < self.capacity()`, so nothing can
         // overflow.
         let tail_outside = (target_cap + 1..=self.capacity()).contains(&(self.head + self.len));
+        // Used in the drop guard below.
+        let old_head = self.head;
 
         if self.len == 0 {
             self.head = 0;
@@ -1030,12 +1032,74 @@ impl<T, A: Allocator> VecDeque<T, A> {
             }
             self.head = new_head;
         }
-        self.buf.shrink_to_fit(target_cap);
+
+        struct Guard<'a, T, A: Allocator> {
+            deque: &'a mut VecDeque<T, A>,
+            old_head: usize,
+            target_cap: usize,
+        }
+
+        impl<T, A: Allocator> Drop for Guard<'_, T, A> {
+            #[cold]
+            fn drop(&mut self) {
+                unsafe {
+                    // SAFETY: This is only called if `buf.shrink_to_fit` unwinds,
+                    // which is the only time it's safe to call `abort_shrink`.
+                    self.deque.abort_shrink(self.old_head, self.target_cap)
+                }
+            }
+        }
+
+        let guard = Guard { deque: self, old_head, target_cap };
+
+        guard.deque.buf.shrink_to_fit(target_cap);
+
+        // Don't drop the guard if we didn't unwind.
+        mem::forget(guard);
 
         debug_assert!(self.head < self.capacity() || self.capacity() == 0);
         debug_assert!(self.len <= self.capacity());
     }
 
+    /// Reverts the deque back into a consistent state in case `shrink_to` failed.
+    /// This is necessary to prevent UB if the backing allocator returns an error
+    /// from `shrink` and `handle_alloc_error` subsequently unwinds (see #123369).
+    ///
+    /// `old_head` refers to the head index before `shrink_to` was called. `target_cap`
+    /// is the capacity that it was trying to shrink to.
+    unsafe fn abort_shrink(&mut self, old_head: usize, target_cap: usize) {
+        // Moral equivalent of self.head + self.len <= target_cap. Won't overflow
+        // because `self.len <= target_cap`.
+        if self.head <= target_cap - self.len {
+            // The deque's buffer is contiguous, so no need to copy anything around.
+            return;
+        }
+
+        // `shrink_to` already copied the head to fit into the new capacity, so this won't overflow.
+        let head_len = target_cap - self.head;
+        // `self.head > target_cap - self.len` => `self.len > target_cap - self.head =: head_len` so this must be positive.
+        let tail_len = self.len - head_len;
+
+        if tail_len <= cmp::min(head_len, self.capacity() - target_cap) {
+            // There's enough spare capacity to copy the tail to the back (because `tail_len < self.capacity() - target_cap`),
+            // and copying the tail should be cheaper than copying the head (because `tail_len <= head_len`).
+
+            unsafe {
+                // The old tail and the new tail can't overlap because the head slice lies between them. The
+                // head slice ends at `target_cap`, so that's where we copy to.
+                self.copy_nonoverlapping(0, target_cap, tail_len);
+            }
+        } else {
+            // Either there's not enough spare capacity to make the deque contiguous, or the head is shorter than the tail
+            // (and therefore hopefully cheaper to copy).
+            unsafe {
+                // The old and the new head slice can overlap, so we can't use `copy_nonoverlapping` here.
+                self.copy(self.head, old_head, head_len);
+                self.head = old_head;
+            }
+        }
+    }
+
     /// Shortens the deque, keeping the first `len` elements and dropping
     /// the rest.
     ///

library/alloc/src/collections/vec_deque/tests.rs

@@ -1,4 +1,11 @@
-use core::iter::TrustedLen;
+#![feature(alloc_error_hook)]
+
+use crate::alloc::{AllocError, Layout};
+use core::{iter::TrustedLen, ptr::NonNull};
+use std::{
+    alloc::{set_alloc_error_hook, take_alloc_error_hook, System},
+    panic::{catch_unwind, AssertUnwindSafe},
+};
 
 use super::*;
 
@@ -790,6 +797,52 @@ fn test_shrink_to() {
     }
 }
 
+#[test]
+fn test_shrink_to_unwind() {
+    // This tests that `shrink_to` leaves the deque in a consistent state when
+    // the call to `RawVec::shrink_to_fit` unwinds. The code is adapted from #123369
+    // but changed to hopefully not have any UB even if the test fails.
+
+    struct BadAlloc;
+
+    unsafe impl Allocator for BadAlloc {
+        fn allocate(&self, l: Layout) -> Result<NonNull<[u8]>, AllocError> {
+            // We allocate zeroed here so that the whole buffer of the deque
+            // is always initialized. That way, even if the deque is left in
+            // an inconsistent state, no uninitialized memory should be accessed.
+            System.allocate_zeroed(l)
+        }
+
+        unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
+            unsafe { System.deallocate(ptr, layout) }
+        }
+
+        unsafe fn shrink(
+            &self,
+            _ptr: NonNull<u8>,
+            _old_layout: Layout,
+            _new_layout: Layout,
+        ) -> Result<NonNull<[u8]>, AllocError> {
+            Err(AllocError)
+        }
+    }
+
+    // preserve the old error hook just in case.
+    let old_error_hook = take_alloc_error_hook();
+    set_alloc_error_hook(|_| panic!("alloc error"));
+
+    let mut v = VecDeque::with_capacity_in(15, BadAlloc);
+    v.push_back(1);
+    v.push_front(2);
+    // This should unwind because it calls `BadAlloc::shrink` and then `handle_alloc_error` which unwinds.
+    assert!(catch_unwind(AssertUnwindSafe(|| v.shrink_to_fit())).is_err());
+    // This should only pass if the deque is left in a consistent state.
+    assert_eq!(v, [2, 1]);
+
+    // restore the old error hook.
+    set_alloc_error_hook(old_error_hook);
+}
+
 #[test]
 fn test_shrink_to_fit() {
     // This test checks that every single combination of head and tail position,

library/alloc/src/lib.rs

@@ -92,6 +92,7 @@
 // tidy-alphabetical-start
 #![cfg_attr(not(no_global_oom_handling), feature(const_alloc_error))]
 #![cfg_attr(not(no_global_oom_handling), feature(const_btree_len))]
+#![cfg_attr(test, feature(alloc_error_hook))]
 #![cfg_attr(test, feature(is_sorted))]
 #![cfg_attr(test, feature(new_uninit))]
 #![feature(alloc_layout_extra)]

library/core/src/fmt/mod.rs

@@ -860,10 +860,10 @@ pub trait Binary {
 /// Basic usage with `i32`:
 ///
 /// ```
-/// let x = 42; // 42 is '2a' in hex
+/// let y = 42; // 42 is '2a' in hex
 ///
-/// assert_eq!(format!("{x:x}"), "2a");
-/// assert_eq!(format!("{x:#x}"), "0x2a");
+/// assert_eq!(format!("{y:x}"), "2a");
+/// assert_eq!(format!("{y:#x}"), "0x2a");
 ///
 /// assert_eq!(format!("{:x}", -16), "fffffff0");
 /// ```
@@ -915,10 +915,10 @@ pub trait LowerHex {
 /// Basic usage with `i32`:
 ///
 /// ```
-/// let x = 42; // 42 is '2A' in hex
+/// let y = 42; // 42 is '2A' in hex
 ///
-/// assert_eq!(format!("{x:X}"), "2A");
-/// assert_eq!(format!("{x:#X}"), "0x2A");
+/// assert_eq!(format!("{y:X}"), "2A");
+/// assert_eq!(format!("{y:#X}"), "0x2A");
 ///
 /// assert_eq!(format!("{:X}", -16), "FFFFFFF0");
 /// ```

library/std/src/os/unix/net/addr.rs

@@ -107,6 +107,16 @@ impl SocketAddr {
         addr: libc::sockaddr_un,
         mut len: libc::socklen_t,
     ) -> io::Result<SocketAddr> {
+        if cfg!(target_os = "openbsd") {
+            // on OpenBSD, getsockname(2) returns the actual size of the socket address,
+            // and not the len of the content. Figure out the length for ourselves.
+            // https://marc.info/?l=openbsd-bugs&m=170105481926736&w=2
+            let sun_path: &[u8] =
+                unsafe { mem::transmute::<&[libc::c_char], &[u8]>(&addr.sun_path) };
+            len = core::slice::memchr::memchr(0, sun_path)
+                .map_or(len, |new_len| (new_len + sun_path_offset(&addr)) as libc::socklen_t);
+        }
+
         if len == 0 {
             // When there is a datagram from unnamed unix socket
             // linux returns zero bytes of address

library/std/src/sys/pal/unix/thread.rs

@@ -709,7 +709,7 @@ mod cgroups {
 // is created in an application with big thread-local storage requirements.
 // See #6233 for rationale and details.
 #[cfg(all(target_os = "linux", target_env = "gnu"))]
-fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize {
+unsafe fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize {
     // We use dlsym to avoid an ELF version dependency on GLIBC_PRIVATE. (#23628)
     // We shouldn't really be using such an internal symbol, but there's currently
     // no other way to account for the TLS size.
@@ -723,11 +723,11 @@ fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize {
 
 // No point in looking up __pthread_get_minstack() on non-glibc platforms.
 #[cfg(all(not(all(target_os = "linux", target_env = "gnu")), not(target_os = "netbsd")))]
-fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
+unsafe fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
     libc::PTHREAD_STACK_MIN
 }
 
 #[cfg(target_os = "netbsd")]
-fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
+unsafe fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
     2048 // just a guess
 }

library/std/src/sys/sync/rwlock/queue.rs

@@ -202,7 +202,7 @@ impl Node {
     fn prepare(&mut self) {
         // Fall back to creating an unnamed `Thread` handle to allow locking in
         // TLS destructors.
-        self.thread.get_or_init(|| thread::try_current().unwrap_or_else(|| Thread::new(None)));
+        self.thread.get_or_init(|| thread::try_current().unwrap_or_else(Thread::new_unnamed));
         self.completed = AtomicBool::new(false);
     }