Questions about soundness

[Veetaha]

Hi. We are using your library at rust-analyzer, specifically rowan red-green syntax tree crate.

We have stumbled with invalid free() (here is the link to that issue).

I just took a quick look into the places where we use unsafe code and stepped into your crate. I am not entirely sure whether this comes from here, otherwise, I would've created a bug report.

I just have some questions about these lines of code here:

thin-dst/src/lib.rs

Lines 111 to 121 in 1da8b7a

	unsafe fn fatten_const(ptr: ErasedPtr) -> NonNull<Self> {
	let len = ptr::read(Self::len(ptr).as_ptr());
	let slice = make_slice(ptr.cast::<SliceItem>().as_ptr(), len);
	NonNull::new_unchecked(slice as *const Self as *mut Self)
	}
	unsafe fn fatten_mut(ptr: ErasedPtr) -> NonNull<Self> {
	let len = ptr::read(Self::len(ptr).as_ptr());
	let slice = make_slice_mut(ptr.cast::<SliceItem>().as_ptr(), len);
	NonNull::new_unchecked(slice as *mut Self)
	}

First of alI, I am not a pro in unsafe counterpart of Rust, please just correct me if I am wrong.
My questions are:

1. What is the intent of having two functions that have the same signature but operate on *const _ and *mut _ pointers in their body respectively? I've heard this has something to do with pointer provenance, though I am not entirely sure in this context.
2. It seems that we are casting ptr to usize and to SliceItem at the first and the second lines of these functions (which is like we are doing type punning), but I don't see why we would do that. Could you please elaborate?

[CAD97]

Hi! I actually published this library specifically to add it in rowan; I was the one pushing to get the inline allocation-based size reductions in.

I try to keep this library under "sanitization" with miri, and of course I believe the API I've published is fully sound, but there's always the possibility that I missed something.

1. The reason for this split is, in fact, at least related to pointer provenance. However, that's not the whole story. The functions that make this split unnecessary (by working directly on pointers and never touching references) are recently FCP-merge complete. The big reason that the split is required is that make_slice creates a shared reference and make_slice_mut creates a mutable reference. fatten_mut is used for &mut-like types that have unique, mutable access to the pointee, and fatten_const is used for &-like types that have shared, immutable access to the pointee. Because fatten_const creates a shared reference, the pointer that it creates cannot be used mutably. Because fatten_mut creates a unique reference, it requires that the pointer actually is not aliased and is derived from any other live pointers (i.e. is &mut or "&own" (box)).

   Using ptr::slice_from_raw_parts(_mut) makes this an unnecessary split, as they can do the *const T -> *const [T] transformation without actually manifesting a reference, which is what is actually putting the restrictions on the pointers here. Unfortunately, those functions are not stable until 1.42. The purpose of the polyfill module is ease of switching between the only-nightly-currently implementations and works-on-stable solutions transparently.

2. This is what the crate is about: we are in fact, type punning. The way the crate works is effectively by turning &T (or other pointer types) into *mut u8, erasing the type information. Which is how we erase the fat-pointer metadata and store just a thin (untyped) pointer. The fat pointer data is added back in by the fatten methods, which use make_slice(_mut) to turn *const T into *const [T]. The job of these methods is recovering the type information that we've hidden from the type system for storage purposes.

I'm actually working on a revision of this system of type erasure which should hopefully end up more powerful and more obvious.

If you can provide a small(er) reproduction case for the issue, I'd be happy to try to trace down the cause. I take pride in the correctness of my evil code 😜

[CAD97]

I'm going to go ahead and close this as most likely not our issue.