Repository: Ralith/hecs
Pull request #4 (2019-12-14 04:05:37)

Run miri in CI


Patch:
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 93b62e8..d8e9c21 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -62,3 +62,24 @@ jobs:
           command: clippy
           args: --all-features -- -D warnings
 
+  miri:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v1
+
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: nightly
+          override: true
+          components: miri, rust-src
+
+      - uses: actions-rs/cargo@v1
+        with:
+          command: install
+          args: xargo
+
+      - uses: actions-rs/cargo@v1
+        with:
+          command: miri
+          args: test --all-features
diff --git a/src/archetype.rs b/src/archetype.rs
index e023599..72b9151 100644
--- a/src/archetype.rs
+++ b/src/archetype.rs
@@ -12,10 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::alloc::{alloc, Layout};
+use std::alloc::{alloc, dealloc, Layout};
 use std::any::TypeId;
 use std::cell::UnsafeCell;
-use std::mem::MaybeUninit;
+use std::mem;
 use std::ptr::{self, NonNull};
 
 use fxhash::FxHashMap;
@@ -30,7 +30,8 @@ pub struct Archetype {
     entities: Box<[u32]>,
     // UnsafeCell allows unique references into `data` to be constructed while shared references
     // containing the `Archetype` exist
-    data: UnsafeCell<Box<[MaybeUninit<u8>]>>,
+    data: UnsafeCell<NonNull<u8>>,
+    data_size: usize,
 }
 
 impl Archetype {
@@ -44,7 +45,8 @@ impl Archetype {
             offsets: FxHashMap::default(),
             entities: Box::new([]),
             len: 0,
-            data: UnsafeCell::new(Box::new([])),
+            data: UnsafeCell::new(NonNull::dangling()),
+            data_size: 0,
         }
     }
 
