Forbid unsafe in most crates in the engine (bevyengine#12684)

# Objective Resolves bevyengine#3824. `unsafe` code should be the exception, not the norm in Rust. It's obviously needed for various use cases as it's interfacing with platforms and essentially running the borrow checker at runtime in the ECS, but the touted benefits of Bevy is that we are able to heavily leverage Rust's safety, and we should be holding ourselves accountable to that by minimizing our unsafe footprint. ## Solution Deny `unsafe_code` workspace wide. Add explicit exceptions for the following crates, and forbid it in almost all of the others. * bevy_ecs - Obvious given how much unsafe is needed to achieve performant results * bevy_ptr - Works with raw pointers, even more low level than bevy_ecs. * bevy_render - due to needing to integrate with wgpu * bevy_window - due to needing to integrate with raw_window_handle * bevy_utils - Several unsafe utilities used by bevy_ecs. Ideally moved into bevy_ecs instead of made publicly usable. * bevy_reflect - Required for the unsafe type casting it's doing. * bevy_transform - for the parallel transform propagation * bevy_gizmos - For the SystemParam impls it has. * bevy_assets - To support reflection. Might not be required, not 100% sure yet. * bevy_mikktspace - due to being a conversion from a C library. Pending safe rewrite. * bevy_dynamic_plugin - Inherently unsafe due to the dynamic loading nature. Several uses of unsafe were rewritten, as they did not need to be using them: * bevy_text - a case of `Option::unchecked` could be rewritten as a normal for loop and match instead of an iterator. * bevy_color - the Pod/Zeroable implementations were replaceable with bytemuck's derive macros.
robtfm · Mar 27, 2024 · 56bcbb0 · 56bcbb0
1 parent 025e8e6
commit 56bcbb0
Show file tree

Hide file tree

Showing 44 changed files with 69 additions and 50 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -47,6 +47,7 @@ ptr_cast_constness = "warn"
 [workspace.lints.rust]
 unsafe_op_in_unsafe_fn = "warn"
 missing_docs = "warn"
+unsafe_code = "deny"
 
 [lints]
 workspace = true

diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_app/src/lib.rs b/crates/bevy_app/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_asset/src/reflect.rs b/crates/bevy_asset/src/reflect.rs
@@ -46,6 +46,7 @@ impl ReflectAsset {
     }
 
     /// Equivalent of [`Assets::get_mut`]
+    #[allow(unsafe_code)]
     pub fn get_mut<'w>(
         &self,
         world: &'w mut World,
@@ -82,6 +83,7 @@ impl ReflectAsset {
     /// violating Rust's aliasing rules. To avoid this:
     /// * Only call this method if you know that the [`UnsafeWorldCell`] may be used to access the corresponding `Assets<T>`
     /// * Don't call this method more than once in the same scope.
+    #[allow(unsafe_code)]
     pub unsafe fn get_unchecked_mut<'w>(
         &self,
         world: UnsafeWorldCell<'w>,
@@ -135,6 +137,7 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
             get_unchecked_mut: |world, handle| {
                 // SAFETY: `get_unchecked_mut` must be called with `UnsafeWorldCell` having access to `Assets<A>`,
                 // and must ensure to only have at most one reference to it live at all times.
+                #[allow(unsafe_code)]
                 let assets = unsafe { world.get_resource_mut::<Assets<A>>().unwrap().into_inner() };
                 let asset = assets.get_mut(&handle.typed_debug_checked());
                 asset.map(|asset| asset as &mut dyn Reflect)

diff --git a/crates/bevy_color/src/lib.rs b/crates/bevy_color/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_color/src/linear_rgba.rs b/crates/bevy_color/src/linear_rgba.rs
@@ -11,7 +11,7 @@ use bytemuck::{Pod, Zeroable};
 /// <div>
 #[doc = include_str!("../docs/diagrams/model_graph.svg")]
 /// </div>
-#[derive(Debug, Clone, Copy, PartialEq, Reflect)]
+#[derive(Debug, Clone, Copy, PartialEq, Reflect, Pod, Zeroable)]
 #[reflect(PartialEq, Default)]
 #[cfg_attr(
     feature = "serialize",
@@ -373,33 +373,6 @@ impl encase::private::CreateFrom for LinearRgba {
     }
 }
 
-/// A [`Zeroable`] type is one whose bytes can be filled with zeroes while remaining valid.
-///
-/// SAFETY: [`LinearRgba`] is inhabited
-/// SAFETY: [`LinearRgba`]'s all-zero bit pattern is a valid value
-unsafe impl Zeroable for LinearRgba {
-    fn zeroed() -> Self {
-        LinearRgba {
-            red: 0.0,
-            green: 0.0,
-            blue: 0.0,
-            alpha: 0.0,
-        }
-    }
-}
-
-/// The [`Pod`] trait is [`bytemuck`]'s marker for types that can be safely transmuted from a byte array.
-///
-/// It is intended to only be implemented for types which are "Plain Old Data".
-///
-/// SAFETY: [`LinearRgba`] is inhabited.
-/// SAFETY: [`LinearRgba`] permits any bit value.
-/// SAFETY: [`LinearRgba`] does not have padding bytes.
-/// SAFETY: all of the fields of [`LinearRgba`] are [`Pod`], as f32 is [`Pod`].
-/// SAFETY: [`LinearRgba`] is `repr(C)`
-/// SAFETY: [`LinearRgba`] does not permit interior mutability.
-unsafe impl Pod for LinearRgba {}
-
 impl encase::ShaderSize for LinearRgba {}
 
 #[cfg(test)]

diff --git a/crates/bevy_core/src/lib.rs b/crates/bevy_core/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_core_pipeline/src/lib.rs b/crates/bevy_core_pipeline/src/lib.rs
@@ -1,5 +1,6 @@
 // FIXME(3492): remove once docs are ready
 #![allow(missing_docs)]
+#![forbid(unsafe_code)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",

diff --git a/crates/bevy_derive/src/lib.rs b/crates/bevy_derive/src/lib.rs
@@ -1,5 +1,6 @@
 // FIXME(3492): remove once docs are ready
 #![allow(missing_docs)]
+#![forbid(unsafe_code)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",

diff --git a/crates/bevy_dev_tools/src/lib.rs b/crates/bevy_dev_tools/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_diagnostic/src/lib.rs b/crates/bevy_diagnostic/src/lib.rs
@@ -1,6 +1,7 @@
 // FIXME(3492): remove once docs are ready
 #![allow(missing_docs)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_dynamic_plugin/src/loader.rs b/crates/bevy_dynamic_plugin/src/loader.rs
@@ -1,3 +1,5 @@
+#![allow(unsafe_code)]
+
 use libloading::{Library, Symbol};
 use std::ffi::OsStr;
 use thiserror::Error;

diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs
@@ -2,6 +2,7 @@
 #![allow(unsafe_op_in_unsafe_fn)]
 #![doc = include_str!("../README.md")]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![allow(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_encase_derive/src/lib.rs b/crates/bevy_encase_derive/src/lib.rs
@@ -1,5 +1,6 @@
 // FIXME(3492): remove once docs are ready
 #![allow(missing_docs)]
+#![forbid(unsafe_code)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",

diff --git a/crates/bevy_gilrs/src/lib.rs b/crates/bevy_gilrs/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_gizmos/src/gizmos.rs b/crates/bevy_gizmos/src/gizmos.rs
@@ -49,6 +49,8 @@ type GizmosState<T> = (
 pub struct GizmosFetchState<T: GizmoConfigGroup> {
     state: <GizmosState<T> as SystemParam>::State,
 }
+
+#[allow(unsafe_code)]
 // SAFETY: All methods are delegated to existing `SystemParam` implementations
 unsafe impl<T: GizmoConfigGroup> SystemParam for Gizmos<'_, '_, T> {
     type State = GizmosFetchState<T>;
@@ -90,6 +92,8 @@ unsafe impl<T: GizmoConfigGroup> SystemParam for Gizmos<'_, '_, T> {
         }
     }
 }
+
+#[allow(unsafe_code)]
 // Safety: Each field is `ReadOnlySystemParam`, and Gizmos SystemParam does not mutate world
 unsafe impl<'w, 's, T: GizmoConfigGroup> ReadOnlySystemParam for Gizmos<'w, 's, T>
 where

diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_hierarchy/src/lib.rs b/crates/bevy_hierarchy/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_input/src/lib.rs b/crates/bevy_input/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_internal/src/lib.rs b/crates/bevy_internal/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_log/src/android_tracing.rs b/crates/bevy_log/src/android_tracing.rs
@@ -73,6 +73,7 @@ impl<S: Subscriber + for<'a> LookupSpan<'a>> Layer<S> for AndroidLayer {
         }
     }
 
+    #[allow(unsafe_code)]
     fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) {
         let mut recorder = StringRecorder::new();
         event.record(&mut recorder);

diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs
@@ -1,4 +1,4 @@
-#![deny(unsafe_code)]
+#![forbid(unsafe_code)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",

diff --git a/crates/bevy_math/src/lib.rs b/crates/bevy_math/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_mikktspace/src/generated.rs b/crates/bevy_mikktspace/src/generated.rs
@@ -41,7 +41,8 @@
     non_upper_case_globals,
     unused_mut,
     unused_assignments,
-    unused_variables
+    unused_variables,
+    unsafe_code
 )]
 
 use std::ptr::null_mut;

