[Oilpan] Add use-after-free detector in Member<>

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

Index: third_party/WebKit/Source/platform/heap/Heap.h
diff --git a/third_party/WebKit/Source/platform/heap/Heap.h b/third_party/WebKit/Source/platform/heap/Heap.h
index 67a8bc873ef643166d19459cecc0c07fab89dab3..756fcb2638d988beda98f0b496753dbe6f973253 100644
--- a/third_party/WebKit/Source/platform/heap/Heap.h
+++ b/third_party/WebKit/Source/platform/heap/Heap.h
@@ -47,6 +47,8 @@ template<typename T> class Member;
 template<typename T> class WeakMember;
 template<typename T> class UntracedMember;
 
+// TODO(peria): Refactor following two sets of templates.
+
 template<typename T, bool = NeedsAdjustAndMark<T>::value> class ObjectAliveTrait;
 
 template<typename T>
@@ -64,11 +66,38 @@ class ObjectAliveTrait<T, true> {
 public:
     static bool isHeapObjectAlive(T* object)
     {
-        static_assert(sizeof(T), "T must be fully defined");
         return object->isHeapObjectAlive();
     }
 };
 
+template<typename T, bool = IsGarbageCollectedMixin<T>::value> class HeapObjectHeaderTrait;
+
+template<typename T>
+class HeapObjectHeaderTrait<T, true> {
+public:
+    static HeapObjectHeader* heapObjectHeader(T* obj)
+    {
+        static_assert(sizeof(T), "T must be fully defined");
+        // TODO(peria): This ASSERT() is too restrictive.  The ASSERT forbids
+        // to call heapObjectHeader() for an object while another
+        // (totally independent) mixin object is under construction.
+        ASSERT(!ThreadState::current()->isConstructingGCMixin());
+        return obj->heapObjectHeader();
+    }
+};
+
+template<typename T>
+class HeapObjectHeaderTrait<T, false> {

[peria, 2015/11/17 01:36:49]

Yuta-san,
Is it safe to instantiate HeapObjectHeaderTrait without having a fully-defined
T?

[Yuta Kitamura, 2015/11/17 06:25:12]

If the lines 95-96 are NOT present, I think it's safe, because there's nothing
in
this specific specialization depends on whether T is fully defined or not; in
other
words, this specialization only depends on the existance of T* and
T* -> void* conversion, both of which can be instantiated without T's full
declaration.

With lines 95-96 things get more complex, because this piece of code changes the
behavior
depending on whether T is fully defined *at the point of first instantiation*.

Consider the following example:

    class A;
    bool a1 = IsFullyDefined<A>::value;
    class A { };
    bool a2 = IsFullyDefined<A>::value;

The values of a1 and a2 are *both* false, because IsFullyDefined<A>::value is
instantiated
at the place of its first use, and not again on the later uses.

Therefore, your code with lines 95-96 would effectively cause the
heapObjectHeader
function to return nullptr for some random "T"s and return a valid header for
the other
"T"s. You probably don't want that behavior, nor lines 95-96, I guess.

[peria, 2015/11/17 07:24:57]

Thank you for the clarification and additional explanation in off-line
communication.
As you said, using IsFullyDefined<A>::value has another problem in link phase.
(Linker will choose which instantiation to link.)

As a result, I will add #include to define T and put a static assert to check it
here.

+public:
+    static HeapObjectHeader* heapObjectHeader(T* obj)
+    {
+        ASSERT(!ThreadState::current()->isConstructingGCMixin());
+        if (!IsFullyDefined<T>::value)
+            return nullptr;
+        return HeapObjectHeader::fromPayload(obj);
+    }
+};
+
 class PLATFORM_EXPORT Heap {
 public:
     static void init();
@@ -96,17 +125,17 @@ public:
     template<typename T>
     static inline bool isHeapObjectAlive(const Member<T>& member)
     {
-        return isHeapObjectAlive(member.get());
+        return isHeapObjectAlive(member.unsafeGet());
     }
     template<typename T>
     static inline bool isHeapObjectAlive(const WeakMember<T>& member)
     {
-        return isHeapObjectAlive(member.get());
+        return isHeapObjectAlive(member.unsafeGet());
     }
     template<typename T>
     static inline bool isHeapObjectAlive(const UntracedMember<T>& member)
     {
-        return isHeapObjectAlive(member.get());
+        return isHeapObjectAlive(member.unsafeGet());
     }
     template<typename T>
     static inline bool isHeapObjectAlive(const RawPtr<T>& ptr)
@@ -259,9 +288,7 @@ public:
     static void reportMemoryUsageHistogram();
     static void reportMemoryUsageForTracing();
 
-#if ENABLE(ASSERT)
-    static uint16_t gcGeneration() { return s_gcGeneration; }
-#endif
+    static uint32_t gcGeneration() { return s_gcGeneration; }
 
 private:
     // A RegionTree is a simple binary search tree of PageMemoryRegions sorted
@@ -308,9 +335,7 @@ private:
     static size_t s_collectedWrapperCount;
     static size_t s_partitionAllocSizeAtLastGC;
     static double s_estimatedMarkingTimePerByte;
-#if ENABLE(ASSERT)
-    static uint16_t s_gcGeneration;
-#endif
+    static uint32_t s_gcGeneration;
 
     friend class ThreadState;
 };
@@ -455,7 +480,7 @@ inline Address Heap::allocateOnHeapIndex(ThreadState* state, size_t size, int he
     ASSERT(state->isAllocationAllowed());
     ASSERT(heapIndex != BlinkGC::LargeObjectHeapIndex);
     NormalPageHeap* heap = static_cast<NormalPageHeap*>(state->heap(heapIndex));
-    return heap->allocateObject(allocationSizeFromSize(size), gcInfoIndex);
+    return heap->allocateObject(allocationSizeFromSize(size), gcInfoIndex, gcGeneration());
 }
 
 template<typename T>