cast_slice is not safe

[bluss]

Hi, I'm looking at gfx-rs and thinking of using it, but I can't drop my usual habit of reviewing unsafe code.

cast_slice is not a safe abstraction, it should not be exposed as a safe function. If you make this function private, then it's only a concern for your library internals, but it's not good that it's public.

Example breakage. usize and &T (for any T) are both Copy, but of course transmuting arbitrary integer into a reference does not make a valid reference (it may be dangling, mutabily aliased, not aligned, or null for example).

Other issues: Breaks type safety. A type may be Copy but only allow construction through explicitly exposed functions, that preserve certain invariants for example.

Recommended resolution: Demote the function to private if possible. If you need "pod casting", make an unsafe trait that is implemented for explicitly Pod compatible types, like the integer types, and only allow the cast to those types.

[kvark]

Agreed, thanks for looking into it @bluss!

[kvark]

Actually, wouldn't cast_slice be safe if T: 'static + Copy?

[bluss]

No. It still violates type safety which means it defeats all kinds of smart abstractions people have built.

(And it still allows creating invalid &'static T references for any T: 'static. For example: Making a &'static String from arbitrary bytes.)

[kvark]

@bluss you can't have static String, and so you can't have &'static String either, not without unsafe code at least. Could you show a simple example where using fn cast_slice<T: 'static + Copy> would violate the safety?

[bluss]

You're wrong about that not being possible.

T: 'a means "T outlives 'a" and is true when T does not contain references or when all references in type T outlive 'a.

So for the String type we have String: 'static because it does not contain references; which we can also understand as "we have an indefinite lease for values of String type; we can keep them around for as long as we want".

T: 'a allows the type &'a T, and thus &'static String is allowed.

I have to use your example since it is a simple null dereference crash:

https://play.rust-lang.org/?gist=34793fd61ed9c6a260bad5e33a9d01e2&version=nightly&backtrace=0

[kvark]

Hmm, you are right. I wonder if it's possible to say that T is a POD type. Obviously, references like &'static X are not POD.

[kvark]

Doesn't look like there is a trait to help. Oh well, I don't mind marking cast_slice as unsafe.

[bluss]

References are not the only problem, just an example. Here's another type invariant that must be preserved: only values 0, 1 can be stored in a bool. cast_slice as it is can easily circumvent that. Add to that all type invariants that a user library may construct and rely on.