Re: [PATCH v2 2/2] rust: arc: remove `ArcBorrow` in favour of `WithRef`

From: Boqun Feng
Date: Tue Sep 26 2023 - 14:21:19 EST


On Tue, Sep 26, 2023 at 05:15:52PM +0000, Benno Lossin wrote:
[...]
> >
> > But here the difference it that we only derive a `*mut` from a `&`,
> > rather than transmute to a `&mut`, right? We only use `&` to get a
> > pointer value (a usize), so I don't think that rule applies here? Or in
> > other words, does the following implemenation look good to you?
> >
> > impl<T: ?Sized> Arc<T> {
> > pub fn as_with_ref(&self) -> &WithRef<T> {
> > // expose
> > let _ = self.ptr.as_ptr() as usize;
> > unsafe { self.ptr.as_ref() }
> > }
> > }
> >
> > impl<T: ?Sized> From<&WithRef<T>> for Arc<T> {
> > fn from(b: &WithRef<T>) -> Self {
> > // from exposed
> > let ptr = unsafe { NonNull::new_unchecked(b as *const _ as usize as *mut _) };
> > // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
> > // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
> > // increment.
> > ManuallyDrop::new(unsafe { Arc::from_inner(ptr) })
> > .deref()
> > .clone()
> > }
> > }
> >
> >
> > An equivalent code snippet is as below (in case anyone wants to try it
> > in miri):
> > ```rust
> > let raw = Box::into_raw(arc);
> >
> > // as_with_ref()
> > let _ = raw as usize;
> > let reference = unsafe { &*raw };
> >
> > // from()
> > let raw: *mut T = reference as *const _ as usize as *mut _ ;
> >
> > // drop()
> > let arc = unsafe { Box::from_raw(raw) };
> > ```
>
> I don't understand why we are trying to use ptr2int to fix this.
> Simply wrapping the `T` field inside `WithRef` with `UnsafeCell`
> should be enough.
>

BTW, how do you fix this with only wrapping `T` field in `WithRef`?

Let say `WithRef` is defined as:

struct WithRef<T> {
refcount: Opaque<bindings::refcount_t>,
data: UnsafeCell<T>,
}


impl<T: ?Sized> From<&WithRef<T>> for Arc<T> {
fn from(b: &WithRef<T>) -> Self {
let data_ptr: *mut T = b.data.get();

let ptr = ?; // how to get a pointer to `WithRef<T>` with the
// provenance to the whole data?

ManuallyDrop::new(unsafe { Arc::from_inner(ptr) })
.deref()
.clone()
}
}

The `data_ptr` above only has provenance to part of the struct for the
similar reason that my proposal of (ab)using `b.refcount.get()`. Am I
missing something here?

Regards,
Boqun

> --
> Cheers,
> Benno
>
>