Cell, references, and struct layout

Ok, so now I'm confused, why do you think that my implementation might be unsound?

I haven't said anything about your implementation's soundness since my first parenthetical and this response. :slight_smile: But on that subject: I'm less sure that it's unsound now, but I'm still not convinced you need &mut.

1 Like

Ok, sorry for the noise. Jumped the gun a bit, :sweat_smile:

That is a surprisingly short macro :smile:! I don't see any issues with the mutable reference in it.

It's also a question of whether the testcase actually manages to exhibit the bad interaction. :wink: For example, the playground snippet doesn't even set the field.

However, even with that, I cannot get Miri to complain. Still the &mut doesn't seem entirely kosher to me, and I think it would be better avoided: the &mut T is (in some sense) live as long as any of its derived pointers is alive (after all, they will all be above the &mut T on the borrow stack), so I would expect that any write to a pointer further down the stack will invalidate the &mut and any pointer derived from it. That this does not seem to happen here might have to do with the fact that Miri does not precisely track raw pointers currently. But morally, I think this code does violate uniqueness of &mut.

5 Likes

In that case I think we need to wait for &raw mut before we can safely project through Cell or use offsets like @rpjohnst did.

What I was getting at here...

...is that you shouldn't need to explicitly calculate an offset like dioptre; instead you should be able to take a shared reference to the field and then cast it to include the Cell wrapper.

This is basically the &mut approach, but without the &mut. Admittedly I'm not an expert on stacked borrows, but I thought that would be sufficient to prevent invalidation of the new &Cell.

This is definitely wrong because &T should be immutable, so casting it to a mutable reference is UB.

But, again, it would never be cast to a mutable reference.

&Cell<T> is a mutable reference, a shared mutable reference

&mut T is also a mutable reference, but a unique mutable reference

...but only after the last time it is used. There are no other &Fields, and the fact that the &Cell<Field> is derived from it shouldn't have the same problem as in the &mut case because of how stacked borrows treats shared references differently. (Again that last bit is just my current understanding.)

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=86ff9864f9cd15d117261b4cccc3e596

fn main() {
    use std::cell::Cell;
    
    let x = 5;
    
    let x_ref = &x;
    let x_cell_ref: &Cell<u32> = unsafe { std::mem::transmute(x_ref) };
}

This fails MIRI, just creating a &Cell<T> from a &T is UB. Even if you never use it

That's not what I'm suggesting you do, though! Your example creates a &Cell pointing to something that was neither originally in a Cell nor mutable. This variant passes MIRI: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f03b760990a023f1a746cddc6903338b

Edit: Maybe this is just the imprecision Ralf mentioned though, because this fails: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=54ef15f841ed2a75fea0c10bc462dbbf

This suggests that either a) none of this works at all, and the offset approach only passes MIRI because of the raw pointer imprecision, or b) the offset approach is okay because it lets the &Cell<Field> be derived directly from the &Cell<Struct>.

1 Like

I think this is it unfortunately

Yeah, casting a &T to &Cell<T> and then writing to that is UB, no matter where the &T originally came from. Shared references are read-only.

This has to be done with raw pointers and raw pointers only, and lacking &raw I agree with @Yato that the best approach is (outer_ref.as_ptr() as *mut u8).offset(field_offset), and then casting that to &Cell<Field>. That should work, as there are no intermediate references.

5 Likes