Bug 1332594 - Check for poison and other corruption during realloc. r=jandem
☠☠ backed out by af973c7f5361 ☠ ☠
authorEmanuel Hoogeveen <emanuel.hoogeveen@gmail.com>
Fri, 20 Jan 2017 08:05:00 -0500
changeset 377789 209d492c3ec50bdfd4397f7d646e8fb2991386ed
parent 377788 31511bed6415b051384c0f6825f5b6514f01e2f6
child 377790 2301f25d15957f24d36b68103b1b4e9786344392
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1332594
milestone53.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 1332594 - Check for poison and other corruption during realloc. r=jandem
js/src/ds/PageProtectingVector.h
js/src/jit/x86-shared/AssemblerBuffer-x86-shared.h
--- a/js/src/ds/PageProtectingVector.h
+++ b/js/src/ds/PageProtectingVector.h
@@ -3,16 +3,17 @@
  * 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/. */
 
 #ifndef ds_PageProtectingVector_h
 #define ds_PageProtectingVector_h
 
 #include "mozilla/Atomics.h"
+#include "mozilla/PodOperations.h"
 #include "mozilla/Vector.h"
 
 #include "ds/MemoryProtectionExceptionHandler.h"
 #include "gc/Memory.h"
 #include "js/Utility.h"
 
 namespace js {
 
@@ -534,59 +535,55 @@ template<typename T, size_t N, class A, 
 MOZ_NEVER_INLINE void
 PageProtectingVector<T, N, A, P, Q, G, D, I, X>::lockSlow() const
 {
     MOZ_CRASH("Cannot access PageProtectingVector from more than one thread at a time!");
 }
 
 class ProtectedReallocPolicy
 {
-    /* We hardcode the page size here to minimize administrative overhead. */
-    static const size_t pageShift = 12;
-    static const size_t pageSize = 1 << pageShift;
-    static const size_t pageMask = pageSize - 1;
+    static const uint8_t PoisonPattern = 0xe5;
 
   public:
     template <typename T> T* maybe_pod_malloc(size_t numElems) {
         return js_pod_malloc<T>(numElems);
     }
     template <typename T> T* maybe_pod_calloc(size_t numElems) {
         return js_pod_calloc<T>(numElems);
     }
     template <typename T> T* maybe_pod_realloc(T* oldAddr, size_t oldSize, size_t newSize) {
         MOZ_ASSERT_IF(oldAddr, oldSize);
-        MOZ_ASSERT(gc::SystemPageSize() == pageSize);
         if (MOZ_UNLIKELY(!newSize))
             return nullptr;
         if (MOZ_UNLIKELY(!oldAddr))
             return js_pod_malloc<T>(newSize);
 
-        T* newAddr = nullptr;
-        size_t initPage = (uintptr_t(oldAddr - 1) >> pageShift) + 1;
-        size_t lastPage = (uintptr_t(oldAddr + oldSize) >> pageShift) - 1;
-        size_t toCopy = (newSize >= oldSize ? oldSize : newSize) * sizeof(T);
-        if (MOZ_UNLIKELY(oldSize >= 32 * 1024 && lastPage >= initPage)) {
-            T* protectAddr = reinterpret_cast<T*>(initPage << pageShift);
-            size_t protectSize = (lastPage - initPage + 1) << pageShift;
-            MemoryProtectionExceptionHandler::addRegion(protectAddr, protectSize);
-            gc::MakePagesReadOnly(protectAddr, protectSize);
-            newAddr = js_pod_malloc<T>(newSize);
-            if (MOZ_LIKELY(newAddr))
-                memcpy(newAddr, oldAddr, toCopy);
-            gc::UnprotectPages(protectAddr, protectSize);
-            MemoryProtectionExceptionHandler::removeRegion(protectAddr);
-            if (MOZ_LIKELY(newAddr))
-                js_free(oldAddr);
-        } else {
-            newAddr = js_pod_malloc<T>(newSize);
-            if (MOZ_LIKELY(newAddr)) {
-                memcpy(newAddr, oldAddr, toCopy);
-                js_free(oldAddr);
-            }
+        T* tmpAddr = js_pod_malloc<T>(newSize);
+        if (MOZ_UNLIKELY(!tmpAddr))
+            return js_pod_realloc<T>(oldAddr, oldSize, newSize);
+
+        size_t bytes = (newSize >= oldSize ? oldSize : newSize) * sizeof(T);
+        memcpy(tmpAddr, oldAddr, bytes);
+
+        T* newAddr = js_pod_realloc<T>(oldAddr, oldSize, newSize);
+        if (MOZ_UNLIKELY(!newAddr)) {
+            js_free(tmpAddr);
+            return js_pod_realloc<T>(oldAddr, oldSize, newSize);
         }
+
+        const uint8_t* newAddrBytes = reinterpret_cast<const uint8_t*>(newAddr);
+        const uint8_t* tmpAddrBytes = reinterpret_cast<const uint8_t*>(tmpAddr);
+        if (!mozilla::PodEqual(tmpAddrBytes, newAddrBytes, bytes)) {
+            if (oldAddr == newAddr)
+                MOZ_CRASH("New buffer doesn't match the old buffer (newAddr == oldAddr)!");
+            else
+                MOZ_CRASH("New buffer doesn't match the old buffer (newAddr != oldAddr)!");
+        }
+
+        js_free(tmpAddr);
         return newAddr;
     }
 
     template <typename T> T* pod_malloc(size_t numElems) { return maybe_pod_malloc<T>(numElems); }
     template <typename T> T* pod_calloc(size_t numElems) { return maybe_pod_calloc<T>(numElems); }
     template <typename T> T* pod_realloc(T* p, size_t oldSize, size_t newSize) {
         return maybe_pod_realloc<T>(p, oldSize, newSize);
     }
--- a/js/src/jit/x86-shared/AssemblerBuffer-x86-shared.h
+++ b/js/src/jit/x86-shared/AssemblerBuffer-x86-shared.h
@@ -121,54 +121,29 @@ namespace jit {
             return m_buffer.length();
         }
 
         bool oom() const
         {
             return m_oom;
         }
 
-#ifndef RELEASE_OR_BETA
-        const unsigned char* acquireBuffer() const
-        {
-            MOZ_RELEASE_ASSERT(!m_oom);
-            return m_buffer.acquire();
-        }
-        void releaseBuffer() const { m_buffer.release(); }
-        unsigned char* acquireData() { return m_buffer.acquire(); }
-        void releaseData() const { m_buffer.release(); }
-        void disableProtection() { m_buffer.disableProtection(); }
-        void enableProtection() { m_buffer.enableProtection(); }
-        void setLowerBoundForProtection(size_t size)
-        {
-            m_buffer.setLowerBoundForProtection(size);
-        }
-        void unprotectRegion(unsigned char* first, size_t size)
-        {
-            m_buffer.unprotectRegion(first, size);
-        }
-        void reprotectRegion(unsigned char* first, size_t size)
-        {
-            m_buffer.reprotectRegion(first, size);
-        }
-#else
         const unsigned char* acquireBuffer() const
         {
             MOZ_RELEASE_ASSERT(!m_oom);
             return m_buffer.begin();
         }
         void releaseBuffer() const {}
         unsigned char* acquireData() { return m_buffer.begin(); }
         void releaseData() const {}
         void disableProtection() {}
         void enableProtection() {}
         void setLowerBoundForProtection(size_t) {}
         void unprotectRegion(unsigned char*, size_t) {}
         void reprotectRegion(unsigned char*, size_t) {}
-#endif
 
     protected:
         /*
          * OOM handling: This class can OOM in the ensureSpace() method trying
          * to allocate a new buffer. In response to an OOM, we need to avoid
          * crashing and report the error. We also want to make it so that
          * users of this class need to check for OOM only at certain points
          * and not after every operation.
@@ -181,20 +156,17 @@ namespace jit {
          * See also the |buffer| method.
          */
         void oomDetected() {
             m_oom = true;
             m_buffer.clear();
         }
 
 #ifndef RELEASE_OR_BETA
-        PageProtectingVector<unsigned char, 256, ProtectedReallocPolicy,
-                             /* ProtectUsed = */ false, /* ProtectUnused = */ false,
-                             /* GuardAgainstReentrancy = */ true, /* DetectPoison = */ true,
-                             /* InitialLowerBound = */ 32 * 1024> m_buffer;
+        mozilla::Vector<unsigned char, 256, ProtectedReallocPolicy> m_buffer;
 #else
         mozilla::Vector<unsigned char, 256, SystemAllocPolicy> m_buffer;
 #endif
         bool m_oom;
     };
 
     class GenericAssembler
     {