Bug 1529922 - Add guard pages following huge allocations. r=glandium
authorGian-Carlo Pascutto <gcp@mozilla.com>
Wed, 20 Mar 2019 10:45:15 +0000
changeset 465438 becf3a27c2bb39c1250a8d51c4875edeacf46025
parent 465437 579548b422c67343d524825027c6f684b6e81267
child 465439 dc8c94d906a60c6bd5b76d729ac0ccf2006a1a9a
push id35738
push userccoroiu@mozilla.com
push dateThu, 21 Mar 2019 21:59:09 +0000
treeherdermozilla-central@7eb8e627961c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs1529922
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1529922 - Add guard pages following huge allocations. r=glandium Differential Revision: https://phabricator.services.mozilla.com/D23292
memory/build/mozjemalloc.cpp
memory/gtest/TestJemalloc.cpp
--- a/memory/build/mozjemalloc.cpp
+++ b/memory/build/mozjemalloc.cpp
@@ -1265,16 +1265,18 @@ static inline void ApplyZeroOrJunk(void*
 static inline void pages_decommit(void* aAddr, size_t aSize) {
 #ifdef XP_WIN
   // The region starting at addr may have been allocated in multiple calls
   // to VirtualAlloc and recycled, so decommitting the entire region in one
   // go may not be valid. However, since we allocate at least a chunk at a
   // time, we may touch any region in chunksized increments.
   size_t pages_size = std::min(aSize, kChunkSize - GetChunkOffsetForPtr(aAddr));
   while (aSize > 0) {
+    // This will cause Access Violation on read and write and thus act as a
+    // guard page or region as well.
     if (!VirtualFree(aAddr, pages_size, MEM_DECOMMIT)) {
       MOZ_CRASH();
     }
     aAddr = (void*)((uintptr_t)aAddr + pages_size);
     aSize -= pages_size;
     pages_size = std::min(aSize, kChunkSize);
   }
 #else
@@ -1723,52 +1725,18 @@ static void* chunk_alloc_mmap(size_t siz
 
 // Purge and release the pages in the chunk of length `length` at `addr` to
 // the OS.
 // Returns whether the pages are guaranteed to be full of zeroes when the
 // function returns.
 // The force_zero argument explicitly requests that the memory is guaranteed
 // to be full of zeroes when the function returns.
 static bool pages_purge(void* addr, size_t length, bool force_zero) {
-#ifdef MALLOC_DECOMMIT
   pages_decommit(addr, length);
   return true;
-#else
-#  ifndef XP_LINUX
-  if (force_zero) {
-    memset(addr, 0, length);
-  }
-#  endif
-#  ifdef XP_WIN
-  // The region starting at addr may have been allocated in multiple calls
-  // to VirtualAlloc and recycled, so resetting the entire region in one
-  // go may not be valid. However, since we allocate at least a chunk at a
-  // time, we may touch any region in chunksized increments.
-  size_t pages_size = std::min(length, kChunkSize - GetChunkOffsetForPtr(addr));
-  while (length > 0) {
-    VirtualAlloc(addr, pages_size, MEM_RESET, PAGE_READWRITE);
-    addr = (void*)((uintptr_t)addr + pages_size);
-    length -= pages_size;
-    pages_size = std::min(length, kChunkSize);
-  }
-  return force_zero;
-#  else
-#    ifdef XP_LINUX
-#      define JEMALLOC_MADV_PURGE MADV_DONTNEED
-#      define JEMALLOC_MADV_ZEROS true
-#    else  // FreeBSD and Darwin.
-#      define JEMALLOC_MADV_PURGE MADV_FREE
-#      define JEMALLOC_MADV_ZEROS force_zero
-#    endif
-  int err = madvise(addr, length, JEMALLOC_MADV_PURGE);
-  return JEMALLOC_MADV_ZEROS && err == 0;
-#    undef JEMALLOC_MADV_PURGE
-#    undef JEMALLOC_MADV_ZEROS
-#  endif
-#endif
 }
 
 static void* chunk_recycle(size_t aSize, size_t aAlignment, bool* aZeroed) {
   extent_node_t key;
 
   size_t alloc_size = aSize + aAlignment - kChunkSize;
   // Beware size_t wrap-around.
   if (alloc_size < aSize) {
@@ -1827,25 +1795,24 @@ static void* chunk_recycle(size_t aSize,
 
   gRecycledSize -= aSize;
 
   chunks_mtx.Unlock();
 
   if (node) {
     base_node_dealloc(node);
   }
-#ifdef MALLOC_DECOMMIT
   if (!pages_commit(ret, aSize)) {
     return nullptr;
   }
   // pages_commit is guaranteed to zero the chunk.
   if (aZeroed) {
     *aZeroed = true;
   }
-#endif
+
   return ret;
 }
 
 #ifdef XP_WIN
 // On Windows, calls to VirtualAlloc and VirtualFree must be matched, making it
 // awkward to recycle allocations of varying sizes. Therefore we only allow
 // recycling when the size equals the chunksize, unless deallocation is entirely
 // disabled.
@@ -3539,41 +3506,46 @@ void* arena_t::MallocHuge(size_t aSize, 
 
 void* arena_t::PallocHuge(size_t aSize, size_t aAlignment, bool aZero) {
   void* ret;
   size_t csize;
   size_t psize;
   extent_node_t* node;
   bool zeroed;
 
-  // Allocate one or more contiguous chunks for this request.
-  csize = CHUNK_CEILING(aSize);
-  if (csize == 0) {
+  // We're going to configure guard pages in the region between the
+  // page-aligned size and the chunk-aligned size, so if those are the same
+  // then we need to force that region into existence.
+  csize = CHUNK_CEILING(aSize + gPageSize);
+  if (csize < aSize) {
     // size is large enough to cause size_t wrap-around.
     return nullptr;
   }
 
   // Allocate an extent node with which to track the chunk.
   node = base_node_alloc();
   if (!node) {
     return nullptr;
   }
 
+  // Allocate one or more contiguous chunks for this request.
   ret = chunk_alloc(csize, aAlignment, false, &zeroed);
   if (!ret) {
     base_node_dealloc(node);
     return nullptr;
   }
+  psize = PAGE_CEILING(aSize);
   if (aZero) {
-    chunk_ensure_zero(ret, csize, zeroed);
+    // We will decommit anything past psize so there is no need to zero
+    // further.
+    chunk_ensure_zero(ret, psize, zeroed);
   }
 
   // Insert node into huge.
   node->mAddr = ret;
-  psize = PAGE_CEILING(aSize);
   node->mSize = psize;
   node->mArena = this;
 
   {
     MutexAutoLock lock(huge_mtx);
     huge.Insert(node);
 
     // Although we allocated space for csize bytes, we indicate that we've
@@ -3593,45 +3565,36 @@ void* arena_t::PallocHuge(size_t aSize, 
     // psize above, malloc_usable_size will return psize, not csize, and
     // the program will (hopefully) never touch bytes in excess of psize.
     // Thus those bytes won't take up space in physical memory, and we can
     // reasonably claim we never "allocated" them in the first place.
     huge_allocated += psize;
     huge_mapped += csize;
   }
 
-#ifdef MALLOC_DECOMMIT
-  if (csize - psize > 0) {
-    pages_decommit((void*)((uintptr_t)ret + psize), csize - psize);
-  }
-#endif
+  pages_decommit((void*)((uintptr_t)ret + psize), csize - psize);
 
   if (!aZero) {
-#ifdef MALLOC_DECOMMIT
     ApplyZeroOrJunk(ret, psize);
-#else
-    ApplyZeroOrJunk(ret, csize);
-#endif
   }
 
   return ret;
 }
 
 void* arena_t::RallocHuge(void* aPtr, size_t aSize, size_t aOldSize) {
   void* ret;
   size_t copysize;
 
   // Avoid moving the allocation if the size class would not change.
   if (aOldSize > gMaxLargeClass &&
-      CHUNK_CEILING(aSize) == CHUNK_CEILING(aOldSize)) {
+      CHUNK_CEILING(aSize + gPageSize) == CHUNK_CEILING(aOldSize + gPageSize)) {
     size_t psize = PAGE_CEILING(aSize);
     if (aSize < aOldSize) {
       memset((void*)((uintptr_t)aPtr + aSize), kAllocPoison, aOldSize - aSize);
     }
-#ifdef MALLOC_DECOMMIT
     if (psize < aOldSize) {
       extent_node_t key;
 
       pages_decommit((void*)((uintptr_t)aPtr + psize), aOldSize - psize);
 
       // Update recorded size.
       MutexAutoLock lock(huge_mtx);
       key.mAddr = const_cast<void*>(aPtr);
@@ -3642,26 +3605,20 @@ void* arena_t::RallocHuge(void* aPtr, si
       huge_allocated -= aOldSize - psize;
       // No need to change huge_mapped, because we didn't (un)map anything.
       node->mSize = psize;
     } else if (psize > aOldSize) {
       if (!pages_commit((void*)((uintptr_t)aPtr + aOldSize),
                         psize - aOldSize)) {
         return nullptr;
       }
-    }
-#endif
-
-    // Although we don't have to commit or decommit anything if
-    // DECOMMIT is not defined and the size class didn't change, we
-    // do need to update the recorded size if the size increased,
-    // so malloc_usable_size doesn't return a value smaller than
-    // what was requested via realloc().
-    if (psize > aOldSize) {
-      // Update recorded size.
+
+      // We need to update the recorded size if the size increased,
+      // so malloc_usable_size doesn't return a value smaller than
+      // what was requested via realloc().
       extent_node_t key;
       MutexAutoLock lock(huge_mtx);
       key.mAddr = const_cast<void*>(aPtr);
       extent_node_t* node = huge.Search(&key);
       MOZ_ASSERT(node);
       MOZ_ASSERT(node->mSize == aOldSize);
       MOZ_RELEASE_ASSERT(node->mArena == this);
       huge_allocated += psize - aOldSize;
@@ -3694,34 +3651,36 @@ void* arena_t::RallocHuge(void* aPtr, si
     memcpy(ret, aPtr, copysize);
   }
   idalloc(aPtr, this);
   return ret;
 }
 
 static void huge_dalloc(void* aPtr, arena_t* aArena) {
   extent_node_t* node;
+  size_t mapped = 0;
   {
     extent_node_t key;
     MutexAutoLock lock(huge_mtx);
 
     // Extract from tree of huge allocations.
     key.mAddr = aPtr;
     node = huge.Search(&key);
     MOZ_RELEASE_ASSERT(node, "Double-free?");
     MOZ_ASSERT(node->mAddr == aPtr);
     MOZ_RELEASE_ASSERT(!aArena || node->mArena == aArena);
     huge.Remove(node);
 
+    mapped = CHUNK_CEILING(node->mSize + gPageSize);
     huge_allocated -= node->mSize;
-    huge_mapped -= CHUNK_CEILING(node->mSize);
+    huge_mapped -= mapped;
   }
 
   // Unmap chunk.
-  chunk_dealloc(node->mAddr, CHUNK_CEILING(node->mSize), HUGE_CHUNK);
+  chunk_dealloc(node->mAddr, mapped, HUGE_CHUNK);
 
   base_node_dealloc(node);
 }
 
 static size_t GetKernelPageSize() {
   static size_t kernel_page_size = ([]() {
 #ifdef XP_WIN
     SYSTEM_INFO info;
--- a/memory/gtest/TestJemalloc.cpp
+++ b/memory/gtest/TestJemalloc.cpp
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/mozalloc.h"
 #include "mozilla/UniquePtr.h"
+#include "mozilla/Unused.h"
 #include "mozilla/Vector.h"
 #include "mozmemory.h"
 #include "nsCOMPtr.h"
 #include "nsICrashReporter.h"
 #include "nsServiceManagerUtils.h"
 #include "Utils.h"
 
 #include "gtest/gtest.h"
@@ -356,18 +357,18 @@ class SizeClassesBetween {
 };
 
 #define ALIGNMENT_CEILING(s, alignment) \
   (((s) + (alignment - 1)) & (~(alignment - 1)))
 
 static bool IsSameRoundedHugeClass(size_t aSize1, size_t aSize2,
                                    jemalloc_stats_t& aStats) {
   return (aSize1 > aStats.large_max && aSize2 > aStats.large_max &&
-          ALIGNMENT_CEILING(aSize1, aStats.chunksize) ==
-              ALIGNMENT_CEILING(aSize2, aStats.chunksize));
+          ALIGNMENT_CEILING(aSize1 + aStats.page_size, aStats.chunksize) ==
+              ALIGNMENT_CEILING(aSize2 + aStats.page_size, aStats.chunksize));
 }
 
 static bool CanReallocInPlace(size_t aFromSize, size_t aToSize,
                               jemalloc_stats_t& aStats) {
   if (aFromSize == malloc_good_size(aToSize)) {
     // Same size class: in-place.
     return true;
   }
@@ -415,16 +416,22 @@ TEST(Jemalloc, InPlace) {
 
 // Bug 1474254: disable this test for windows ccov builds because it leads to
 // timeout.
 #if !defined(XP_WIN) || !defined(MOZ_CODE_COVERAGE)
 TEST(Jemalloc, JunkPoison) {
   jemalloc_stats_t stats;
   jemalloc_stats(&stats);
 
+#  ifdef HAS_GDB_SLEEP_DURATION
+  // Avoid death tests adding some unnecessary (long) delays.
+  unsigned int old_gdb_sleep_duration = _gdb_sleep_duration;
+  _gdb_sleep_duration = 0;
+#  endif
+
   // Create buffers in a separate arena, for faster comparisons with
   // bulk_compare.
   arena_id_t buf_arena = moz_create_arena();
   char* junk_buf = (char*)moz_arena_malloc(buf_arena, stats.page_size);
   // Depending on its configuration, the allocator will either fill the
   // requested allocation with the junk byte (0xe4) or with zeroes, or do
   // nothing, in which case, since we're allocating in a fresh arena,
   // we'll be getting zeroes.
@@ -489,40 +496,44 @@ TEST(Jemalloc, JunkPoison) {
     SCOPED_TRACE(testing::Message() << "from_size = " << from_size);
     for (size_t to_size : sSizes) {
       SCOPED_TRACE(testing::Message() << "to_size = " << to_size);
       if (CanReallocInPlace(from_size, to_size, stats)) {
         char* ptr = (char*)moz_arena_malloc(arena, from_size);
         memset(ptr, fill, moz_malloc_usable_size(ptr));
         char* ptr2 = (char*)moz_arena_realloc(arena, ptr, to_size);
         ASSERT_EQ(ptr, ptr2);
+        // Shrinking allocation
         if (from_size >= to_size) {
           ASSERT_NO_FATAL_FAILURE(
               bulk_compare(ptr, 0, to_size, fill_buf, stats.page_size));
-          // On Windows (MALLOC_DECOMMIT), in-place realloc of huge allocations
-          // decommits extra pages, writing to them becomes an error.
-#  ifdef XP_WIN
+          // Huge allocations have guards and will crash when accessing
+          // beyond the valid range.
           if (to_size > stats.large_max) {
             size_t page_limit = ALIGNMENT_CEILING(to_size, stats.page_size);
             ASSERT_NO_FATAL_FAILURE(bulk_compare(ptr, to_size, page_limit,
                                                  poison_buf, stats.page_size));
             ASSERT_DEATH_WRAP(ptr[page_limit] = 0, "");
-          } else
-#  endif
-          {
+          } else {
             ASSERT_NO_FATAL_FAILURE(bulk_compare(ptr, to_size, from_size,
                                                  poison_buf, stats.page_size));
           }
         } else {
+          // Enlarging allocation
           ASSERT_NO_FATAL_FAILURE(
               bulk_compare(ptr, 0, from_size, fill_buf, stats.page_size));
           if (stats.opt_junk || stats.opt_zero) {
             ASSERT_NO_FATAL_FAILURE(bulk_compare(ptr, from_size, to_size,
                                                  junk_buf, stats.page_size));
           }
+          // Huge allocation, so should have a guard page following
+          if (to_size > stats.large_max) {
+            ASSERT_DEATH_WRAP(
+                ptr[ALIGNMENT_CEILING(to_size, stats.page_size)] = 0, "");
+          }
         }
         moz_arena_free(arena, ptr2);
       }
     }
   }
 
   // Growing to a different size class should poison the old allocation,
   // preserve the original bytes, and junk the new bytes in the new allocation.
@@ -595,10 +606,14 @@ TEST(Jemalloc, JunkPoison) {
 
   // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
   // moz_dispose_arena(arena);
 
   moz_arena_free(buf_arena, poison_buf);
   moz_arena_free(buf_arena, junk_buf);
   // Until Bug 1364359 is fixed it is unsafe to call moz_dispose_arena.
   // moz_dispose_arena(buf_arena);
+
+#  ifdef HAS_GDB_SLEEP_DURATION
+  _gdb_sleep_duration = old_gdb_sleep_duration;
+#  endif
 }
 #endif