Skip to content

Commit 0b2e80f

Browse files
committed
Fix undefined behavior in deferred component insertions
Detected by Miri.
1 parent 707572e commit 0b2e80f

3 files changed

Lines changed: 19 additions & 35 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Deferred component insertions no longer cause undefined behavior.
13+
1014
## [0.39.3] - 2026-04-01
1115

1216
### Fixed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ typeid = "1.0"
3838
bytes = { version = "1.10", default-features = false }
3939
serde = { version = "1.0", default-features = false }
4040
bitflags = { version = "2.6", features = ["serde"] }
41+
bumpalo = "3.20"
4142
smallbitvec = "2.6"
4243
postcard = { version = "1.1", default-features = false, features = [
4344
"experimental-derive",

src/shared/replication/deferred_entity.rs

Lines changed: 14 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
use core::{alloc::Layout, ptr::NonNull};
1+
use core::ptr::NonNull;
22

33
use bevy::{
44
ecs::component::{ComponentId, Mutable},
55
prelude::*,
6-
ptr::PtrMut,
6+
ptr::OwningPtr,
77
};
8+
use bumpalo::Bump;
89

910
/// Like [`EntityWorldMut`], but buffers all structural changes.
1011
///
@@ -106,9 +107,9 @@ impl DeferredChanges {
106107
/// Buffered insertions stored in type-erased way.
107108
#[derive(Default)]
108109
pub(crate) struct DeferredInsertions {
110+
ptrs: Vec<NonNull<u8>>,
109111
ids: Vec<ComponentId>,
110-
offsets: Vec<usize>,
111-
data: Vec<u8>,
112+
bump: Bump,
112113
}
113114

114115
impl DeferredInsertions {
@@ -119,39 +120,14 @@ impl DeferredInsertions {
119120
/// Component ID should correspond to the passed type, otherwise [`Self::apply`] won't
120121
/// write the data correctly.
121122
unsafe fn insert<C: Component>(&mut self, component: C, component_id: ComponentId) {
122-
let layout = Layout::new::<C>();
123-
124-
// If items would otherwise not be aligned, add alignment.
125-
let align = layout.align();
126-
let extra_offset = if !self.data.len().is_multiple_of(align) {
127-
align - (self.data.len() % align)
128-
} else {
129-
0
130-
};
131-
132-
let grow = layout.size() + extra_offset;
133-
let offset = self.data.len() + extra_offset;
134-
123+
let ptr: NonNull<_> = self.bump.alloc(component).into();
124+
self.ptrs.push(ptr.cast());
135125
self.ids.push(component_id);
136-
self.offsets.push(offset);
137-
self.data.resize(self.data.len() + grow, 0);
138-
139-
// SAFETY: pointer references a properly allocated memory.
140-
unsafe {
141-
let ptr = self.data.as_mut_ptr().byte_add(offset) as *mut C;
142-
ptr.write(component);
143-
}
144126
}
145127

146128
fn apply(&mut self, entity: &mut EntityWorldMut) {
147-
// SAFETY: iterator produces valid pointers for each component ID.
148-
unsafe {
149-
let iter = self.offsets.iter().map(|&offset| {
150-
let ptr = PtrMut::new(NonNull::new_unchecked(self.data.as_mut_ptr()));
151-
ptr.byte_add(offset).promote()
152-
});
153-
entity.insert_by_ids(&self.ids, iter);
154-
}
129+
// SAFETY: each pointer is valid and points to a value of the type corresponding to its ID.
130+
unsafe { entity.insert_by_ids(&self.ids, self.ptrs.iter().map(|&p| OwningPtr::new(p))) };
155131
}
156132

157133
fn is_empty(&self) -> bool {
@@ -160,11 +136,14 @@ impl DeferredInsertions {
160136

161137
fn clear(&mut self) {
162138
self.ids.clear();
163-
self.offsets.clear();
164-
self.data.clear();
139+
self.ptrs.clear();
140+
self.bump.reset();
165141
}
166142
}
167143

144+
// SAFETY: `NonNull` pointers are unique and allocated by a private arena.
145+
unsafe impl Send for DeferredInsertions {}
146+
168147
#[cfg(test)]
169148
mod tests {
170149
use alloc::sync::Arc;

0 commit comments

Comments
 (0)