CheckedPtr should require FastMalloc, Part 1

https://bugs.webkit.org/show_bug.cgi?id=271950 Reviewed by Ryosuke Niwa. This will enable zombie-mode CheckedPtr. It's also just good performance hygiene. * Source/WTF/wtf/CheckedPtr.h: (WTF::CheckedPtr::refIfNotNull): (WTF::CheckedPtr::derefIfNotNull): Use static_assert to verify use of FastMalloc. This is a good spot because it provides a complete type definition for T and it's our lowest level funnel. (Not enabled yet because there are a few complex cases left to work through.) (WTF::CheckedPtr::operator UnspecifiedBoolType const): Deleted. (WTF::CheckedPtr::unspecifiedBoolTypeInstance const): Deleted. Took the opportunity to remove these. 'explicit operator bool()' obsoletes them. * Source/WTF/wtf/FastMalloc.h: Capitalized our type name since that's WebKit style. Renamed to WTFIsFastAllocated to match the macro WTF_MAKE_FAST_ALLOCATED. * Source/WTF/wtf/StdLibExtras.h: (WTF::makeUnique): (WTF::makeUniqueWithoutRefCountedCheck): (WTF::makeUniqueWithoutFastMallocCheck): Adopted new typename for WTFIsFastAllocated and added a little more instruction to the error message. * Source/WTF/wtf/UniqueRef.h: (WTF::makeUniqueRef): Ditto * Source/bmalloc/bmalloc/IsoHeap.h: * Source/bmalloc/bmalloc/IsoHeapInlines.h: * Source/bmalloc/bmalloc/TZoneHeap.h: * Source/bmalloc/bmalloc/TZoneHeapInlines.h: Ditto * Source/WebCore/platform/ScrollableArea.cpp: * Source/WebCore/platform/graphics/TextRun.cpp: * Source/WebCore/rendering/LegacyRootInlineBox.cpp: * Source/WebCore/rendering/MarkedText.h: * Source/WebCore/rendering/RegionContext.h: Adopt FastMalloc. * Source/WebCore/rendering/RenderLayerCompositor.cpp: (WebCore::RenderLayerCompositor::adjustOverflowScrollbarContainerLayers): Fixed a type safety bug uncovered by removing the UnspecifiedBoolType operator. This code certainly did something before, but probably not what the author expected! :P * Source/WebCore/rendering/RenderObject.cpp: * Source/WebKit/Shared/WebPreferencesStore.h: Adopt FastMalloc. Canonical link: https://commits.webkit.org/276869@main
WebKit · Apr 1, 2024 · 1bcc994 · 1bcc994
1 parent 4563c83
commit 1bcc994
Show file tree

Hide file tree

