Skip to content

Commit 61dffa9

Browse files
authored
Use an unsafe trait to reinterpret packed struct references as bytes (#7336)
## Summary This is a cosmetic change to make it clearer what is happening when re-interpreting references as raw bytes to serialize plans, making it harder to mis-use by making it a more specific trait. Signed-off-by: Adam Gutglick <adam@spiraldb.com>
1 parent b4dca29 commit 61dffa9

File tree

1 file changed

+20
-16
lines changed
  • vortex-cuda/src/dynamic_dispatch

1 file changed

+20
-16
lines changed

vortex-cuda/src/dynamic_dispatch/mod.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -86,22 +86,26 @@ pub fn tag_to_ptype(tag: PTypeTag) -> PType {
8686
}
8787
}
8888

89-
/// Serialize a `#[repr(C)]` struct to a byte vector for the packed plan.
89+
/// Reinterpret a reference to a packed type as a raw bytes slice.
9090
///
91-
/// Copies field data into a pre-zeroed buffer so padding holes are
92-
/// deterministically zero, avoiding UB from reading uninitialised bytes.
93-
fn as_bytes<T: Sized>(val: &T) -> Vec<u8> {
94-
let n = size_of::<T>();
95-
let mut buf = vec![0u8; n];
96-
// SAFETY: T is a bindgen-generated #[repr(C)] struct with only
97-
// integer/float/enum fields. We overwrite the zeroed buffer with
98-
// the struct's bytes; padding holes keep their zero value.
99-
unsafe {
100-
std::ptr::copy_nonoverlapping(std::ptr::addr_of!(*val).cast::<u8>(), buf.as_mut_ptr(), n);
101-
}
102-
buf
91+
/// # Safety
92+
///
93+
/// The caller must ensure `T` is a `#[repr(C)]` type whose layout is
94+
/// compatible with the C ABI. All the types we serialise (`PlanHeader`,
95+
/// `PackedStage`, `ScalarOp`) are bindgen-generated `#[repr(C)]` structs.
96+
/// Padding bytes may be uninitialised on the Rust side, but the C reader
97+
/// never inspects them, so the values are irrelevant.
98+
unsafe trait AsPackedBytes: Sized {
99+
/// Reinterpret a `&T` as a byte slice for serialization into the packed plan.
100+
fn as_packed_bytes(&self) -> &[u8] {
101+
unsafe { std::slice::from_raw_parts(std::ptr::addr_of!(*self).cast(), size_of::<Self>()) }
102+
}
103103
}
104104

105+
unsafe impl AsPackedBytes for PlanHeader {}
106+
unsafe impl AsPackedBytes for PackedStage {}
107+
unsafe impl AsPackedBytes for ScalarOp {}
108+
105109
/// A stage used to build a [`CudaDispatchPlan`] on the host side.
106110
///
107111
/// This is NOT a C ABI struct — it exists purely on the Rust side and is
@@ -210,7 +214,7 @@ impl CudaDispatchPlan {
210214
output_ptype,
211215
plan_size_bytes: total_size as u16,
212216
};
213-
buffer.extend_from_slice(&as_bytes(&header));
217+
buffer.extend_from_slice(header.as_packed_bytes());
214218

215219
for stage in &stages {
216220
let packed_stage = PackedStage {
@@ -221,9 +225,9 @@ impl CudaDispatchPlan {
221225
num_scalar_ops: stage.scalar_ops.len() as u8,
222226
source_ptype: stage.source_ptype,
223227
};
224-
buffer.extend_from_slice(&as_bytes(&packed_stage));
228+
buffer.extend_from_slice(packed_stage.as_packed_bytes());
225229
for op in &stage.scalar_ops {
226-
buffer.extend_from_slice(&as_bytes(op));
230+
buffer.extend_from_slice(op.as_packed_bytes());
227231
}
228232
}
229233

0 commit comments

Comments
 (0)