Fix misaligned pointer dereference when storing UserData (#211)

* Fix misaligned pointer dereference when storing UserData

closes #210

* chore: run cargo fmt in physx-sys/pxbind

Author: rlidwka

physx-sys/pxbind/src/consumer.rs

@@ -299,7 +299,9 @@ impl<'ast> AstConsumer<'ast> {
                     // If a record decl doesn't have any inner nodes, it's just
                     // a foreward declaration and we can skip it
                     if inn.inner.is_empty() || rec.definition_data.is_none() {
-                        let Some(name) = rec.name.as_deref() else { continue; };
+                        let Some(name) = rec.name.as_deref() else {
+                            continue;
+                        };
 
                         if !self.classes.contains_key(name) {
                             self.recs

physx-sys/pxbind/src/consumer/enums.rs

@@ -182,7 +182,9 @@ impl<'ast> super::AstConsumer<'ast> {
         // PhysX uses a PxFlags<> template typedef to create a bitfield type for
         // a specific enum, we use this typedef to also generate an appropriate
         // bitflags that can be transparently passed between the FFI boundary
-        let Some(flags) = super::no_physx(&td.kind.qual_type).strip_prefix("PxFlags<") else { return Ok(()) };
+        let Some(flags) = super::no_physx(&td.kind.qual_type).strip_prefix("PxFlags<") else {
+            return Ok(());
+        };
         // Get rid of `>`
         let flags = &flags[..flags.len() - 1];
 

physx-sys/pxbind/src/consumer/record.rs

@@ -58,7 +58,9 @@ impl Constructor {
             }
         });
 
-        let Some(first) = iter.next() else { return false };
+        let Some(first) = iter.next() else {
+            return false;
+        };
 
         if first.ends_with(" &&") {
             return true;
@@ -443,7 +445,9 @@ impl<'ast> super::AstConsumer<'ast> {
             return Ok(());
         }
 
-        let Some(rname) = rec.name.as_deref() else { return Ok(()) };
+        let Some(rname) = rec.name.as_deref() else {
+            return Ok(());
+        };
 
         anyhow::ensure!(
             rec.definition_data.is_some(),
@@ -674,7 +678,9 @@ impl<'ast> super::AstConsumer<'ast> {
         template_types: &[(&str, &super::TemplateArg<'ast>)],
         fields: &mut Vec<FieldBinding<'ast>>,
     ) -> anyhow::Result<()> {
-        let Some(rname) = rec.name.as_deref() else { return Ok(()) };
+        let Some(rname) = rec.name.as_deref() else {
+            return Ok(());
+        };
         let mut is_public = !matches!(rec.tag_used, Some(Tag::Class));
 
         for inn in &node.inner {

physx/CHANGELOG.md

@@ -12,6 +12,9 @@ into mutable and immutable variants.
   - `ActorMap::as_rigid_static` -> `ActorMap::as_rigid_static_mut`
   - `ActorMap::as_articulation_link` -> `ActorMap::as_articulation_link_mut`
 
+### Fixed
+- [PR#211](https://github.com/EmbarkStudios/physx-rs/pull/211) fixed misaligned pointer dereference when using `UserData` with small-size values.
+
 ## [0.18.0] - 2023-03-03
 ### Changed
 - [PR#191](https://github.com/EmbarkStudios/physx-rs/pull/191) replaced `PxCooking` with regular functions as `PxCooking` is deprecated in the C++ code.

physx/src/traits/user_data.rs

@@ -21,14 +21,11 @@ pub unsafe trait UserData: Sized {
     unsafe fn init_user_data(&mut self, user_data: Self::UserData) -> &mut Self {
         if size_of::<Self::UserData>() > size_of::<*mut c_void>() {
             // Too big to pack into a *mut c_void, kick it to the heap.
-            let data = Box::new(user_data);
-            *self.user_data_ptr_mut() = Box::into_raw(data) as *mut c_void;
+            let data = Box::into_raw(Box::new(user_data));
+            *(self.user_data_ptr_mut() as *mut *mut c_void as *mut *mut Self::UserData) = data;
         } else {
             // DATA_SIZE <= VOID_SIZE
-            unsafe {
-                *self.user_data_ptr_mut() =
-                    *(&user_data as *const Self::UserData as *const *mut c_void)
-            }
+            *(self.user_data_ptr_mut() as *mut *mut c_void as *mut Self::UserData) = user_data;
         }
         self
     }
@@ -49,6 +46,7 @@ pub unsafe trait UserData: Sized {
     }
 
     /// # Safety
+    ///
     /// The user data field must have previously been initialized via `init_user_data`.
     unsafe fn get_user_data_mut(this: &mut Self) -> &mut Self::UserData {
         unsafe {
@@ -63,3 +61,54 @@ pub unsafe trait UserData: Sized {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::{ffi::c_void, fmt::Debug, marker::PhantomData, ptr::null_mut};
+
+    use super::UserData;
+
+    struct TestUserData<U> {
+        user_data: *mut c_void,
+        phantom: PhantomData<U>,
+    }
+
+    impl<U> Default for TestUserData<U> {
+        fn default() -> Self {
+            Self {
+                user_data: null_mut(),
+                phantom: PhantomData,
+            }
+        }
+    }
+
+    unsafe impl<U> UserData for TestUserData<U> {
+        type UserData = U;
+
+        fn user_data_ptr(&self) -> &*mut c_void {
+            &self.user_data
+        }
+
+        fn user_data_ptr_mut(&mut self) -> &mut *mut c_void {
+            &mut self.user_data
+        }
+    }
+
+    fn do_test<U: PartialEq + Clone + Debug>(user_data: U) {
+        unsafe {
+            let mut object: TestUserData<U> = TestUserData::default();
+            object.init_user_data(user_data.clone());
+
+            assert_eq!(UserData::get_user_data(&object), &user_data);
+            assert_eq!(UserData::get_user_data_mut(&mut object), &user_data);
+        }
+    }
+
+    #[test]
+    fn test_user_data() {
+        do_test(()); // unit type
+        do_test(100u8); // smaller than pointer
+        do_test(100usize); // same size as pointer
+        do_test([100usize; 4]); // larger than pointer
+    }
+}