-
-
Notifications
You must be signed in to change notification settings - Fork 15k
Unsoundness: explicit pin projection into a #[repr(packed)] field breaks Pin without #[pin_v2] #157615
Copy link
Copy link
Open
Labels
A-alignArea: alignment control (`repr(align(N))` and so on)Area: alignment control (`repr(align(N))` and so on)A-destructorsArea: Destructors (`Drop`, …)Area: Destructors (`Drop`, …)A-pinArea: PinArea: PinA-repr-packedArea: the naughtiest reprArea: the naughtiest reprC-bugCategory: This is a bug.Category: This is a bug.F-pin_ergonomics`#![feature(pin_ergonomics)]``#![feature(pin_ergonomics)]`I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Metadata
Metadata
Assignees
Labels
A-alignArea: alignment control (`repr(align(N))` and so on)Area: alignment control (`repr(align(N))` and so on)A-destructorsArea: Destructors (`Drop`, …)Area: Destructors (`Drop`, …)A-pinArea: PinArea: PinA-repr-packedArea: the naughtiest reprArea: the naughtiest reprC-bugCategory: This is a bug.Category: This is a bug.F-pin_ergonomics`#![feature(pin_ergonomics)]``#![feature(pin_ergonomics)]`I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Type
Fields
Give feedbackNo fields configured for issues without a type.
Projects
StatusShow more project fields
No status
So when the destructor of a
repr(packed)struct runs, a field that's unaligned and has a destructor gets moved to an aligned place before that field's destructor runs. (See also #157011, #143411, taiki-e/pin-project-lite#89, taiki-e/pin-project#34.)#157011 covers this for
#[pin_v2], and the fix there (#157542) just bans#[pin_v2]+#[repr(packed)]. But afaict the#[pin_v2]attribute only gates the implicit projection (the default binding-mode shift, like plainFoo { x, y }matched against aPin<&mut Foo>). The explicit&pin mut/ref pin mutpatterns project in place no matter whether the type opted in, so banning the attribute combo doesn't really close this. You can hit the exact same move-on-drop with no#[pin_v2]anywhere. Filing this as the standalone issue for the leftover case that #157542 left open under the pin ergonomics tracking issue #130494.The projection soundness check lives in
rustc_hir_typeck/src/pat.rsand only runs underdefault_binding_modes. Spelling the deref out (&pin mut .. ref pin mut ..) kinda walks right past it, which is why no attribute is needed and why the #157011 fix doesn't catch this.I tried this code:
I expected to see this happen: the explicit
&pin mut/ref pin mutprojection through a packed type should get rejected, like the#[pin_v2]case in #157011. APin<&mut Thing>you hand out should sure keep a stable address for the rest of the value's life.Instead, this happened: it just compiles and runs, and the pinned-access addr and the drop addr come out different, so the
Pininvariant is broken.Oneispacked(4), so itsThingleaf is aligned to 4. That's fine on its own, sounaligned_referencesdoesn't complain. The thing that actually moves on drop is the parentTwo(align 4096), and we never take a reference toTwo, so nothing catches it. (Onehas to be generic to dodge E0588.)Couple more notes: I used
align(4096)instead of #157011's65536just so it prints cleanly. At65536it straight up segfaults on my machine (stack realignment during the drop-glue move), same unsoundness, just louder. Also the&pin mut <place>borrow operator (like&pin mut one.0.0) does not repro this, imo because it copies the packed field out to an aligned temp first, so it stays safe. It's specifically the pattern form that pins in place.Meta
rustc --version --verbose: