Oilpan: fix tracing of un-initialized part objects during conservative GCs.

If a part object has a virtual trace method we need to check the vtable
before calling the trace method. This is necessary because conservative
GCs can see the part object before it has been constructed (if there
is an allocation during the construction of the containing object.)

Added simple regression test that illustrates the issue.

R=erik.corry@gmail.com, zerny@chromium.org

Review URL: https://codereview.chromium.org/455363002

git-svn-id: svn://svn.chromium.org/blink/trunk@180335 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/platform/heap/Heap.cpp

@@ -521,23 +521,42 @@ void LargeHeapObject<Header>::checkAndMarkPointer(Visitor* visitor, Address addr
     mark(visitor);
 }
 
+#if ENABLE(ASSERT)
+static bool isUninitializedMemory(void* objectPointer, size_t objectSize)
+{
+    // Scan through the object's fields and check that they are all zero.
+    Address* objectFields = reinterpret_cast<Address*>(objectPointer);
+    for (size_t i = 0; i < objectSize / sizeof(Address); ++i) {
+        if (objectFields[i] != 0)
+            return false;
+    }
+    return true;
+}
+#endif
+
 template<>
 void LargeHeapObject<FinalizedHeapObjectHeader>::mark(Visitor* visitor)
 {
-    if (heapObjectHeader()->hasVTable() && !vTableInitialized(payload()))
-        visitor->markConservatively(heapObjectHeader());
-    else
+    if (heapObjectHeader()->hasVTable() && !vTableInitialized(payload())) {
+        FinalizedHeapObjectHeader* header = heapObjectHeader();
+        visitor->markNoTracing(header);
+        ASSERT(isUninitializedMemory(header->payload(), header->payloadSize()));
+    } else {
         visitor->mark(heapObjectHeader(), heapObjectHeader()->traceCallback());
+    }
 }
 
 template<>
 void LargeHeapObject<HeapObjectHeader>::mark(Visitor* visitor)
 {
     ASSERT(gcInfo());
-    if (gcInfo()->hasVTable() && !vTableInitialized(payload()))
-        visitor->markConservatively(heapObjectHeader());
-    else
+    if (gcInfo()->hasVTable() && !vTableInitialized(payload())) {
+        HeapObjectHeader* header = heapObjectHeader();
+        visitor->markNoTracing(header);
+        ASSERT(isUninitializedMemory(header->payload(), header->payloadSize()));
+    } else {
         visitor->mark(heapObjectHeader(), gcInfo()->m_trace);
+    }
 }
 
 template<>
@@ -1362,10 +1381,12 @@ void HeapPage<Header>::checkAndMarkPointer(Visitor* visitor, Address address)
 #if ENABLE(GC_PROFILE_MARKING)
     visitor->setHostInfo(&address, "stack");
 #endif
-    if (hasVTable(header) && !vTableInitialized(header->payload()))
-        visitor->markConservatively(header);
-    else
+    if (hasVTable(header) && !vTableInitialized(header->payload())) {
+        visitor->markNoTracing(header);
+        ASSERT(isUninitializedMemory(header->payload(), header->payloadSize()));
+    } else {
         visitor->mark(header, traceCallback(header));
+    }
 }
 
 #if ENABLE(GC_PROFILE_MARKING)
@@ -1780,35 +1801,6 @@ public:
         visitHeader(header, header->payload(), callback);
     }
 
-
-    inline void visitConservatively(HeapObjectHeader* header, void* objectPointer, size_t objectSize)
-    {
-        ASSERT(header);
-        ASSERT(objectPointer);
-        if (header->isMarked())
-            return;
-        header->mark();
-
-        // Scan through the object's fields and visit them conservatively.
-        Address* objectFields = reinterpret_cast<Address*>(objectPointer);
-        for (size_t i = 0; i < objectSize / sizeof(Address); ++i)
-            Heap::checkAndMarkPointer(this, objectFields[i]);
-    }
-
-    virtual void markConservatively(HeapObjectHeader* header)
-    {
-        // We need both the HeapObjectHeader and FinalizedHeapObjectHeader
-        // version to correctly find the payload.
-        visitConservatively(header, header->payload(), header->payloadSize());
-    }
-
-    virtual void markConservatively(FinalizedHeapObjectHeader* header)
-    {
-        // We need both the HeapObjectHeader and FinalizedHeapObjectHeader
-        // version to correctly find the payload.
-        visitConservatively(header, header->payload(), header->payloadSize());
-    }
-
     virtual void registerWeakMembers(const void* closure, const void* containingObject, WeakPointerCallback callback) OVERRIDE
     {
         Heap::pushWeakObjectPointerCallback(const_cast<void*>(closure), const_cast<void*>(containingObject), callback);

third_party/WebKit/Source/platform/heap/HeapTest.cpp

@@ -264,16 +264,6 @@ public:
         m_count++;
     }
 
-    virtual void markConservatively(HeapObjectHeader* header) OVERRIDE
-    {
-        ASSERT_NOT_REACHED();
-    }
-
-    virtual void markConservatively(FinalizedHeapObjectHeader* header) OVERRIDE
-    {
-        ASSERT_NOT_REACHED();
-    }
-
     virtual void registerWeakMembers(const void*, const void*, WeakPointerCallback) OVERRIDE { }
     virtual void registerWeakTable(const void*, EphemeronCallback, EphemeronCallback) OVERRIDE { }
 #if ENABLE(ASSERT)
@@ -4849,12 +4839,19 @@ TEST(HeapTest, ObjectDeadBit)
     DeadBitTester::test();
 }
 
+static bool allocateAndReturnBool()
+{
+    Heap::collectGarbage(ThreadState::HeapPointersOnStack);
+    return true;
+}
+
 class MixinWithGarbageCollectionInConstructor : public GarbageCollectedMixin {
 public:
-    MixinWithGarbageCollectionInConstructor()
+    MixinWithGarbageCollectionInConstructor() : m_dummy(allocateAndReturnBool())
     {
-        Heap::collectGarbage(ThreadState::HeapPointersOnStack);
     }
+private:
+    bool m_dummy;
 };
 
 class ClassWithGarbageCollectingMixinConstructor
@@ -5049,4 +5046,24 @@ TEST(HeapTest, TraceIfNeeded)
     }
 }
 
+class PartObjectWithVirtualMethod {
+public:
+    virtual void trace(Visitor*) { }
+};
+
+class ObjectWithVirtualPartObject : public GarbageCollected<ObjectWithVirtualPartObject> {
+public:
+    ObjectWithVirtualPartObject() : m_dummy(allocateAndReturnBool()) { }
+    void trace(Visitor* visitor) { visitor->trace(m_part); }
+private:
+    bool m_dummy;
+    PartObjectWithVirtualMethod m_part;
+};
+
+TEST(HeapTest, PartObjectWithVirtualMethod)
+{
+    ObjectWithVirtualPartObject* object = new ObjectWithVirtualPartObject();
+    EXPECT_TRUE(object);
+}
+
 } // namespace blink

third_party/WebKit/Source/platform/heap/Visitor.h

@@ -289,6 +289,11 @@ public:
     template<typename T>
     void trace(const T& t)
     {
+        if (WTF::IsPolymorphic<T>::value) {
+            intptr_t vtable = *reinterpret_cast<const intptr_t*>(&t);
+            if (!vtable)
+                return;
+        }
         const_cast<T&>(t).trace(this);
     }
 
@@ -329,8 +334,6 @@ public:
     // Used to mark objects during conservative scanning.
     virtual void mark(HeapObjectHeader*, TraceCallback) = 0;
     virtual void mark(FinalizedHeapObjectHeader*, TraceCallback) = 0;
-    virtual void markConservatively(HeapObjectHeader*) = 0;
-    virtual void markConservatively(FinalizedHeapObjectHeader*) = 0;
 
     // If the object calls this during the regular trace callback, then the
     // WeakPointerCallback argument may be called later, when the strong roots