Attachment 370247 Details for Bug 198035 – Patch

[patch] Patch

bug-198035-20190520113510.patch (text/plain), 4.36 KB, created by Tadeu Zagallo on 2019-05-20 02:35:11 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-05-20 02:35:11 PDT

Size: 4.36 KB

patch

obsolete

>Subversion Revision: 245396
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 5b4da472e0ce993c154193f9b8fee4044e796e59..d8e68e75b8b45f0153e86cb660cc3aa24b5a4ae7 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2019-05-20  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Fix 32-bit btyecode cache crashes
>+        https://bugs.webkit.org/show_bug.cgi?id=198035
>+        <rdar://problem/49905560>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        There were 2 32-bit issues with the bytecode cache:
>+        - UnlinkedFunctionExecutable::m_cachedCodeBlockForConstructOffset was not initialized.
>+          The code was relying on the other member of the union, `m_unlinkedCodeBlockForConstruct`,
>+          initializing both m_cachedCodeBlockForCallOffset and m_cachedCodeBlockForConstructOffset.
>+          This is undefined behavior and is also incorrect in 32-bit. Since m_unlinkedCodeBlockForConstruct
>+          is 32-bit, it only initializes the first member of the struct.
>+        - Encoder::Page was not aligned at the end. This lead to unaligned allocations on subsequent
>+          pages, since the start of the following page would not be aligned.
>+
>+        * runtime/CachedTypes.cpp:
>+        (JSC::Encoder::release):
>+        (JSC::Encoder::Page::alignEnd):
>+        (JSC::Encoder::allocateNewPage):
>+        (JSC::VariableLengthObject::buffer const):
>+        (JSC::VariableLengthObject::allocate):
>+        (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):
>+
> 2019-05-15  Saam Barati  <sbarati@apple.com>
> 
>         Bound liveness of SetArgumentMaybe nodes when maximal flush insertion phase is enabled
>diff --git a/Source/JavaScriptCore/runtime/CachedTypes.cpp b/Source/JavaScriptCore/runtime/CachedTypes.cpp
>index 3b610a8d7778a7d7b81ba0be6c137a4bd7636a3d..29e21c8d40785299ef38a6c516fb5c01a55d8d73 100644
>--- a/Source/JavaScriptCore/runtime/CachedTypes.cpp
>+++ b/Source/JavaScriptCore/runtime/CachedTypes.cpp
>@@ -146,6 +146,10 @@ public:
> 
>     Ref<CachedBytecode> release()
>     {
>+        if (!m_currentPage)
>+            return CachedBytecode::create();
>+
>+        m_currentPage->alignEnd();
>         size_t size = m_baseOffset + m_currentPage->size();
>         MallocPtr<uint8_t> buffer = MallocPtr<uint8_t>::malloc(size);
>         unsigned offset = 0;
>@@ -193,6 +197,15 @@ private:
>             return false;
>         }
> 
>+        void alignEnd()
>+        {
>+            ptrdiff_t size = roundUpToMultipleOf(alignof(std::max_align_t), m_offset);
>+            if (size == m_offset)
>+                return;
>+            ASSERT(static_cast<size_t>(size) <= m_capacity);
>+            m_offset = size;
>+        }
>+
>     private:
>         MallocPtr<uint8_t> m_buffer;
>         ptrdiff_t m_offset;
>@@ -202,8 +215,10 @@ private:
>     void allocateNewPage(size_t size = 0)
>     {
>         static size_t minPageSize = pageSize();
>-        if (m_currentPage)
>+        if (m_currentPage) {
>+            m_currentPage->alignEnd();
>             m_baseOffset += m_currentPage->size();
>+        }
>         if (size < minPageSize)
>             size = minPageSize;
>         else
>@@ -383,6 +398,7 @@ protected:
>     template<typename T>
>     const T* buffer() const
>     {
>+        ASSERT(!(bitwise_cast<uintptr_t>(buffer()) % alignof(T)));
>         return bitwise_cast<const T*>(buffer());
>     }
> 
>@@ -403,6 +419,7 @@ protected:
>     T* allocate(Encoder& encoder, unsigned size = 1)
>     {
>         uint8_t* result = allocate(encoder, sizeof(T) * size);
>+        ASSERT(!(bitwise_cast<uintptr_t>(result) % alignof(T)));
>         return new (result) T[size];
>     }
> 
>@@ -2100,8 +2117,11 @@ ALWAYS_INLINE UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(Decoder& de
>                 codeBlockOffset = offset;
>                 m_isCached = true;
>                 leafExecutables--;
>+                return;
>             }
>         }
>+
>+        codeBlockOffset = 0;
>     };
> 
>     if (!cachedExecutable.unlinkedCodeBlockForCall().isEmpty() || !cachedExecutable.unlinkedCodeBlockForConstruct().isEmpty()) {
>@@ -2109,6 +2129,8 @@ ALWAYS_INLINE UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(Decoder& de
>         checkBounds(m_cachedCodeBlockForConstructOffset, cachedExecutable.unlinkedCodeBlockForConstruct());
>         if (m_isCached)
>             m_decoder = &decoder;
>+        else
>+            m_decoder = nullptr;
>     }
> 
>     if (leafExecutables)

Actions: View | Formatted Diff | Diff

Attachments on bug 198035: 370247