Changeset 196186 in webkit

Timestamp:

Feb 5, 2016, 1:34:27 PM (10 years ago)

Author:

sbarati@apple.com

Message:

bmalloc: largeMax calculation is wrong on iOS
​https://bugs.webkit.org/show_bug.cgi?id=153923

Reviewed by Mark Lam.

Our number for largeMax was larger than what we had
space to actually allocate inside the LargeChunk. This made
it so that we would allocate a large object for something
that really should be extra large. Previously:
largeMax + sizeof(LargeChunk) > 1MB
which meant that when we would grow() to accommodate an allocation
of a particular size inside a LargeObject despite the fact that
the allocation size would be too large to actually fit in the LargeObject.
This would manifest when we had an allocation size in the range:
1MB - sizeof(LargeChunk) < allocation size < largeMax

We fix this bug by being precise in our calculation of largeMax
instead of just assuming largeChunkSize * 99/100 is enough
space for the metadata.

• bmalloc/LargeChunk.h:

(bmalloc::LargeChunk::get):

• bmalloc/Sizes.h:

Location:

trunk/Source/bmalloc

Files:

• ChangeLog (modified) (1 diff)
• bmalloc/LargeChunk.h (modified) (1 diff)
• bmalloc/Sizes.h (modified) (1 diff)
• bmalloc/VMHeap.cpp (modified) (1 diff)

trunk/Source/bmalloc/ChangeLog

@@ +1 @@
+2016-02-05  Saam barati  <sbarati@apple.com>
+
+        bmalloc: largeMax calculation is wrong on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=153923
+
+        Reviewed by Mark Lam.
+
+        Our number for largeMax was larger than what we had
+        space to actually allocate inside the LargeChunk. This made 
+        it so that we would allocate a large object for something
+        that really should be extra large. Previously:
+        largeMax + sizeof(LargeChunk) > 1MB
+        which meant that when we would grow() to accommodate an allocation
+        of a particular size inside a LargeObject despite the fact that
+        the allocation size would be too large to actually fit in the LargeObject.
+        This would manifest when we had an allocation size in the range:
+        1MB - sizeof(LargeChunk) < allocation size < largeMax
+
+        We fix this bug by being precise in our calculation of largeMax
+        instead of just assuming largeChunkSize * 99/100 is enough
+        space for the metadata.
+
+        * bmalloc/LargeChunk.h:
+        (bmalloc::LargeChunk::get):
+        * bmalloc/Sizes.h:
+
 2016-01-31  Dan Bernstein  <mitz@apple.com>
 

trunk/Source/bmalloc/bmalloc/LargeChunk.h

@@ -79 +79 @@
 };
 
+static_assert(largeChunkMetadataSize == sizeof(LargeChunk), "'largeChunkMetadataSize' should be the same number as sizeof(LargeChunk) or our computation in Sizes.h for 'largeMax' is wrong");
+static_assert(largeChunkMetadataSize + largeMax <= largeChunkSize, "We will think we can accommodate larger objects than we can in reality");
+
 inline LargeChunk* LargeChunk::get(void* object)
 {

trunk/Source/bmalloc/bmalloc/Sizes.h

@@ -73 +73 @@
 
     static const size_t largeChunkSize = superChunkSize / 2;
+#if BPLATFORM(IOS)
+    static const size_t largeChunkMetadataSize = 16 * kB;
+#else
+    static const size_t largeChunkMetadataSize = 4 * kB;
+#endif
     static const size_t largeChunkOffset = 0;
     static const size_t largeChunkMask = ~(largeChunkSize - 1ul);
 
     static const size_t largeAlignment = 64;
-    static const size_t largeMax = largeChunkSize * 99 / 100; // Plenty of room for metadata.
+    static const size_t largeMax = largeChunkSize - largeChunkMetadataSize;
     static const size_t largeMin = mediumMax;
     

trunk/Source/bmalloc/bmalloc/VMHeap.cpp

@@ -54 +54 @@
 
     LargeChunk* largeChunk = superChunk->largeChunk();
-    m_largeObjects.insert(LargeObject(LargeObject::init(largeChunk).begin()));
+    LargeObject result(LargeObject::init(largeChunk).begin());
+    BASSERT(result.size() == largeMax);
+    m_largeObjects.insert(result);
 }