Skip to content

Commit

Permalink
examples/export: Replace unsound to_padded_byte_vector() implementa…
Browse files Browse the repository at this point in the history
…tion with `bytemuck`.

The function `to_padded_byte_vector()` is unsound because:

* It accepts an arbitrary `Vec<T>` without checking that `T` contains no
  padding, which is UB to read in any way including by reinterpreting as
  `u8`s.
* It produces a `Vec` which thinks it has a different alignment than the
  allocation was actually created with.

To fix these problems, this change:

* Uses `bytemuck` to check the no-padding condition.
* Creates a new `Vec` instead of trying to reuse the existing one.
  (Conditional reuse would be possible, but more complex.)

An alternative to `bytemuck` would be to make `to_padded_byte_vector()`
an `unsafe fn` (or to accept `Vertex` only instead of `T`). However, I
think it is valuable to demonstrate how to do this conversion using safe
tools, to encourage people to use safe code instead of writing unsafe
code without fully understanding the requirements.
  • Loading branch information
kpreid committed Jan 13, 2025
1 parent 88e719d commit 24bcd54
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 9 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ members = ["gltf-derive", "gltf-json"]

[dev-dependencies]
approx = "0.5"
bytemuck = { version = "1.21.0", features = ["derive"] }

[dependencies]
base64 = { optional = true, version = "0.13" }
Expand Down
17 changes: 8 additions & 9 deletions examples/export/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ enum Output {
Binary,
}

#[derive(Copy, Clone, Debug)]
#[derive(Copy, Clone, Debug, bytemuck::NoUninit)]
#[repr(C)]
struct Vertex {
position: [f32; 3],
Expand All @@ -42,15 +42,14 @@ fn align_to_multiple_of_four(n: &mut usize) {
*n = (*n + 3) & !3;
}

fn to_padded_byte_vector<T>(vec: Vec<T>) -> Vec<u8> {
let byte_length = vec.len() * mem::size_of::<T>();
let byte_capacity = vec.capacity() * mem::size_of::<T>();
let alloc = vec.into_boxed_slice();
let ptr = Box::<[T]>::into_raw(alloc) as *mut u8;
let mut new_vec = unsafe { Vec::from_raw_parts(ptr, byte_length, byte_capacity) };
fn to_padded_byte_vector<T: bytemuck::NoUninit>(data: &[T]) -> Vec<u8> {
let byte_slice: &[u8] = bytemuck::cast_slice(data);
let mut new_vec: Vec<u8> = byte_slice.to_owned();

while new_vec.len() % 4 != 0 {
new_vec.push(0); // pad to multiple of four bytes
}

new_vec
}

Expand Down Expand Up @@ -171,7 +170,7 @@ fn export(output: Output) {
let writer = fs::File::create("triangle/triangle.gltf").expect("I/O error");
json::serialize::to_writer_pretty(writer, &root).expect("Serialization error");

let bin = to_padded_byte_vector(triangle_vertices);
let bin = to_padded_byte_vector(&triangle_vertices);
let mut writer = fs::File::create("triangle/buffer0.bin").expect("I/O error");
writer.write_all(&bin).expect("I/O error");
}
Expand All @@ -188,7 +187,7 @@ fn export(output: Output) {
.try_into()
.expect("file size exceeds binary glTF limit"),
},
bin: Some(Cow::Owned(to_padded_byte_vector(triangle_vertices))),
bin: Some(Cow::Owned(to_padded_byte_vector(&triangle_vertices))),
json: Cow::Owned(json_string.into_bytes()),
};
let writer = std::fs::File::create("triangle.glb").expect("I/O error");
Expand Down

0 comments on commit 24bcd54

Please sign in to comment.