Skip to content

Commit

Permalink
Fix overflowing length in Vec<ZST> to VecDeque
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
cuviper committed Mar 17, 2021
1 parent 04ae501 commit c07955c
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 19 deletions.
37 changes: 18 additions & 19 deletions library/alloc/src/collections/vec_deque/mod.rs
Expand Up @@ -2783,27 +2783,26 @@ impl<T> From<Vec<T>> for VecDeque<T> {
/// This avoids reallocating where possible, but the conditions for that are
/// strict, and subject to change, and so shouldn't be relied upon unless the
/// `Vec<T>` came from `From<VecDeque<T>>` and hasn't been reallocated.
fn from(other: Vec<T>) -> Self {
unsafe {
let mut other = ManuallyDrop::new(other);
let other_buf = other.as_mut_ptr();
let mut buf = RawVec::from_raw_parts(other_buf, other.capacity());
let len = other.len();

// We need to extend the buf if it's not a power of two, too small
// or doesn't have at least one free space.
// We check if `T` is a ZST in the first condition,
// because `usize::MAX` (the capacity returned by `capacity()` for ZST)
// is not a power of two and thus it'll always try
// to reserve more memory which will panic for ZST (rust-lang/rust#78532)
if (!buf.capacity().is_power_of_two() && mem::size_of::<T>() != 0)
|| (buf.capacity() < (MINIMUM_CAPACITY + 1))
|| (buf.capacity() == len)
{
let cap = cmp::max(buf.capacity() + 1, MINIMUM_CAPACITY + 1).next_power_of_two();
buf.reserve_exact(len, cap - len);
fn from(mut other: Vec<T>) -> Self {
let len = other.len();
if mem::size_of::<T>() == 0 {
// There's no actual allocation for ZSTs to worry about capacity,
// but `VecDeque` can't handle as much length as `Vec`.
assert!(len < MAXIMUM_ZST_CAPACITY, "capacity overflow");
} else {
// We need to resize if the capacity is not a power of two, too small or
// doesn't have at least one free space. We do this while it's still in
// the `Vec` so the items will drop on panic.
let min_cap = cmp::max(MINIMUM_CAPACITY, len) + 1;
let cap = cmp::max(min_cap, other.capacity()).next_power_of_two();
if other.capacity() != cap {
other.reserve_exact(cap - len);
}
}

unsafe {
let (other_buf, len, capacity) = other.into_raw_parts();
let buf = RawVec::from_raw_parts(other_buf, capacity);
VecDeque { tail: 0, head: len, buf }
}
}
Expand Down
15 changes: 15 additions & 0 deletions library/alloc/src/collections/vec_deque/tests.rs
Expand Up @@ -457,6 +457,21 @@ fn test_from_vec() {
assert!(vd.into_iter().eq(vec));
}
}

let vec = Vec::from([(); MAXIMUM_ZST_CAPACITY - 1]);
let vd = VecDeque::from(vec.clone());
assert!(vd.cap().is_power_of_two());
assert_eq!(vd.len(), vec.len());
}

#[test]
#[should_panic = "capacity overflow"]
fn test_from_vec_zst_overflow() {
use crate::vec::Vec;
let vec = Vec::from([(); MAXIMUM_ZST_CAPACITY]);
let vd = VecDeque::from(vec.clone()); // no room for +1
assert!(vd.cap().is_power_of_two());
assert_eq!(vd.len(), vec.len());
}

#[test]
Expand Down

0 comments on commit c07955c

Please sign in to comment.