Skip to content

Commit

Permalink
fix: unaligned read for Metal buffers (#882)
Browse files Browse the repository at this point in the history
After Rust 1.77 `u128` starts requiring 16 bytes aligned values. This
breaks Metal, where `retrieve_contents` assumed it could just cast the
pointer and treat it as a slice.
Instead, we need to assume no alignment by doing the following:
1. Read the `i-th` element from the buffer with
   `ptr.add(i).read_unaligned()`;
2. `clone` the read value, as the bitwise copy may cause aliasing
   otherwise;
3. `push` the `clone`d value into the vector;
4. `mem::forget` the element we read to avoid calling its `drop`
   implementation twice, one for the copy and one for the `Buffer`.
  • Loading branch information
Oppen authored Jul 19, 2024
1 parent 2a68c19 commit 07657e9
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions gpu/src/metal/abstractions/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,21 @@ impl MetalState {
/// Returns a vector of a copy of the data that `buffer` holds, interpreting it into a specific
/// type `T`.
///
/// BEWARE: this function uses an unsafe function for retrieveing the data, if the buffer's
/// SAFETY: this function uses an unsafe function for retrieveing the data, if the buffer's
/// contents don't match the specified `T`, expect undefined behaviour. Always make sure the
/// buffer you are retreiving from holds data of type `T`.
pub fn retrieve_contents<T: Clone>(buffer: &Buffer) -> Vec<T> {
let ptr = buffer.contents() as *const T;
let len = buffer.length() as usize / mem::size_of::<T>();
let slice = unsafe { std::slice::from_raw_parts(ptr, len) };

slice.to_vec()
let mut contents = Vec::with_capacity(len);
for i in 0..len {
// 1. Read possibly unaligned data producing a bitwise copy
let val = unsafe { ptr.add(i).read_unaligned() };
// 2. Clone into the vector to avoid both `contents` and `buffer` dropping it
contents.push(val.clone());
// 3. Forget the bitwise copy to avoid both `val` and `buffer` dropping it
core::mem::forget(val);
}
contents
}
}

0 comments on commit 07657e9

Please sign in to comment.