Do not store LazyNeverDestroyed objects as member variables

https://bugs.webkit.org/show_bug.cgi?id=298814

Reviewed by Darin Adler.

Stop using LazyNeverDestroyed for member variables in StaticCSSValuePool.

This triggers undefined behavior in ASSERT_ENABLED builds, since the
'm_isConstructed' member of LazyNeverDestroyed<T> is not initialized
in that case. GCC 14 correctly warned about this, breaking the build
on e.g. Ubuntu 25.04, where GCC 14 is default.

The straightforward solution was to directly use AlignedStorage<T>
as type for the pool member variables instead of LazyNeverDestroyed<T>,
preserving the current performance characteristics (no dynamic
allocations, etc.). The "canonical" solution of using e.g.
Vector<RefPtr<CSSPrimitiveValue>> would re-introduce the memory
problems which were previously migitiated by introducing
std::array<LazyNeverDestroyed<...>, capacity> -- thus it is not
applicable here.

Covered by existing tests.

* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::valueFromPool):
* Source/WebCore/css/CSSValuePool.cpp:
(WebCore::StaticCSSValuePool::StaticCSSValuePool):
(WebCore::CSSValuePool::createColorValue):
* Source/WebCore/css/CSSValuePool.h:
(WebCore::CSSPrimitiveValue::implicitInitialValue):
(WebCore::CSSPrimitiveValue::create):

Canonical link: https://commits.webkit.org/299936@main

Author: nikolaszimmermann

Source/WebCore/css/CSSPrimitiveValue.cpp

@@ -423,14 +423,14 @@ Ref<CSSPrimitiveValue> CSSPrimitiveValue::create(CSSPropertyID propertyID)
     return adoptRef(*new CSSPrimitiveValue(propertyID));
 }
 
-static CSSPrimitiveValue* valueFromPool(std::span<LazyNeverDestroyed<CSSPrimitiveValue>> pool, double value)
+static CSSPrimitiveValue* valueFromPool(std::span<AlignedStorage<CSSPrimitiveValue>> pool, double value)
 {
     // Casting to a signed integer first since casting a negative floating point value to an unsigned
     // integer is undefined behavior.
     unsigned poolIndex = static_cast<unsigned>(static_cast<int>(value));
     double roundTripValue = poolIndex;
     if (equalSpans(asByteSpan(value), asByteSpan(roundTripValue)) && poolIndex < pool.size())
-        return &pool[poolIndex].get();
+        return pool[poolIndex].get();
     return nullptr;
 }
 

Source/WebCore/css/CSSValuePool.cpp

@@ -37,20 +37,18 @@ DEFINE_ALLOCATOR_WITH_HEAP_IDENTIFIER(CSSValuePool);
 LazyNeverDestroyed<StaticCSSValuePool> staticCSSValuePool;
 
 StaticCSSValuePool::StaticCSSValuePool()
+    : m_implicitInitialValue(CSSValue::StaticCSSValue, CSSPrimitiveValue::ImplicitInitialValue)
+    , m_transparentColor(CSSValue::StaticCSSValue, WebCore::Color::transparentBlack)
+    , m_whiteColor(CSSValue::StaticCSSValue, WebCore::Color::white)
+    , m_blackColor(CSSValue::StaticCSSValue, WebCore::Color::black)
 {
-    m_implicitInitialValue.construct(CSSValue::StaticCSSValue, CSSPrimitiveValue::ImplicitInitialValue);
-    
-    m_transparentColor.construct(CSSValue::StaticCSSValue, WebCore::Color::transparentBlack);
-    m_whiteColor.construct(CSSValue::StaticCSSValue, WebCore::Color::white);
-    m_blackColor.construct(CSSValue::StaticCSSValue, WebCore::Color::black);
-
     for (auto keyword : allCSSValueKeywords())
-        m_identifierValues[enumToUnderlyingType(keyword)].construct(CSSValue::StaticCSSValue, keyword);
+        new (m_identifierValues[enumToUnderlyingType(keyword)].get()) CSSPrimitiveValue { CSSValue::StaticCSSValue, keyword };
 
     for (unsigned i = 0; i <= maximumCacheableIntegerValue; ++i) {
-        m_pixelValues[i].construct(CSSValue::StaticCSSValue, i, CSSUnitType::CSS_PX);
-        m_percentageValues[i].construct(CSSValue::StaticCSSValue, i, CSSUnitType::CSS_PERCENTAGE);
-        m_numberValues[i].construct(CSSValue::StaticCSSValue, i, CSSUnitType::CSS_NUMBER);
+        new (m_pixelValues[i].get()) CSSPrimitiveValue(CSSValue::StaticCSSValue, i, CSSUnitType::CSS_PX);
+        new (m_percentageValues[i].get()) CSSPrimitiveValue(CSSValue::StaticCSSValue, i, CSSUnitType::CSS_PERCENTAGE);
+        new (m_numberValues[i].get()) CSSPrimitiveValue(CSSValue::StaticCSSValue, i, CSSUnitType::CSS_NUMBER);
     }
 }
 
