[Pre-RFC] `field_projection`

To be clear, I am not preferring #[pin], I am still torn whether to use it or #[unpin].

If you place no #[pin] on your fields, then all projections would result in unwrapping. So there would be projections.

TBH I am not very fond of the example. Many things become unsound when you expose internal fields that have invariants associated with them. That is the reason why I was talking about encapsulation.

I think I understand where you are coming from: the proposal removes the need for unsafe at the projection site. Hence projections are safe. It now enables me to exhibit UB without writing any additional unsafe code.

Would this produce a compile error then?

let u: Pin<&mut Unmovable> = ...;
let _ = &mut u.data;
//           ^^^^^^ error cannot access field `data`

Ok I think I finally got what you wanted to tell me... (sorry that it took so long)

I have following library:

pub struct BufPtr {
    buf: [u8; 64],
    // null, or points into buf
    ptr: *const u8,
    // for some legacy reasons this is `pub` and cannot be changed
    pub len: u8,
    _pin: PhantomPinned,
}

impl BufPtr {
    pub fn new(buf: [u8; 64]) -> Self {
        Self {
            buf,
            ptr: ptr::null(),
            len: 0,
            _pin: PhantomPinned,
        }
    }

    pub fn next(self: Pin<&mut Self>) -> Option<u8> {
        // SAFETY: we do not move out of `ptr`
        let this = unsafe { Pin::get_unchecked_mut(self) };
        if this.ptr.is_null() {
            let buf: *const [u8] = &this.buf[1..];
            this.ptr = buf.cast();
            // here we set the len and it cannot be changed, since we are `!Unpin` and we are pinned.
            this.len = 1;
            Some(this.buf[0])
        } else if this.len >= 64 {
            None
        } else {
            this.len += 1;
            // SAFETY: `ptr` is not null, so it points into `buf`
            let res = Some(unsafe { *this.ptr });
            this.ptr = this.ptr.wrapping_add(1);
            res
        }
    }
}

And now the feature gets stabilized. Now users can write:

fn main() {
    let mut buf = Box::pin(BufPtr::new([0; 64]));
    loop {
        // field projection used here:
        buf.as_mut().len = 0;
        buf.as_mut().next(); // at some point we read some bad address
    }
}
1 Like

Thanks for bringing this up!

I guess our only options are:

  • require opt-in to pin projection (like you suggested), we could remove this in an upcoming edition (I think),
  • require explicit #[pin] and #[unpin] on fields (and not project the rest at all).
1 Like

Glad we're finally on the same page.

Yes, I suggest this to be a compile error. After thinking about it a bit, I think turning off field access entirely may be too restrictive. Instead, I'd propose that "excluded" fields can only be projected within an unsafe context. This way, you still get to use all of that nice syntax.

I think it would have to be field specific. And I support the form of requiring marking which fields get projected with wrapping (#[pin]), projected without wrapping #[unpin], and projected only in unsafe (no attribute).

I've also come around on the "importable attributes" idea. It could allow for some custom checks, like what I suggest below. I would probably name #[pin] / #[unpin] something like #[pin::project_wrapped] / #[pin::project_unwrapped] instead, though. I just think having use std::pin::{pin, unpin}; is pretty weird, but that's debatable.

I think that #[pin] / #[pin::project_wrapped] should trigger a warning (#[warn(useless_projection_wrapping)]) if used on a field with an Unpin type, since it will essentially allow moving out that value through projection anyways.

1 Like

Let's continue the unsoundness discussion on zulip.

I agree that this API satisfies the property that one cannot cause UB using this API and only safe code. That is certainly a necessary condition for soundness, but it is not entirely clear that it is sufficient. (A precise definition of soundness is super hard -- a first version of that definition takes more than 100 pages in my PhD thesis -- so we usually just say that one must not be able to cause UB from safe code. But in this case I think that leads to the wrong conclusion.)

For example (not sure if this example was already brought up here; it is by Quy Nguyen), a client might call Pin::get_unchecked_mut and then re-assign the len field. This needs unsafe code, but all requirements of get_unchecked_mut are satisfied -- if we had a formal system for this, we could prove this code correct.

So clearly, if we combine the unsafe module that declares this pinned type, and the unsafe module that uses get_unchecked_mut, we can have UB. Which of the two modules is buggy? I think a good argument can be made that the get_unchecked_mut module is perfectly fine, since it diligently added safety comments to each unsafe operation and everything checks out. Therefore it must be the other module which is buggy -- the "soundness proof" of that module relied on an invariant involving the len field, but public fields cannot carry custom invariants.

Therefore I don't think pin projections are to blame here, this module was unsound to begin with.

5 Likes