Prevent GCs when constructing GC mixin objects.

Source/platform/heap/Heap.h

Index: Source/platform/heap/Heap.h
diff --git a/Source/platform/heap/Heap.h b/Source/platform/heap/Heap.h
index 76052f32432746a783d92fe4ed12282d7b68b9d3..58ae3b949f05b35f67b6ac3e46655709e3458cca 100644
--- a/Source/platform/heap/Heap.h
+++ b/Source/platform/heap/Heap.h
@@ -1031,6 +1031,74 @@ private:
     friend class ThreadState;
 };
 
+template<typename T>
+struct HeapIndexTrait {
+    static int index() { return NormalPageHeapIndex; };
+};
+
+// FIXME: The forward declaration is layering violation.
+#define DEFINE_TYPED_HEAP_TRAIT(Type)                   \
+    class Type;                                         \
+    template <> struct HeapIndexTrait<class Type> {     \
+        static int index() { return Type##HeapIndex; }; \
+    };
+FOR_EACH_TYPED_HEAP(DEFINE_TYPED_HEAP_TRAIT)
+#undef DEFINE_TYPED_HEAP_TRAIT
+
+template<typename T, typename Enabled = void>
+class AllocateObjectTrait {
+public:
+    static inline Address allocate(size_t size)
+    {
+        return Heap::allocateOnHeapIndex<T>(size, HeapIndexTrait<T>::index(), GCInfoTrait<T>::index());
+    }
+
+    static inline void constructor()
+    {
+    }
+};
+
+template<typename T>
+class AllocateObjectTrait<T, typename WTF::EnableIf<blink::IsGarbageCollectedMixin<T>::value>::Type> {
+public:
+    // An object which implements GarbageCollectedMixin is marked
+    // and traced during GC by first adjusting object references to
+    // it to refer to the leftmost base for the object (which would
+    // be a GarbageCollected-derived class.) The prefixed object header
+    // can be located after that adjustment and its trace() vtbl slot
+    // will be used to fully trace the object, if not already marked.
+    //
+    // A C++ object's vptr will be initialized to its leftmost base's
+    // vtabke after the constructors of all its subclasses have run,

[haraken, 2015/02/23 08:24:23]

vtable

[sof, 2015/02/23 10:29:02]

Done.

+    // so if a subclass constructor tries to access any of the vtbl
+    // entries of its leftmost base prematurely, it'll find an as-yet
+    // incorrect vptr and fail. Which is exactly what a garbage collector
+    // will try to do if it tries to access the leftmost base while one
+    // of the subclass constructors of a GC mixin object triggers a GC.
+    // It is consequently not safe to allow any GCs while these objects
+    // are under (sub constructor) construction.
+    //
+    // To achieve that, the construction of mixins are handled in a
+    // special manner:
+    //
+    //  - The initial allocation of the mixin object will enter a no GC scope.
+    //  - The constructor for the leftmost base, which is when the mixin
+    //    object is in a state ready for a GC, leaves that GC scope.
+    //  - no-GC scope entering/leaving must support nesting.
+    //
+    static inline Address allocate(size_t size)
+    {
+        Address object = Heap::allocateOnHeapIndex<T>(size, HeapIndexTrait<T>::index(), GCInfoTrait<T>::index());
+        ThreadState::current()->enterNoGCAllowedScope();

[haraken, 2015/02/23 08:24:22]

Are compilers smart enough to remove the two ThreadState::current() lookups (one
in Heap::allocateOnHeapIndex and the other here)?

[sof, 2015/02/23 10:29:02]

Reasonable to assume so, but we might as well manually CSE to remove the
possibility that it won't be. Now done.

I left in a templated version of allocateOnHeapIndex<T>; please check if you
find that agreeable.

+        return object;
+    }
+
+    static inline void constructor()
+    {
+        ThreadState::current()->leaveNoGCAllowedScope();
+    }
+};
+
 // Base class for objects allocated in the Blink garbage-collected heap.
 //
 // Defines a 'new' operator that allocates the memory in the heap.  'delete'
@@ -1079,6 +1147,7 @@ public:
 protected:
     GarbageCollected()
     {
+        AllocateObjectTrait<T>::constructor();
     }
 };
 
@@ -1376,20 +1445,6 @@ Address NormalPageHeap::allocate(size_t size, size_t gcInfoIndex)
 }
 
 template<typename T>
-struct HeapIndexTrait {
-    static int index() { return NormalPageHeapIndex; };
-};
-
-// FIXME: The forward declaration is layering violation.
-#define DEFINE_TYPED_HEAP_TRAIT(Type)                   \
-    class Type;                                         \
-    template <> struct HeapIndexTrait<class Type> {     \
-        static int index() { return Type##HeapIndex; }; \
-    };
-FOR_EACH_TYPED_HEAP(DEFINE_TYPED_HEAP_TRAIT)
-#undef DEFINE_TYPED_HEAP_TRAIT
-
-template<typename T>
 Address Heap::allocateOnHeapIndex(size_t size, int heapIndex, size_t gcInfoIndex)
 {
     ThreadState* state = ThreadStateFor<ThreadingTrait<T>::Affinity>::state();
@@ -1400,7 +1455,7 @@ Address Heap::allocateOnHeapIndex(size_t size, int heapIndex, size_t gcInfoIndex
 template<typename T>
 Address Heap::allocate(size_t size)
 {
-    return allocateOnHeapIndex<T>(size, HeapIndexTrait<T>::index(), GCInfoTrait<T>::index());
+    return AllocateObjectTrait<T>::allocate(size);
 }
 
 template<typename T>