Skip to content

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Mar 17, 2021

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.

`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.
@rust-highfive
Copy link
Contributor

r? @KodrAus

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 17, 2021
@camelid camelid added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Mar 17, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Mar 18, 2021

r? @m-ou-se

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 18, 2021

📌 Commit c07955c has been approved by m-ou-se

@rust-highfive rust-highfive assigned m-ou-se and unassigned KodrAus Mar 18, 2021
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 19, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#79986 (Only build help popup when it's really needed)
 - rust-lang#82570 (Add `as_str` method for split whitespace str iterators)
 - rust-lang#83244 (Fix overflowing length in Vec<ZST> to VecDeque)
 - rust-lang#83254 (Include output stream in `panic!()` documentation)
 - rust-lang#83269 (Revert the second deprecation of collections::Bound)
 - rust-lang#83277 (Mark early otherwise optimization unsound)
 - rust-lang#83285 (Update LLVM to bring in SIMD updates for WebAssembly)
 - rust-lang#83297 (Do not ICE on ty::Error as an error must already have been reported)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1a0e32f into rust-lang:master Mar 20, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 20, 2021
@cuviper cuviper deleted the vec_deque-zst branch September 21, 2021 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VecDeque from Vec with ZST elements breaks internal invariants
7 participants