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

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

Index: third_party/WebKit/Source/platform/heap/HeapPage.h
diff --git a/third_party/WebKit/Source/platform/heap/HeapPage.h b/third_party/WebKit/Source/platform/heap/HeapPage.h
index e1f17dea22cd8dd78930fcde1bad120963e296f5..befc46e563586253fc5ab7ceb386ca60276911dd 100644
--- a/third_party/WebKit/Source/platform/heap/HeapPage.h
+++ b/third_party/WebKit/Source/platform/heap/HeapPage.h
@@ -116,12 +116,6 @@ const uint8_t reuseForbiddenZapValue = 0x2c;
 #define CHECK_MEMORY_INACCESSIBLE(address, size) do { } while (false)
 #endif
 
-#if !ENABLE(ASSERT) && CPU(64BIT)
-#define USE_4BYTE_HEADER_PADDING 1
-#else
-#define USE_4BYTE_HEADER_PADDING 0
-#endif
-
 class CallbackStack;
 class FreePagePool;
 class NormalPageHeap;
@@ -168,11 +162,9 @@ class PLATFORM_EXPORT HeapObjectHeader {
 public:
     // If gcInfoIndex is 0, this header is interpreted as a free list header.
     NO_SANITIZE_ADDRESS
-    HeapObjectHeader(size_t size, size_t gcInfoIndex)
+    HeapObjectHeader(size_t size, size_t gcInfoIndex, uint32_t generation)
+        : m_gcGeneration(generation)
     {
-#if ENABLE(ASSERT)
-        m_magic = magic;
-#endif
         // sizeof(HeapObjectHeader) must be equal to or smaller than
         // allocationGranurarity, because HeapObjectHeader is used as a header
         // for an freed entry.  Given that the smallest entry size is
@@ -212,52 +204,29 @@ public:
 
 #if ENABLE(ASSERT)
     bool checkHeader() const;
-    // Zap magic number with a new magic number that means there was once an
-    // object allocated here, but it was freed because nobody marked it during
-    // GC.
-    void zapMagic();
 #endif
+    NO_SANITIZE_ADDRESS
+    uint32_t gcGeneration() const { return m_gcGeneration; }
 
     void finalize(Address, size_t);
     static HeapObjectHeader* fromPayload(const void*);
 
-    static const uint16_t magic = 0xfff1;
-    static const uint16_t zappedMagic = 0x4321;
-
 private:
     uint32_t m_encoded;
-#if ENABLE(ASSERT)
-    uint16_t m_magic;
-#endif
-
-    // In 64 bit architectures, we intentionally add 4 byte padding immediately
-    // after the HeapHeaderObject. This is because:
-    //
-    // | HeapHeaderObject (4 byte) | padding (4 byte) | object payload (8 * n byte) |
-    // ^8 byte aligned                                ^8 byte aligned
-    //
-    // is better than:
-    //
-    // | HeapHeaderObject (4 byte) | object payload (8 * n byte) | padding (4 byte) |
-    // ^4 byte aligned             ^8 byte aligned               ^4 byte aligned
-    //
-    // since the former layout aligns both header and payload to 8 byte.
-#if USE_4BYTE_HEADER_PADDING
-public:
-    uint32_t m_padding;
-#endif
+    // m_gcGeneration keeps track of the number of GC cycle where the object gets

[haraken, 2015/11/16 02:47:50]

GC cycles

gets allocated

[peria, 2015/11/16 05:33:26]

Done.

+    // allocated. m_gcGeneration == 0 indicates that the object has already been freed.
+    uint32_t m_gcGeneration;
 };
 
 class FreeListEntry final : public HeapObjectHeader {
 public:
     NO_SANITIZE_ADDRESS
     explicit FreeListEntry(size_t size)
-        : HeapObjectHeader(size, gcInfoIndexForFreeListHeader)
+        : HeapObjectHeader(size, gcInfoIndexForFreeListHeader, 0)
         , m_next(nullptr)
     {
 #if ENABLE(ASSERT)
         ASSERT(size >= sizeof(HeapObjectHeader));
-        zapMagic();
 #endif
     }
 
@@ -703,7 +672,7 @@ public:
 #endif
     void takeFreelistSnapshot(const String& dumpBaseName) override;
 
-    Address allocateObject(size_t allocationSize, size_t gcInfoIndex);
+    Address allocateObject(size_t allocationSize, size_t gcInfoIndex, uint32_t generation);
 
     void freePage(NormalPage*);
 
@@ -783,7 +752,8 @@ size_t HeapObjectHeader::size() const
 NO_SANITIZE_ADDRESS inline
 bool HeapObjectHeader::checkHeader() const
 {
-    return !pageFromObject(this)->orphaned() && m_magic == magic;
+    ASSERT(isFree() == (m_gcGeneration == 0));

[haraken, 2015/11/16 02:47:50]

Looks much nicer than the predicate I suggested!

[peria, 2015/11/16 05:33:26]

:)

+    return !pageFromObject(this)->orphaned();
 }
 #endif
 
@@ -855,14 +825,14 @@ void HeapObjectHeader::markDead()
     m_encoded |= headerDeadBitMask;
 }
 
-inline Address NormalPageHeap::allocateObject(size_t allocationSize, size_t gcInfoIndex)
+inline Address NormalPageHeap::allocateObject(size_t allocationSize, size_t gcInfoIndex, uint32_t generation)

[haraken, 2015/11/16 02:47:50]

Why do we need to pass the generation parameter to allocateObject()? Can't we
just call Heap::gcGeneration in allocateObject()?

[peria, 2015/11/16 05:33:26]

This is just a dependency problem of .h files.
We already have a strong dependency from Heap.h to HeapPage.h, so we can't make
a dependency from HeapPage.h to Heap.h.

 {
     if (LIKELY(allocationSize <= m_remainingAllocationSize)) {
         Address headerAddress = m_currentAllocationPoint;
         m_currentAllocationPoint += allocationSize;
         m_remainingAllocationSize -= allocationSize;
         ASSERT(gcInfoIndex > 0);
-        new (NotNull, headerAddress) HeapObjectHeader(allocationSize, gcInfoIndex);
+        new (NotNull, headerAddress) HeapObjectHeader(allocationSize, gcInfoIndex, generation);
         Address result = headerAddress + sizeof(HeapObjectHeader);
         ASSERT(!(reinterpret_cast<uintptr_t>(result) & allocationMask));