Attachment 347100 Details for Bug 188571 – Patch

[patch] Patch

bug-188571-20180815035507.patch (text/plain), 5.99 KB, created by Yusuke Suzuki on 2018-08-14 11:55:08 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-08-14 11:55:08 PDT

Size: 5.99 KB

patch

obsolete

>Subversion Revision: 234857
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 7112d156486a698de856617305d9d934c791f94e..66d0dd17a6371d4212d0ed46a2763cb4da5bab1b 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-08-14  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        [YARR] Align allocation size in BumpPointerAllocator with sizeof(void*)
>+        https://bugs.webkit.org/show_bug.cgi?id=188571
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        UBSan finds YarrInterpreter performs misaligned accesses. This is because YarrInterpreter
>+        allocates DisjunctionContext and ParenthesesDisjunctionContext from BumpPointerAllocator
>+        without considering alignment of them. This patch adds DisjunctionContext::allocationSize
>+        and ParenthesesDisjunctionContext::allocationSize to calculate allocation sizes for them.
>+        The size is always rounded to `sizeof(void*)` so that these classes are always allocated
>+        with `sizeof(void*)` alignment. We also ensure the alignments of both classes are less
>+        than or equal to `sizeof(void*)` by `static_assert`.
>+
>+        * yarr/YarrInterpreter.cpp:
>+        (JSC::Yarr::Interpreter::DisjunctionContext::allocationSize):
>+        (JSC::Yarr::Interpreter::allocDisjunctionContext):
>+        (JSC::Yarr::Interpreter::ParenthesesDisjunctionContext::ParenthesesDisjunctionContext):
>+        (JSC::Yarr::Interpreter::ParenthesesDisjunctionContext::getDisjunctionContext):
>+        (JSC::Yarr::Interpreter::ParenthesesDisjunctionContext::allocationSize):
>+        (JSC::Yarr::Interpreter::allocParenthesesDisjunctionContext):
>+        (JSC::Yarr::Interpreter::Interpreter):
>+        (JSC::Yarr::Interpreter::DisjunctionContext::DisjunctionContext): Deleted.
>+
> 2018-08-14  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         [JSC] GetByIdStatus::m_wasSeenInJIT is touched in GetByIdStatus::slowVersion
>diff --git a/Source/JavaScriptCore/yarr/YarrInterpreter.cpp b/Source/JavaScriptCore/yarr/YarrInterpreter.cpp
>index d6cb80debb00bc300b7b56ebc877b52f3c439bed..e6e8b6e12b6909be9b90ec618ff032700ced3405 100644
>--- a/Source/JavaScriptCore/yarr/YarrInterpreter.cpp
>+++ b/Source/JavaScriptCore/yarr/YarrInterpreter.cpp
>@@ -67,17 +67,20 @@ class Interpreter {
> 
>     struct DisjunctionContext
>     {
>-        DisjunctionContext()
>-            : term(0)
>-        {
>-        }
>+        DisjunctionContext() = default;
> 
>         void* operator new(size_t, void* where)
>         {
>             return where;
>         }
> 
>-        int term;
>+        static size_t allocationSize(size_t numberOfFrames)
>+        {
>+            static_assert(alignof(DisjunctionContext) <= sizeof(void*), "");
>+            return roundUpToMultipleOf<sizeof(void*)>(sizeof(DisjunctionContext) - sizeof(uintptr_t) + numberOfFrames * sizeof(uintptr_t));
>+        }
>+
>+        int term { 0 };
>         unsigned matchBegin;
>         unsigned matchEnd;
>         uintptr_t frame[1];
>@@ -85,7 +88,7 @@ class Interpreter {
> 
>     DisjunctionContext* allocDisjunctionContext(ByteDisjunction* disjunction)
>     {
>-        size_t size = sizeof(DisjunctionContext) - sizeof(uintptr_t) + disjunction->m_frameSize * sizeof(uintptr_t);
>+        size_t size = DisjunctionContext::allocationSize(disjunction->m_frameSize);
>         allocatorPool = allocatorPool->ensureCapacity(size);
>         RELEASE_ASSERT(allocatorPool);
>         return new (allocatorPool->alloc(size)) DisjunctionContext();
>@@ -99,7 +102,6 @@ class Interpreter {
>     struct ParenthesesDisjunctionContext
>     {
>         ParenthesesDisjunctionContext(unsigned* output, ByteTerm& term)
>-            : next(0)
>         {
>             unsigned firstSubpatternId = term.atom.subpatternId;
>             unsigned numNestedSubpatterns = term.atom.parenthesesDisjunction->m_numSubpatterns;
>@@ -125,16 +127,22 @@ class Interpreter {
> 
>         DisjunctionContext* getDisjunctionContext(ByteTerm& term)
>         {
>-            return reinterpret_cast<DisjunctionContext*>(&(subpatternBackup[term.atom.parenthesesDisjunction->m_numSubpatterns << 1]));
>+            return bitwise_cast<DisjunctionContext*>(bitwise_cast<uintptr_t>(this) + allocationSize(term.atom.parenthesesDisjunction->m_numSubpatterns));
>+        }
>+
>+        static size_t allocationSize(size_t numberOfSubpatterns)
>+        {
>+            static_assert(alignof(ParenthesesDisjunctionContext) <= sizeof(void*), "");
>+            return roundUpToMultipleOf<sizeof(void*)>(sizeof(ParenthesesDisjunctionContext) - sizeof(unsigned) + (numberOfSubpatterns << 1) * sizeof(unsigned));
>         }
> 
>-        ParenthesesDisjunctionContext* next;
>+        ParenthesesDisjunctionContext* next { nullptr };
>         unsigned subpatternBackup[1];
>     };
> 
>     ParenthesesDisjunctionContext* allocParenthesesDisjunctionContext(ByteDisjunction* disjunction, unsigned* output, ByteTerm& term)
>     {
>-        size_t size = sizeof(ParenthesesDisjunctionContext) - sizeof(unsigned) + (term.atom.parenthesesDisjunction->m_numSubpatterns << 1) * sizeof(unsigned) + sizeof(DisjunctionContext) - sizeof(uintptr_t) + static_cast<size_t>(disjunction->m_frameSize) * sizeof(uintptr_t);
>+        size_t size = ParenthesesDisjunctionContext::allocationSize(term.atom.parenthesesDisjunction->m_numSubpatterns) + DisjunctionContext::allocationSize(disjunction->m_frameSize);
>         allocatorPool = allocatorPool->ensureCapacity(size);
>         RELEASE_ASSERT(allocatorPool);
>         return new (allocatorPool->alloc(size)) ParenthesesDisjunctionContext(output, term);
>@@ -1630,7 +1638,6 @@ class Interpreter {
>         , unicode(pattern->unicode())
>         , output(output)
>         , input(input, start, length, pattern->unicode())
>-        , allocatorPool(0)
>         , startOffset(start)
>         , remainingMatchCount(matchLimit)
>     {
>@@ -1641,7 +1648,7 @@ class Interpreter {
>     bool unicode;
>     unsigned* output;
>     InputStream input;
>-    BumpPointerPool* allocatorPool;
>+    BumpPointerPool* allocatorPool { nullptr };
>     unsigned startOffset;
>     unsigned remainingMatchCount;
> };

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188571: 347100