Showing 16 changed files with 33 additions and 20 deletions.
diff --git a/Source/WTF/wtf/CheckedPtr.h b/Source/WTF/wtf/CheckedPtr.h
@@ -134,10 +134,6 @@ class CheckedPtr {
 
     bool isHashTableDeletedValue() const { return PtrTraits::isHashTableDeletedValue(m_ptr); }
 
-    // This conversion operator allows implicit conversion to bool but not to other integer types.
-    using UnspecifiedBoolType = void (CheckedPtr::*)() const;
-    operator UnspecifiedBoolType() const { return m_ptr ? &CheckedPtr::unspecifiedBoolTypeInstance : nullptr; }
-
     ALWAYS_INLINE explicit operator bool() const { return PtrTraits::unwrap(m_ptr); }
 
     ALWAYS_INLINE T* get() const { return PtrTraits::unwrap(m_ptr); }
@@ -233,16 +229,20 @@ class CheckedPtr {
 private:
     template<typename OtherType, typename OtherPtrTraits> friend class CheckedPtr;
 
-    void unspecifiedBoolTypeInstance() const { }
-
     ALWAYS_INLINE void refIfNotNull()
     {
+        // FIXME: Enable this assertion once all clients conform.
+        // static_assert(std::is_same<typename T::WTFIsFastAllocated, int>::value, "Objects that use CheckedPtr must use FastMalloc (WTF_MAKE_FAST_ALLOCATED)");
+
         if (T* ptr = PtrTraits::unwrap(m_ptr); LIKELY(ptr))
             ptr->incrementPtrCount();
     }
 
     ALWAYS_INLINE void derefIfNotNull()
     {
+        // FIXME: Enable this assertion once all clients conform.
+        // static_assert(std::is_same<typename T::WTFIsFastAllocated, int>::value, "Objects that use CheckedPtr must use FastMalloc (WTF_MAKE_FAST_ALLOCATED)");
+
         if (T* ptr = PtrTraits::unwrap(m_ptr); LIKELY(ptr))
             ptr->decrementPtrCount();
     }

diff --git a/Source/WTF/wtf/FastMalloc.h b/Source/WTF/wtf/FastMalloc.h
@@ -448,7 +448,7 @@ using WTF::fastCompactAlignedMalloc;
     { \
         ::WTF::fastFree(p); \
     } \
-    using webkitFastMalloced = int; \
+    using WTFIsFastAllocated = int; \
 
 #define WTF_MAKE_FAST_COMPACT_ALLOCATED_IMPL \
     WTF_ALLOW_COMPACT_POINTERS_IMPL; \
@@ -483,7 +483,7 @@ using WTF::fastCompactAlignedMalloc;
     { \
         ::WTF::fastFree(p); \
     } \
-    using webkitFastMalloced = int; \
+    using WTFIsFastAllocated = int; \
 
 // FIXME: WTF_MAKE_FAST_ALLOCATED should take class name so that we can create malloc_zone per this macro.
 // https://bugs.webkit.org/show_bug.cgi?id=205702
@@ -541,7 +541,7 @@ using __thisIsHereToForceASemicolonAfterThisMacro UNUSED_TYPE_ALIAS = int
     { \
         classname##Malloc::free(p); \
     } \
-    using webkitFastMalloced = int; \
+    using WTFIsFastAllocated = int; \
 
 #define WTF_MAKE_FAST_ALLOCATED_WITH_HEAP_IDENTIFIER(classname) \
 public: \

diff --git a/Source/WTF/wtf/StdLibExtras.h b/Source/WTF/wtf/StdLibExtras.h
@@ -597,22 +597,22 @@ template<typename T> class TypeHasRefMemberFunction {
 template<class T, class... Args>
 ALWAYS_INLINE decltype(auto) makeUnique(Args&&... args)
 {
-    static_assert(std::is_same<typename T::webkitFastMalloced, int>::value, "T is FastMalloced");
-    static_assert(!TypeHasRefMemberFunction<T>::value, "T should not be refcounted");
+    static_assert(std::is_same<typename T::WTFIsFastAllocated, int>::value, "T sould use FastMalloc (WTF_MAKE_FAST_ALLOCATED)");
+    static_assert(!TypeHasRefMemberFunction<T>::value, "T should not be RefCounted");
     return std::make_unique<T>(std::forward<Args>(args)...);
 }
 
 template<class T, class... Args>
 ALWAYS_INLINE decltype(auto) makeUniqueWithoutRefCountedCheck(Args&&... args)
 {
-    static_assert(std::is_same<typename T::webkitFastMalloced, int>::value, "T is FastMalloced");
+    static_assert(std::is_same<typename T::WTFIsFastAllocated, int>::value, "T sould use FastMalloc (WTF_MAKE_FAST_ALLOCATED)");
     return std::make_unique<T>(std::forward<Args>(args)...);
 }
 
 template<class T, class... Args>
 ALWAYS_INLINE decltype(auto) makeUniqueWithoutFastMallocCheck(Args&&... args)
 {
-    static_assert(!TypeHasRefMemberFunction<T>::value, "T should not be refcounted");
+    static_assert(!TypeHasRefMemberFunction<T>::value, "T should not be RefCounted");
     return std::make_unique<T>(std::forward<Args>(args)...);
 }
 

diff --git a/Source/WTF/wtf/UniqueRef.h b/Source/WTF/wtf/UniqueRef.h
@@ -43,7 +43,7 @@ UniqueRef<T> makeUniqueRefWithoutFastMallocCheck(Args&&... args)
 template<typename T, class... Args>
 UniqueRef<T> makeUniqueRef(Args&&... args)
 {
-    static_assert(std::is_same<typename T::webkitFastMalloced, int>::value, "T is FastMalloced");
+    static_assert(std::is_same<typename T::WTFIsFastAllocated, int>::value, "T sould use FastMalloc (WTF_MAKE_FAST_ALLOCATED)");
     return makeUniqueRefWithoutFastMallocCheck<T>(std::forward<Args>(args)...);
 }
 

diff --git a/Source/WebCore/platform/ScrollableArea.cpp b/Source/WebCore/platform/ScrollableArea.cpp
@@ -50,6 +50,8 @@
 namespace WebCore {
 
 struct SameSizeAsScrollableArea : public CanMakeWeakPtr<SameSizeAsScrollableArea>, public CanMakeCheckedPtr {
+    WTF_MAKE_STRUCT_FAST_ALLOCATED;
+
     ~SameSizeAsScrollableArea() { }
     SameSizeAsScrollableArea() { }
     void* pointer[3];

diff --git a/Source/WebCore/platform/graphics/TextRun.cpp b/Source/WebCore/platform/graphics/TextRun.cpp
@@ -29,6 +29,8 @@
 namespace WebCore {
 
 struct ExpectedTextRunSize : public CanMakeCheckedPtr {
+    WTF_MAKE_STRUCT_FAST_ALLOCATED;
+
     String text;
     TabSize tabSize;
     float float1;

diff --git a/Source/WebCore/rendering/LegacyRootInlineBox.cpp b/Source/WebCore/rendering/LegacyRootInlineBox.cpp
@@ -43,6 +43,8 @@ namespace WebCore {
 WTF_MAKE_ISO_ALLOCATED_IMPL(LegacyRootInlineBox);
 
 struct SameSizeAsLegacyRootInlineBox : LegacyInlineFlowBox, CanMakeWeakPtr<LegacyRootInlineBox>, CanMakeCheckedPtr {
+    WTF_MAKE_STRUCT_FAST_ALLOCATED;
+
     int layoutUnits[4];
 };
 

diff --git a/Source/WebCore/rendering/MarkedText.h b/Source/WebCore/rendering/MarkedText.h
@@ -38,6 +38,8 @@ class RenderedDocumentMarker;
 struct TextBoxSelectableRange;
 
 struct MarkedText : public CanMakeCheckedPtr {
+    WTF_MAKE_STRUCT_FAST_ALLOCATED;
+
     // Sorted by paint order
     enum class Type : uint8_t {
         Unmarked,

diff --git a/Source/WebCore/rendering/RegionContext.h b/Source/WebCore/rendering/RegionContext.h
@@ -35,6 +35,7 @@ namespace WebCore {
 class Path;
 
 class RegionContext : public CanMakeCheckedPtr {
+    WTF_MAKE_FAST_ALLOCATED;
 public:
     virtual ~RegionContext() = default;
 

diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
@@ -1714,7 +1714,7 @@ void RenderLayerCompositor::adjustOverflowScrollbarContainerLayers(RenderLayer&
 
         if (lastDescendantLayerIndex && scrollerLayerIndex) {
             auto insertionIndex = std::max(lastDescendantLayerIndex.value() + 1, scrollerLayerIndex.value() + 1);
-            LOG_WITH_STREAM(Compositing, stream << "Moving overflow controls layer for " << overflowScrollingLayer << " to appear after " << lastContainedDescendant);
+            LOG_WITH_STREAM(Compositing, stream << "Moving overflow controls layer for " << *overflowScrollingLayer << " to appear after " << *lastContainedDescendant);
             layerChildren.insert(insertionIndex, *overflowContainerLayer);
         }
 

diff --git a/Source/WebCore/rendering/RenderObject.cpp b/Source/WebCore/rendering/RenderObject.cpp
@@ -111,6 +111,8 @@ RenderObject::SetLayoutNeededForbiddenScope::~SetLayoutNeededForbiddenScope()
 #endif
 
 struct SameSizeAsRenderObject : public CachedImageClient, public CanMakeCheckedPtr {
+    WTF_MAKE_STRUCT_FAST_ALLOCATED;
+
     virtual ~SameSizeAsRenderObject() = default; // Allocate vtable pointer.
 #if ASSERT_ENABLED
     unsigned m_debugBitfields : 2;

diff --git a/Source/WebKit/Shared/WebPreferencesStore.h b/Source/WebKit/Shared/WebPreferencesStore.h
@@ -36,6 +36,8 @@
 namespace WebKit {
 
 struct WebPreferencesStore : public CanMakeCheckedPtr {
+    WTF_MAKE_STRUCT_FAST_ALLOCATED;
+
     using Value = std::variant<String, bool, uint32_t, double>;
     using ValueMap = MemoryCompactRobinHoodHashMap<String, Value>;
 

diff --git a/Source/bmalloc/bmalloc/IsoHeap.h b/Source/bmalloc/bmalloc/IsoHeap.h
@@ -192,7 +192,7 @@ public: \
     \
     exportMacro static void freeAfterDestruction(void*); \
     \
-    using webkitFastMalloced = int; \
+    using WTFIsFastAllocated = int; \
 private: \
     using __makeBisoMallocedMacroSemicolonifier BUNUSED_TYPE_ALIAS = int
 
@@ -218,7 +218,7 @@ public: \
     \
     exportMacro static void freeAfterDestruction(void*); \
     \
-    using webkitFastMalloced = int; \
+    using WTFIsFastAllocated = int; \
 private: \
     using __makeBisoMallocedMacroSemicolonifier BUNUSED_TYPE_ALIAS = int
 

diff --git a/Source/bmalloc/bmalloc/IsoHeapInlines.h b/Source/bmalloc/bmalloc/IsoHeapInlines.h
@@ -145,7 +145,7 @@ public: \
         bisoHeap().deallocate(p); \
     } \
     \
-    using webkitFastMalloced = int; \
+    using WTFIsFastAllocated = int; \
 private: \
     using __makeBisoMallocedInlineMacroSemicolonifier BUNUSED_TYPE_ALIAS = int
 

diff --git a/Source/bmalloc/bmalloc/TZoneHeap.h b/Source/bmalloc/bmalloc/TZoneHeap.h
@@ -204,7 +204,7 @@ public: \
     } \
     exportMacro static void freeAfterDestruction(void*); \
     \
-    using webkitFastMalloced = int; \
+    using WTFIsFastAllocated = int; \
 private: \
     using __makeTZoneMallocedMacroSemicolonifier BUNUSED_TYPE_ALIAS = int
 

diff --git a/Source/bmalloc/bmalloc/TZoneHeapInlines.h b/Source/bmalloc/bmalloc/TZoneHeapInlines.h
@@ -154,7 +154,7 @@ public: \
         btzoneHeap().deallocate(p); \
     } \
     \
-    using webkitFastMalloced = int; \
+    using WTFIsFastAllocated = int; \
 private: \
     using __makeBtzoneMallocedInlineMacroSemicolonifier BUNUSED_TYPE_ALIAS = int