Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Ralf Jung]

Hi all,

On 01.05.25 20:55, Benno Lossin wrote:
> On Thu May 1, 2025 at 7:12 PM CEST, Christian Schrefl wrote:
> > On 30.04.25 10:36 AM, Christian Schrefl wrote:
> > > +/// This type provides a way to opt-out of typical aliasing rules;
> > > +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
> > > +///
> > > +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
> > > +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
> > > +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
> > > +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
> > > +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
> > > +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
> > > +///
> > > +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
> > > +/// the public API of a library. It is an internal implementation detail of libraries that need to
> > > +/// support aliasing mutable references.
> > > +///
> > > +/// Further note that this does *not* lift the requirement that shared references must be read-only!
> > > +/// Use [`UnsafeCell`] for that.
> >
> > [CC Ralf]
> >
> > Ralf has replied to me on Github that this will most likely change [0]. How should this be handled?
> >
> > I would fine with submitting a patch once it changes on the rust side (possibly waiting until the
> > feature is close to stabilization). I think it is better to only add this guarantee later as it
> > will be easier to remove unnecessary `UnsafeCell`s than it would be to later add them back in ever
> > case where they would be needed (in case rust doesn't change `UnsafePinned` to act like `UnsafeCell`).
>
> Agreed, unless Ralf suggests a different way, we should do it like this.

Sorry, I replied only on github since that was easier to do more quickly. ;)

Yes I would recommend for now to keep the `UnsafeCell` in the kernel sources, 
until the compiler actually implements the change that removes `noalias` from 
`&UnsafePinned`. And even then you should probably keep the `UnsafeCell` when 
building for older compiler versions from before that patch (or keep it until 
you drop support for those older compiler versions).

This is not a spec-only change, codegen really has to be adjusted to make 
`&UnsafePinned` properly compatible with mutable aliasing, and I see no reason 
to risk potential problems here by prematurely removing that `UnsafeCell`.

Kind regards,
Ralf

> ---
> Cheers,
> Benno
>
> > Also see to the tracking issue [1] for the reason why `UnsafeCell` behavior is most likely required.
> >
> > [0]: https://github.com/rust-lang/rust/issues/125735#issuecomment-2842926832
> > [1]: https://github.com/rust-lang/rust/issues/137750
> >
> > Cheers
> > Christian