Skip to content

Commit 582543d

Browse files
authored
mark register_info, register_contributed_bundle_info unsafe (#20790)
# Objective calling `register_info` and `register_contributed_bundle_info` with incorrect arguments results in panics in 100% safe code part of breaking up #20739 ## Solution Mark `register_info` and `register_contributed_bundle_info` as unsafe. ## Testing cargo test in crates/bevy_ecs --- It's pretty obvious why this is unsafe, but the function doesn't explicitly forbid it: ```rust fn safe_bundle_register_world_unsafety() { #![forbid(unsafe_code)] let mut world = World::new(); let mut antiworld = World::new(); #[derive(crate::prelude::Component, Debug, PartialEq, Eq)] struct A(u32); let mut world_components = world.components_registrator(); let _ = antiworld .bundles .register_info::<(A,)>(&mut world_components, &mut antiworld.storages); // unsound beyond this point let e = antiworld.spawn((A(3),)); let a = e.get::<A>().unwrap(); assert_eq!(a, &A(3)); } ```
1 parent 914bcff commit 582543d

File tree

6 files changed

+72
-26
lines changed

6 files changed

+72
-26
lines changed

crates/bevy_ecs/src/bundle/info.rs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,14 @@ impl Bundles {
418418
/// Registers a new [`BundleInfo`] for a statically known type.
419419
///
420420
/// Also registers all the components in the bundle.
421-
pub(crate) fn register_info<T: Bundle>(
421+
///
422+
/// # Safety
423+
///
424+
/// `components` and `storages` must be from the same [`World`] as `self`.
425+
///
426+
/// [`World`]: crate::world::World
427+
#[deny(unsafe_op_in_unsafe_fn)]
428+
pub(crate) unsafe fn register_info<T: Bundle>(
422429
&mut self,
423430
components: &mut ComponentsRegistrator,
424431
storages: &mut Storages,
@@ -442,15 +449,25 @@ impl Bundles {
442449
/// Registers a new [`BundleInfo`], which contains both explicit and required components for a statically known type.
443450
///
444451
/// Also registers all the components in the bundle.
445-
pub(crate) fn register_contributed_bundle_info<T: Bundle>(
452+
///
453+
/// # Safety
454+
///
455+
/// `components` and `storages` must be from the same [`World`] as `self`.
456+
///
457+
/// [`World`]: crate::world::World
458+
#[deny(unsafe_op_in_unsafe_fn)]
459+
pub(crate) unsafe fn register_contributed_bundle_info<T: Bundle>(
446460
&mut self,
447461
components: &mut ComponentsRegistrator,
448462
storages: &mut Storages,
449463
) -> BundleId {
450464
if let Some(id) = self.contributed_bundle_ids.get(&TypeId::of::<T>()).cloned() {
451465
id
452466
} else {
453-
let explicit_bundle_id = self.register_info::<T>(components, storages);
467+
// SAFETY: as per the guarantees of this function, components and
468+
// storages are from the same world as self
469+
let explicit_bundle_id = unsafe { self.register_info::<T>(components, storages) };
470+
454471
// SAFETY: reading from `explicit_bundle_id` and creating new bundle in same time. Its valid because bundle hashmap allow this
455472
let id = unsafe {
456473
let (ptr, len) = {

crates/bevy_ecs/src/bundle/insert.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,13 @@ impl<'w> BundleInserter<'w> {
4040
// SAFETY: These come from the same world. `world.components_registrator` can't be used since we borrow other fields too.
4141
let mut registrator =
4242
unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) };
43-
let bundle_id = world
44-
.bundles
45-
.register_info::<T>(&mut registrator, &mut world.storages);
43+
44+
// SAFETY: `registrator`, `world.bundles`, and `world.storages` all come from the same world
45+
let bundle_id = unsafe {
46+
world
47+
.bundles
48+
.register_info::<T>(&mut registrator, &mut world.storages)
49+
};
4650
// SAFETY: We just ensured this bundle exists
4751
unsafe { Self::new_with_id(world, archetype_id, bundle_id, change_tick) }
4852
}

crates/bevy_ecs/src/bundle/remove.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ impl<'w> BundleRemover<'w> {
3333
/// # Safety
3434
/// Caller must ensure that `archetype_id` is valid
3535
#[inline]
36+
#[deny(unsafe_op_in_unsafe_fn)]
3637
pub(crate) unsafe fn new<T: Bundle>(
3738
world: &'w mut World,
3839
archetype_id: ArchetypeId,
@@ -41,9 +42,13 @@ impl<'w> BundleRemover<'w> {
4142
// SAFETY: These come from the same world. `world.components_registrator` can't be used since we borrow other fields too.
4243
let mut registrator =
4344
unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) };
44-
let bundle_id = world
45-
.bundles
46-
.register_info::<T>(&mut registrator, &mut world.storages);
45+
46+
// SAFETY: `registrator`, `world.storages`, and `world.bundles` all come from the same world.
47+
let bundle_id = unsafe {
48+
world
49+
.bundles
50+
.register_info::<T>(&mut registrator, &mut world.storages)
51+
};
4752
// SAFETY: we initialized this bundle_id in `init_info`, and caller ensures archetype is valid.
4853
unsafe { Self::new_with_id(world, archetype_id, bundle_id, require_all) }
4954
}

crates/bevy_ecs/src/bundle/spawner.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,13 @@ impl<'w> BundleSpawner<'w> {
2929
// SAFETY: These come from the same world. `world.components_registrator` can't be used since we borrow other fields too.
3030
let mut registrator =
3131
unsafe { ComponentsRegistrator::new(&mut world.components, &mut world.component_ids) };
32-
let bundle_id = world
33-
.bundles
34-
.register_info::<T>(&mut registrator, &mut world.storages);
32+
33+
// SAFETY: `registrator`, `world.bundles`, and `world.storages` all come from the same world.
34+
let bundle_id = unsafe {
35+
world
36+
.bundles
37+
.register_info::<T>(&mut registrator, &mut world.storages)
38+
};
3539
// SAFETY: we initialized this bundle_id in `init_info`
3640
unsafe { Self::new_with_id(world, bundle_id, change_tick) }
3741
}

crates/bevy_ecs/src/world/entity_ref.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2304,7 +2304,10 @@ impl<'w> EntityWorldMut<'w> {
23042304
let mut registrator = unsafe {
23052305
ComponentsRegistrator::new(&mut self.world.components, &mut self.world.component_ids)
23062306
};
2307-
let bundle_id = bundles.register_contributed_bundle_info::<T>(&mut registrator, storages);
2307+
2308+
// SAFETY: `storages`, `bundles` and `registrator` come from the same world.
2309+
let bundle_id =
2310+
unsafe { bundles.register_contributed_bundle_info::<T>(&mut registrator, storages) };
23082311

23092312
// SAFETY: We just created the bundle, and the archetype is valid, since we are in it.
23102313
let Some(mut remover) = (unsafe {
@@ -2351,10 +2354,13 @@ impl<'w> EntityWorldMut<'w> {
23512354
ComponentsRegistrator::new(&mut self.world.components, &mut self.world.component_ids)
23522355
};
23532356

2354-
let retained_bundle = self
2355-
.world
2356-
.bundles
2357-
.register_info::<T>(&mut registrator, storages);
2357+
// SAFETY: `storages`, `bundles` and `registrator` come from the same world.
2358+
let retained_bundle = unsafe {
2359+
self.world
2360+
.bundles
2361+
.register_info::<T>(&mut registrator, storages)
2362+
};
2363+
23582364
// SAFETY: `retained_bundle` exists as we just initialized it.
23592365
let retained_bundle_info = unsafe { self.world.bundles.get_unchecked(retained_bundle) };
23602366
let old_archetype = &mut archetypes[old_location.archetype_id];

crates/bevy_ecs/src/world/mod.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2305,9 +2305,12 @@ impl World {
23052305
// SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too.
23062306
let mut registrator =
23072307
unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) };
2308-
let bundle_id = self
2309-
.bundles
2310-
.register_info::<B>(&mut registrator, &mut self.storages);
2308+
2309+
// SAFETY: `registrator`, `self.bundles`, and `self.storages` all come from this world.
2310+
let bundle_id = unsafe {
2311+
self.bundles
2312+
.register_info::<B>(&mut registrator, &mut self.storages)
2313+
};
23112314

23122315
let mut batch_iter = batch.into_iter();
23132316

@@ -2450,9 +2453,12 @@ impl World {
24502453
// SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too.
24512454
let mut registrator =
24522455
unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) };
2453-
let bundle_id = self
2454-
.bundles
2455-
.register_info::<B>(&mut registrator, &mut self.storages);
2456+
2457+
// SAFETY: `registrator`, `self.bundles`, and `self.storages` all come from this world.
2458+
let bundle_id = unsafe {
2459+
self.bundles
2460+
.register_info::<B>(&mut registrator, &mut self.storages)
2461+
};
24562462

24572463
let mut invalid_entities = Vec::<Entity>::new();
24582464
let mut batch_iter = batch.into_iter();
@@ -3072,9 +3078,13 @@ impl World {
30723078
// SAFETY: These come from the same world. `Self.components_registrator` can't be used since we borrow other fields too.
30733079
let mut registrator =
30743080
unsafe { ComponentsRegistrator::new(&mut self.components, &mut self.component_ids) };
3075-
let id = self
3076-
.bundles
3077-
.register_info::<B>(&mut registrator, &mut self.storages);
3081+
3082+
// SAFETY: `registrator`, `self.storages` and `self.bundles` all come from this world.
3083+
let id = unsafe {
3084+
self.bundles
3085+
.register_info::<B>(&mut registrator, &mut self.storages)
3086+
};
3087+
30783088
// SAFETY: We just initialized the bundle so its id should definitely be valid.
30793089
unsafe { self.bundles.get(id).debug_checked_unwrap() }
30803090
}

0 commit comments

Comments
 (0)