bmalloc IsoHeap needs to allow a thread to deallocate some size for the first time
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Dec 2017 22:50:47 +0000 (22:50 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Dec 2017 22:50:47 +0000 (22:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180443

Reviewed by Saam Barati.
Source/bmalloc:

It's true that we can expect a heap to already be initialized if we try to deallocate in it.  But it
may not have its deallocator initialized on this thread yet.

This is easily fixed by adding a null check on the deallocate path. That's probably not going to
change perf at all. But doing that allows me to get rid of a lot of weird stuff I previously did to
avoid that null check, like creating a dummy TLS in the DebugHeap case.

* bmalloc/IsoTLS.cpp:
(bmalloc::IsoTLS::debugFree):
(bmalloc::IsoTLS::deallocateSlow): Deleted.
* bmalloc/IsoTLS.h:
* bmalloc/IsoTLSInlines.h:
(bmalloc::IsoTLS::allocateImpl):
(bmalloc::IsoTLS::allocateSlow):
(bmalloc::IsoTLS::deallocateImpl):
(bmalloc::IsoTLS::deallocateSlow):
(bmalloc::IsoTLS::ensureHeapAndEntries):

Source/WTF:

With this change it's possible to reenable isoheaps on iOS.

* wtf/IsoMalloc.h:
* wtf/IsoMallocInlines.h:

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

Source/WTF/ChangeLog
Source/WTF/wtf/IsoMalloc.h
Source/WTF/wtf/IsoMallocInlines.h
Source/bmalloc/ChangeLog
Source/bmalloc/bmalloc/IsoTLS.cpp
Source/bmalloc/bmalloc/IsoTLS.h
Source/bmalloc/bmalloc/IsoTLSInlines.h

index 59760ce..941c948 100644 (file)
@@ -1,3 +1,15 @@
+2017-12-05  Filip Pizlo  <fpizlo@apple.com>
+
+        bmalloc IsoHeap needs to allow a thread to deallocate some size for the first time
+        https://bugs.webkit.org/show_bug.cgi?id=180443
+
+        Reviewed by Saam Barati.
+        
+        With this change it's possible to reenable isoheaps on iOS.
+
+        * wtf/IsoMalloc.h:
+        * wtf/IsoMallocInlines.h:
+
 2017-12-05  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [WTF] Remove m_atomicStringTableDestructor in Thread by querying the condition at thread destruction
index d11df2a..38e3d83 100644 (file)
@@ -25,7 +25,7 @@
 
 #pragma once
 
-#if (defined(USE_SYSTEM_MALLOC) && USE_SYSTEM_MALLOC) || PLATFORM(IOS)
+#if (defined(USE_SYSTEM_MALLOC) && USE_SYSTEM_MALLOC)
 
 #include <wtf/FastMalloc.h>
 
index de35ee4..1a8200b 100644 (file)
@@ -25,7 +25,7 @@
 
 #pragma once
 
-#if (defined(USE_SYSTEM_MALLOC) && USE_SYSTEM_MALLOC) || PLATFORM(IOS)
+#if (defined(USE_SYSTEM_MALLOC) && USE_SYSTEM_MALLOC)
 
 #include <wtf/FastMalloc.h>
 
index dff2ab9..f52b3de 100644 (file)
@@ -1,3 +1,28 @@
+2017-12-05  Filip Pizlo  <fpizlo@apple.com>
+
+        bmalloc IsoHeap needs to allow a thread to deallocate some size for the first time
+        https://bugs.webkit.org/show_bug.cgi?id=180443
+
+        Reviewed by Saam Barati.
+
+        It's true that we can expect a heap to already be initialized if we try to deallocate in it.  But it
+        may not have its deallocator initialized on this thread yet.
+        
+        This is easily fixed by adding a null check on the deallocate path. That's probably not going to
+        change perf at all. But doing that allows me to get rid of a lot of weird stuff I previously did to
+        avoid that null check, like creating a dummy TLS in the DebugHeap case.
+
+        * bmalloc/IsoTLS.cpp:
+        (bmalloc::IsoTLS::debugFree):
+        (bmalloc::IsoTLS::deallocateSlow): Deleted.
+        * bmalloc/IsoTLS.h:
+        * bmalloc/IsoTLSInlines.h:
+        (bmalloc::IsoTLS::allocateImpl):
+        (bmalloc::IsoTLS::allocateSlow):
+        (bmalloc::IsoTLS::deallocateImpl):
+        (bmalloc::IsoTLS::deallocateSlow):
+        (bmalloc::IsoTLS::ensureHeapAndEntries):
+
 2017-12-01  Michael Saboff  <msaboff@apple.com>
 
         Gigacage should not be enabled for ARM64_32
index ec3fcab..2259a8c 100644 (file)
@@ -52,15 +52,6 @@ IsoTLS::IsoTLS()
 {
 }
 
-void IsoTLS::deallocateSlow(void* p)
-{
-    // If we go down this path and we aren't in debug heap mode, then this means we have some corruption.
-    // Think of this as really being an assertion about offset < tls->m_extent.
-    RELEASE_BASSERT(PerProcess<Environment>::get()->isDebugHeapEnabled());
-    
-    PerProcess<DebugHeap>::get()->free(p);
-}
-
 IsoTLS* IsoTLS::ensureEntries(unsigned offset)
 {
     RELEASE_BASSERT(!get() || offset >= get()->m_extent);
@@ -189,5 +180,14 @@ auto IsoTLS::debugMalloc(size_t size) -> DebugMallocResult
     return result;
 }
 
+bool IsoTLS::debugFree(void* p)
+{
+    if (isUsingDebugHeap()) {
+        PerProcess<DebugHeap>::get()->free(p);
+        return true;
+    }
+    return false;
+}
+
 } // namespace bmalloc
 
index 99a4501..788087c 100644 (file)
@@ -72,7 +72,8 @@ private:
     template<typename Config>
     void deallocateFast(unsigned offset, void* p);
     
-    BEXPORT static void deallocateSlow(void* p);
+    template<typename Config, typename Type>
+    static void deallocateSlow(api::IsoHeap<Type>&, void* p);
     
     static IsoTLS* get();
     static void set(IsoTLS*);
@@ -92,7 +93,7 @@ private:
     template<typename Func>
     void forEachEntry(const Func&);
     
-    BEXPORT static bool isUsingDebugHeap();
+    static bool isUsingDebugHeap();
     
     struct DebugMallocResult {
         void* ptr { nullptr };
@@ -100,6 +101,7 @@ private:
     };
     
     BEXPORT static DebugMallocResult debugMalloc(size_t);
+    BEXPORT static bool debugFree(void*);
     
     IsoTLSEntry* m_lastEntry { nullptr };
     unsigned m_extent { 0 };
index 6dbcb61..39d81f0 100644 (file)
@@ -67,7 +67,7 @@ void* IsoTLS::allocateImpl(api::IsoHeap<Type>& handle, bool abortOnFailure)
     unsigned offset = handle.allocatorOffset();
     IsoTLS* tls = get();
     if (!tls || offset >= tls->m_extent)
-        return allocateSlow<typename api::IsoHeap<Type>::Config>(handle, abortOnFailure);
+        return allocateSlow<Config>(handle, abortOnFailure);
     return tls->allocateFast<Config>(offset, abortOnFailure);
 }
 
