Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix overflowing length in Vec<ZST> to VecDeque #83244

Merged
merged 1 commit into from
Mar 20, 2021

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
Collaborator

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
Contributor

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