UnsafePinned: allow aliasing of pinned mutable references

[RalfJung]

Add a type UnsafePinned that indicates to the compiler that this field is "pinned" and there might be pointers elsewhere that point to the same memory. This means, in particular, that &mut UnsafePinned is not necessarily a unique pointer, and thus the compiler cannot just make aliasing assumptions. However, &mut UnsafePinned can still be mem::swaped, so this is not a free ticket for arbitrary aliasing of mutable references. You need to use mechanisms such as Pin to ensure that mutable references cannot be used in incorrect ways by clients.

This type is then used in generator lowering, finally fixing #63818.

Thanks to @rust-lang/opsem for a bunch of useful feedback!

Rendered

[RalfJung]

That example doesn't even touch on any of the problems discussed in this RFC. For instance, there are no mutable references.

[comex]

Strongly in favor. But bikeshed time:

unlike types like MaybeUninit or MaybeDangling that have an effect when wrapped around the pointer.

This wording initially threw me for a loop, given that it's much more common to see &mut MaybeUninit<T> than MaybeUninit<&mut T>. Then I remembered that when you want MaybeUninit to affect pointer validity requirements specifically, it has to be wrapped around a pointer. And in fact from a pure opsem perspective, &mut MaybeUninit<T> behaves the same as &mut T (right?). But I'd argue that line of reasoning sees the world through overly opsem-tinted glasses. From the perspective of intended or safe usage, &mut MaybeUninit<T> very much implies weaker validity rules for the pointee than &mut T; it's just that those rules aren't enforced until you dereference them at their respective types.

That said, I still think UnsafeAliased is a good name.

[RalfJung]

&mut MaybeUninit<T> very much implies weaker validity rules for the pointee than &mut T

Hm, yeah... however the aliasing and dereferenceable rules are still in full effect on &mut MaybeUninit<T> and that's what this RFC is call about.

I tried to clarify this in the RFC text.

[clarfonthey]

So, I definitely feel like UnsafeAliased isn't that great of a name, but for the primary reason that I think that a good unsafe API would have a safe counterpart, which here would just be called Aliased.

And when I think over and over again about what that kind of API would look like, I keep coming back to the same conclusion: isn't that just RefCell? I mean, obviously, they're not the same. But they feel the same in the sense that RefCell is accomplishing the same thing. Here, our data is inside the cell and the pointer is an Option<Ref<T>>. Obviously, it doesn't work this way, but this line of reasoning makes it feel weird to simply refer to the value as "aliased," even though that's the name of the optimisation (or rather, lack thereof) that gets applied.

Like, the confusion, I think, is the fact that "aliased" implies that the issue is someone else having a reference to the value, when the real issue is that the value itself cannot be referenced safely. So, in the first example given, the issue isn't that we've got the reference ptr_to_data, but rather the fact that data thinks it owns its value when it's lent its data to someone else.

I tried a bit to sketch this safe Aliased API, but I couldn't wrap my head around the details. Although the terminology that made sense to me was to consider it a "Slot," since the value could be "moved" out of its slot and has to be "moved" back into it before the slot can be used again. Internally, this would just be accomplished via noalias, but a term like UnsafeSlot to me says something along the lines of, I can take the value out of the slot (get a pointer to it), but in order to use the value again, I must put it back (relinquish borrow). This will solve some of the confusion around UB with multiple aliases (you can't have multiple aliases, it's just that any actions done around the value can't be assumed to be of something other than the value itself, since one of them might be "putting it back in").

I had a rough time properly formulating this since, although noalias is a simple optimisation, the actual way it breaks the borrow mechanics are hard to describe. I probably did a terrible job, but maybe it helps improve the way this is explained.

[RalfJung]

So, I definitely feel like UnsafeAliased isn't that great of a name, but for the primary reason that I think that a good unsafe API would have a safe counterpart, which here would just be called Aliased.

A safe counterpart would be some Pin-based construction that allows self-referential pointers. That's why UnsafePinned is a proposed alternative. But I don't think that's necessarily a good name since there might be uses of this that do not involve pinning.

