Skip to content

Add functions cast::transmute_slice, cast::transmute_mut_slice #9972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

lilyball
Copy link
Contributor

No description provided.

By adding io to the hidden std re-exports, this allows the use of
println!() inside of libstd.
#[inline]
pub unsafe fn transmute_slice<'a,L,G>(thing: &'a [L]) -> &'a [G] {
let mut slice: Slice<G> = transmute_copy(&thing);
let bytes = slice.len * mem::nonzero_size_of::<L>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment about why this can't overflow?

Also fix rand::isaac to transmute its slice properly. That didn't get
updated when the slice repr changed, so it was not properly seeding
itself.
@thestinger
Copy link
Contributor

I'm not convinced we should expose convenience functions for this. The usage in rand could be replacing by integer manipulation, and it would optimize down to the same code. In most cases, this pattern doesn't work because the data isn't equally distributed entropy where endianness doesn't matter.

In C and C++, casts like this are undefined behaviour because they break the type punning rules implemented to enable type-based alias analysis

@lilyball
Copy link
Contributor Author

@thestinger The problem with not exposing this is people will try it using transmute(), and it will fail horribly (and they may not even realize, e.g. the rand:isaac seed was broken without anyone noticing). No, it's not something people should be doing often, but then again, neither is transmuting in general. As long as we give people the tools to shoot themselves in the foot, we should try to help them avoid some of the more obvious blunders.

@lilyball
Copy link
Contributor Author

After talking with @thestinger, I get the impression that his primary objection is that TBAA makes most casts like this into undefined behavior (which is why C makes it undefined). Although technically it's only accessing the resulting object type through the pointer that's undefined, and not the cast itself (at least in C).

My response is twofold:

  1. If we don't provide this functionality, anyone who wants to coerce slices of one type to slices of another will find a way to do so regardless. Whether it's using the Slice repr, or converting to *T and then coercing that, or some other way, they will do it. This is dangerous and hard to catch errors in, as we've already seen by the fact that rand:isaac did this.
  2. Providing this functionality does not preclude introducing TBAA later. It just means that we have to restrict the ability to transmute slices once TBAA is introduced. This could potentially be done by introducing some form of compiler-provided trait CompatibleType<A,B> that is implemented for all compatible types A and B, thus allowing transmuting of slices only between compatible types, just as one example.

I'm also willing to restrict this to something like unsafe fn transmute_slice_to_u8<'a,T>(thing: &'a [V]) -> &'a [u8] (which is expressly allowed by C), as that's really the only cast that I explicitly need (this is the cast that rand uses as well). But that reintroduces my worry that anyone who needs a different coercion will simply do so in a way that is error-prone and may break later.

@lilyball
Copy link
Contributor Author

Upon further reflection, transmute_slice_to_u8() is not sufficient, because OSRng.fill_bytes() needs to cast back in reverse, from a &[u8] to a &[u64] and &[u32].

@huonw
Copy link
Member

huonw commented Oct 22, 2013

FWIW, I've replaced the default implementation of fill_bytes with something that doesn't do the [u8][u64] and [u32] casts in #10015, since they seem to be very unsafe; and those casts there entirely ignored any alignment concerns. Also, I've replaced the ISAAC seeding with vec::raw::{to_mut_ptr, mut_buf_as_slice}, which should avoid any problems with transmute making things unhappy.

@lilyball
Copy link
Contributor Author

I'm going to close this out in favor of #10015. I think we should have an official way of coercing a slice to &[u8], but it's not worth trying to get consensus on this right now.

@lilyball lilyball closed this Oct 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants