gfx_hal::memory::cast_slice can cause UB

[Lokathor]

I was pointed to use cast_slice to easily send push constants to a pipeline, but upon inspection it became clear that the method doesn't respect alignment and can easily trigger UB.

Corrected code is as follows:

pub fn cast_slice<T: Pod, U: Pod>(ts: &[T]) -> Option<&[U]> {
  use core::mem::{align_of, size_of};
  // Handle ZST (this all const folds)
  if size_of::<T>() == 0 || size_of::<U>() == 0 {
    if size_of::<T>() == size_of::<U>() {
      unsafe {
        return Some(core::slice::from_raw_parts(
          ts.as_ptr() as *const U,
          ts.len(),
        ));
      }
    } else {
      return None;
    }
  }
  // Handle alignments (this const folds)
  if align_of::<U>() > align_of::<T>() {
    // possible mis-alignment at the new type (this is a real runtime check)
    if (ts.as_ptr() as usize) % align_of::<U>() != 0 {
      return None;
    }
  }
  if size_of::<T>() == size_of::<U>() {
    // same size, so we direct cast, keeping the old length
    unsafe {
      Some(core::slice::from_raw_parts(
        ts.as_ptr() as *const U,
        ts.len(),
      ))
    }
  } else {
    // we might have slop, which would cause us to fail
    let byte_size = size_of::<T>() * ts.len();
    let (new_count, new_overflow) = (byte_size / size_of::<U>(), byte_size % size_of::<U>());
    if new_overflow > 0 {
      return None;
    } else {
      unsafe {
        Some(core::slice::from_raw_parts(
          ts.as_ptr() as *const U,
          new_count,
        ))
      }
    }
  }
}

Please note that this version returns an Option<_> instead of immediately doing a panic, and the end user can handle the Option layer in whatever way they feel is appropriate.

[zakarumych]

This one is still pretty UB-ish.
The only one safely acceptable conversion is to &[u8] which in turn shouldn't have any alignment issues.

[zakarumych]

Btw. Casting from ZST to non-ZST type should just give empty slice, instead of None

[Lokathor]

I guess you could make it give an empty slice. (Edit: the current setup has the property that if you do get a non-None output, you can then cast that output back and perfectly round trip)

But where is the other UB?

[zakarumych]

Aliasing rules doesn't allow to reinterpret T as arbitrary U.
IIRC Rust follows C's strict aliasing rules here.
This means that casting &f32 into &i32 can confuse compiler as it assumes &f32 and &i32 never points to the same memory location.
Casting to chars, signed and unsigned alike, is always valid in C to inspect memory as raw bytes. Those types are i8 and u8 in Rust.

[Lokathor]

Rust doesn't do Type Based Alias Analysis and also it would be breaking to add it, so we can pretty much assume that it will never be added.

Further, "Turning an &mut T into an &mut U" is in the transmute docs as an example of something that transmute _could_ do but which you should accomplish using safer techniques instead (since you can just do pointer casts)

[kvark]

If we can't provide a simple helper, let's not provide any at all. The responsibility to produce the slice that is well aligned would be on user's shoulders.

[msiglreith]

can easily trigger UB

are there any issues with the current POD types or does this refer to users manually implementaing the POD trait for structs?

[Lokathor]

Sorry, I should have explained that part. Existing Pod impls can trigger UB because it's UB to have any unaligned reference. If you have a four element &[u8] at (for example) bytes 1,2,3,4 and then you cast that into a single element &[u32] you'd be at byte 1, which is misaligned. This is a common concern whenever you're casting to a larger alignment type via any style, in any language (even in langs without references it's UB to read from an unaligned pointer).

[Lokathor]

Updated the code to have improved clarity about what case is what, and it const optimizes down to the correct stuff for a particular monomorphization without actually "doing" the match if statement.