@@ -76,12 +74,12 @@ Ref<CSSColorValue> CSSValuePool::createColorValue(const WebCore::Color& color)
 {
     // These are the empty and deleted values of the hash table.
     if (color == WebCore::Color::transparentBlack)
-        return staticCSSValuePool->m_transparentColor.get();
+        return staticCSSValuePool->m_transparentColor;
     if (color == WebCore::Color::white)
-        return staticCSSValuePool->m_whiteColor.get();
+        return staticCSSValuePool->m_whiteColor;
     // Just because it is common.
     if (color == WebCore::Color::black)
-        return staticCSSValuePool->m_blackColor.get();
+        return staticCSSValuePool->m_blackColor;
 
     // Remove one entry at random if the cache grows too large.
     // FIXME: Use TinyLRUCache instead?

Source/WebCore/css/CSSValuePool.h

@@ -48,18 +48,18 @@ class StaticCSSValuePool {
 private:
     StaticCSSValuePool();
 
-    LazyNeverDestroyed<CSSPrimitiveValue> m_implicitInitialValue;
+    CSSPrimitiveValue m_implicitInitialValue;
 
-    LazyNeverDestroyed<CSSColorValue> m_transparentColor;
-    LazyNeverDestroyed<CSSColorValue> m_whiteColor;
-    LazyNeverDestroyed<CSSColorValue> m_blackColor;
+    CSSColorValue m_transparentColor;
+    CSSColorValue m_whiteColor;
+    CSSColorValue m_blackColor;
 
     static constexpr int maximumCacheableIntegerValue = 255;
 
-    std::array<LazyNeverDestroyed<CSSPrimitiveValue>, maximumCacheableIntegerValue + 1> m_pixelValues;
-    std::array<LazyNeverDestroyed<CSSPrimitiveValue>, maximumCacheableIntegerValue + 1> m_percentageValues;
-    std::array<LazyNeverDestroyed<CSSPrimitiveValue>, maximumCacheableIntegerValue + 1> m_numberValues;
-    std::array<LazyNeverDestroyed<CSSPrimitiveValue>, numCSSValueKeywords> m_identifierValues;
+    std::array<AlignedStorage<CSSPrimitiveValue>, maximumCacheableIntegerValue + 1> m_pixelValues;
+    std::array<AlignedStorage<CSSPrimitiveValue>, maximumCacheableIntegerValue + 1> m_percentageValues;
+    std::array<AlignedStorage<CSSPrimitiveValue>, maximumCacheableIntegerValue + 1> m_numberValues;
+    std::array<AlignedStorage<CSSPrimitiveValue>, numCSSValueKeywords> m_identifierValues;
 };
 
 WEBCORE_EXPORT extern LazyNeverDestroyed<StaticCSSValuePool> staticCSSValuePool;
@@ -85,13 +85,13 @@ class CSSValuePool {
 
 inline CSSPrimitiveValue& CSSPrimitiveValue::implicitInitialValue()
 {
-    return staticCSSValuePool->m_implicitInitialValue.get();
+    return staticCSSValuePool->m_implicitInitialValue;
 }
 
 inline Ref<CSSPrimitiveValue> CSSPrimitiveValue::create(CSSValueID identifier)
 {
     RELEASE_ASSERT(identifier < numCSSValueKeywords);
-    return staticCSSValuePool->m_identifierValues[identifier].get();
+    return *staticCSSValuePool->m_identifierValues[identifier];
 }
 
 } // namespace WebCore