Repository navigation
Fix repr(packed) drop UBs - #165010
Fix repr(packed) drop UBs#165010Jules-Bertholet wants to merge 3 commits into
repr(packed) drop UBs#165010Conversation
|
cc @bjorn3
cc @rust-lang/miri Some changes occurred to the CTFE machinery Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred to constck cc @fee1-dead This PR changes MIR cc @oli-obk, @RalfJung, @JakobDegen, @vakaras This PR changes rustc_public cc @oli-obk, @celinval, @ouz-a, @makai410 Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri
cc @rust-lang/clippy |
rustbot
added
S-waiting-on-review
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
rustbot
added
A-destructors
46bcae7 to
28ef878
Compare
This comment has been minimized.
This comment has been minimized.
28ef878 to
9c76242
Compare
This comment has been minimized.
This comment has been minimized.
Jules-Bertholet
changed the title
Box field
Jules-Bertholet
changed the title
Jules-Bertholet
changed the title
repr(packed) drop UBs
9b352df to
dfca5fb
Compare
| UnwrapUnsafeBinder(T), | ||
| /// Transmute the type of the place. This is a projection of a place, | ||
| /// so it doesn't necessarily constitute a move out of the source type. | ||
| /// Used for unsafe binders, among other things. |
There was a problem hiding this comment.
"transmute on a place" is an interesting operation. This is basically transmuting a pointer.
Which constraints does it have? Probably none on the actual value, that will be checked only if the place is actually loaded-from. But I assume the old and new type both have to be sized? Or at least, they must have the same metadata type. Otherwise, it's unclear where the metadata should come from.
There was a problem hiding this comment.
Yes, they both need to be Sized, that is checked in validate.
| span_bug!(self.cur_span(), "PhantomDeref encountered after borrowck") | ||
| } | ||
| UnwrapUnsafeBinder(target) => base.transmute(self.layout_of(target)?, self)?, | ||
| Transmute(target) => base.transmute(self.layout_of(target)?, self)?, |
There was a problem hiding this comment.
This will check that base and target have the same size. If that's a constraint we want to impose, it should be mentioned in the MIR syntax docs.
| if util::place_unalignment_2(tcx, self.elaborator.typing_env(), self.place, |place| { | ||
| self.place_ref_ty(place) |
There was a problem hiding this comment.
That function name surely is something.^^
Why do we not have a LocalDecls here?
There was a problem hiding this comment.
Because the locals are split between the MIR body, and the MIR patch we are still building up and haven't applied to it yet.
|
@rustbot reroll |
This comment has been minimized.
This comment has been minimized.
7329986 to
9e89e35
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Co-authored-by: Ralf Jung <post@ralfj.de>
9e89e35 to
1a346ba
Compare
Fixes #143411, fixes #164561.
This also replaces the
UnwrapUnsafeBinderMIR projection with a more generalTransmuteprojection. I'm not sure if there is anything extra I need to implement for that, e.g. for Miri checks.@rustbot label A-destructors A-MIR A-repr-packed