Closed steffahn closed 8 months ago
On that second part of the issue: I supposed, Deref
can be salvaged though. Removing the Copy
& Clone
from Slice<T>
by virtue of replacing it by some
#[repr_transparent]
pub struct Slice<T>(RawSlice<T>);
wrapper that can remove the Copy
&Clone
(so only the contained hidden RawSlice<T>
, not exposed to users, keeps the Copy
impl), while internal usage, e.g. for implementing SliceRef
, can still use RawSlice<T>
to keep the needed Copy
implementation.
Thank you for pointing this out. It is difficult to predict all the failure modes for unsafe
and this is one that hadn't been on my radar. The Nomicon is great but I wish there was some kind of footgun checklist. I will try to address this soon. It seems that Vec
gets around this issue because slice
is unsized, which I tried really hard to make work with DST structs but ultimately gave up on. Maybe something like that is still possible, but for now I will mitigate the issue.
I think I've managed to address this. There is now a regression test showing that the offending code no longer compiles:
https://github.com/tim-harding/soapy/blob/9f3fb757dc055a47bc1883a8c8d4ecdf526deb39/src/borrow_tests.rs#L84-L94
The trick was making Slice
dependently-sized, with a defaulted type parameter that makes Slice
dynamically sized. Users code will always get the dynamically sized type for references, meaning that mem::swap
is no longer possible. Internally, the type parameter can be set to a zero-sized type like ()
when slices need to be held by value, and internal code allows for casting between different representations. The dynamically-sized field type used is [()] so that it takes up no space, but we can still embed the slice length in the fat pointer with some casting trickery. Feel free to reopen if you think this is still unsound. I'm still pretty new to the pitfalls of unsafe
in Rust.
…i.e. never forget the existence of
mem::swap
. Here’s a problematic code exampleUnless of course
Soa
was somehow made to deal with blatantly incorrectcapacity
values… which seems impossible becausecapacity
is safety-critical information, e.g. instead of out-of-bounds access like above (which can be checked by looking at thelen
), access of uninitialized data is the whole point of whatcapacity
tries to prevent.This unfortunately kills the deref-based API, at least as far as
DerefMut
is concerned, at least you can’t move out of a shared reference so theDeref
part does still stand?Nevermind. As I’m writing this, I’m seeing the next problem:
the type
Slice<T>
isCopy
, and thus can just be copied out from behind even a shared immutable reference, kept without a lifetime, and lead to use-after free.