I also think UnsafeCell is not a great name, since it says basically nothing about what is actually happening. (That's also why I don't like "slot" at all. "slot" and "cell" aren't even obviously referring to different concepts; UnsafeSlot would be just as good a name for our interior mutability primitive as UnsafeCell.) UnsafeSharedMutable would be a lot better. So then maybe this one here should be UnsafeMutableAliased?

Like, the confusion, I think, is the fact that "aliased" implies that the issue is someone else having a reference to the value, when the real issue is that the value itself cannot be referenced safely.

I don't understand the distinction you are making between a reference existing to the value vs the value being referenced.

the issue isn't that we've got the reference ptr_to_data, but rather the fact that data thinks it owns its value when it's lent its data to someone else

So... UnsafeMaybeBorrowed?

[clarfonthey]

A safe counterpart would be some Pin-based construction that allows self-referential pointers. That's why UnsafePinned is a proposed alternative. But I don't think that's necessarily a good name since there might be uses of this that do not involve pinning.

I mean, even such construction would be incomplete, TBQH. In the second example given, for example, data is accessed without first "dropping" the borrowed pointer, and this would be required if there were an actual mutable reference to the data. The mutability is precisely what requires this extra construct; self-referentiality on its own isn't enough.

I also think UnsafeCell is not a great name, since it says basically nothing about what is actually happening. (That's also why I don't like "slot" at all. "slot" and "cell" aren't even obviously referring to different concepts; UnsafeSlot would be just as good a name for our interior mutability primitive as UnsafeCell.) UnsafeSharedMutable would be a lot better. So then maybe this one here should be UnsafeMutableAliased?

I sympathise with the concept of "cell" being a horrible name, although what I like about it is that it has actually two names: Cell, and "interior mutability." While Cell isn't the safe counterpart for UnsafeCell, it's the simplest one, and without Copy semantics you need to branch into more complicated abstractions like RefCell, Mutex, etc.

In terms of the "second" name here, what we're offering is effectively an alternative to &own references or &move references via an unsafe API. What we're doing is effectively an owning reference, since simple self-referentiality can be accomplished without the need for these unsafe-aliasing features.

I don't understand the distinction you are making between a reference existing to the value vs the value being referenced.

In terms of ownership, ptr_to_data has exclusive control over the value whereas data does not. So, if someone accesses the value via data, they're technically violating those ownership guarantees, since ptr_to_data is in control. So, effectively, obtaining a mutable reference to data has to come with the unsafe assertion that it actually does own its own data, and no mutable references to the data exist elsewhere. This also comes with a compile-time indication that operations on the newly-created reference can't be reordered with respect to other mutations, since any one of them could be the one that actually owned the data beforehand.

So... UnsafeMaybeBorrowed?

One other term I thought of was "disowned" since data is effectively no longer in control of its own ownership. This also closer resembles the other versions of this feature which were proposed, like references which actually move values.

[RalfJung]

Usage which can be made sound without UnsafeCell semantics are quite rare2

That's wrong. I am not aware of a single usage that needs UnsafeCell semantics. The current implementation of the Unpin hack does not have UnsafeCell semantics, and Miri is perfectly happy with self-referential generators and async fn and all sorts of other pinning shenanigans.

[VitWW]

UnsafeAliased<T> is not a good naming. AlreadyBorrowed<T> / ConsiderBorrowed<T> / AssumeBorrowed<T> is much better name

[RalfJung]

@clarfonthey

I mean, even such construction would be incomplete, TBQH. In the second example given, for example, data is accessed without first "dropping" the borrowed pointer, and this would be required if there were an actual mutable reference to the data.

ptr_to_data is a raw pointer, not a reference, so it doesn't have to be "dropped". Using both it and the field back-and-forth is intended to be legal.

And even if it was a reference, this example would be fine -- the lifetime of the reference ends after the write(42). Generators (and by extension async fn) will generate code that does exactly that. The example matches the following generator:

let mut data = 0;
let ptr_to_data = &mut data;
yield;
*ptr_to_data = 42;
println!("{}", data);
return;

The mutability is precisely what requires this extra construct; self-referentiality on its own isn't enough.

This is trivially true in the sense that if everything is read-only then there are no aliasing violations. Self-referential webs of shared references can be created in safe code.

But for better or worse, we need self-referential data with mutable references, e.g. to support the Future::poll API.

Maybe you want to say that UnsafeAliased is an incomplete name since "aliased" is fine, we allow aliasing all the time via shared references? I agree. It should be UnsafeAliasedDespiteMutableReference. That's a bit too long though...

I sympathise with the concept of "cell" being a horrible name, although what I like about it is that it has actually two names: Cell, and "interior mutability." While Cell isn't the safe counterpart for UnsafeCell, it's the simplest one, and without Copy semantics you need to branch into more complicated abstractions like RefCell, Mutex, etc.

In terms of the "second" name here, what we're offering is effectively an alternative to https://github.com/rust-lang/rfcs/issues/998 or https://github.com/rust-lang/rfcs/pull/1646 via an unsafe API. What we're doing is effectively an owning reference, since simple self-referentiality can be accomplished without the need for these unsafe-aliasing features.

Insofar as "interior mutability" is a (terrible) name for "mutability behind shared references", the corresponding thing for UnsafeAliased would be "aliasing / alternative incoming pointers despite mutable references". I don't think there is any relation to &own/&move.

In terms of ownership, ptr_to_data has exclusive control over the value whereas data does not. So, if someone accesses the value via data, they're technically violating those ownership guarantees, since ptr_to_data is in control. So, effectively, obtaining a mutable reference to data has to come with the unsafe assertion that it actually does own its own data, and no mutable references to the data exist elsewhere. This also comes with a compile-time indication that operations on the newly-created reference can't be reordered with respect to other mutations, since any one of them could be the one that actually owned the data beforehand.

Nobody is creating a mutable reference to data though. People are creating mutable references to S, of which data is a field, and we somehow have to indicate that this should not be considered as also making the usual "mutable reference promises" for data.

One other term I thought of was "disowned" since data is effectively no longer in control of its own ownership. This also closer resembles the other versions of this feature which were proposed, like references which actually move values.

It would be MaybeDisowned though -- some of the time the field does have all the ownership. In fact when you have a regular &mut S you still get all the ownership for all the fields -- otherwise mem::swap would be unsound.

You can't really go with any kind of ownership story for this RFC since the ownership of &mut T is fully determined already. That's why self-referential futures need pinning -- they need to get away from that mutable reference and its strong ownership requirements.

[RalfJung]

The other potential name which I'm partial to is UnsafePinCell. This leans more into naming based on usage instead of effect, as well as leaning on PhantomPinned (which could be potentially defined as UnsafePinCell<()>). This becomes more interesting of a naming direction if a) we commit to infectious cell semantics and b) provide a PhantomMut as UnsafeCell<()> / Freeze optout.

I think we should avoid "cell", but I like UnsafePinned and it is an alternative mentioned in the RFC. If we impl !Unpin for UnsafePinned, we could define PhantomPinned as UnsafePinCell and that would be backwards compatible. We could add a new unsafe trait that detects UnsafePinned the way Freeze detects UnsafeCell (called Unique in the RFC but I hope there are better names), and switch codegen over to using that to opt-out of noalias for &mut, and that would be backwards compatible except for code that does impl !Unpin (which is unstable).

Without infectious cell semantics, switching Miri over to Unique would not be backwards compatible, but that's okay.

So yeah I really like the idea of re-defining PhantomPinned as UnsafePinned<()> (no matter the name we end up picking).

[ahicks92]

SO something I don't like about this RFC is that it requires tracking the provenance of a reference through all code you control, in case that reference somehow aliases.

My guess is that it's only a matter of time until someone uses this wrong internally, but in a important crate like arc_swap (as an example--they don't need it I think), and that relies on the current optimization behavior of the compiler at whatever time this happens. If you consider one person writing unsafe code in one sitting it's possible to get this right, but if given to a team or maintained over a long period of time, I feel like this asymptotically approaches very hard to spot problems.

Unfortunately I absolutely don't have a better suggestion here. I'd offer one if I could. The drawbacks section does acknowledge this, but the core crate concern makes me flinch a bit. I don't want to be the one who finds miscompilation because of something that's a 5-times dependency and we decided to upgrade Rust. I'm not sure how we'd even track that back to the problem. Unfortunately something like this is probably necessary. I just don't like what appears to me to be a major incident waiting to happen. Imagine if this took out tokio.

[Darksonn]

Imagine if this took out tokio.

It's interesting that you mention Tokio. I'm one of the maintainers of Tokio. Tokio does have unsafe code that relies on the so-called Unpin hack to write custom self-referential futures (well, mostly futures with intrusive linked lists). In fact, I suspect that Tokio is the single largest user of the Unpin hack on crates.io.

We justify this because it lets us eliminate a bunch of allocations, and I have been monitoring the state of these issues for a very long time, and I have always been ready to replace the Unpin hack with a proper solution once one is stabilized. For example, see this gist that I wrote back in Jan 2021 advocating for pretty much the solution proposed here.

[ahicks92]

@Darksonn I'm not saying Tokio itself will get this wrong. Tokio also has much less dependencies than I expected. I used it as the example because it is a core, popular crate that would be disastrous to many people if it broke. In terms of me trusting Tokio itself, I do.

For the sake of not conflating me picking on a specific crate, let's just say that there is some popular crate x with many dependencies. Consider:

• T+0: big team incorporated or whatever depends on x.
• whenever this rfc is stable: some dependency of x uses this rfc wrong.
• 6 months or more down the line: Rustc starts getting more aggressive in a way that breaks the broken dependency of x. In the meantime, x has done a major release and no longer supports the version we started with, and none of their devs are interested in trying to refactor the older version.
• 1 year after this rfc: big team just upgrades the Rust compiler. Their software now crashes in weird "wait is this actually C++" heisenbug ways.

We have team members who do reverse engineering of pdbs and the like even, all of whom know Rust at this point. I suppose Miri can in some cases help track this back. I will admit that I am not up to date on the formal verification tooling, so maybe this is easy. But getting as far as "it's a miscompilation" would take days with my current knowledge and getting as far as "from this dependency" would take...infinite time? I'm not sure how we would take that last step even if we figured out the first one.

Maybe I'm being too paranoid. I expect this to land anyway because clearly the current state of affairs is worse than this hypothetical scenario. But I did at least want to come out of my cave to represent this as someone who has done and code reviewed unsafe, taught Rust to others, etc. in the context of a medium-sized corp who isn't interested in Rust because Rust. Our Rust upgrades have gone 100% smoothly even over a year ago when we had to use nightly (we no longer use nightly; long story, tldr rocket for initial POC of adoption). I don't want to lose that, at least in so much as current Rust unsafe hasn't lead to a catastrophe for the ecosystem yet.

[RalfJung]

@ahicks92 I think your concerns are very valid, but this might be the wrong RFC to express them on. You seem to be concerned about having (more or less) strict aliasing rules in general, since those are very subtle forms of UB. However, rejecting this RFC won't help with your concern at all -- it will only make it worse since some code (like self-referential generators) becomes even harder to write!

Eventually we'll have to RFC an aliasing model, and decide what exactly the rules are that &mut and & follow throughout the program. That is where your concern applies. But right now, Rust had an informal idea of "we have aliasing restrictions" since 1.0 ("at least as restrictive as is needed to justify noalias annotations"), and self-referential generators have violated those rules since they exist (even the fairly lax LLVM noalias rules, let alone the much more strict Stacked Borrows rules), and this RFC provides a way to fix that. I share your general uneasiness about complicated aliasing rules, but

• I think the chances of us wanting to drop the LLVM noalias annotations are basically 0, so the simplest possible aliasing model we can have is noalias. (The ones we have currently are more complicated, with the goal of providing more optimizations.)
• This RFC is needed even for that simplest possible aliasing model.

So I don't see what the goal of discussing general concerns about aliasing model complications is in the context of this RFC. Unless you want to advocate for the alternative of "just don't have aliasing rules", this is not the right venue for your concerns, I think.

[RalfJung]

Our Rust upgrades have gone 100% smoothly even over a year ago when we had to use nightly (we no longer use nightly; long story, tldr rocket for initial POC of adoption). I don't want to lose that, at least in so much as current Rust unsafe hasn't lead to a catastrophe for the ecosystem yet.

That sounds like you are concerned about the upgrade path if this lands. I have recently edited that section -- I think we have an upgrade plan that ensures that code which is LVM-UB-free today will remain LLVM-UB-free. In other words, people running Miri might see failures where there were none before, but this change itself will not introduce any new chances of miscompilation. In fact it will remove the potential miscompilations that currently come with an entirely safe impl Unpin.

[ahicks92]

I was trying to make a more specific point, but now I am re-learning that Rust aliasing even in unsafe is currently somewhat ill-defined, and I suppose that you're right that what I'm saying is isomorphic to discussing whether Rust should allow aliasing everywhere. Looking at this again I do see that there are other foot-guns I just haven't encountered. That wasn't my goal, and in fact I think Rust probably should disallow aliasing. Given that we were apparently having different conversations, carry on; apparently I didn't understand things I thought I understood.

[823984418]

What is the difference between this and UnsafeCell without UnsafeCell::get_mut?

[RalfJung]

The RFC talks a lot about how this is different from UnsafeCell, from the initial paragraph to the overview here.

[823984418]

Attention:

UnsafeCell without UnsafeCell::get_mut

---

I am not asking if there is a semantic difference between the two.

That's entirely analogous to how UnsafeCell lets the compiler know that shared references to this field might undergo mutation.

What I mean is, is UnsafeCell::get_mut the only difference in implementation between the two.

[programmerjake]

What I mean is, is UnsafeCell::get_mut the only difference in implementation between the two.

maybe in the library but not in the compiler.

[Darksonn]

@823984418 They differ in their lang attributes:

• UnsafeAliased uses #[lang = "unsafe_aliased"]
• UnsafeCell uses #[lang = "unsafe_cell"]

This will make the computer treat them differently in the ways described in the rfc.

[823984418]

Of course, using language item is one way to implement it, but what I want to ask is whether encapsulating UnsafeCell and remove UnsafeCell::get_mut is another way.

Just like:

use std::cell::UnsafeCell;

struct UnsafeAliased<T: ?Sized> {
    value: UnsafeCell<T>,
}

impl<T: ?Sized> UnsafeAliased<T> {

    // all others

    // delete it
    // pub fn get_mut(&mut self) -> &mut T {
    //     self.value.get_mut()
    // }
}

[Darksonn]

No, the lang attribute is important. What you described gives you something that makes immutable references allow mutation (like UnsafeCell), but what we actually want us something that makes mutable references allow aliasing. These are different things.

[RalfJung]

In particular: when the compiler decides whether to add noalias to an &T argument, it searches T for UnsafeCell. When it decides whether to add noaloas to an &mut T argument, it searches T for UnsafeAliased. Those are completely different roles played by these types, and by looking only at the library API you will not understand what these types are for.

[ahicks92]

@823984418 It may be helpful to think of these types as weird syntax for attributes, as an analogy (emphasis analogy, though). For one thing the type lets the attribute "follow" the value around, which is why it's not just an attribute.

Like, Arc<UnsafeCell<T>> is Arc<#[unsafe_cell]T>.

The methods are relevant in so much as you get an API to drive it correctly, which is one reason it can't ever be an attribute, but the methods aren't the point of having either.

[withoutboats]

(NOT A CONTRIBUTION)

I think this is a great and sensible solution to the problem that self-referential future bodies raised, and in retrospect an obviously missing API addition. I'm curious what other library improvements might be enabled by it in the future - e.g. maybe using this instead of raw pointers for intrusive data structures will have some benefit.

As to the name, I think the current name is fine. If anything, it reveals that UnsafeCell was sort of misnamed, and should maybe have been called something like UnsafeMutable instead. C'est la vie.

[823984418]

I think I can understand them, struct has UnsafeCell field that refuse to add no_alias when transfer immutable borrow argument, and UnsafeAliased is further the same when transfer mutable borrow argument.

But, like UnsafeCell, it has a more 'rusty' semantic meaning. The purpose of adding no_alias is only to present runtime reference alias rules internally, there is no no_alias in this process, no_alias is just a means to implement it in LLVM.

But UnsafeAliased directly violates this point, as in the current expression, it allows for multiple mutable references.

My idea is that maybe his name is MaybeBorrowed better, change its semantics to: "the contents may have been borrowed".

After that, the difference between UnsafeCell and MaybeBorrowed is that &mut UnsafeCell can believing that the interior has not been borrowed, but &mut MaybeBorrowed can't.

[RalfJung]

"borrowed" is somewhat ambiguous here, since borrowing usually refers to references in Rust, but generally speaking with UnsafeAliased, the aliasing pointers will be raw pointers.

I think UnsafePinned expresses well the idea that "there are other pointers pointing here" and points to Pin as a potential way to achieve a safe abstraction, which is why I increasingly gravitate to that name. Just to get a read of the room -- can we get a quick vote? :+1: for UnsafePinned, :-1: for UnsafeAliased. (I don't plan to do language design by vote/committee, but I am curious how people generally feel about these two names.)

[Darksonn]

Yeah, I don't think borrowed is the right word. This type is concerned with the rules that even unsafe code must follow, which is a different ruleset from the borrow checker rules. However, the word "borrowed" only really makes sense when using the borrow checker rules.

[823984418]

Wait, for a struct T that includes UnsafeXXX, is it sufficient to remove no_alias for Pin<&mut T>?

Is it unnecessary to remove no_alias for all &mut T?

If explained in this way, I think UnsafePinned would become a better name.