Assertion failure in bmalloc::vmRevokePermissions(void*, unsigned long).
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 May 2016 19:43:56 +0000 (19:43 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 May 2016 19:43:56 +0000 (19:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157047

Reviewed by Filip Pizlo.

The previous fix aligned the guard page sizes correctly but forgot to
align the guard page start address correctly.

* bmalloc/Algorithm.h:
(bmalloc::roundUpToMultipleOfSloppy): Use a new helper method to round
up when not working with a power of two, instead of writing out the
math by hand.

* bmalloc/VMHeap.cpp:
(bmalloc::VMHeap::allocateSmallChunk): Make sure to round up the guard
page start address in addition to its size. Assert at the very end to
try to catch more bugs.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@200385 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/bmalloc/ChangeLog
Source/bmalloc/bmalloc/Algorithm.h
Source/bmalloc/bmalloc/VMHeap.cpp

index c7a2b31..4c9c972 100644 (file)
@@ -1,3 +1,23 @@
+2016-05-03  Geoffrey Garen  <ggaren@apple.com>
+
+        Assertion failure in bmalloc::vmRevokePermissions(void*, unsigned long).
+        https://bugs.webkit.org/show_bug.cgi?id=157047
+
+        Reviewed by Filip Pizlo.
+
+        The previous fix aligned the guard page sizes correctly but forgot to
+        align the guard page start address correctly.
+
+        * bmalloc/Algorithm.h:
+        (bmalloc::roundUpToMultipleOfSloppy): Use a new helper method to round
+        up when not working with a power of two, instead of writing out the
+        math by hand.
+
+        * bmalloc/VMHeap.cpp:
+        (bmalloc::VMHeap::allocateSmallChunk): Make sure to round up the guard
+        page start address in addition to its size. Assert at the very end to
+        try to catch more bugs.
+
 2016-04-27  Geoffrey Garen  <ggaren@apple.com>
 
         Assertion failure in bmalloc::vmRevokePermissions(void*, unsigned long).
index c23b94a..8d4e2a7 100644 (file)
@@ -101,6 +101,11 @@ template<typename T> inline T divideRoundingUp(T numerator, T denominator)
     return (numerator + denominator - 1) / denominator;
 }
 
+template<typename T> inline T roundUpToMultipleOfSloppy(size_t divisor, T x)
+{
+    return divideRoundingUp(x, divisor) * divisor;
+}
+
 // Version of sizeof that returns 0 for empty classes.
 
 template<typename T> inline constexpr size_t sizeOf()
index c873cab..9975c90 100644 (file)
@@ -61,25 +61,26 @@ void VMHeap::allocateSmallChunk(std::lock_guard<StaticMutex>& lock, size_t pageC
     size_t pageSize = bmalloc::pageSize(pageClass);
     size_t smallPageCount = pageSize / smallPageSize;
 
-    // We align to our page size in order to guarantee that we can service
-    // aligned allocation requests at equal and smaller powers of two.
-    size_t metadataSize = divideRoundingUp(sizeof(Chunk), pageSize) * pageSize;
-
     void* memory = vmAllocate(chunkSize, chunkSize);
     Chunk* chunk = static_cast<Chunk*>(memory);
 
+    // We align to our page size in order to honor OS APIs and in order to
+    // guarantee that we can service aligned allocation requests at equal
+    // and smaller powers of two.
+    size_t vmPageSize = roundUpToMultipleOf(bmalloc::vmPageSize(), pageSize);
+    size_t metadataSize = roundUpToMultipleOfSloppy(vmPageSize, sizeof(Chunk));
+
     Object begin(chunk, metadataSize);
     Object end(chunk, chunkSize);
 
     // Establish guard pages before writing to Chunk memory to work around
     // an edge case in the Darwin VM system (<rdar://problem/25910098>).
-    size_t guardSize = roundUpToMultipleOf(vmPageSize(), pageSize);
-    BASSERT(chunkSize >= 2 * guardSize + pageSize);
-    vmRevokePermissions(begin.address(), guardSize);
-    vmRevokePermissions(end.address() - guardSize, guardSize);
-
-    begin = begin + guardSize;
-    end = end - guardSize;
+    vmRevokePermissions(begin.address(), vmPageSize);
+    vmRevokePermissions(end.address() - vmPageSize, vmPageSize);
+    
+    begin = begin + vmPageSize;
+    end = end - vmPageSize;
+    BASSERT(begin <= end && end.offset() - begin.offset() >= pageSize);
 
     new (chunk) Chunk(lock);