Skip to content

Commit 9524b6e

Browse files
committed
Auto merge of #123910 - matthiaskrgr:rollup-8imjhnf, r=matthiaskrgr
Rollup of 9 pull requests Successful merges: - #123651 (Thread local updates for idiomatic examples) - #123699 (run-make-support: tidy up support library) - #123779 (OpenBSD fix long socket addresses) - #123803 (Fix `VecDeque::shrink_to` UB when `handle_alloc_error` unwinds.) - #123875 (Doc: replace x with y for hexa-decimal fmt) - #123879 (Add missing `unsafe` to some internal `std` functions) - #123889 (reduce tidy overheads in run-make checks) - #123898 (Generic associated consts: Check regions earlier when comparing impl with trait item def) - #123902 (compiletest: Update rustfix to 0.8.1) r? `@ghost` `@rustbot` modify labels: rollup
2 parents f3c6608 + 504f534 commit 9524b6e

File tree

20 files changed

+409
-208
lines changed

20 files changed

+409
-208
lines changed

Cargo.lock

+15-3
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ dependencies = [
766766
"miropt-test-tools",
767767
"once_cell",
768768
"regex",
769-
"rustfix",
769+
"rustfix 0.8.1",
770770
"serde",
771771
"serde_json",
772772
"tracing",
@@ -4855,6 +4855,18 @@ dependencies = [
48554855
"serde_json",
48564856
]
48574857

4858+
[[package]]
4859+
name = "rustfix"
4860+
version = "0.8.1"
4861+
source = "registry+https://github.com/rust-lang/crates.io-index"
4862+
checksum = "81864b097046da5df3758fdc6e4822bbb70afa06317e8ca45ea1b51cb8c5e5a4"
4863+
dependencies = [
4864+
"serde",
4865+
"serde_json",
4866+
"thiserror",
4867+
"tracing",
4868+
]
4869+
48584870
[[package]]
48594871
name = "rustfmt-config_proc_macro"
48604872
version = "0.3.0"
@@ -5896,7 +5908,7 @@ dependencies = [
58965908
"prettydiff",
58975909
"regex",
58985910
"rustc_version",
5899-
"rustfix",
5911+
"rustfix 0.6.1",
59005912
"serde",
59015913
"serde_json",
59025914
"tempfile",
@@ -5923,7 +5935,7 @@ dependencies = [
59235935
"prettydiff",
59245936
"regex",
59255937
"rustc_version",
5926-
"rustfix",
5938+
"rustfix 0.6.1",
59275939
"serde",
59285940
"serde_json",
59295941
"spanned",

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -1723,6 +1723,7 @@ pub(super) fn compare_impl_const_raw(
17231723

17241724
compare_number_of_generics(tcx, impl_const_item, trait_const_item, false)?;
17251725
compare_generic_param_kinds(tcx, impl_const_item, trait_const_item, false)?;
1726+
check_region_bounds_on_impl_item(tcx, impl_const_item, trait_const_item, false)?;
17261727
compare_const_predicate_entailment(tcx, impl_const_item, trait_const_item, impl_trait_ref)
17271728
}
17281729

@@ -1763,8 +1764,6 @@ fn compare_const_predicate_entailment<'tcx>(
17631764
let impl_ct_predicates = tcx.predicates_of(impl_ct.def_id);
17641765
let trait_ct_predicates = tcx.predicates_of(trait_ct.def_id);
17651766

1766-
check_region_bounds_on_impl_item(tcx, impl_ct, trait_ct, false)?;
1767-
17681767
// The predicates declared by the impl definition, the trait and the
17691768
// associated const in the trait are assumed.
17701769
let impl_predicates = tcx.predicates_of(impl_ct_predicates.parent.unwrap());
@@ -1866,6 +1865,7 @@ pub(super) fn compare_impl_ty<'tcx>(
18661865
let _: Result<(), ErrorGuaranteed> = try {
18671866
compare_number_of_generics(tcx, impl_ty, trait_ty, false)?;
18681867
compare_generic_param_kinds(tcx, impl_ty, trait_ty, false)?;
1868+
check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?;
18691869
compare_type_predicate_entailment(tcx, impl_ty, trait_ty, impl_trait_ref)?;
18701870
check_type_bounds(tcx, trait_ty, impl_ty, impl_trait_ref)?;
18711871
};
@@ -1886,8 +1886,6 @@ fn compare_type_predicate_entailment<'tcx>(
18861886
let impl_ty_predicates = tcx.predicates_of(impl_ty.def_id);
18871887
let trait_ty_predicates = tcx.predicates_of(trait_ty.def_id);
18881888

1889-
check_region_bounds_on_impl_item(tcx, impl_ty, trait_ty, false)?;
1890-
18911889
let impl_ty_own_bounds = impl_ty_predicates.instantiate_own(tcx, impl_args);
18921890
if impl_ty_own_bounds.len() == 0 {
18931891
// Nothing to check.

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

+65-1
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,8 @@ impl<T, A: Allocator> VecDeque<T, A> {
978978
// `head` and `len` are at most `isize::MAX` and `target_cap < self.capacity()`, so nothing can
979979
// overflow.
980980
let tail_outside = (target_cap + 1..=self.capacity()).contains(&(self.head + self.len));
981+
// Used in the drop guard below.
982+
let old_head = self.head;
981983

982984
if self.len == 0 {
983985
self.head = 0;
@@ -1030,12 +1032,74 @@ impl<T, A: Allocator> VecDeque<T, A> {
10301032
}
10311033
self.head = new_head;
10321034
}
1033-
self.buf.shrink_to_fit(target_cap);
1035+
1036+
struct Guard<'a, T, A: Allocator> {
1037+
deque: &'a mut VecDeque<T, A>,
1038+
old_head: usize,
1039+
target_cap: usize,
1040+
}
1041+
1042+
impl<T, A: Allocator> Drop for Guard<'_, T, A> {
1043+
#[cold]
1044+
fn drop(&mut self) {
1045+
unsafe {
1046+
// SAFETY: This is only called if `buf.shrink_to_fit` unwinds,
1047+
// which is the only time it's safe to call `abort_shrink`.
1048+
self.deque.abort_shrink(self.old_head, self.target_cap)
1049+
}
1050+
}
1051+
}
1052+
1053+
let guard = Guard { deque: self, old_head, target_cap };
1054+
1055+
guard.deque.buf.shrink_to_fit(target_cap);
1056+
1057+
// Don't drop the guard if we didn't unwind.
1058+
mem::forget(guard);
10341059

10351060
debug_assert!(self.head < self.capacity() || self.capacity() == 0);
10361061
debug_assert!(self.len <= self.capacity());
10371062
}
10381063

1064+
/// Reverts the deque back into a consistent state in case `shrink_to` failed.
1065+
/// This is necessary to prevent UB if the backing allocator returns an error
1066+
/// from `shrink` and `handle_alloc_error` subsequently unwinds (see #123369).
1067+
///
1068+
/// `old_head` refers to the head index before `shrink_to` was called. `target_cap`
1069+
/// is the capacity that it was trying to shrink to.
1070+
unsafe fn abort_shrink(&mut self, old_head: usize, target_cap: usize) {
1071+
// Moral equivalent of self.head + self.len <= target_cap. Won't overflow
1072+
// because `self.len <= target_cap`.
1073+
if self.head <= target_cap - self.len {
1074+
// The deque's buffer is contiguous, so no need to copy anything around.
1075+
return;
1076+
}
1077+
1078+
// `shrink_to` already copied the head to fit into the new capacity, so this won't overflow.
1079+
let head_len = target_cap - self.head;
1080+
// `self.head > target_cap - self.len` => `self.len > target_cap - self.head =: head_len` so this must be positive.
1081+
let tail_len = self.len - head_len;
1082+
1083+
if tail_len <= cmp::min(head_len, self.capacity() - target_cap) {
1084+
// There's enough spare capacity to copy the tail to the back (because `tail_len < self.capacity() - target_cap`),
1085+
// and copying the tail should be cheaper than copying the head (because `tail_len <= head_len`).
1086+
1087+
unsafe {
1088+
// The old tail and the new tail can't overlap because the head slice lies between them. The
1089+
// head slice ends at `target_cap`, so that's where we copy to.
1090+
self.copy_nonoverlapping(0, target_cap, tail_len);
1091+
}
1092+
} else {
1093+
// Either there's not enough spare capacity to make the deque contiguous, or the head is shorter than the tail
1094+
// (and therefore hopefully cheaper to copy).
1095+
unsafe {
1096+
// The old and the new head slice can overlap, so we can't use `copy_nonoverlapping` here.
1097+
self.copy(self.head, old_head, head_len);
1098+
self.head = old_head;
1099+
}
1100+
}
1101+
}
1102+
10391103
/// Shortens the deque, keeping the first `len` elements and dropping
10401104
/// the rest.
10411105
///

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

+54-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
use core::iter::TrustedLen;
1+
#![feature(alloc_error_hook)]
2+
3+
use crate::alloc::{AllocError, Layout};
4+
use core::{iter::TrustedLen, ptr::NonNull};
5+
use std::{
6+
alloc::{set_alloc_error_hook, take_alloc_error_hook, System},
7+
panic::{catch_unwind, AssertUnwindSafe},
8+
};
29

310
use super::*;
411

@@ -790,6 +797,52 @@ fn test_shrink_to() {
790797
}
791798
}
792799

800+
#[test]
801+
fn test_shrink_to_unwind() {
802+
// This tests that `shrink_to` leaves the deque in a consistent state when
803+
// the call to `RawVec::shrink_to_fit` unwinds. The code is adapted from #123369
804+
// but changed to hopefully not have any UB even if the test fails.
805+
806+
struct BadAlloc;
807+
808+
unsafe impl Allocator for BadAlloc {
809+
fn allocate(&self, l: Layout) -> Result<NonNull<[u8]>, AllocError> {
810+
// We allocate zeroed here so that the whole buffer of the deque
811+
// is always initialized. That way, even if the deque is left in
812+
// an inconsistent state, no uninitialized memory should be accessed.
813+
System.allocate_zeroed(l)
814+
}
815+
816+
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
817+
unsafe { System.deallocate(ptr, layout) }
818+
}
819+
820+
unsafe fn shrink(
821+
&self,
822+
_ptr: NonNull<u8>,
823+
_old_layout: Layout,
824+
_new_layout: Layout,
825+
) -> Result<NonNull<[u8]>, AllocError> {
826+
Err(AllocError)
827+
}
828+
}
829+
830+
// preserve the old error hook just in case.
831+
let old_error_hook = take_alloc_error_hook();
832+
set_alloc_error_hook(|_| panic!("alloc error"));
833+
834+
let mut v = VecDeque::with_capacity_in(15, BadAlloc);
835+
v.push_back(1);
836+
v.push_front(2);
837+
// This should unwind because it calls `BadAlloc::shrink` and then `handle_alloc_error` which unwinds.
838+
assert!(catch_unwind(AssertUnwindSafe(|| v.shrink_to_fit())).is_err());
839+
// This should only pass if the deque is left in a consistent state.
840+
assert_eq!(v, [2, 1]);
841+
842+
// restore the old error hook.
843+
set_alloc_error_hook(old_error_hook);
844+
}
845+
793846
#[test]
794847
fn test_shrink_to_fit() {
795848
// This test checks that every single combination of head and tail position,

library/alloc/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
// tidy-alphabetical-start
9393
#![cfg_attr(not(no_global_oom_handling), feature(const_alloc_error))]
9494
#![cfg_attr(not(no_global_oom_handling), feature(const_btree_len))]
95+
#![cfg_attr(test, feature(alloc_error_hook))]
9596
#![cfg_attr(test, feature(is_sorted))]
9697
#![cfg_attr(test, feature(new_uninit))]
9798
#![feature(alloc_layout_extra)]

library/core/src/fmt/mod.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -860,10 +860,10 @@ pub trait Binary {
860860
/// Basic usage with `i32`:
861861
///
862862
/// ```
863-
/// let x = 42; // 42 is '2a' in hex
863+
/// let y = 42; // 42 is '2a' in hex
864864
///
865-
/// assert_eq!(format!("{x:x}"), "2a");
866-
/// assert_eq!(format!("{x:#x}"), "0x2a");
865+
/// assert_eq!(format!("{y:x}"), "2a");
866+
/// assert_eq!(format!("{y:#x}"), "0x2a");
867867
///
868868
/// assert_eq!(format!("{:x}", -16), "fffffff0");
869869
/// ```
@@ -915,10 +915,10 @@ pub trait LowerHex {
915915
/// Basic usage with `i32`:
916916
///
917917
/// ```
918-
/// let x = 42; // 42 is '2A' in hex
918+
/// let y = 42; // 42 is '2A' in hex
919919
///
920-
/// assert_eq!(format!("{x:X}"), "2A");
921-
/// assert_eq!(format!("{x:#X}"), "0x2A");
920+
/// assert_eq!(format!("{y:X}"), "2A");
921+
/// assert_eq!(format!("{y:#X}"), "0x2A");
922922
///
923923
/// assert_eq!(format!("{:X}", -16), "FFFFFFF0");
924924
/// ```

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

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

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,7 @@ mod cgroups {
709709
// is created in an application with big thread-local storage requirements.
710710
// See #6233 for rationale and details.
711711
#[cfg(all(target_os = "linux", target_env = "gnu"))]
712-
fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize {
712+
unsafe fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize {
713713
// We use dlsym to avoid an ELF version dependency on GLIBC_PRIVATE. (#23628)
714714
// We shouldn't really be using such an internal symbol, but there's currently
715715
// no other way to account for the TLS size.
@@ -723,11 +723,11 @@ fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize {
723723

724724
// No point in looking up __pthread_get_minstack() on non-glibc platforms.
725725
#[cfg(all(not(all(target_os = "linux", target_env = "gnu")), not(target_os = "netbsd")))]
726-
fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
726+
unsafe fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
727727
libc::PTHREAD_STACK_MIN
728728
}
729729

730730
#[cfg(target_os = "netbsd")]
731-
fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
731+
unsafe fn min_stack_size(_: *const libc::pthread_attr_t) -> usize {
732732
2048 // just a guess
733733
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ impl Node {
202202
fn prepare(&mut self) {
203203
// Fall back to creating an unnamed `Thread` handle to allow locking in
204204
// TLS destructors.
205-
self.thread.get_or_init(|| thread::try_current().unwrap_or_else(|| Thread::new(None)));
205+
self.thread.get_or_init(|| thread::try_current().unwrap_or_else(Thread::new_unnamed));
206206
self.completed = AtomicBool::new(false);
207207
}
208208

0 commit comments

Comments
 (0)