diff --git a/crates/bevy_mikktspace/src/lib.rs b/crates/bevy_mikktspace/src/lib.rs
@@ -67,6 +67,7 @@ pub trait Geometry {
 ///
 /// Returns `false` if the geometry is unsuitable for tangent generation including,
 /// but not limited to, lack of vertices.
+#[allow(unsafe_code)]
 pub fn generate_tangents<I: Geometry>(geometry: &mut I) -> bool {
     unsafe { generated::genTangSpace(geometry, 180.0) }
 }

diff --git a/crates/bevy_pbr/src/meshlet/persistent_buffer.rs b/crates/bevy_pbr/src/meshlet/persistent_buffer.rs
@@ -1,3 +1,5 @@
+#![allow(unsafe_code)]
+
 use bevy_render::{
     render_resource::{
         BindingResource, Buffer, BufferAddress, BufferDescriptor, BufferUsages,

diff --git a/crates/bevy_pbr/src/meshlet/persistent_buffer_impls.rs b/crates/bevy_pbr/src/meshlet/persistent_buffer_impls.rs
@@ -1,3 +1,4 @@
+#![allow(unsafe_code)]
 #![allow(clippy::undocumented_unsafe_blocks)]
 
 use super::{persistent_buffer::PersistentGpuBufferable, Meshlet, MeshletBoundingSphere};

diff --git a/crates/bevy_ptr/src/lib.rs b/crates/bevy_ptr/src/lib.rs
@@ -1,6 +1,7 @@
 #![doc = include_str!("../README.md")]
 #![no_std]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![allow(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_reflect/src/path/parse.rs b/crates/bevy_reflect/src/path/parse.rs
@@ -65,6 +65,7 @@ impl<'a> PathParser<'a> {
         //   the last byte before an ASCII utf-8 character (ie: it is a char
         //   boundary).
         // - The slice always starts after a symbol ie: an ASCII character's boundary.
+        #[allow(unsafe_code)]
         let ident = unsafe { from_utf8_unchecked(ident) };
 
         self.remaining = remaining;

diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs
@@ -663,6 +663,7 @@ pub struct ReflectFromPtr {
     from_ptr_mut: unsafe fn(PtrMut) -> &mut dyn Reflect,
 }
 
+#[allow(unsafe_code)]
 impl ReflectFromPtr {
     /// Returns the [`TypeId`] that the [`ReflectFromPtr`] was constructed for.
     pub fn type_id(&self) -> TypeId {
@@ -714,6 +715,7 @@ impl ReflectFromPtr {
     }
 }
 
+#[allow(unsafe_code)]
 impl<T: Reflect> FromType<T> for ReflectFromPtr {
     fn from_type() -> Self {
         ReflectFromPtr {
@@ -733,6 +735,7 @@ impl<T: Reflect> FromType<T> for ReflectFromPtr {
 }
 
 #[cfg(test)]
+#[allow(unsafe_code)]
 mod test {
     use crate::{GetTypeRegistration, ReflectFromPtr};
     use bevy_ptr::{Ptr, PtrMut};

diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs
@@ -1,5 +1,6 @@
 // FIXME(3492): remove once docs are ready
 #![allow(missing_docs)]
+#![allow(unsafe_code)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",

diff --git a/crates/bevy_scene/src/lib.rs b/crates/bevy_scene/src/lib.rs
@@ -1,4 +1,5 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_sprite/src/lib.rs b/crates/bevy_sprite/src/lib.rs
@@ -1,6 +1,7 @@
 // FIXME(3492): remove once docs are ready
 #![allow(missing_docs)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_tasks/src/single_threaded_task_pool.rs b/crates/bevy_tasks/src/single_threaded_task_pool.rs
@@ -94,6 +94,7 @@ impl TaskPool {
     /// to spawn tasks. This function will await the completion of all tasks before returning.
     ///
     /// This is similar to `rayon::scope` and `crossbeam::scope`
+    #[allow(unsafe_code)]
     pub fn scope_with_executor<'env, F, T>(
         &self,
         _tick_task_pool_executor: bool,

diff --git a/crates/bevy_tasks/src/task_pool.rs b/crates/bevy_tasks/src/task_pool.rs
@@ -334,6 +334,7 @@ impl TaskPool {
         })
     }
 
+    #[allow(unsafe_code)]
     fn scope_with_executor_inner<'env, F, T>(
         &self,
         tick_task_pool_executor: bool,

diff --git a/crates/bevy_text/src/lib.rs b/crates/bevy_text/src/lib.rs
@@ -1,6 +1,7 @@
 // FIXME(3492): remove once docs are ready
 #![allow(missing_docs)]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_text/src/pipeline.rs b/crates/bevy_text/src/pipeline.rs
@@ -129,32 +129,25 @@ impl TextMeasureInfo {
         scale_factor: f32,
     ) -> Result<TextMeasureInfo, TextError> {
         let sections = &text.sections;
-        for section in sections {
-            if !fonts.contains(&section.style.font) {
-                return Err(TextError::NoSuchFont);
-            }
-        }
-        let (auto_fonts, sections) = sections
-            .iter()
-            .enumerate()
-            .map(|(i, section)| {
-                // SAFETY: we exited early earlier in this function if
-                // one of the fonts was missing.
-                let font = unsafe { fonts.get(&section.style.font).unwrap_unchecked() };
-                (
-                    font.font.clone(),
-                    TextMeasureSection {
+        let mut auto_fonts = Vec::with_capacity(sections.len());
+        let mut out_sections = Vec::with_capacity(sections.len());
+        for (i, section) in sections.iter().enumerate() {
+            match fonts.get(&section.style.font) {
+                Some(font) => {
+                    auto_fonts.push(font.font.clone());
+                    out_sections.push(TextMeasureSection {
                         font_id: FontId(i),
                         scale: scale_value(section.style.font_size, scale_factor),
                         text: section.value.clone().into_boxed_str(),
-                    },
-                )
-            })
-            .unzip();
+                    });
+                }
+                None => return Err(TextError::NoSuchFont),
+            }
+        }
 
         Ok(Self::new(
             auto_fonts,
-            sections,
+            out_sections,
             text.justify,
             text.linebreak_behavior.into(),
         ))

diff --git a/crates/bevy_time/src/lib.rs b/crates/bevy_time/src/lib.rs
@@ -1,5 +1,6 @@
 #![doc = include_str!("../README.md")]
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![forbid(unsafe_code)]
 #![doc(
     html_logo_url = "https://bevyengine.org/assets/icon.png",
     html_favicon_url = "https://bevyengine.org/assets/icon.png"

diff --git a/crates/bevy_transform/src/systems.rs b/crates/bevy_transform/src/systems.rs
@@ -79,6 +79,7 @@ pub fn propagate_transforms(
                 // - Since each root entity is unique and the hierarchy is consistent and forest-like,
                 //   other root entities' `propagate_recursive` calls will not conflict with this one.
                 // - Since this is the only place where `transform_query` gets used, there will be no conflicting fetches elsewhere.
+                #[allow(unsafe_code)]
                 unsafe {
                     propagate_recursive(
                         &global_transform,
@@ -106,6 +107,7 @@ pub fn propagate_transforms(
 /// nor any of its descendants.
 /// - The caller must ensure that the hierarchy leading to `entity`
 /// is well-formed and must remain as a tree or a forest. Each entity must have at most one parent.
+#[allow(unsafe_code)]
 unsafe fn propagate_recursive(
     parent: &GlobalTransform,
     transform_query: &Query<
-Original file line number
+Diff line change
@@ Expand Up / @@ -334,6 +334,7 @@ impl TaskPool { @@
             })
         }
+        #[allow(unsafe_code)]
         fn scope_with_executor_inner<'env, F, T>(
             &self,
             tick_task_pool_executor: bool,
@@ Expand Down @@