Oilpan: improve handling of ASan contiguous container annotations.

Source/platform/heap/Heap.cpp

Index: Source/platform/heap/Heap.cpp
diff --git a/Source/platform/heap/Heap.cpp b/Source/platform/heap/Heap.cpp
index dfe3e0e773fac5a0817483677da6f0fd45d8706c..96d1a1549973d86eef1d3467c0910d00bcb2c51e 100644
--- a/Source/platform/heap/Heap.cpp
+++ b/Source/platform/heap/Heap.cpp
@@ -40,6 +40,7 @@
 #include "public/platform/Platform.h"
 #include "wtf/AddressSpaceRandomization.h"
 #include "wtf/Assertions.h"
+#include "wtf/ContainerAnnotations.h"
 #include "wtf/LeakAnnotations.h"
 #include "wtf/PassOwnPtr.h"
 #if ENABLE(GC_PROFILING)
@@ -59,6 +60,17 @@
 #include <windows.h>
 #endif
 
+#ifdef ANNOTATE_CONTIGUOUS_CONTAINER
+#define ENABLE_ASAN_CONTAINER_ANNOTATIONS 1
+// Remove the contiguous container annotation, as non-inlined vector backings
+// aren't finalized.
+#define ASAN_RETIRE_CONTAINER_ANNOTATION(heapIndex, payload, payloadSize) \

[haraken, 2015/02/20 20:57:56]

You might remove the heapIndex parameter from ASAN_RETIRE_CONTAINER_ANNOTATION.
Then I think you can move ASAN_RETIRE_CONTAINER_ANNOTATION(payload, payloadSize)
into finalize().

The macro can do something like:

BasePage* page = pageFromObject(payload);
if (page->heapIndex() == VectorHeapIndex || (page->heapIndex() ==
LargeObjectHeapIndex &&
static_cast<LargeObjectPage>(page)->isVectorBackingPage())) {
    ANNOTATE_DELETE_BUFFER();
}

[sof, 2015/02/21 08:52:34]

Done.

+    if (heapIndex == VectorHeapIndex) \
+        ANNOTATE_DELETE_BUFFER(payload, payloadSize, 0)
+#else
+#define ASAN_RETIRE_CONTAINER_ANNOTATION(heapIndex, payload, payloadSize)
+#endif
+
 namespace blink {
 
 #if ENABLE(GC_PROFILING)
@@ -889,6 +901,7 @@ void NormalPageHeap::promptlyFreeObject(HeapObjectHeader* header)
 
     {
         ThreadState::SweepForbiddenScope forbiddenScope(threadState());
+        ASAN_RETIRE_CONTAINER_ANNOTATION(heapIndex(), payload, payloadSize);
         header->finalize(payload, payloadSize);
         if (address + size == m_currentAllocationPoint) {
             m_currentAllocationPoint = address;
@@ -1019,8 +1032,17 @@ Address NormalPageHeap::outOfLineAllocate(size_t allocationSize, size_t gcInfoIn
 #endif
 
     // 1. If this allocation is big enough, allocate a large object.
-    if (allocationSize >= largeObjectSizeThreshold)
-        return static_cast<LargeObjectHeap*>(threadState()->heap(LargeObjectHeapIndex))->allocateLargeObjectPage(allocationSize, gcInfoIndex);
+    if (allocationSize >= largeObjectSizeThreshold) {
+        Address largeObject = static_cast<LargeObjectHeap*>(threadState()->heap(LargeObjectHeapIndex))->allocateLargeObjectPage(allocationSize, gcInfoIndex);
+#if ENABLE(ASAN_CONTAINER_ANNOTATIONS)
+        if (heapIndex() == VectorHeapIndex) {

[haraken, 2015/02/20 20:57:56]

Can we move this logic to doAllocateLargeObject, which actually does a
LargeObject allocation?

[sof, 2015/02/20 21:06:43]

Don't see how without passing in the heapIndex as an argument, which seems a bit
artificial?

[haraken, 2015/02/20 21:11:09]

You can get heapIndex from payload. payload => page => heapIndex.

(Given that this is only for ASAN code and thus not performance-sensitive, I'd
prefer encapsulating the complexity into the macro.)

[sof, 2015/02/20 21:45:17]

I don't get it; payload of what? LargeObjectHeap has a generic&unrelated heap
index; this has to be the heap index of the origining NormalPageHeap.

[haraken, 2015/02/21 06:56:35]

As commented above, I think you can get a right heapIndex by:

BasePage* page = pageFromObject(payload);
if (page->heapIndex() == VectorHeapIndex || (page->heapIndex() ==
LargeObjectHeapIndex &&
static_cast<LargeObjectPage>(page)->isVectorBackingPage())) {
    ANNOTATE_DELETE_BUFFER();
}

[sof, 2015/02/21 08:52:34]

Addressed in the tidiest manner I could think of.

+            BasePage* largePage = pageFromObject(largeObject);
+            ASSERT(largePage->isLargeObjectPage());
+            static_cast<LargeObjectPage*>(largePage)->setIsVectorBackingPage();
+        }
+#endif
+        return largeObject;
+    }
 
     // 2. Check if we should trigger a GC.
     updateRemainingAllocationSize();
@@ -1156,6 +1178,10 @@ Address LargeObjectHeap::doAllocateLargeObjectPage(size_t allocationSize, size_t
 
 void LargeObjectHeap::freeLargeObjectPage(LargeObjectPage* object)
 {
+#if ENABLE(ASAN_CONTAINER_ANNOTATIONS)
+    if (object->isVectorBackingPage())
+        ASAN_RETIRE_CONTAINER_ANNOTATION(VectorHeapIndex, object->payload(), object->payloadSize());
+#endif
     object->heapObjectHeader()->finalize(object->payload(), object->payloadSize());
     Heap::decreaseAllocatedSpace(object->size());
 
@@ -1515,6 +1541,7 @@ void NormalPage::sweep()
             // finalizer to operate on the object, but not have other finalizers
             // be allowed to access it.
             ASAN_UNPOISON_MEMORY_REGION(payload, payloadSize);
+            ASAN_RETIRE_CONTAINER_ANNOTATION(heap()->heapIndex(), payload, payloadSize);
             header->finalize(payload, payloadSize);
             // This memory will be added to the freelist. Maintain the invariant
             // that memory on the freelist is zero filled.
@@ -1774,6 +1801,9 @@ NormalPageHeap* NormalPage::heapForNormalPage()
 LargeObjectPage::LargeObjectPage(PageMemory* storage, BaseHeap* heap, size_t payloadSize)
     : BasePage(storage, heap)
     , m_payloadSize(payloadSize)
+#if ENABLE(ASAN_CONTAINER_ANNOTATIONS)
+    , m_isVectorBackingPage(false)
+#endif
 {
 }