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 panic-safety in specialized Zip::next_back #86452

Merged
merged 2 commits into from
Jun 21, 2021

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Jun 19, 2021

This was unsound since a panic in a.next_back() would result in the
length not being updated which would then lead to the same element
being revisited in the side-effect preserving code.

fixes #86443

This was unsound since a panic in a.next_back() would result in the
length not being updated which would then lead to the same element
being revisited in the side-effect preserving code.
@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(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 Jun 19, 2021
@the8472 the8472 added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 19, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jun 19, 2021

Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Jun 19, 2021

📌 Commit 8b51854 has been approved by m-ou-se

@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 Jun 19, 2021
@m-ou-se m-ou-se assigned m-ou-se and unassigned joshtriplett Jun 19, 2021
@bors
Copy link
Contributor

bors commented Jun 20, 2021

⌛ Testing commit 8b51854 with merge 1c8899cd89868eb5cd168a631594fcfdf1dfbe47...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jun 20, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 20, 2021
@the8472

This comment has been minimized.

@the8472
Copy link
Member Author

the8472 commented Jun 20, 2021

I have been informed that this is due to wasm not supporting unwinding. Added cfg(panic = "unwind") to the test to exclude all such platforms.

@m-ou-se
Copy link
Member

m-ou-se commented Jun 20, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jun 20, 2021

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

@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 Jun 20, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 21, 2021
fix panic-safety in specialized Zip::next_back

This was unsound since a panic in a.next_back() would result in the
length not being updated which would then lead to the same element
being revisited in the side-effect preserving code.

fixes rust-lang#86443
This was referenced Jun 21, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 21, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#83739 (Account for bad placeholder errors on consts/statics with trait objects)
 - rust-lang#85637 (document PartialEq, PartialOrd, Ord requirements more explicitly)
 - rust-lang#86152 (Lazify is_really_default condition in the RustdocGUI bootstrap step)
 - rust-lang#86156 (Fix a bug in the linkchecker)
 - rust-lang#86427 (Updated release note)
 - rust-lang#86452 (fix panic-safety in specialized Zip::next_back)
 - rust-lang#86484 (Do not set depth to 0 in fully_expand_fragment)
 - rust-lang#86491 (expand: Move some more derive logic to rustc_builtin_macros)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 504c378 into rust-lang:master Jun 21, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 21, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 22, 2021
…fJung

Add comments around code where ordering is important due for panic-safety

Iterators contain arbitrary code which may panic. Unsafe code has to be
careful to do its state updates at the right point between calls that may panic.

As requested in rust-lang#86452 (comment)

r? `@RalfJung`
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.

Panic safety issue in Zip::next_back() TrustedRandomAccess specialization
8 participants