Skip to content

Commit c07955c

Browse files
committed
Fix overflowing length in Vec<ZST> to VecDeque
`Vec` can hold up to `usize::MAX` ZST items, but `VecDeque` has a lower limit to keep its raw capacity as a power of two, so we should check that in `From<Vec<T>> for VecDeque<T>`. We can also simplify the capacity check for the remaining non-ZST case. Before this fix, the new test would fail on the length: ``` thread 'collections::vec_deque::tests::test_from_vec_zst_overflow' panicked at 'assertion failed: `(left == right)` left: `0`, right: `9223372036854775808`', library/alloc/src/collections/vec_deque/tests.rs:474:5 note: panic did not contain expected string panic message: `"assertion failed: `(left == right)`\n left: `0`,\n right: `9223372036854775808`"`, expected substring: `"capacity overflow"` ``` That was a result of `len()` using a mask `& (size - 1)` with the improper length. Now we do get a "capacity overflow" panic as soon as that `VecDeque::from(vec)` is attempted.
1 parent 04ae501 commit c07955c

File tree

2 files changed

+33
-19
lines changed

2 files changed

+33
-19
lines changed

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

+18-19
Original file line numberDiff line numberDiff line change
@@ -2783,27 +2783,26 @@ impl<T> From<Vec<T>> for VecDeque<T> {
27832783
/// This avoids reallocating where possible, but the conditions for that are
27842784
/// strict, and subject to change, and so shouldn't be relied upon unless the
27852785
/// `Vec<T>` came from `From<VecDeque<T>>` and hasn't been reallocated.
2786-
fn from(other: Vec<T>) -> Self {
2787-
unsafe {
2788-
let mut other = ManuallyDrop::new(other);
2789-
let other_buf = other.as_mut_ptr();
2790-
let mut buf = RawVec::from_raw_parts(other_buf, other.capacity());
2791-
let len = other.len();
2792-
2793-
// We need to extend the buf if it's not a power of two, too small
2794-
// or doesn't have at least one free space.
2795-
// We check if `T` is a ZST in the first condition,
2796-
// because `usize::MAX` (the capacity returned by `capacity()` for ZST)
2797-
// is not a power of two and thus it'll always try
2798-
// to reserve more memory which will panic for ZST (rust-lang/rust#78532)
2799-
if (!buf.capacity().is_power_of_two() && mem::size_of::<T>() != 0)
2800-
|| (buf.capacity() < (MINIMUM_CAPACITY + 1))
2801-
|| (buf.capacity() == len)
2802-
{
2803-
let cap = cmp::max(buf.capacity() + 1, MINIMUM_CAPACITY + 1).next_power_of_two();
2804-
buf.reserve_exact(len, cap - len);
2786+
fn from(mut other: Vec<T>) -> Self {
2787+
let len = other.len();
2788+
if mem::size_of::<T>() == 0 {
2789+
// There's no actual allocation for ZSTs to worry about capacity,
2790+
// but `VecDeque` can't handle as much length as `Vec`.
2791+
assert!(len < MAXIMUM_ZST_CAPACITY, "capacity overflow");
2792+
} else {
2793+
// We need to resize if the capacity is not a power of two, too small or
2794+
// doesn't have at least one free space. We do this while it's still in
2795+
// the `Vec` so the items will drop on panic.
2796+
let min_cap = cmp::max(MINIMUM_CAPACITY, len) + 1;
2797+
let cap = cmp::max(min_cap, other.capacity()).next_power_of_two();
2798+
if other.capacity() != cap {
2799+
other.reserve_exact(cap - len);
28052800
}
2801+
}
28062802

2803+
unsafe {
2804+
let (other_buf, len, capacity) = other.into_raw_parts();
2805+
let buf = RawVec::from_raw_parts(other_buf, capacity);
28072806
VecDeque { tail: 0, head: len, buf }
28082807
}
28092808
}

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

+15
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,21 @@ fn test_from_vec() {
457457
assert!(vd.into_iter().eq(vec));
458458
}
459459
}
460+
461+
let vec = Vec::from([(); MAXIMUM_ZST_CAPACITY - 1]);
462+
let vd = VecDeque::from(vec.clone());
463+
assert!(vd.cap().is_power_of_two());
464+
assert_eq!(vd.len(), vec.len());
465+
}
466+
467+
#[test]
468+
#[should_panic = "capacity overflow"]
469+
fn test_from_vec_zst_overflow() {
470+
use crate::vec::Vec;
471+
let vec = Vec::from([(); MAXIMUM_ZST_CAPACITY]);
472+
let vd = VecDeque::from(vec.clone()); // no room for +1
473+
assert!(vd.cap().is_power_of_two());
474+
assert_eq!(vd.len(), vec.len());
460475
}
461476

462477
#[test]

0 commit comments

Comments
 (0)