Skip to content

Fix unsoundness in VecDeque::from_iter(vec::IntoIter) - #162462

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
maxdexh:issue-162452-vec-into-iter-into-deque
Sep 8, 2026
Merged

Fix unsoundness in VecDeque::from_iter(vec::IntoIter)#162462
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
maxdexh:issue-162452-vec-into-iter-into-deque

Conversation

@maxdexh

@maxdexh maxdexh commented Sep 8, 2026

Copy link
Copy Markdown
Member

Adds a safety requirement to VecDeque::from_contiguous_raw_parts_in to ensure that the VecDeque it creates upholds the safety invariants. In particular, either head < capacity or head == capacity == 0.
Refactors vec::IntoIter::into_vecdeque to uphold that invariant (and splits the unsafe blocks into multiple parts, because the safety comment was messy & incomplete). Actual fix is the addition of || len == 0.

I would add a regression test, but the specialization is behind cfg(not(test)). I cannot see why this was done, as there are no comments documenting this...

Fixes #162452

r? libs

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 8, 2026
@maxdexh
maxdexh force-pushed the issue-162452-vec-into-iter-into-deque branch from 46f693a to 5d7774e Compare September 8, 2026 09:48
@maxdexh

maxdexh commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

The cfg(not(test)) on the relevant code was added in #136642, but that also doesn't give me any insight into why it's needed.

cc @bjorn3

Edit: Sorry for the ping, I just got confused by the fact that debug assertions are apparently off in alloctests, which made me believe that the method wasn't getting run.

Comment thread library/alloc/src/collections/vec_deque/mod.rs
@bjorn3

bjorn3 commented Sep 8, 2026

Copy link
Copy Markdown
Member

The cfg(not(test)) on the relevant code was added in #136642, but that also doesn't give me any insight into why it's needed.

I don't remember why for this particular case. For a couple of other cases it was necessary to avoid errors from the #[path = "../alloc/src/collections/mod.rs"] mod collections; in alloctests like violating the orphan rule due to the implemented trait being included from liballoc rather than in the local crate.

@Darksonn Darksonn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general looks good. One nit below, otherwise r=me if CI passes

View changes since this review

Comment thread library/alloc/src/vec/into_iter.rs Outdated
@maxdexh

maxdexh commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

otherwise r=me if CI passes

I don't have bors privileges ^^

@maxdexh
maxdexh force-pushed the issue-162452-vec-into-iter-into-deque branch from 898722c to 8bd0a5a Compare September 8, 2026 11:36
@Darksonn

Darksonn commented Sep 8, 2026

Copy link
Copy Markdown
Member

I'll r+ it once CI passes 👍

@Darksonn

Darksonn commented Sep 8, 2026

Copy link
Copy Markdown
Member

@bors r+

@rust-bors

rust-bors Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 8bd0a5a has been approved by Darksonn

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 5. This pull request will be tested once the tree is reopened.

Reason for tree closure: spurious failures

@rust-bors rust-bors Bot 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 Sep 8, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 8, 2026
…uwer

Rollup of 12 pull requests

Successful merges:

 - #162462 (Fix unsoundness in `VecDeque::from_iter(vec::IntoIter)`)
 - #162468 (mir_build: Rename `lower_let_expr` to `lower_fallible_let`, and clarify)
 - #161806 (Add tests and docs for `#[derive(GenericTypeVisitable)]`)
 - #161912 (run `extern "tail"` with `byval` argument test)
 - #161932 (Only check `#[allow(dead_code)]` on the trait)
 - #162236 (Call renumberMetadataForAssembly() in LLVMRustPrintModule())
 - #162435 (windows-gnu: document libgcc requirement)
 - #162439 (Update books)
 - #162451 (Add regression test for item-local diagnostic attribute lint levels)
 - #162455 (Update codegen-llvm/inline-debuginfo.rs for distinct DILocation)
 - #162459 (docs(time): replace "method" with "function")
 - #162465 (Fix my duplicate thanks entry)
@rust-bors
rust-bors Bot merged commit 29b15e6 into rust-lang:main Sep 8, 2026
13 checks passed
rust-bors Bot pushed a commit that referenced this pull request Sep 8, 2026
Rollup merge of #162462 - maxdexh:issue-162452-vec-into-iter-into-deque, r=Darksonn

Fix unsoundness in `VecDeque::from_iter(vec::IntoIter)`

Adds a safety requirement to `VecDeque::from_contiguous_raw_parts_in` to ensure that the `VecDeque` it creates upholds the safety invariants. In particular, either `head < capacity` or `head == capacity == 0`.
Refactors `vec::IntoIter::into_vecdeque` to uphold that invariant (and splits the unsafe blocks into multiple parts, because the safety comment was messy & incomplete). Actual fix is the addition of `|| len == 0`.

I would add a regression test, but the specialization is behind `cfg(not(test))`. I cannot see why this was done, as there are no comments documenting this...

Fixes #162452

r? libs
@rustbot rustbot added this to the 1.100.0 milestone Sep 8, 2026
@maxdexh
maxdexh deleted the issue-162452-vec-into-iter-into-deque branch September 8, 2026 17:11
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: collecting an exhausted vec::IntoIter yields head == capacity, later failing wrap_index's debug assertion

4 participants