fix: let cppgc align to 16 bytes (#1677)

examples/cppgc-object.rs

@@ -27,7 +27,7 @@ fn main() {
   v8::V8::initialize_platform(platform.clone());
   v8::V8::initialize();
 
-  v8::cppgc::initalize_process(platform.clone());
+  v8::cppgc::initialize_process(platform.clone());
 
   {
     let heap =

examples/cppgc.rs

@@ -46,7 +46,7 @@ fn main() {
   let platform = v8::new_default_platform(0, false).make_shared();
   v8::V8::initialize_platform(platform.clone());
   v8::V8::initialize();
-  v8::cppgc::initalize_process(platform.clone());
+  v8::cppgc::initialize_process(platform.clone());
 
   {
     // Create a managed heap.

src/binding.cc

@@ -3976,9 +3976,19 @@ void cppgc__heap__collect_garbage_for_testing(
   heap->CollectGarbageForTesting(stack_state);
 }
 
-RustObj* cppgc__make_garbage_collectable(v8::CppHeap* heap, size_t size) {
+class alignas(16) RustObjButAlign16 : public RustObj {};
-  return cppgc::MakeGarbageCollected<RustObj>(heap->GetAllocationHandle(),
+
-                                              cppgc::AdditionalBytes(size));
+RustObj* cppgc__make_garbage_collectable(v8::CppHeap* heap, size_t size,
+                                         size_t alignment) {
+  if (alignment <= 8) {
+    return cppgc::MakeGarbageCollected<RustObj>(heap->GetAllocationHandle(),
+                                                cppgc::AdditionalBytes(size));
+  }
+  if (alignment <= 16) {
+    return cppgc::MakeGarbageCollected<RustObjButAlign16>(
+        heap->GetAllocationHandle(), cppgc::AdditionalBytes(size));
+  }
+  return nullptr;
 }
 
 void cppgc__Visitor__Trace__Member(cppgc::Visitor* visitor,

src/cppgc.rs

@@ -27,6 +27,7 @@ extern "C" {
   fn cppgc__make_garbage_collectable(
     heap: *mut Heap,
     size: usize,
+    alignment: usize,
   ) -> *mut RustObj;
 
   fn cppgc__heap__enable_detached_garbage_collections_for_testing(
@@ -145,7 +146,7 @@ unsafe fn get_object_from_rust_obj<T: GarbageCollected>(
 /// creating a Heap.
 ///
 /// Can be called multiple times when paired with `ShutdownProcess()`.
-pub fn initalize_process(platform: SharedRef<Platform>) {
+pub fn initialize_process(platform: SharedRef<Platform>) {
   unsafe {
     cppgc__initialize_process(&*platform as *const Platform as *mut _);
   }
@@ -155,7 +156,7 @@ pub fn initalize_process(platform: SharedRef<Platform>) {
 ///
 /// Must be called after destroying the last used heap. Some process-global
 /// metadata may not be returned and reused upon a subsequent
-/// `initalize_process()` call.
+/// `initialize_process()` call.
 pub unsafe fn shutdown_process() {
   cppgc__shutdown_process();
 }
@@ -343,6 +344,11 @@ pub unsafe fn make_garbage_collected<T: GarbageCollected + 'static>(
   heap: &Heap,
   obj: T,
 ) -> Ptr<T> {
+  const {
+    // max alignment in cppgc is 16
+    assert!(std::mem::align_of::<T>() <= 16);
+  }
+
   let additional_bytes = (object_offset_for_rust_obj::<T>()
     - std::mem::size_of::<RustObj>())
     + std::mem::size_of::<T>();
@@ -351,9 +357,12 @@ pub unsafe fn make_garbage_collected<T: GarbageCollected + 'static>(
     cppgc__make_garbage_collectable(
       heap as *const Heap as *mut _,
       additional_bytes,
+      std::mem::align_of::<T>(),
     )
   };
 
+  assert!(!pointer.is_null());
+
   unsafe {
     let inner = get_object_from_rust_obj::<T>(pointer);
     inner.write(obj);

src/support.h

@@ -193,8 +193,8 @@ struct three_pointers_t {
 
 #endif  // SUPPORT_H_
 
-class RustObj final : public cppgc::GarbageCollected<RustObj>,
+class RustObj : public cppgc::GarbageCollected<RustObj>,
-                      public cppgc::NameProvider {
+                public cppgc::NameProvider {
  public:
   ~RustObj();
   void Trace(cppgc::Visitor* visitor) const;

tests/test_cppgc.rs

@@ -2,177 +2,219 @@
 use std::sync::atomic::{AtomicUsize, Ordering};
 use v8::cppgc::{GarbageCollected, Visitor};
 
-struct CppGCGuard {
+mod setup {
-  pub platform: v8::SharedRef<v8::Platform>,
+  use std::sync::Once;
-}
+  use std::sync::RwLock;
+  // use std::sync::RwLockReadGuard;
+  use std::sync::RwLockWriteGuard;
 
-fn initalize_test() -> CppGCGuard {
+  static PROCESS_LOCK: RwLock<()> = RwLock::new(());
-  v8::V8::set_flags_from_string("--no_freeze_flags_after_init --expose-gc");
-  let platform = v8::new_unprotected_default_platform(0, false).make_shared();
-  v8::V8::initialize_platform(platform.clone());
-  v8::V8::initialize();
-  v8::cppgc::initalize_process(platform.clone());
 
-  CppGCGuard { platform }
+  /*
-}
+  /// Set up global state for a test that can run in parallel with other tests.
+  pub(super) fn parallel_test() -> SetupGuard<RwLockReadGuard<'static, ()>> {
+    initialize_once();
+    SetupGuard::new(PROCESS_LOCK.read().unwrap())
+  }
+  */
 
-impl Drop for CppGCGuard {
+  /// Set up global state for a test that must be the only test running.
-  fn drop(&mut self) {
+  pub(super) fn sequential_test() -> SetupGuard<RwLockWriteGuard<'static, ()>> {
-    unsafe {
+    initialize_once();
-      v8::cppgc::shutdown_process();
+    SetupGuard::new(PROCESS_LOCK.write().unwrap())
-      v8::V8::dispose();
+  }
+
+  fn initialize_once() {
+    static START: Once = Once::new();
+    START.call_once(|| {
+      assert!(v8::icu::set_common_data_74(align_data::include_aligned!(
+        align_data::Align16,
+        "../third_party/icu/common/icudtl.dat"
+      ))
+      .is_ok());
+      v8::V8::set_flags_from_string(
+        "--no_freeze_flags_after_init --expose_gc --harmony-import-assertions --harmony-shadow-realm --allow_natives_syntax --turbo_fast_api_calls",
+      );
+
+      let platform = v8::new_unprotected_default_platform(0, false).make_shared();
+      v8::V8::initialize_platform(platform.clone());
+      v8::V8::initialize();
+      v8::cppgc::initialize_process(platform.clone());
+    });
+  }
+
+  #[must_use]
+  pub(super) struct SetupGuard<G> {
+    _inner: G,
+  }
+
+  impl<G> SetupGuard<G> {
+    fn new(inner: G) -> Self {
+      Self { _inner: inner }
     }
-    v8::V8::dispose_platform();
   }
 }
 
 const TAG: u16 = 1;
 
-#[test]
+macro_rules! test {
-fn cppgc_object_wrap() {
+  ( $( $decln:ident : $declt:ty )?, $( $initn:ident : $inite:expr )? ) => {{
-  let guard = initalize_test();
+      let _guard = setup::sequential_test();
 
-  static TRACE_COUNT: AtomicUsize = AtomicUsize::new(0);
+      static TRACE_COUNT: AtomicUsize = AtomicUsize::new(0);
-  static DROP_COUNT: AtomicUsize = AtomicUsize::new(0);
+      static DROP_COUNT: AtomicUsize = AtomicUsize::new(0);
 
-  struct Wrap {
+      struct Wrap {
-    value: v8::TracedReference<v8::Value>,
+        $( #[allow(unused)] $decln: $declt , )?
-  }
+        value: v8::TracedReference<v8::Value>,
-
-  impl GarbageCollected for Wrap {
-    fn trace(&self, visitor: &Visitor) {
-      TRACE_COUNT.fetch_add(1, Ordering::SeqCst);
-      visitor.trace(&self.value);
-    }
-
-    fn get_name(&self) -> Option<&'static std::ffi::CStr> {
-      Some(c"Eyecatcher")
-    }
-  }
-
-  impl Drop for Wrap {
-    fn drop(&mut self) {
-      DROP_COUNT.fetch_add(1, Ordering::SeqCst);
-    }
-  }
-
-  fn op_wrap(
-    scope: &mut v8::HandleScope,
-    args: v8::FunctionCallbackArguments,
-    mut rv: v8::ReturnValue<v8::Value>,
-  ) {
-    fn empty(
-      _scope: &mut v8::HandleScope,
-      _args: v8::FunctionCallbackArguments,
-      _rv: v8::ReturnValue<v8::Value>,
-    ) {
-    }
-    let templ = v8::FunctionTemplate::new(scope, empty);
-    let func = templ.get_function(scope).unwrap();
-    let obj = func.new_instance(scope, &[]).unwrap();
-    assert!(obj.is_api_wrapper());
-
-    let wrap = Wrap {
-      value: v8::TracedReference::new(scope, args.get(0)),
-    };
-    let member = unsafe {
-      v8::cppgc::make_garbage_collected(scope.get_cpp_heap().unwrap(), wrap)
-    };
-
-    unsafe {
-      v8::Object::wrap::<TAG, Wrap>(scope, obj, &member);
-    }
-
-    rv.set(obj.into());
-  }
-
-  fn op_unwrap(
-    scope: &mut v8::HandleScope,
-    args: v8::FunctionCallbackArguments,
-    mut rv: v8::ReturnValue,
-  ) {
-    let obj = args.get(0).try_into().unwrap();
-    let member = unsafe { v8::Object::unwrap::<TAG, Wrap>(scope, obj) };
-    rv.set(member.unwrap().value.get(scope).unwrap());
-  }
-
-  {
-    // Create a managed heap.
-    let mut heap = v8::cppgc::Heap::create(
-      guard.platform.clone(),
-      v8::cppgc::HeapCreateParams::default(),
-    );
-    let isolate = &mut v8::Isolate::new(v8::CreateParams::default());
-    isolate.attach_cpp_heap(&mut heap);
-
-    {
-      let handle_scope = &mut v8::HandleScope::new(isolate);
-      let context = v8::Context::new(handle_scope, Default::default());
-      let scope = &mut v8::ContextScope::new(handle_scope, context);
-      let global = context.global(scope);
-      {
-        let func = v8::Function::new(scope, op_wrap).unwrap();
-        let name = v8::String::new(scope, "wrap").unwrap();
-        global.set(scope, name.into(), func.into()).unwrap();
-      }
-      {
-        let func = v8::Function::new(scope, op_unwrap).unwrap();
-        let name = v8::String::new(scope, "unwrap").unwrap();
-        global.set(scope, name.into(), func.into()).unwrap();
       }
 
-      execute_script(
+      impl GarbageCollected for Wrap {
-        scope,
+        fn trace(&self, visitor: &Visitor) {
-        r#"
+          TRACE_COUNT.fetch_add(1, Ordering::SeqCst);
-      {
+          visitor.trace(&self.value);
-        const x = {};
+        }
-        const y = unwrap(wrap(x)); // collected
+
-        if (x !== y) {
+        fn get_name(&self) -> Option<&'static std::ffi::CStr> {
-          throw new Error('mismatch');
+          Some(c"Eyecatcher")
         }
       }
 
-      globalThis.wrapped = wrap(wrap({})); // not collected
+      impl Drop for Wrap {
-    "#,
+        fn drop(&mut self) {
-      );
+          DROP_COUNT.fetch_add(1, Ordering::SeqCst);
-
+        }
-      assert_eq!(DROP_COUNT.load(Ordering::SeqCst), 0);
-
-      {
-        let mut vec = Vec::<u8>::new();
-        scope.take_heap_snapshot(|chunk| {
-          vec.extend_from_slice(chunk);
-          true
-        });
-        let s = std::str::from_utf8(&vec).unwrap();
-        assert!(s.contains("Eyecatcher"));
       }
 
-      scope.request_garbage_collection_for_testing(
+      fn op_wrap(
-        v8::GarbageCollectionType::Full,
+        scope: &mut v8::HandleScope,
-      );
+        args: v8::FunctionCallbackArguments,
+        mut rv: v8::ReturnValue<v8::Value>,
+      ) {
+        fn empty(
+          _scope: &mut v8::HandleScope,
+          _args: v8::FunctionCallbackArguments,
+          _rv: v8::ReturnValue<v8::Value>,
+        ) {
+        }
+        let templ = v8::FunctionTemplate::new(scope, empty);
+        let func = templ.get_function(scope).unwrap();
+        let obj = func.new_instance(scope, &[]).unwrap();
+        assert!(obj.is_api_wrapper());
 
-      assert!(TRACE_COUNT.load(Ordering::SeqCst) > 0);
+        let wrap = Wrap {
-      assert_eq!(DROP_COUNT.load(Ordering::SeqCst), 1);
+          $( $initn: $inite , )?
+          value: v8::TracedReference::new(scope, args.get(0)),
+        };
+        let member = unsafe {
+          v8::cppgc::make_garbage_collected(scope.get_cpp_heap().unwrap(), wrap)
+        };
 
-      execute_script(
+        unsafe {
-        scope,
+          v8::Object::wrap::<TAG, Wrap>(scope, obj, &member);
-        r#"
+        }
-      globalThis.wrapped = undefined;
-    "#,
-      );
 
-      scope.request_garbage_collection_for_testing(
+        rv.set(obj.into());
-        v8::GarbageCollectionType::Full,
+      }
-      );
 
-      assert_eq!(DROP_COUNT.load(Ordering::SeqCst), 3);
+      fn op_unwrap(
-    }
+        scope: &mut v8::HandleScope,
+        args: v8::FunctionCallbackArguments,
+        mut rv: v8::ReturnValue,
+      ) {
+        let obj = args.get(0).try_into().unwrap();
+        let member = unsafe { v8::Object::unwrap::<TAG, Wrap>(scope, obj) };
+        rv.set(member.unwrap().value.get(scope).unwrap());
+      }
 
-    isolate.detach_cpp_heap();
+      {
-    heap.terminate();
+        // Create a managed heap.
-    drop(heap);
+        let mut heap = v8::cppgc::Heap::create(
-  }
+          v8::V8::get_current_platform(),
+          v8::cppgc::HeapCreateParams::default(),
+        );
+        let isolate = &mut v8::Isolate::new(v8::CreateParams::default());
+        isolate.attach_cpp_heap(&mut heap);
+
+        {
+          let handle_scope = &mut v8::HandleScope::new(isolate);
+          let context = v8::Context::new(handle_scope, Default::default());
+          let scope = &mut v8::ContextScope::new(handle_scope, context);
+          let global = context.global(scope);
+          {
+            let func = v8::Function::new(scope, op_wrap).unwrap();
+            let name = v8::String::new(scope, "wrap").unwrap();
+            global.set(scope, name.into(), func.into()).unwrap();
+          }
+          {
+            let func = v8::Function::new(scope, op_unwrap).unwrap();
+            let name = v8::String::new(scope, "unwrap").unwrap();
+            global.set(scope, name.into(), func.into()).unwrap();
+          }
+
+          execute_script(
+            scope,
+            r#"
+          {
+            const x = {};
+            const y = unwrap(wrap(x)); // collected
+            if (x !== y) {
+              throw new Error('mismatch');
+            }
+          }
+
+          globalThis.wrapped = wrap(wrap({})); // not collected
+        "#,
+          );
+
+          assert_eq!(DROP_COUNT.load(Ordering::SeqCst), 0);
+
+          {
+            let mut vec = Vec::<u8>::new();
+            scope.take_heap_snapshot(|chunk| {
+              vec.extend_from_slice(chunk);
+              true
+            });
+            let s = std::str::from_utf8(&vec).unwrap();
+            assert!(s.contains("Eyecatcher"));
+          }
+
+          scope.request_garbage_collection_for_testing(
+            v8::GarbageCollectionType::Full,
+          );
+
+          assert!(TRACE_COUNT.load(Ordering::SeqCst) > 0);
+          assert_eq!(DROP_COUNT.load(Ordering::SeqCst), 1);
+
+          execute_script(
+            scope,
+            r#"
+          globalThis.wrapped = undefined;
+        "#,
+          );
+
+          scope.request_garbage_collection_for_testing(
+            v8::GarbageCollectionType::Full,
+          );
+
+          assert_eq!(DROP_COUNT.load(Ordering::SeqCst), 3);
+        }
+
+        isolate.detach_cpp_heap();
+        heap.terminate();
+        drop(heap);
+      }
+  }}
+}
+
+#[test]
+fn cppgc_object_wrap8() {
+  test!(,);
+}
+
+#[test]
+fn cppgc_object_wrap16() {
+  test!(big: u128, big: 0);
 }
 
 fn execute_script(