Skip to content

Commit addc36f

Browse files
committed
Add safety comments to usages of byte_add (Ptr, PtrMut, OwningPtr) (#7214)
# Objective The usages of the unsafe function `byte_add` are not properly documented. Follow-up to #7151. ## Solution Add safety comments to each call-site.
1 parent 5fd628e commit addc36f

File tree

1 file changed

+31
-11
lines changed

1 file changed

+31
-11
lines changed

crates/bevy_ecs/src/storage/blob_vec.rs

+31-11
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,8 @@ impl BlobVec {
233233
#[must_use = "The returned pointer should be used to dropped the removed element"]
234234
pub unsafe fn swap_remove_and_forget_unchecked(&mut self, index: usize) -> OwningPtr<'_> {
235235
debug_assert!(index < self.len());
236+
// Since `index` must be strictly less than `self.len` and `index` is at least zero,
237+
// `self.len` must be at least one. Thus, this cannot underflow.
236238
let new_len = self.len - 1;
237239
let size = self.item_layout.size();
238240
if index != new_len {
@@ -244,6 +246,10 @@ impl BlobVec {
244246
}
245247
self.len = new_len;
246248
// Cannot use get_unchecked here as this is technically out of bounds after changing len.
249+
// SAFETY:
250+
// - `new_len` is less than the old len, so it must fit in this vector's allocation.
251+
// - `size` is a multiple of the erased type's alignment,
252+
// so adding a multiple of `size` will preserve alignment.
247253
self.get_ptr_mut().byte_add(new_len * size).promote()
248254
}
249255

@@ -285,16 +291,27 @@ impl BlobVec {
285291
#[inline]
286292
pub unsafe fn get_unchecked(&self, index: usize) -> Ptr<'_> {
287293
debug_assert!(index < self.len());
288-
self.get_ptr().byte_add(index * self.item_layout.size())
294+
let size = self.item_layout.size();
295+
// SAFETY:
296+
// - The caller ensures that `index` fits in this vector,
297+
// so this operation will not overflow the original allocation.
298+
// - `size` is a multiple of the erased type's alignment,
299+
// so adding a multiple of `size` will preserve alignment.
300+
self.get_ptr().byte_add(index * size)
289301
}
290302

291303
/// # Safety
292304
/// It is the caller's responsibility to ensure that `index` is < self.len()
293305
#[inline]
294306
pub unsafe fn get_unchecked_mut(&mut self, index: usize) -> PtrMut<'_> {
295307
debug_assert!(index < self.len());
296-
let layout_size = self.item_layout.size();
297-
self.get_ptr_mut().byte_add(index * layout_size)
308+
let size = self.item_layout.size();
309+
// SAFETY:
310+
// - The caller ensures that `index` fits in this vector,
311+
// so this operation will not overflow the original allocation.
312+
// - `size` is a multiple of the erased type's alignment,
313+
// so adding a multiple of `size` will preserve alignment.
314+
self.get_ptr_mut().byte_add(index * size)
298315
}
299316

300317
/// Gets a [`Ptr`] to the start of the vec
@@ -326,15 +343,18 @@ impl BlobVec {
326343
// accidentally drop elements twice in the event of a drop impl panicking.
327344
self.len = 0;
328345
if let Some(drop) = self.drop {
329-
let layout_size = self.item_layout.size();
346+
let size = self.item_layout.size();
330347
for i in 0..len {
331-
// SAFETY: `i * layout_size` is inbounds for the allocation, and the item is left unreachable so it can be safely promoted to an `OwningPtr`
332-
unsafe {
333-
// NOTE: this doesn't use self.get_unchecked(i) because the debug_assert on index
334-
// will panic here due to self.len being set to 0
335-
let ptr = self.get_ptr_mut().byte_add(i * layout_size).promote();
336-
(drop)(ptr);
337-
}
348+
// SAFETY:
349+
// * 0 <= `i` < `len`, so `i * size` must be in bounds for the allocation.
350+
// * `size` is a multiple of the erased type's alignment,
351+
// so adding a multiple of `size` will preserve alignment.
352+
// * The item is left unreachable so it can be safely promoted to an `OwningPtr`.
353+
// NOTE: `self.get_unchecked_mut(i)` cannot be used here, since the `debug_assert`
354+
// would panic due to `self.len` being set to 0.
355+
let item = unsafe { self.get_ptr_mut().byte_add(i * size).promote() };
356+
// SAFETY: `item` was obtained from this `BlobVec`, so its underlying type must match `drop`.
357+
unsafe { drop(item) };
338358
}
339359
}
340360
}

0 commit comments

Comments
 (0)