about summary refs log tree commit diff
diff options
context:
space:
mode:
authorDylan DPC <dylan.dpc@gmail.com>2021-03-19 23:01:37 +0100
committerGitHub <noreply@github.com>2021-03-19 23:01:37 +0100
commit1a0e32f4bc30318f4e15d200039bdbc2ea659fdb (patch)
tree7ed7b88459086ffb2ae66fee9a37fca8e3482b0f
parentf7febc8865a2cb2287b034f6588d7617fa0fc6e9 (diff)
parentc07955c6b6a8901fc59d9c22004127b30a965407 (diff)
downloadrust-1a0e32f4bc30318f4e15d200039bdbc2ea659fdb.tar.gz
rust-1a0e32f4bc30318f4e15d200039bdbc2ea659fdb.zip
Rollup merge of #83244 - cuviper:vec_deque-zst, r=m-ou-se
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.

Fixes #80167.
-rw-r--r--library/alloc/src/collections/vec_deque/mod.rs37
-rw-r--r--library/alloc/src/collections/vec_deque/tests.rs15
2 files changed, 33 insertions, 19 deletions
diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/library/alloc/src/collections/vec_deque/mod.rs
index f7cefdce278..7a0de74eb23 100644
--- a/library/alloc/src/collections/vec_deque/mod.rs
+++ b/library/alloc/src/collections/vec_deque/mod.rs
@@ -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 }
         }
     }
diff --git a/library/alloc/src/collections/vec_deque/tests.rs b/library/alloc/src/collections/vec_deque/tests.rs
index 87e06fa394d..6116cfe1d01 100644
--- a/library/alloc/src/collections/vec_deque/tests.rs
+++ b/library/alloc/src/collections/vec_deque/tests.rs
@@ -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]