@@ -80,14 +80,13 @@ void* IsoTLS::allocateFast(unsigned offset, bool abortOnFailure)
 template<typename Config, typename Type>
 BNO_INLINE void* IsoTLS::allocateSlow(api::IsoHeap<Type>& handle, bool abortOnFailure)
 {
-    IsoTLS* tls = ensureHeapAndEntries(handle);
-    
     auto debugMallocResult = debugMalloc(Config::objectSize);
     if (debugMallocResult.usingDebugHeap)
         return debugMallocResult.ptr;
     
-    unsigned offset = handle.allocatorOffset();
-    return tls->allocateFast<Config>(offset, abortOnFailure);
+    IsoTLS* tls = ensureHeapAndEntries(handle);
+    
+    return tls->allocateFast<Config>(handle.allocatorOffset(), abortOnFailure);
 }
 
 template<typename Config, typename Type>
@@ -97,8 +96,8 @@ void IsoTLS::deallocateImpl(api::IsoHeap<Type>& handle, void* p)
     IsoTLS* tls = get();
     // Note that this bounds check would be here even if we didn't have to support DebugHeap,
     // since we don't want unpredictable behavior if offset or m_extent ever got corrupted.
-    if (offset >= tls->m_extent)
-        deallocateSlow(p);
+    if (!tls || offset >= tls->m_extent)
+        deallocateSlow<Config>(handle, p);
     else
         tls->deallocateFast<Config>(offset, p);
 }
@@ -109,6 +108,19 @@ void IsoTLS::deallocateFast(unsigned offset, void* p)
     reinterpret_cast<IsoDeallocator<Config>*>(m_data + offset)->deallocate(p);
 }
 
+template<typename Config, typename Type>
+BNO_INLINE void IsoTLS::deallocateSlow(api::IsoHeap<Type>& handle, void* p)
+{
+    if (debugFree(p))
+        return;
+    
+    RELEASE_BASSERT(handle.isInitialized());
+    
+    IsoTLS* tls = ensureEntries(std::max(handle.allocatorOffset(), handle.deallocatorOffset()));
+    
+    tls->deallocateFast<Config>(handle.deallocatorOffset(), p);
+}
+
 inline IsoTLS* IsoTLS::get()
 {
 #if HAVE_PTHREAD_MACHDEP_H
@@ -151,16 +163,8 @@ BNO_INLINE IsoTLS* IsoTLS::ensureHeapAndEntries(api::IsoHeap<Type>& handle)
         !get()
         || handle.allocatorOffset() >= get()->m_extent
         || handle.deallocatorOffset() >= get()->m_extent);
-    unsigned offset;
-    if (isUsingDebugHeap()) {
-        if (IsoTLS* result = get())
-            return result;
-        offset = 0;
-    } else {
-        ensureHeap(handle);
-        offset = std::max(handle.allocatorOffset(), handle.deallocatorOffset());
-    }
-    return ensureEntries(offset);
+    ensureHeap(handle);
+    return ensureEntries(std::max(handle.allocatorOffset(), handle.deallocatorOffset()));
 }
 
 } // namespace bmalloc