Skip to content

Fix leak in Vec::extend_from_within #82760

New issue

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

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

Already on GitHub? # to your account

Merged
merged 2 commits into from
Mar 13, 2021

Conversation

WaffleLapkin
Copy link
Member

Fixes #82533

Previously vec's len was updated only after full copy, making the method
leak if T::clone panic!s.

This commit makes `Vec::extend_from_within` (or, more accurately, it's
`T: Clone` specialization) update vec's len on every iteration, fixing
the issue.

`T: Copy` specialization was not affected by the issue b/c it doesn't
call user specified code (as, e.g. `T::clone`), and instead calls
`ptr::copy_nonoverlapping`.
@rust-highfive
Copy link
Contributor

r? @kennytm

(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 4, 2021
@kennytm
Copy link
Member

kennytm commented Mar 13, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 13, 2021

📌 Commit 1f031d9 has been approved by kennytm

@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 13, 2021
@bors
Copy link
Collaborator

bors commented Mar 13, 2021

⌛ Testing commit 1f031d9 with merge ec487bf...

@bors
Copy link
Collaborator

bors commented Mar 13, 2021

☀️ Test successful - checks-actions
Approved by: kennytm
Pushing ec487bf to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 13, 2021
@bors bors merged commit ec487bf into rust-lang:master Mar 13, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 13, 2021

/// Safety: changing returned .2 (&mut usize) is considered the same as calling `.set_len(_)`.
///
/// This method is used to have unique access to all vec parts at once in `extend_from_within`.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/is used to have/provides/ would be clearer

@WaffleLapkin WaffleLapkin deleted the unleak_extend_from_within branch March 24, 2021 09:21
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 18, 2021
…vink

Slightly change wording in doc comment and fix typo in vec/mod.rs

Suggested by `@pickfire` in rust-lang#82760
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 18, 2021
…vink

Slightly change wording in doc comment and fix typo in vec/mod.rs

Suggested by ``@pickfire`` in rust-lang#82760
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 18, 2021
…vink

Slightly change wording in doc comment and fix typo in vec/mod.rs

Suggested by ```@pickfire``` in rust-lang#82760
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2021
Slightly change wording in doc comment and fix typo in vec/mod.rs

Suggested by `@pickfire` in rust-lang#82760
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extend_from_within leaks elements on panic
6 participants