Writing through a pointer derived from a shared reference, after that reference is dead

[RalfJung]

Using Miri, I found an interesting violation of my latest Stacked Borrows rules in the bytes crate:

impl Inner {
    /// Pointer to the start of the inline buffer
    unsafe fn inline_ptr(&self) -> *mut u8 {
        (self as *const Inner as *mut Inner as *mut u8)
            .offset(INLINE_DATA_OFFSET)
    }

    /// Return a mutable slice for the handle's view into the shared buffer
    /// including potentially uninitialized bytes.    
    unsafe fn as_raw(&mut self) -> &mut [u8] {
        debug_assert!(!self.is_static());
        if self.is_inline() {
            slice::from_raw_parts_mut(self.inline_ptr(), INLINE_CAP)
        } else {
            slice::from_raw_parts_mut(self.ptr, self.cap)
        }
    }
}

// Elsewhere
                let mut inner: Inner = mem::uninitialized();
                // Set inline mask
                inner.arc = AtomicPtr::new(KIND_INLINE as *mut Shared);
                inner.set_inline_len(len);
                inner.as_raw()[0..len].copy_from_slice(src);

What happens here is that Inner::inline_ptr takes a shared reference. It turns that into a *mut, which then is then returned to Inner::as_raw. That one creates an &mut, and then writes into it using copy_from_slice.

IOW, this function writes into a pointer that was created from a shared reference. Miri flags this as violation of the aliasing rules. The fix is to make inline_ptr take &mut.

What makes this interesting is that my usual answer for “what is interior mutability” has been “writing to a location where a live shared reference to the same location exists”, but that is not the case here: The &self is long dead by the time the write happens. What happens here, instead, is that creating a raw pointer from a shared reference makes the location readable but not writeable with raw pointers.

What do y’all think, who is right – Miri or the code? I think what Miri does makes sense here, but I could also imagine tweaks to the model that allow this code. But do we want to allow it?

Cc @carllerche

[RalfJung]

In fact, this is just another instance of the NonNull issue that I worked around, but that is really a deeper issue with our coercions.

[kornel]

Hmm, that’s a tough one. One one hand I’d prefer analysis that’s closer to checking whether it’s actually unsafe, rather than theoretically unsafe.

OTOH that unsafe function uses a “safe” Rust type, so it’s not too weird to require sticking to safe Rust semantics for “safe” types.

What would the fix look like if &mut self wasn’t possible? (e.g. because it was in a &self-taking function, similar to RefCell::borrow_mut()) Should it take *const Self?

[crlf0710]

I think miri is actually correct here. If user want to write bytes through this pointer, i think it’s reasonable for asking the code to prove that it “has the write permission for writing” at one point. Providing a inline_ptr returning * const and an inline_ptr_mut taking &mut seems reasonable.

However i think we’ve not raised this requirement (formally) before.

[RalfJung]

One one hand I’d prefer analysis that’s closer to checking whether it’s actually unsafe, rather than theoretically unsafe.

I don’t understand the distinction you are making here. This effort is about defining what is actually unsafe and what is not.

What would the fix look like if &mut self wasn’t possible? (e.g. because it was in a &self -taking function, similar to RefCell::borrow_mut() ) Should it take *const Self ?

In that case you’d have to use UnsafeCell, which is what RefCell does.

[kornel]

I don’t understand the distinction you are making here.

By “actually safe” (which now I see is a poor wording) I’ve meant when a piece of code happens to give the expected result on my machine with my particular version of the compiler, regardless of what theoretically could happen with other compiler, other optimizations, on other machine.

As a programmer, for my piece of mind, I’d prefer rules to be as close as possible to behavior of a simple, naive, non-optimizing implementation.

The gap of understanding between “pointer type casts do nothing, because they’re all just the same integer” and “pointer casts have subtle implications for aliasing rules that affect surrounding code and can cause UB which allows the compiler to replace the whole code with nasal demons” is scary.

[RalfJung]

As a programmer, for my piece of mind, I’d prefer rules to be as close as possible to behavior of a simple, naive, non-optimizing implementation.

That’s fair. However, unfortunately, the only way I know to make that true is to actually use a simple, naive, non-optimizing compiler. To realize the performance goals Rust is aspiring to hit, we need optimizations, and many of the interesting ones are inherently incompatible with a naive model.

“pointer type casts do nothing, because they’re all just the same integer”

Note that pointers are far from being just integers even if you ignore Stacked Borrows, and even in C.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.