fix vec::IntoIter::drop on high-alignment ZST#106084
Conversation
rustbot
added
S-waiting-on-review
|
The Miri subtree was changed cc @rust-lang/miri Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
library/alloc/src/vec/into_iter.rs
Outdated
There was a problem hiding this comment.
FWIW I don't think we have a test for this (with a ZST that needs alignment)
There was a problem hiding this comment.
Wouldn't it be better to add the tests to std instead of the miri testsuite then?
There was a problem hiding this comment.
I'm not sure how this test would be written in std in a way that would actually test the behavior except under miri.
There was a problem hiding this comment.
Maybe by adding an unsafe precondition debug assert to drop_in_place?
There was a problem hiding this comment.
I would not be opposed to that, but I don't think it needs to be done in this PR.
library/alloc/src/vec/into_iter.rs
Outdated
There was a problem hiding this comment.
FWIW I don't think we have a test for this (with a ZST that needs alignment)
|
Subtle, nice catch. @bors r+ rollup |
bors
added
S-waiting-on-bors
fix vec::IntoIter::drop on high-alignment ZST This fixes a soundness bug: IntoIter would call `drop_in_place` on an insufficiently aligned pointer. So if a ZST with alignment greater 1 had drop glue, that would be called with an unaligned reference. Since rust-lang#103957, Miri checks alignment even if the type does not have drop glue, which is how this bug was found. r? `@thomcc`
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#105970 (docs/test: add UI test and long-form error docs for E0462) - rust-lang#105975 (rustc: Remove needless lifetimes) - rust-lang#106069 (rustdoc: use a more evocative name for CSS/JS `#titles`) - rust-lang#106084 (fix vec::IntoIter::drop on high-alignment ZST) - rust-lang#106091 (Use correct CSS pseudo-element selector) - rust-lang#106093 (rustdoc: remove no-op CSS from `.docblock-short`) - rust-lang#106102 (Fix `triagebot.toml`) Failed merges: - rust-lang#106028 (docs/test: add UI test and long-form error docs for `E0461`) r? `@ghost` `@rustbot` modify labels: rollup
|
There is a way to write a test without Miri: define a ZST with high alignment and give it an impl Drop that checks alignment.
|
add lib tests for vec::IntoIter alignment issues This adds non-Miri tests for the issue fixed in rust-lang#106084 r? `@thomcc`
fix vec::IntoIter::drop on high-alignment ZST This fixes a soundness bug: IntoIter would call `drop_in_place` on an insufficiently aligned pointer. So if a ZST with alignment greater 1 had drop glue, that would be called with an unaligned reference. Since rust-lang#103957, Miri checks alignment even if the type does not have drop glue, which is how this bug was found. r? ``@thomcc``
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#105970 (docs/test: add UI test and long-form error docs for E0462) - rust-lang#105975 (rustc: Remove needless lifetimes) - rust-lang#106069 (rustdoc: use a more evocative name for CSS/JS `#titles`) - rust-lang#106084 (fix vec::IntoIter::drop on high-alignment ZST) - rust-lang#106091 (Use correct CSS pseudo-element selector) - rust-lang#106093 (rustdoc: remove no-op CSS from `.docblock-short`) - rust-lang#106102 (Fix `triagebot.toml`) Failed merges: - rust-lang#106028 (docs/test: add UI test and long-form error docs for `E0461`) r? `@ghost` `@rustbot` modify labels: rollup
add lib tests for vec::IntoIter alignment issues This adds non-Miri tests for the issue fixed in rust-lang/rust#106084 r? `@thomcc`
5 participants
This fixes a soundness bug: IntoIter would call
drop_in_placeon an insufficiently aligned pointer. So if a ZST with alignment greater 1 had drop glue, that would be called with an unaligned reference. Since #103957, Miri checks alignment even if the type does not have drop glue, which is how this bug was found.r? @thomcc