Attachment 369586 Details for Bug 197756 – Patch

[patch] Patch

patch197756 (text/plain), 2.43 KB, created by Robin Morisset on 2019-05-10 13:07:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2019-05-10 13:07:13 PDT

Size: 2.43 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 245189)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2019-05-10  Robin Morisset  <rmorisset@apple.com>
>+
>+        testb3 failing with crash in JSC::B3::BasicBlock::appendNonTerminal
>+        https://bugs.webkit.org/show_bug.cgi?id=197756
>+        <rdar://problem/50641659>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When I added https://bugs.webkit.org/show_bug.cgi?id=197265 I assumed that which block is the root does not change in the middle of strength reduction.
>+        But specializeSelect can use splitForward, which uses a new block for the first half of the given block.
>+        So if the block being split is the root block I must update m_root and erase the m_valueInConstant cache.
>+        Erasing the cache cannot cause wrong results: at most it can make us miss some optimization opportunities in this iteration of the fixpoint.
>+
>+        * b3/B3ReduceStrength.cpp:
>+
> 2019-05-09  Keith Miller  <keith_miller@apple.com>
> 
>         Fix crashes related to pointer authentication for primitive gigacage
>Index: Source/JavaScriptCore/b3/B3ReduceStrength.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3ReduceStrength.cpp	(revision 245066)
>+++ Source/JavaScriptCore/b3/B3ReduceStrength.cpp	(working copy)
>@@ -2162,6 +2162,7 @@
>                 m_valueForConstant.add(key, m_value);
>             else {
>                 Value* constInRoot = m_proc.clone(m_value);
>+                ASSERT(m_root && m_root->size() >= 1);
>                 m_root->appendNonTerminal(constInRoot);
>                 m_valueForConstant.add(key, constInRoot);
>                 m_value->replaceWithIdentity(constInRoot);
>@@ -2224,8 +2225,11 @@
> 
>         // This mutates startIndex to account for the fact that m_block got the front of it
>         // chopped off.
>-        BasicBlock* predecessor =
>-            m_blockInsertionSet.splitForward(m_block, m_index, &m_insertionSet);
>+        BasicBlock* predecessor = m_blockInsertionSet.splitForward(m_block, m_index, &m_insertionSet);
>+        if (m_block == m_root) {
>+            m_root = predecessor;
>+            m_valueForConstant.clear();
>+        }
> 
>         // Splitting will commit the insertion set, which changes the exact position of the
>         // source. That's why we do the search after splitting.

Actions: View | Formatted Diff | Diff

Attachments on bug 197756: 369526 | 369586