@@ -104,7 +106,7 @@ impl Archetype {
         debug_assert!(index < self.len);
         Some(NonNull::new_unchecked(
             (*self.data.get())
-                .as_mut_ptr()
+                .as_ptr()
                 .add(*self.offsets.get(&ty)? + size * index as usize)
                 .cast::<u8>(),
         ))
@@ -125,34 +127,32 @@ impl Archetype {
         new_entities[0..old_count].copy_from_slice(&self.entities);
         self.entities = new_entities;
 
-        let mut data_size = 0;
+        let old_data_size = mem::replace(&mut self.data_size, 0);
         let mut offsets = FxHashMap::with_capacity_and_hasher(self.types.len(), Default::default());
         for ty in &self.types {
-            data_size = align(data_size, ty.layout.align());
-            offsets.insert(ty.id, data_size);
-            data_size += ty.layout.size() * count;
+            self.data_size = align(self.data_size, ty.layout.align());
+            offsets.insert(ty.id, self.data_size);
+            self.data_size += ty.layout.size() * count;
         }
-        let raw = if data_size == 0 {
-            Box::<[MaybeUninit<u8>]>::into_raw(Box::new([MaybeUninit::<u8>::uninit(); 0]))
+        let new_data = if self.data_size == 0 {
+            NonNull::dangling()
         } else {
-            let ptr = alloc(
+            NonNull::new(alloc(
                 Layout::from_size_align(
-                    data_size,
+                    self.data_size,
                     self.types.first().map_or(1, |x| x.layout.align()),
                 )
                 .unwrap(),
-            )
-            .cast::<MaybeUninit<u8>>();
-            std::slice::from_raw_parts_mut(ptr, data_size)
+            ))
+            .unwrap()
         };
-        let mut new_data = Box::from_raw(raw);
-        if !(*self.data.get()).is_empty() {
+        if old_data_size != 0 {
             for ty in &self.types {
                 let old_off = *self.offsets.get(&ty.id).unwrap();
                 let new_off = *offsets.get(&ty.id).unwrap();
                 ptr::copy_nonoverlapping(
                     (*self.data.get()).as_ptr().add(old_off),
-                    new_data.as_mut_ptr().add(new_off),
+                    new_data.as_ptr().add(new_off),
                     ty.layout.size() * old_count,
                 );
             }
@@ -237,6 +237,17 @@ impl Archetype {
 impl Drop for Archetype {
     fn drop(&mut self) {
         self.clear();
+        if self.data_size != 0 {
+            unsafe {
+                dealloc(
+                    (*self.data.get()).as_ptr().cast(),
+                    Layout::from_size_align_unchecked(
+                        self.data_size,
+                        self.types.first().map_or(1, |x| x.layout.align()),
+                    ),
+                );
+            }
+        }
     }
 }
 
diff --git a/src/entity_builder.rs b/src/entity_builder.rs
index 1a6d257..5b53d14 100644
--- a/src/entity_builder.rs
+++ b/src/entity_builder.rs
@@ -12,9 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::alloc::{alloc, Layout};
+use std::alloc::{alloc, dealloc, Layout};
 use std::any::TypeId;
 use std::mem::{self, MaybeUninit};
+use std::ptr;
 
 use crate::archetype::TypeInfo;
 use crate::{Component, DynamicBundle};
@@ -58,8 +59,9 @@ impl EntityBuilder {
         }
         if mem::size_of::<T>() != 0 {
             unsafe {
-                self.storage[self.cursor]
+                self.storage
                     .as_mut_ptr()
+                    .add(self.cursor)
                     .cast::<T>()
                     .write_unaligned(component);
             }
@@ -104,18 +106,23 @@ impl EntityBuilder {
             .unwrap_or(0);
         unsafe {
             // Suitably aligned storage for drop
-            let mut tmp = if max_size > 0 {
-                Box::from_raw(std::slice::from_raw_parts_mut(
-                    alloc(Layout::from_size_align(max_size, max_align).unwrap()).cast(),
-                    max_size,
-                ))
+            let tmp = if max_size > 0 {
+                alloc(Layout::from_size_align(max_size, max_align).unwrap()).cast()
             } else {
-                Box::new([])
+                max_align as *mut _
             };
             for (ty, offset) in self.info.drain(..) {
-                tmp[0..ty.layout().size()]
-                    .copy_from_slice(&self.storage[offset..offset + ty.layout().size()]);
-                ty.drop(tmp.as_mut_ptr().cast::<u8>());
+                ptr::copy_nonoverlapping(
+                    self.storage[offset..offset + ty.layout().size()]
+                        .as_ptr()
+                        .cast(),
+                    tmp,
+                    ty.layout().size(),
+                );
+                ty.drop(tmp);
+            }
+            if max_size > 0 {
+                dealloc(tmp, Layout::from_size_align(max_size, max_align).unwrap())
             }
         }
     }
@@ -153,8 +160,8 @@ impl DynamicBundle for BuiltEntity<'_> {
     }
 
     unsafe fn put(self, mut f: impl FnMut(*mut u8, TypeId, usize) -> bool) {
-        for (ty, component) in self.builder.info.drain(..) {
-            let ptr = self.builder.storage[component].as_mut_ptr().cast();
+        for (ty, offset) in self.builder.info.drain(..) {
+            let ptr = self.builder.storage.as_mut_ptr().add(offset).cast();
             if !f(ptr, ty.id(), ty.layout().size()) {
                 ty.drop(ptr);
             }


Test patch:
diff --git a/tests/tests.rs b/tests/tests.rs
index 7deb2a4..ca4a75b 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -100,7 +100,8 @@ fn query_optional_component() {
 fn build_entity() {
     let mut world = World::new();
     let mut entity = EntityBuilder::new();
-    entity.add("abc").add(123);
+    entity.add("abc");
+    entity.add(123);
     let e = world.spawn(entity.build());
     entity.add("def");
     entity.add([0u8; 1024]);
@@ -221,6 +222,7 @@ fn derived_query() {
 }
 
 #[test]
+#[cfg_attr(miri, ignore)]
 fn spawn_many() {
     let mut world = World::new();
     const N: usize = 100_000;