Is this program sound?

I'v been working on a module to allow safely extending the lifetime of an immutable reference to a value; My current implementation realises on the fact that it is impossible to distinguish between an immutable reference to a bitwise copy of a value, and moving a the value to a new location, doing whatever operation requires said immutable reference, and then moving it back.

Implementation

1 Like
unsafe impl<T: ?Sized> Freeze for &mut T {}
unsafe impl<T:?Sized+Freeze> Share for T{}

It's UB to duplicate a &mut. More generally if you want that blanket impl, the safety requirements for Freeze need to include those for Share.

There's also some safety comments about a CopyToShare which isn't otherwise present that imply there may be even more requirements.

Miri flags this as UB.

4 Likes

After having read about Stacked Borrows, I think that this is a case in which Stacked Borrows is wrong. The Stacked Borrows violation occurs because MiRi assumes that at some point the &mut T inside the MannualyDrop is going to be mutably dereferenced at some point, which, if it ever occurred, would be Undefined Behaviour. However, the only method that touches the field containing the &mut T is deref which only exposes an immutable reference to the &mut T, which prevents the user from mutably dereferencing it. As long as the &mut T is never mutably dereferenced, it is effectively just a &T.

As a work-around, I have removed the Share trait from &mut to prevent MiRi from complaining about Stacked Borrows violations. The code is safe, but it seems like it is just MiRi complaining about the fact that it could be a Stacked Borrows violation. I have also added a HideMutFromMiRi struct to the program, which has exactly the same behaviour as a &mut T except it doesn't cause unnecessary Stacked Borrows violations.

Updated version

PS: The Freeze impls were just copied from core(line 717 in core::marker). The impl for Share is justified on the basis that there is no way of distinguishing between two references that point to the same data without comparing their addresses,which would be unreliable because it relies on the data not being moved. The Freeze bound exists because otherwise it would be extremely obvious that they were different because otherwise changes in one would not reflect in the other.

PPS:Copy to share was something I was working on for when a type couldn't implement Share directly but transparently wrapped something that could. It became obvious relatively quickly that the only type for which this was usefully true was Cell<T:Drop> which combined with the fact that it was hard to develop led to it being dropped.

That's not correct, because of the exclusivity.

If you want to reason just about the accesses made, then you want to wrap pointers. But &mut is special because even if you never use it, it still needs to be exclusive.

3 Likes

Is having multiple aliasing &mut T a safety invariant or a validity invariant.

Rust will optimize based on it at the language level, so I'm pretty sure it's validity.

If you want a safety invariant instead, you spell that something like struct FakeMutRef<'a, T>(*mut T, PhantomData<&mut 'a T>);.

1 Like

After looking through the rust reference, I think it is safe because the exclusivity of &mut T is based on LLVM's noalias attribute. According to the documentation "This guarantee only holds for memory locations that are modified , by any means, during the execution of the function.". So if only a &&mut T is available, it prevents modification through the mutable reference, cancelling out the noalias guarantee.

First of all, that's "how the implementation details work today" level reasoning; it doesn't mean it's not UB on the Rust level.

Second of all, you're still violating &mut exclusivity by allowing someone to hold on to a &&mut while the underlying &mut gets used.

    let i_can_see_you = y.as_ref(); // println!("{:?}", y);
    *rx += 1;
    println!("{rx} @ {rx:p}");
    println!("{i_can_see_you:?} @ {:p}", *i_can_see_you);

Or if you prefer, here's a data race.

7 Likes

I think I fixed that bug in the updated version. Line 73 new should require a lifetime of 'a for the reference passed in and the corresponding change for new_inplace.

1 Like

That is how it is defined in the rust reference though.

I haven't had a chance to look at your updated example yet, sorry (and sorry for overlooking that on my last reply too).

But on the topic of what exactly is allowed, the situation is, I'm afraid, still up in the air -- not actually defined as Rust has no spec yet, and still actively being discussed. (The reference is non-normative and that particular section gets its own red box too.) And I have definitely seen the assertion that the mere existence of an aliasing &mut and &[mut] (where both are usable at the same time) is UB. So until a weaker model is formally accepted, that's the one I'll be using for any soundness considerations.

Related...

1 Like

Is there a reason (&mut,) is still allowed?

Everything left besides that which implements Freeze also implements Copy, so you could just copy things directly with no unsafe at that point.

Incidentally, HideMutFromMiri could just be

struct HideMutFromMiRi<'a, T: ?Sized>(
    *mut T,
    PhantomData<&'a mut T>,
);

and I'm not sure "hiding" is as healthy an approach as "satisfying".


I think the main thing left after that is new_inplace. I suggest align_to_mut to get rid of the alignment calculations. You also do need the commented repr(transparent) or there's no guarantee superfluous padding or the like wasn't added to your struct.

impl<'a, T: ?Sized + Share + 'a> RefProxy<'a, T> {
    pub fn new_inplace<'b, 'c: 'a + 'b>(
        loc: &'b mut [MaybeUninit<u8>],
        value: &'a T,
    ) -> &'c mut RefProxy<'c, T> {
        // Safety: Both the source and the result are MaybeUninit
        let (_, loc, _): (_, &mut [MaybeUninit<T>], _) = unsafe {
            loc.align_to_mut()
        };

        let loc = &mut loc[0];
        let refmut: &mut T = MaybeUninit::write(loc, *value);
        
        // Safety: repr(transparent)
        unsafe { &mut *( refmut as *mut T as *mut RefProxy<'_, T>) }
    }
}

But this is still unsound due to the lifetime constraints, which allow one to do this:

    let buf: [MaybeUninit<u8>; 100] = MaybeUninit::uninit_array();
    let mut buf = Box::new(buf);
    let local = 10;
    let rp: &'static mut _ = RefProxy::new_inplace(&mut *buf, &local);
    drop(buf);
    println!("{rp:?}");

You can't return a &'c mut that lasts longer than 'b. Perhaps you just got the relationships backwards? Due to the covariance involved, I think this is adequate:

    pub fn new_inplace<'b>(
        loc: &'b mut [MaybeUninit<u8>],
        value: &'a T,
    ) -> &'b mut RefProxy<'a, T> 
    where
        'a: 'b,

Sorry for the late reply. I haven't checked my notifications for a while.

Yes, it's really just a workaround until I can find a better way to fix it.

I thought about using align_to_mut originally, but it requires that the type that you align to is Sized, which defeats the main point of using new_inplace(ie to allow constructing an instance of RefProxy for unsized types.)

Thanks for catching that. I'll fix it;

While Rust's aliasing model is currently unknown, I think this would be sound under the new Tree Borrows(link), because mutable references are first placed it a reserved state when they are created; This means that they can alias other live references without conflict until they are used to mutate, and because the user can only get an immutable reference to the mutable reference, they cannot mutate though it.(I haven't actually tested my program under Tree Borrows yet because I'm having difficulty updating MiRi; This is just based on the blog post the designer of it made.)

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.