Skip to content

Commit

Permalink
bevy_dynamic_plugin: fix unsafe_op_in_unsafe_fn lint (#11622)
Browse files Browse the repository at this point in the history
# Objective

- Part of #11590.

## Solution

- Fix `unsafe_op_in_unsafe_fn` for `bevy_dynamic_plugin`.

---

## Changelog

- Added further restrictions to the safety requirements of
`bevy_dynamic_plugin::dynamically_load_plugin`.

---

I had a few issues, specifically with the safety comment on
`dynamically_load_plugin`. There are three different unsafe functions
called within the function body, and they all need their own
justification / message.

Also, would it be unsound to call `dynamically_load_plugin` multiple
times on the same file? I feel the documentation needs to be more clear.
  • Loading branch information
BD103 authored Feb 12, 2024
1 parent 1c67e02 commit 078dd06
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 8 deletions.
2 changes: 0 additions & 2 deletions crates/bevy_dynamic_plugin/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// FIXME(11590): remove this once the lint is fixed
#![allow(unsafe_op_in_unsafe_fn)]
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]

Expand Down
27 changes: 21 additions & 6 deletions crates/bevy_dynamic_plugin/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,28 @@ pub enum DynamicPluginLoadError {
/// The specified plugin must be linked against the exact same libbevy.so as this program.
/// In addition the `_bevy_create_plugin` symbol must not be manually created, but instead created
/// by deriving `DynamicPlugin` on a unit struct implementing [`Plugin`].
///
/// Dynamically loading plugins is orchestrated through dynamic linking. When linking against foreign
/// code, initialization routines may be run (as well as termination routines when the program exits).
/// The caller of this function is responsible for ensuring these routines are sound. For more
/// information, please see the safety section of [`libloading::Library::new`].
pub unsafe fn dynamically_load_plugin<P: AsRef<OsStr>>(
path: P,
) -> Result<(Library, Box<dyn Plugin>), DynamicPluginLoadError> {
let lib = Library::new(path).map_err(DynamicPluginLoadError::Library)?;
let func: Symbol<CreatePlugin> = lib
.get(b"_bevy_create_plugin")
.map_err(DynamicPluginLoadError::Plugin)?;
let plugin = Box::from_raw(func());
// SAFETY: Caller must follow the safety requirements of Library::new.
let lib = unsafe { Library::new(path).map_err(DynamicPluginLoadError::Library)? };

// SAFETY: Loaded plugins are not allowed to specify `_bevy_create_plugin` symbol manually, but must
// instead automatically generate it through `DynamicPlugin`.
let func: Symbol<CreatePlugin> = unsafe {
lib.get(b"_bevy_create_plugin")
.map_err(DynamicPluginLoadError::Plugin)?
};

// SAFETY: `func` is automatically generated and is guaranteed to return a pointer created using
// `Box::into_raw`.
let plugin = unsafe { Box::from_raw(func()) };

Ok((lib, plugin))
}

Expand All @@ -41,7 +55,8 @@ pub trait DynamicPluginExt {

impl DynamicPluginExt for App {
unsafe fn load_plugin<P: AsRef<OsStr>>(&mut self, path: P) -> &mut Self {
let (lib, plugin) = dynamically_load_plugin(path).unwrap();
// SAFETY: Follows the same safety requirements as `dynamically_load_plugin`.
let (lib, plugin) = unsafe { dynamically_load_plugin(path).unwrap() };
std::mem::forget(lib); // Ensure that the library is not automatically unloaded
plugin.build(self);
self
Expand Down

0 comments on commit 078dd06

Please sign in to comment.