Cell, references, and struct layout

[rpjohnst]

This is a kind of follow-up to this thread:

Idea: DerefPin/DerefCell

Currently, Pin offers the unsafe map method, which is safe to use when returning a Pin to a struct field or other interior pointer that preserves Pin's constraints. There is the idea of a #[derive]-like macro to automatically generate safe accessor methods to make this nicer. Similarly, Cell has RFC 1789, which offers a safe conversion from &Cell<[T]> to &[Cell<T>]. This should also apply to struct fields, but there’s no easy way to express this in the type system. If Pin<'a, T> and &'a Cell<T>…

I wrote this post on Reddit but realized it would probably get a better response here, so I'll quote it:

@Rusky

I've been trying to come up with a way to implement the last point on this list for structs generically (called Cell field projection), based on a derive like the one I did for soak, but it's kind of a mess right now.

The most straightforward way involves getting the offset of the field. But the language spec is not complete enough yet to say what is actually allowed in unsafe code, and the language doesn't provide a primitive for it either. The closest option is demonstrated in memoffset, but it's still unsound.

Cell field projection does have one advantage that offset_of lacks- we know we have a valid object already. So maybe we could do something like Ref::map(_split) with a closure? But even if the closure is unsafe and generated by a macro, at first glance it involves creating an unwrapped reference to the interior of the Cell! Maybe "less" unsound than offset_of, but not ideal.

A further alternative might be for the macro to generate a version of the struct with its fields wrapped in Cells. This might let us cast the original struct to this new one before passing it to the closure. But is that struct guaranteed to have the same layout? And even if it does, what does the macro call this alternative struct and what scope does it go in? A kludge even if it's sound.

Am I understanding the situation correctly? Is the best approach just to wait for memoffset to be made sound, or is there a trick I'm missing?

[pitdicker]

@rpjohnst

But even if the closure is unsafe and generated by a macro, at first glance it involves creating an unwrapped reference to the interior of the Cell! Maybe "less" unsound than offset_of, but not ideal.

