Introduce a PinSafePointer trait that generalizes PinCoerceUnsized - #156935
Conversation
Darksonn
added
T-lang
rustbot
added
the
S-waiting-on-review
| // safely as `Box::pin((*p).clone())`. | ||
| #[unstable(feature = "pin_coerce_unsized_trait", issue = "150112")] | ||
| unsafe impl<T: ?Sized, A: Allocator> PinCoerceUnsized for Box<T, A> {} | ||
| unsafe impl<T: ?Sized, A: Allocator + 'static> PinSafePointer for Box<T, A> {} |
There was a problem hiding this comment.
I added A: 'static bounds to these impls since you generally cannot pin values in memory from a non-static allocator.
There was a problem hiding this comment.
You can create a Pin<Box<T, A>> for a non-'static allocator by first pinning using MyAllocator<'static>, and then use subtyping to turn that into MyAllocator<'a>.
There was a problem hiding this comment.
But that's kind of awkward, then. In that scenario, we have a non-static Pin<Box<T, A<'a>>>, we clone the inner box to create a new Box<T, A<'a>>, and then we wrap the clone in Pin to get a second Pin<Box<T, A<'a>>>. But you can normally only wrap a box in Pin if the allocator is 'static, so the argument I'm using for Box doesn't quite apply anymore.
There was a problem hiding this comment.
This feels unsound, but I haven't figured out yet how.
There was a problem hiding this comment.
It's unsound (even without using the PinCoerceUnsized impl). I filed this as #157089.
| /// [`clone`]: Clone::clone | ||
| /// [`Arc`]: ../../std/sync/struct.Arc.html "Arc" | ||
| /// [`Arc::get_mut`]: ../../std/sync/struct.Arc.html#method.get_mut "Arc::get_mut" | ||
| pub unsafe trait PinSafePointer: Deref + Sized {} |
There was a problem hiding this comment.
I added a Sized bound here due to #156920 (comment). This doesn't actually affect any use-cases since Pin<P> already requires P: Sized, but seemed like a good idea nonetheless.
83c7d24 to
23a34ef
Compare
| /// Calls to [`deref`]/[`deref_mut`] on the same `Pin<P>` instance must always | ||
| /// refer to the same object. That is, the address returned by these methods | ||
| /// must not change. This applies even if the pointer type is moved. |
There was a problem hiding this comment.
So this is still doing the thing where it talks about a DerefMut impl but DerefMut is not a supertrait of this trait. What are the valid ways in which an unsafe impl PinSafePointer can satisfy this requirement?
- There's a
DerefMutfor the same set of types in the same crate: that's easy, we can just check the condition. - There's a negative
DerefMutfor the same set of types in the same crate: that's easy, no impl can exist so the condition is trivially satisfied. - Neither of these hold true. Now we... uh, look at the orphan rule and hope for the best? I am not sure that's a good idea, this is exactly what has bitten us in the past...
The same concern applies to the requirements on Clone, the formatting traits, and any others that I missed. (IMO the docs should call out more explicitly: If this type also implements Foo, then blah. If it also implements Bar, then blah. etc)
There was a problem hiding this comment.
I agree that it's best to avoid this kind of safety requirement, but I think we have no choice. Luckily we do not need to worry about dyn PinSafePointer.
This PR is adding comments to every impl of PinSafePointer explaining why the various trait implementations are compatible or cannot exist for the particular type in question. Please see the comments for various ways we can make these arguments.
There was a problem hiding this comment.
Yeah in std we can do that because we have negative impls. But our users can't really do that... and the moment one wants to impl PinSafePointer for MyType<T> where MyType<T>: DerefMut for some but not all T, I don't see any way to make this work.
There was a problem hiding this comment.
Negative impls are only required for fundamental types, so I'm not too concerned about that.
There was a problem hiding this comment.
With regards to types where MyType<T>: DerefMut for some but not all T, we do in fact have one instance of exactly that. Please see #145608 for the trick to make that sound.
There was a problem hiding this comment.
I have seen that PR and ran away screaming.^^
I really hope the reasoning is sound, but I do not have a coherent proof in my head -- it relies too much on orphan rule details.
| // with `Rc::get_mut`), this means that this type treats `&Rc<T>` as evidence | ||
| // that the `T` is not pinned. The implementations of various traits are written | ||
| // accordingly. Since this type is not fundamental, downstream crates cannot | ||
| // provide malicious implementations of any of the traits relevant for `Pin`. |
There was a problem hiding this comment.
So this relies on something about the orphan rule, right? That makes me uneasy.^^
An explicit impl !DerefMut for Rc would be a lot better IMO.
There was a problem hiding this comment.
Yes, by the orphan rule downstream crates cannot implement DerefMut for Rc<_> under any scenario.
There was a problem hiding this comment.
Those are the current orphan rules. How do we know they will never be relaxed?
The only promise the orphan rules are making is "there will never be more than one impl for the same trait + type". I don't think we should rely on any property that goes beyond this, and I don't see how the argument here follows from that property.
There was a problem hiding this comment.
We can add a negative impl to simplify the reasoning for Rc, but I think there is no way out for the #145608 case unless someone comes up with an entirely different solution. It's simply unsound to add untrusted code to the coherence domain of core because it could implement DerefMut for Pin<LocalType> maliciously.
There was a problem hiding this comment.
Well, even if I add the impl !DerefMut for Rc, what about this one?
impl<T: ?Sized + fmt::Debug, A: Allocator> fmt::Debug for Rc<T, A>
I do not think we can get out of this without orphan-rules based reasoning.
There was a problem hiding this comment.
Yeah, we need new language features to support this properly I think. Maybe if we could have a "dual" negative impl?
impl<T: ?Sized + !fmt::Debug, A> !fmt::Debug for Rc<T, A>Since every concrete type either impls Debug or not, we can always use one or the other impl as justification.
| /// If this pointer type uses `&P` references as evidence that this value is not | ||
| /// pinned, then it must not treat the `&self` argument passed to [`Clone`] or | ||
| /// the formatting traits ([`fmt::Debug`], [`fmt::Display`], [`fmt::Pointer`]) | ||
| /// as such evidence. | ||
| /// | ||
| /// As an example, given a `Pin<Arc<T>>` there is no way to obtain an `&Arc<T>` | ||
| /// (note that `Deref` just gives a `&T`). Because of this, the [`Arc`] type can | ||
| /// assume that an `&Arc<T>` value can only exist if the `T` is not pinned, | ||
| /// which justifies the soundness of the [`Arc::get_mut`] method. |
There was a problem hiding this comment.
I didn't realize get_mut becomes even more subtle when pinning gets involved. Impressive.
23a34ef to
9f84c57
Compare
This comment has been minimized.
This comment has been minimized.
d9f9287 to
83a842a
Compare
| /// refer to the same object. That is, the address returned by these methods | ||
| /// must not change. This applies even if the pointer type is moved. |
There was a problem hiding this comment.
the address returned by these methods must not change
This seems to imply that it's OK for DerefMut to return different fat references with the same address but different metadata (and therefore different underlying concrete types).
There was a problem hiding this comment.
True, the part about concrete types should not be behind an "if the smart pointer can participate in unsizing coercions" and should just be unconditional.
There was a problem hiding this comment.
Is this still pending an edit? I admit I don't quite understand why we have such a strong condition, but if we think it's reasonable then I don't have direct objections. My main concern with all these rules is that I am not sure how to audit for them effectively (especially as we seem to keep changing them as we find more potential problems).
|
I'd like to note that |
|
@rustbot reroll |
|
Sorry, this is completely out of my comfort zone... |
| /// will call `DerefMut::deref_mut` and `Deref::deref` *on the pointer type `Ptr`* | ||
| /// and expect these methods to uphold the pinning invariants. | ||
| /// By using this method, you are also making a promise about several trait | ||
| /// implementations of `Ptr` itself, if they exist. Most importantly, they |
There was a problem hiding this comment.
I'm wondering if there's some formulation of "if they exist" like "if they exist now or could exist in the future"...
There was a problem hiding this comment.
I could add an:
By using this method, you are also making a promise about several trait implementations of
Ptritself, if they exist. For fundamental types, this applies even if a downstream crate implements the trait.
There was a problem hiding this comment.
That seems like a good idea to me.
| /// | ||
| /// As an example, after unsizing coercing a pinned pointer, `deref_mut` must | ||
| /// not return a `#[repr(transparent)]` wrapper around the value it referenced | ||
| /// before being unsized, even if the address is unchanged. |
There was a problem hiding this comment.
Is this accurate? Or is changing to repr(transparent) wrapper fine as long as the impls are all the same on both, i.e., not actually malicious?
There was a problem hiding this comment.
We can choose to disallow obscure cases even if they are sound.
I mean, if the #[repr(transparent)] wrapper behaves identically to the original object, then I suppose it's ok. But I really don't see any reason to allow this kind of thing. In general, I'd expect that if I have a MyBox<MyStruct> and I coerce that to MyBox<dyn MyTrait>, then I would be able to deref it to &dyn MyTrait and invoke Any::downcast to get back a &MyStruct. But if MyBox somehow returns a &dyn MyTrait that actually corresponds to Wrapper<MyStruct>, then that downcast would fail. This kind of type change on coercion seems extremely surprising.
| /// refer to the same object. That is, the address returned by these methods | ||
| /// must not change. This applies even if the pointer type is moved. |
There was a problem hiding this comment.
Is this still pending an edit? I admit I don't quite understand why we have such a strong condition, but if we think it's reasonable then I don't have direct objections. My main concern with all these rules is that I am not sure how to audit for them effectively (especially as we seem to keep changing them as we find more potential problems).
83a842a to
ef9e727
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. |
|
@bors r+ I'm going to go ahead and approve this, I think we can continue iteration in subsequent PRs. The general idea seems like it's moving in the right direction. |
rust-bors
Bot
added
S-waiting-on-bors
…uwer Rollup of 9 pull requests Successful merges: - #156935 (Introduce a `PinSafePointer` trait that generalizes `PinCoerceUnsized`) - #159834 (Apply str debugger visualizer to `*const str`, `*mut str` and `Box<str>`) - #160663 (Suggest add async for function sig with return expr in body) - #160732 (Optimize slice::contains for one-byte BytewiseEq types) - #157944 (Make `char::is_default_ignorable` unstably public) - #158835 (rustc_passes: lint unused `#[path]` attributes on inline modules) - #160754 (Rename more diagnostic files to `diagnostics.rs`) - #160755 (Merge `rustc_lint/lints.rs` into `diagnostics.rs`) - #160757 (Merge `rustc_attr_parsing/session_diagnostics.rs` into `diagnostics.rs`)
Rollup merge of #156935 - Darksonn:pin-safe-pointer, r=Mark-Simulacrum Introduce a `PinSafePointer` trait that generalizes `PinCoerceUnsized` This PR renames `PinCoerceUnsized` to `PinSafePointer` and adds several new safety requirements about the implementations of various safe traits. Closes: #152667 Closes: #147794 ~~With this merged, the only remaining soundness issues with `Pin` are:~~ * #134407 * The fact that `CoercePointee` may be implemented on `Pin<LocalType>` in downstream crates. (unstable only) For your convenience here are the docs for the new trait: # `PinSafePointer` Trait that indicates that this is a pointer that does not misbehave when combined with `Pin`. Note that for backwards compatibility reasons, it is possible to create a `Pin<P>` for pointer types `P` that do not implement this trait. However, this can only be done safely if `<P as Deref>::Target` implements `Unpin`, which means that pinning has no effect. # Safety Types that implement this trait must not provide "malicious" implementations of any safe traits used by `Pin`. ## The pointer must always reference the same object Calls to [`deref`]/[`deref_mut`] on the same `Pin<P>` instance must always refer to the same object. That is, the address returned by these methods must not change. This applies even if the pointer type is moved. Furthermore, if the pointer type can participate in unsizing coercions or dynamic dispatch, then these coercions must also not change the underlying concrete type. Here, the concrete type of a trait object is the type that the vtable corresponds to. The concrete type of a slice is an array of the same element type and the length specified in the metadata. The concrete type of a sized type is the type itself. As an example, after unsizing coercing a pinned pointer, `deref_mut` must not return a `#[repr(transparent)]` wrapper around the value it referenced before being unsized, even if the address is unchanged. ## The pointer must not move its pointee The [`deref_mut`] method and the pointer type's destructor are called with a `&mut self` receiver, but they must behave as-if it was a `self: Pin<&mut Self>` receiver. That is, they must not move out of the underlying value. As an example, `deref_mut` must not invoke `swap` on the inner value. ## Shared access to the pointer If this pointer type uses `&P` references as evidence that this value is not pinned, then it must not treat the `&self` argument passed to [`Clone`] or the formatting traits (`fmt::Debug`, `fmt::Display`, `fmt::Pointer`) as such evidence. As an example, given a `Pin<Arc<T>>` there is no way to obtain an `&Arc<T>` (note that `Deref` just gives a `&T`). Because of this, the [`Arc`] type can assume that an `&Arc<T>` value can only exist if the `T` is not pinned, which justifies the soundness of the [`Arc::get_mut`] method. ## Cloning pinned pointers When a `Pin<P>` is cloned, the `P` pointer value returned by `clone` is passed to [`Pin::new_unchecked`]. The implementation of [`Clone`] must return a value such that this is sound. For example, when a `Pin<&T>` is cloned, the resulting `&T` points at the same value. The value is known to be pinned since a `Pin<&T>` to it exists, so it is safe to wrap the `&T` returned by `clone` in `Pin`. [`deref`]: https://doc.rust-lang.org/stable/core/ops/trait.Deref.html#tymethod.deref [`deref_mut`]: https://doc.rust-lang.org/stable/core/ops/trait.DerefMut.html#tymethod.deref_mut [`clone`]: https://doc.rust-lang.org/stable/core/clone/trait.Clone.html#tymethod.clone [`Clone`]: https://doc.rust-lang.org/stable/core/clone/trait.Clone.html [`Arc`]: https://doc.rust-lang.org/stable/std/sync/struct.Arc.html [`Arc::get_mut`]: https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.get_mut [`Pin::new_unchecked`]: https://doc.rust-lang.org/stable/std/pin/struct.Pin.html#method.new_unchecked r? lcnr
Darksonn
added
the
relnotes
Pkgsrc changes: * Adapt to changes in vendored crate versions. * Version & checksum changes. Upstream changes: Version 1.99.0 (2026-10-01) ========================== Language -------- - [Add allow-by-default `raw_borrows_via_references` lint that checks for references that decay immediately into raw borrows](rust-lang/rust#138230) - [Extend `unconditional_panic` lint to function calls that panic when the chunks/windows size is zero] (rust-lang/rust#153563) - [Stabilize C-variadic function definitions] (rust-lang/rust#155697) - [Stabilize the ability to use `#[unsafe(naked)]` functions to define C-variadic functions (`#![feature(c_variadic_naked_functions)]`).] (rust-lang/rust#159746) - [Trait methods are now resolved on an adjusted never type (producing a FCW)] (rust-lang/rust#156047) - [Coerce from inference variables to trait objects if the inference variable is related via subtyping to a type that is known to be `Sized`] (rust-lang/rust#157820) - [Stabilize `#[my_macro] mod foo;`] (rust-lang/rust#157857). This allows outlined modules (`mod foo;`) anywhere in the body of a custom attribute or derive macro. - [Fix the `overflowing_literals` lint with repeated negation] (rust-lang/rust#158302). For instance, it will now no longer lint on `--128_i8`, which is already detected by the `arithmetic_overflow` lint. - [Add POSIX symbols to the `invalid_runtime_symbol_definitions` and `suspicious_runtime_symbol_definitions` lints] (rust-lang/rust#158522) - [Lint unused `#[path]` attributes on inline modules] (rust-lang/rust#158835) - [Enable `unreachable_cfg_select_predicates` lint as part of `unused` lint group] (rust-lang/rust#159179) - [Stabilize passing 128-bit integers via vector registers with `asm!` on x86] (rust-lang/rust#159525) - [Explicitly document that some allocations are allowed to grow in-place (but none are allowed to shrink)] (rust-lang/rust#159729) - We now [guarantee] (rust-lang/rust#159730) that the contents of an `UnsafeCell` can be accessed without going through `get` - [The `invalid_reference_casting` lint was adjusted accordingly] (rust-lang/rust#159960) - [Account for globally enabled target features in `global_asm!`] (rust-lang/rust#160594) - [Warn if an invalid `doc` attribute is used on a macro invocation] (rust-lang/rust#161003) Compiler -------- - [Convert `-Ctarget-cpu` into a target-modifier for AVR, AMDGCN and NVPTX] (rust-lang/rust#150732) - [Enable `static_position_independent_executables` on all gnu and musl targets] (rust-lang/rust#158510) - When providing a suggestion about a missing method, rustc now prefers an exactly matching name from a [doc alias attribute] (https://doc.rust-lang.org/rustdoc/advanced-features.html#add-aliases-for-an-item-in-documentation-search) over a similarity search from other method names. If your new users sometimes expect a method under a different name, adding a doc alias will now help them find it via rustc suggestions, in addition to helping them find it via rustdoc search: [When suggesting method names, prefer *exact* doc aliases over similar names](rust-lang/rust#160369) Platform Support ---------------- - [Promote `riscv64-unknown-linux-musl` to Tier 2 with host tools] (rust-lang/rust#158766) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. [platform-support-doc]: https://doc.rust-lang.org/rustc/platform-support.html Libraries --------- - Iteration on `RangeInclusive` (`a..=b` ranges) is now [optimized better in some circumstances] (rust-lang/rust#155114). As a side effect of this, the behavior of `RangeInclusive` values that has already been exhausted (as an iterator) has changed. For example, the return values of `start()` and `end()` on such ranges may return different values, and using such ranges as slice indexes may have different behavior. These behaviors were not guaranteed to be stable, so these changes are considered to not be breaking changes. - [Relax `transmute_copy` to accept `?Sized` types] (rust-lang/rust#155989) - [Update `transmute_copy` to use a non-unwinding panic] (rust-lang/rust#155989) - [Don't escape U+FF9E and U+FF9F in `escape_debug_ext`] (rust-lang/rust#158057) - [Re-export `core::fmt::NumBuffer` in `alloc` (and `std`)] (rust-lang/rust#161430) Stabilized APIs --------------- - [`IntoIterator` for `Box<[T; N]>`] (https://doc.rust-lang.org/stable/std/iter/trait.IntoIterator.html#impl-IntoIterator-for-Box%3C%5BT;+N%5D,+A%3E) - [`IntoIterator` for `&Box<[T; N]>`] (https://doc.rust-lang.org/stable/std/iter/trait.IntoIterator.html#impl-IntoIterator-for-%26Box%3C%5BT;+N%5D,+A%3E) - [`IntoIterator` for `&mut Box<[T; N]>`] (https://doc.rust-lang.org/stable/std/iter/trait.IntoIterator.html#impl-IntoIterator-for-%26mut+Box%3C%5BT;+N%5D,+A%3E) - [`VecDeque::retain_back`] (https://doc.rust-lang.org/stable/std/collections/struct.VecDeque.html#method.retain_back) - [`core::ffi::VaList`] (https://doc.rust-lang.org/stable/core/ffi/struct.VaList.html) - [`Box::into_non_null`] (https://doc.rust-lang.org/stable/std/boxed/struct.Box.html#method.into_non_null) - [`Box::from_non_null`] (https://doc.rust-lang.org/stable/std/boxed/struct.Box.html#method.from_non_null) - [`Vec::into_parts`] (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#method.into_parts) - [`Vec::from_parts`] (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#method.from_parts) - [`core::mem::size_of_val_raw`] (https://doc.rust-lang.org/stable/core/mem/fn.size_of_val_raw.html) - [`core::mem::align_of_val_raw`] (https://doc.rust-lang.org/stable/core/mem/fn.align_of_val_raw.html) - [`core::alloc::Layout::for_value_raw`] (https://doc.rust-lang.org/stable/core/alloc/struct.Layout.html#method.for_value_raw) - [`String::from_utf8_lossy_owned`] (https://doc.rust-lang.org/stable/std/string/struct.String.html#method.from_utf8_lossy_owned) - [`string::FromUtf8Error::into_utf8_lossy`] (https://doc.rust-lang.org/stable/std/string/struct.FromUtf8Error.html#method.into_utf8_lossy) - [`FusedIterator for StepBy<I>`] (https://doc.rust-lang.org/stable/std/iter/struct.StepBy.html#impl-FusedIterator-for-StepBy%3CI%3E) - [`std::fs::set_times`] (https://doc.rust-lang.org/stable/std/fs/fn.set_times.html) - [`std::fs::set_times_nofollow`] (https://doc.rust-lang.org/stable/std/fs/fn.set_times_nofollow.html) Cargo ----- - Add a new built-in profile `debug`. This is a preparation for transitioning the `dev` profile away from debugging to give a saner default for faster development iterations. Currently there is no difference between `dev` and `debug` profiles. [docs] (https://doc.rust-lang.org/nightly/cargo/reference/profiles.html#debug-1) [#17214] (rust-lang/cargo#17214) - Workspace members on edition 2024 or later can now override an inherited workspace dependency's `default-features` field. For example, `serde = { workspace = true, default-features = false }` now turns off default features even when the workspace definition enables them. On earlier editions, `default-features = false` is ignored with a warning. ([RFC 3945] (rust-lang/rfcs#3945)) [#17126](rust-lang/cargo#17126) - Incremental compilation is now disabled by default when running in CI. CI is detected via the CI environment variable. [#17220] (rust-lang/cargo#17220) See also the [full Cargo changelog] (https://doc.rust-lang.org/nightly/cargo/CHANGELOG.html#cargo-199-2026-10-01) Rustdoc ----- - [Add new `unused_footnote_definition` rustdoc lint] (rust-lang/rust#137858) - Smarter filtering of trait impls yields performance improvements of 20% on average and up to 40% on some real-world crates. ([1] (rust-lang/rust#159623), [2](rust-lang/rust#159721), [3](rust-lang/rust#159779), [4](rust-lang/rust#159854), [5](rust-lang/rust#159091)) Compatibility Notes ------------------- - [Fully deprecate the legacy integral modules] (rust-lang/rust#146882). For example, `std::i32::MAX` should be accessed via `i32::MAX` instead. - [Upgrade `no_mangle_generic_items` into hard error] (rust-lang/rust#154585) - [The `Pin::new_unchecked` has had its safety invariants changed slightly] (rust-lang/rust#156935) - [Do not promote references to extern statics] (rust-lang/rust#157641) - [Ensure that the inferred types of `let` patterns typecheck] (rust-lang/rust#157841) - [hermit/fs: Return `unsupported()` instead of `from_raw_os_error(22)`] (rust-lang/rust#158247) - [Fixed a bug where `#[repr(simd)]` was accidentally allowed on macro invocations on stable Rust] (rust-lang/rust#158523) - [Abort const-eval when there are generics in the type of the value being produced] (rust-lang/rust#159504) - [Attributes not applying to anything are now an error in code blocks in doc comments] (rust-lang/rust#159849) - [`Box::leak`: tell people to avoid unleaking] (rust-lang/rust#160323) - [PowerPC inline ASM: Fix scalar floats being in the wrong vector lane on little endian] (rust-lang/rust#160441) - [Do not take `doc(cfg())` into account when filtering doctests] (rust-lang/rust#159014) - [Infer anonymous lifetimes in the types of associated consts as `'static`] (rust-lang/rust#156508) - Macros that expand to a semicolon now produce a warning lint (`semicolon_in_expressions_from_non_local_macros`) even when the macro comes from another crate. Previously, such warnings only appeared for macros from the same crate, to avoid showing warnings that can't be fixed locally; however, this masked problems, as integration tests from the crate providing the macro get compiled as a separate crate, so tests often wouldn't reveal this issue. If you encounter a lint like this, please make sure to report it to the crate providing the macro so they can fix it; don't just silence it in your own crate. - [`semicolon_in_expressions_from_macros`: Lint on non-local macros too] (rust-lang/rust#159222) - [Split non-local `semicolon_in_expressions_from_macros` into a separate lint] (rust-lang/rust#159700) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Update to LLVM 23] (rust-lang/rust#158734)
8 participants
View all comments
This PR renames
PinCoerceUnsizedtoPinSafePointerand adds several new safety requirements about the implementations of various safe traits.Closes: #152667
Closes: #147794
With this merged, the only remaining soundness issues withPinare:Pin::new’s check forTarget: Unpinbecomes insufficient #134407CoercePointeemay be implemented onPin<LocalType>in downstream crates. (unstable only)For your convenience here are the docs for the new trait:
PinSafePointerTrait that indicates that this is a pointer that does not misbehave when combined with
Pin.Note that for backwards compatibility reasons, it is possible to create a
Pin<P>for pointer typesPthat do not implement this trait. However, this can only be done safely if<P as Deref>::TargetimplementsUnpin, which means that pinning has no effect.Safety
Types that implement this trait must not provide "malicious" implementations of any safe traits used by
Pin.The pointer must always reference the same object
Calls to
deref/deref_muton the samePin<P>instance must always refer to the same object. That is, the address returned by these methods must not change. This applies even if the pointer type is moved.Furthermore, if the pointer type can participate in unsizing coercions or dynamic dispatch, then these coercions must also not change the underlying concrete type. Here, the concrete type of a trait object is the type that the vtable corresponds to. The concrete type of a slice is an array of the same element type and the length specified in the metadata. The concrete type of a sized type is the type itself.
As an example, after unsizing coercing a pinned pointer,
deref_mutmust not return a#[repr(transparent)]wrapper around the value it referenced before being unsized, even if the address is unchanged.The pointer must not move its pointee
The
deref_mutmethod and the pointer type's destructor are called with a&mut selfreceiver, but they must behave as-if it was aself: Pin<&mut Self>receiver. That is, they must not move out of the underlying value.As an example,
deref_mutmust not invokeswapon the inner value.Shared access to the pointer
If this pointer type uses
&Preferences as evidence that this value is not pinned, then it must not treat the&selfargument passed toCloneor the formatting traits (fmt::Debug,fmt::Display,fmt::Pointer) as such evidence.As an example, given a
Pin<Arc<T>>there is no way to obtain an&Arc<T>(note thatDerefjust gives a&T). Because of this, theArctype can assume that an&Arc<T>value can only exist if theTis not pinned, which justifies the soundness of theArc::get_mutmethod.Cloning pinned pointers
When a
Pin<P>is cloned, thePpointer value returned bycloneis passed toPin::new_unchecked. The implementation ofClonemust return a value such that this is sound.For example, when a
Pin<&T>is cloned, the resulting&Tpoints at the same value. The value is known to be pinned since aPin<&T>to it exists, so it is safe to wrap the&Treturned bycloneinPin.r? lcnr