Changeset 241447 in webkit

Timestamp:

Feb 13, 2019 11:16:36 AM (5 years ago)

Author:

Tadeu Zagallo

Message:

VariableLengthObject::allocate<T> should initialize objects
​https://bugs.webkit.org/show_bug.cgi?id=194534

Reviewed by Michael Saboff.

buffer() should not be called for empty VariableLengthObjects, but
these cases were not being caught due to the objects not being properly
initialized. Fix it so that allocate calls the constructor and fix the
assertion failues.

• runtime/CachedTypes.cpp:

(JSC::CachedObject::operator new):
(JSC::VariableLengthObject::allocate):
(JSC::CachedVector::encode):
(JSC::CachedVector::decode const):
(JSC::CachedUniquedStringImpl::decode const):
(JSC::CachedBitVector::encode):
(JSC::CachedBitVector::decode const):
(JSC::CachedArray::encode):
(JSC::CachedArray::decode const):
(JSC::CachedImmutableButterfly::CachedImmutableButterfly):
(JSC::CachedBigInt::decode const):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/CachedTypes.cpp (modified) (13 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-02-13  Tadeu Zagallo  <tzagallo@apple.com>
+
+        VariableLengthObject::allocate<T> should initialize objects
+        https://bugs.webkit.org/show_bug.cgi?id=194534
+
+        Reviewed by Michael Saboff.
+
+        `buffer()` should not be called for empty VariableLengthObjects, but
+        these cases were not being caught due to the objects not being properly
+        initialized. Fix it so that allocate calls the constructor and fix the
+        assertion failues.
+
+        * runtime/CachedTypes.cpp:
+        (JSC::CachedObject::operator new):
+        (JSC::VariableLengthObject::allocate):
+        (JSC::CachedVector::encode):
+        (JSC::CachedVector::decode const):
+        (JSC::CachedUniquedStringImpl::decode const):
+        (JSC::CachedBitVector::encode):
+        (JSC::CachedBitVector::decode const):
+        (JSC::CachedArray::encode):
+        (JSC::CachedArray::decode const):
+        (JSC::CachedImmutableButterfly::CachedImmutableButterfly):
+        (JSC::CachedBigInt::decode const):
+
 2019-02-13  Tadeu Zagallo  <tzagallo@apple.com>
 

trunk/Source/JavaScriptCore/runtime/CachedTypes.cpp

@@ -307 +307 @@
 class CachedObject {
     WTF_MAKE_NONCOPYABLE(CachedObject<Source>);
-    WTF_FORBID_HEAP_ALLOCATION;
 
 public:
     using SourceType_ = Source;
+
+    CachedObject() = default;
+
+    inline void* operator new(size_t, void* where) { return where; }
+
+    // Copied from WTF_FORBID_HEAP_ALLOCATION, since we only want to allow placement new
+    void* operator new[](size_t, void*) = delete;
+    void* operator new(size_t) = delete;
+    void operator delete(void*) = delete;
+    void* operator new[](size_t size) = delete;
+    void operator delete[](void*) = delete;
+    void* operator new(size_t, NotNullTag, void* location) = delete;
 };
 
@@ -333 +344 @@
     uint8_t* allocate(Encoder& encoder, size_t size)
     {
-        if (!size)
-            return nullptr;
-
         ptrdiff_t offsetOffset = encoder.offsetOf(&m_offset);
         auto result = encoder.malloc(size);
@@ -346 +354 @@
     {
         uint8_t* result = allocate(encoder, sizeof(T) * size);
-        return reinterpret_cast<T*>(result);
+        return new (result) T();
     }
 
@@ -464 +472 @@
     {
         m_size = vector.size();
+        if (!m_size)
+            return;
         T* buffer = this->template allocate<T>(encoder, m_size);
         for (unsigned i = 0; i < m_size; ++i)
@@ -472 +482 @@
     void decode(Decoder& decoder, Vector<SourceType<T>, InlineCapacity, OverflowHandler>& vector, Args... args) const
     {
+        if (!m_size)
+            return;
         vector.resizeToFit(m_size);
         const T* buffer = this->template buffer<T>();
@@ -571 +583 @@
                 return AtomicStringImpl::add(buffer, m_length).leakRef();
 
-            if (!m_length)
-                return &SymbolImpl::createNullSymbol().leakRef();
-
             Identifier ident = Identifier::fromString(&decoder.vm(), buffer, m_length);
             String str = decoder.vm().propertyNames->lookUpPrivateName(ident);
@@ -581 +590 @@
         };
 
+        if (!m_length) {
+            if (m_isSymbol)
+                return &SymbolImpl::createNullSymbol().leakRef();
+            return AtomicStringImpl::add("").leakRef();
+        }
+
         if (m_is8Bit)
             return create(this->buffer<LChar>());
@@ -741 +756 @@
     {
         m_size = bitVector.size();
+        if (!m_size)
+            return;
         uint8_t* buffer = this->allocate(encoder, m_size);
         memcpy(buffer, bitVector.bits(), m_size);
@@ -747 +764 @@
     void decode(Decoder&, BitVector& bitVector) const
     {
+        if (!m_size)
+            return;
         bitVector.ensureSize(m_size);
         memcpy(bitVector.bits(), this->buffer(), m_size);
@@ -861 +880 @@
     void encode(Encoder& encoder, const Source* array, unsigned size)
     {
+        if (!size)
+            return;
         T* dst = this->template allocate<T>(encoder, size);
         for (unsigned i = 0; i < size; ++i)
@@ -869 +890 @@
     void decode(Decoder& decoder, Source* array, unsigned size, Args... args) const
     {
+        if (!size)
+            return;
         const T* buffer = this->template buffer<T>();
         for (unsigned i = 0; i < size; ++i)
@@ -949 +972 @@
 class CachedImmutableButterfly : public CachedObject<JSImmutableButterfly> {
 public:
+    CachedImmutableButterfly()
+        : m_cachedDoubles()
+    {
+    }
+
     void encode(Encoder& encoder, JSImmutableButterfly& immutableButterfly)
     {
@@ -1038 +1066 @@
         JSBigInt* bigInt = JSBigInt::createWithLengthUnchecked(decoder.vm(), m_length);
         bigInt->setSign(m_sign);
-        memcpy(bigInt->dataStorage(), this->buffer(), sizeof(JSBigInt::Digit) * m_length);
+        if (m_length)
+            memcpy(bigInt->dataStorage(), this->buffer(), sizeof(JSBigInt::Digit) * m_length);
         return bigInt;
     }