I believe it is safe to create a reference to the interior inside a method on Cell, because at that time there are no other references (Cell doesn't hand out references), and there are no other methods active at the same time.

[CAD97]

@pitdicker

I believe it is safe to create a reference to the interior inside a method on Cell , because at that time there are no other references ( Cell doesn't hand out references), and there are no other methods active at the same time.

This is safely available as Cell::get_mut: if you have exclusive access to the (Unsafe)Cell, it is sound to provide an internal reference.

Doing this with a shared reference is unsound.

The underlying desired operation (breaking &Cell<Struct> into &Cell<Field>s) is sound, but tricky.

Doing this with raw pointer manipulation (&raw or memoffset + ptr::offset) is the only way to do so without passing through bad internal references.

You can do this 100% soundly given #[repr(C)], or with memoffset. I'd say "just" use memoffset, as it's currently "UB but not exploited", and will be updated to be not fully safe and sound as soon as that becomes possible, and contains safeguards against misuse.

[pitdicker]

We are probably talking about a different thing. Inside a method on Cell, as an implementation detail, it should be safe to use a normal reference. Cell::replace actually does this.

[RalfJung]

@pitdicker

I believe it is safe to create a reference to the interior inside a method on Cell , because at that time there are no other references ( Cell doesn't hand out references), and there are no other methods active at the same time.

Yeah, the key property is that between the time the shared reference is created and the last time it is used, no mutation of the memory it points to happens. Stacked Borrows additionally requires that shared references passed as function arguments also don't have their pointee mutated until the respective function returns. But a temporary shared reference to interior mutable data that is not actually currently being mutated is fine.

[CAD97]

The key point I forgot and overlooked: Cell: !Sync, so only one location in the code is able to work on it at a time. This means Cell<T>::with_ref(impl for<'a> FnOnce(&'a T) -> R) -> R would be sound.

[RalfJung]

@CAD97

The key point I forgot and overlooked: Cell: !Sync , so only one location in the code is able to work on it at a time. This means Cell<T>::with_ref(impl for<'a> FnOnce(&'a T) -> R) -> R would be sound.

No, it's not.

I feel like we should have a collection of unsound ideas somewhere, or else the same proposals are going to come up again and again. Though this time there is a funny timely coincidence where the proposals happened basically at the same time... or, is it really coincidence?^^

[CAD97]

@RalfJung

No, it's not.

Well that shows be to double check and think rather than phone posting while technically on vacation

A resource of subtle "this would be nice and seems ok at first but is actually unsound because..." would be awesome to have!

[Centril]

@RalfJung

I feel like we should have a collection of unsound ideas somewhere, or else the same proposals are going to come up again and again.

-- really great idea! Maybe somewhere in the UCG?

[pitdicker]

To do a shameless plug: the comment from @rpjohnst started as a reply on my blog post collecting extensions to interior mutability types (which was again inspired by a comment from @rpjohnst ), also listing why some are unsound. Maybe a starting point? I am currently interested in collecting a little more.

[RalfJung]

@Centril

-- really great idea! Maybe somewhere in the UCG?

Doesn't sound like the worst place to start, at least.

[rpjohnst]

Thanks to this thread, I threw together a small proof-of-concept crate: https://www.abubalay.com/blog/2020/01/05/cell-field-projection

[RustyYato]

Could Cell projection be done as a macro, like this? Also your dioptre looks a lot like my RFC from a while back, and I made an unpublished crate to go along with the RFC

[rpjohnst]

@RustyYato

Could Cell projection be done as a macro, like this?

Yep! (Though I believe your &mut is unsound.) It's simple enough that I would probably just do that directly in a lot of scenarios, without pulling in a crate.

I published dioptre more because I have some use cases for the crate as a whole, and as a place to point people as an explanation.

[RustyYato]

@rpjohnst

(Though I believe your &mut is unsound.)

How so?

[rpjohnst]

It's a &mut to an object with at least one valid & still live, probably more in actual use. Dioptre instead does this followed by this.

Without the intervening offset calculation you could just go from a &Field to a &Cell<Field> via as, instead of trying to go via Cell::from_mut.

[RustyYato]

@rpjohnst

It's a &mut to an object with at least one valid & still live, probably more in actual use. Dioptre instead does this followed by this.

That doesn't follow. That's like saying that Cell::set is unsound because it creates a &mut T while setting the value. There is nothing about creating this &mut T is wrong. Because for the entire lifetime of the &mut T there is no other way to access the pointee (T) other than going through the &mut T. Also the a new Cell is immediately created from the &mut T, so it doesn't leak to the outside world.

This does pass MIRI (although that isn't proof, it's still strong evidence because this is what MIRI was designed to catch).

@rpjohnst

Without the intervening offset calculation you could just go from a &Field to a &Cell<Field> via as , instead of trying to go via Cell::from_mut .

Now this is UB, because if Field: Freeze, then &Field is immutable. But &Cell<Field> is most certainly not immutable. You must use Cell::from_mut or pointer casts from *mut T to *const Cell<T> to soundly get a Cell.

---

Also, there is a cost to using function pointers, so I'm not sure if that's the best way to do this. You can look into my unpublished crate, where I made a trait for field which handles the projection an lots of type-level meta programming to encode multiple fields and get the rest of the features. Though that may be a bit too heavy weight and hard to maintain.

[rpjohnst]

@RustyYato

Now this is UB, because if Field: Freeze , then &Field is immutable. But &Cell<Field> is most certainly not immutable. You must use Cell::from_mut or pointer casts from *mut T to *const Cell<T> to soundly get a Cell.

But this is exactly what I asked for clarification about in this thread! Ralf seems to say it's sound here: Cell, references, and struct layout - #6 by RalfJung

@RustyYato

there is a cost to using function pointers

They're const, so that shouldn't be an issue. It's certainly much lighter, and more forward-compatible with offset_of, to do it this way.

[RustyYato]

There @RalfJung is talking about shared references into the Cell<T>, i.e. &T not &Cell<T>. In my example, the only things that could alias the &mut T are &Cell<T>. But given that the &mut T was derived from the &Cell<T> this is fine. It's the same as reborrowing. As long as I don't interleave accesses from &Cell<T> and &mut T I'm fine (and I don't interleave accesses).

[rpjohnst]

@RustyYato

There @RalfJung is talking about shared references into the Cell<T> , i.e. &T not &Cell<T> .

So am I...