pointer-utils icon indicating copy to clipboard operation
pointer-utils copied to clipboard

Why is `ErasablePtr` not implemented for `Weak<T>`?

Open Kijewski opened this issue 1 year ago • 3 comments

The build.rs mentions

// NB: Requires this impl to cover T: ?Sized, which is not the case as of 2020-09-01.

But couldn't you add an implementation for a sized T, so one can use Weak<T> together with ptr_union?

Kijewski avatar Mar 22 '23 00:03 Kijewski

Worth noting that Weak::into_raw has covered T: ?Sized since rust 1.51 (of which @CAD97 is no doubt aware since he stabilised it, so presumably the impl can now be added irrespective?

eggyal avatar Mar 15 '24 16:03 eggyal

It just escaped me that this was actually properly possible to implement now. I've been dancing on a small rewrite/restructuring of the erasable crate for a while now, probably contributing to missing this.

The reason I didn't think about limiting the impl to just T: Sized at the time is that I was more thinking about the slice-dst usecase than the ptr-union one.

CAD97 avatar Mar 16 '24 18:03 CAD97

Rotating back to this: ErasablePtr has a requirement that Self::into_raw must be non-null and (critically) dereferenceable (enough to be unerased). Weak does not satisfy either of these properties, as a Weak::new()-created weak reference dangles and Weak::as_ptr specifically allows for the raw pointer form of such Weak to be dangling, unaligned, and/or null. The current implementation returns usize::MAX as *const T, which is dangling+unaligned, but (usize::MAX as *const T).wrapping_byte_add(offset_of!(RcInner<T>, value)) (dangling+unaligned) or 0 as *const T (dangling+null).

Note that it is fully possible to get the canonical dangling Weak for unsized pointee types by going through an unsizing coercion, e.g. Weak<[T; N]> to Weak<[T]>.

Additionally, please note that the dangling Weak relaxation of pointer properties is not limited to only canonical Weak::new() handles, but also extends to Weak handles after the last strong reference has been released. Weak is within its rights to canonicalize dangling references on Weak::into_raw.

For this reason, while impl<T: Sized> Erasable for Weak<T> could be sound, using Weak<T> in a ptr union is at best questionable. Making BuilderN<…, Weak<_>, …> will be unsound unless Weak guarantees dangling weak's into_raw is aligned, and making UnionN<…, Weak<_>, …> will be unsound unless Weak guarantees a pure into_raw. The latter I can see happening, but the former seems unlikely[^1].

[^1]: I see guaranteeing non-null as more likely a guarantee to provide than aligned, as it's an easier niche to utilize. Providing both isn't possible given Ts with both a high alignment and zero size (e.g. [f64x4; 0]) since every non-null aligned address for RcInner<T> is potentially a valid allocation address, as even null().wrapping_sub(1) can dereference to align bytes.

…Actually, this has made me realize that Thin has the same requirement of pure into_raw, and ErasablePtr directly implies it doesn't. I understand the requirements for erasable to be sound much better now than I did when I first wrote the crate, so I suppose it's time for a v2.0 that fixes these issues. Thankfully I believe that it's only ever doc ambiguity rather than true unsoundness.

CAD97 avatar May 02 '24 21:05 CAD97