Never emit vptr for empty/auto traits
#131864
Merged
+36
−14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
You can write tests for the vtable layout (see the failing tests that already do that, not sure if there is a need to add more after blessing them) |
Yeah, the failed CI already told me the answer to that question. Will take a look at it. |
lrh2000
force-pushed
the
upcast_reorder
branch
from
a4f208d
to
76c4711
Compare
This comment has been minimized.
This comment has been minimized.
lrh2000
force-pushed
the
upcast_reorder
branch
from
76c4711
to
781bff0
Compare
WaffleLapkin
approved these changes
Oct 18, 2024
|
@bors r+ |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 18, 2024
Rollup of 3 pull requests Successful merges: - rust-lang#126207 (std::unix::stack_overflow::drop_handler addressing todo through libc …) - rust-lang#131864 (Never emit `vptr` for empty/auto traits) - rust-lang#131870 (compiletest: Store test collection context/state in two structs) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 18, 2024
Rollup merge of rust-lang#131864 - lrh2000:upcast_reorder, r=WaffleLapkin Never emit `vptr` for empty/auto traits Emiting `vptr`s for empty/auto traits is unnecessary (rust-lang#114942) and causes unsoundness in `trait_upcasting` (rust-lang#131813). This PR should ensure that we never emit vtables for such traits. See the linked issues for more details. I'm not sure if I can add tests for the vtable layout. So this PR only adds tests for the soundness hole (i.e., the segmentation fault will disappear after this PR). Fixes rust-lang#114942 Fixes rust-lang#131813 Cc rust-lang#65991 (tracking issue for `trait_upcasting`) r? `@WaffleLapkin` (per rust-lang#131813 (comment))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Emiting
vptrs for empty/auto traits is unnecessary (#114942) and causes unsoundness intrait_upcasting(#131813). This PR should ensure that we never emit vtables for such traits. See the linked issues for more details.I'm not sure if I can add tests for the vtable layout. So this PR only adds tests for the soundness hole (i.e., the segmentation fault will disappear after this PR).
Fixes #114942
Fixes #131813
Cc #65991 (tracking issue for
trait_upcasting)r? @WaffleLapkin (per #131813 (comment))