Inlining of a function that ends in op_unreachable crashes

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

Reviewed by Filip Pizlo.

JSTests:

* stress/inlining-unreachable.js: Added.
(bar):
(baz):
(i.catch):

Source/JavaScriptCore:

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::allocateTargetableBlock):
(JSC::DFG::ByteCodeParser::inlineCall):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@226362 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index ce707bb..6b2440c 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,15 @@
+2018-01-03  Robin Morisset  <rmorisset@apple.com>
+
+        Inlining of a function that ends in op_unreachable crashes
+        https://bugs.webkit.org/show_bug.cgi?id=181027
+
+        Reviewed by Filip Pizlo.
+
+        * stress/inlining-unreachable.js: Added.
+        (bar):
+        (baz):
+        (i.catch):
+
 2018-01-02  Saam Barati  <sbarati@apple.com>
 
         Incorrect assertion inside AccessCase

diff --git a/JSTests/stress/inlining-unreachable.js b/JSTests/stress/inlining-unreachable.js
new file mode 100644 (file)
index 0000000..35bd2cf
--- /dev/null
+++ b/JSTests/stress/inlining-unreachable.js
@@ -0,0 +1,9 @@
+var bar = class Bar { };
+var baz = class Baz {
+    constructor() { bar(); }
+};
+for (var i = 0; i < 10000; i++) {
+    try {
+        new baz();
+    } catch (e) {}
+}

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 5d9d9f7..d740771 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,14 @@
+2018-01-03  Robin Morisset  <rmorisset@apple.com>
+
+        Inlining of a function that ends in op_unreachable crashes
+        https://bugs.webkit.org/show_bug.cgi?id=181027
+
+        Reviewed by Filip Pizlo.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::allocateTargetableBlock):
+        (JSC::DFG::ByteCodeParser::inlineCall):
+
 2018-01-02  Saam Barati  <sbarati@apple.com>
 
         Incorrect assertion inside AccessCase

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
index ea3afa5..0f7f77a 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -139,6 +139,7 @@ private:
     // It is also used when doing an early return from an inlined callee: it is easier to fix the bytecode index later on if needed
     // than to move the right index all the way to the treatment of op_ret.
     BasicBlock* allocateTargetableBlock(unsigned bytecodeIndex);
+    BasicBlock* allocateTargetableBlock(InlineStackEntry*, unsigned bytecodeIndex);
     BasicBlock* allocateUntargetableBlock();
     // An untargetable block can be given a bytecodeIndex to be later managed by linkBlock, but only once, and it can never go in the other direction
     void makeBlockTargetable(BasicBlock*, unsigned bytecodeIndex);
@@ -1144,13 +1145,18 @@ private:
 
 BasicBlock* ByteCodeParser::allocateTargetableBlock(unsigned bytecodeIndex)
 {
+    return allocateTargetableBlock(m_inlineStackTop, bytecodeIndex);
+}
+
+BasicBlock* ByteCodeParser::allocateTargetableBlock(InlineStackEntry* stackEntry, unsigned bytecodeIndex)
+{
     ASSERT(bytecodeIndex != UINT_MAX);
     Ref<BasicBlock> block = adoptRef(*new BasicBlock(bytecodeIndex, m_numArguments, m_numLocals, 1));
     BasicBlock* blockPtr = block.ptr();
     // m_blockLinkingTargets must always be sorted in increasing order of bytecodeBegin
-    if (m_inlineStackTop->m_blockLinkingTargets.size())
-        ASSERT(m_inlineStackTop->m_blockLinkingTargets.last()->bytecodeBegin < bytecodeIndex);
-    m_inlineStackTop->m_blockLinkingTargets.append(blockPtr);
+    if (stackEntry->m_blockLinkingTargets.size())
+        ASSERT(stackEntry->m_blockLinkingTargets.last()->bytecodeBegin < bytecodeIndex);
+    stackEntry->m_blockLinkingTargets.append(blockPtr);
     m_graph.appendBlock(WTFMove(block));
     return blockPtr;
 }
@@ -1652,11 +1658,13 @@ void ByteCodeParser::inlineCall(Node* callTargetNode, int resultOperand, CallVar
 
     linkBlocks(inlineStackEntry.m_unlinkedBlocks, inlineStackEntry.m_blockLinkingTargets);
     
-    // There is an invariant that we use here: every function has at least one op_ret somewhere.
-    // And when parseBlock encounters an op_ret, it setups a continuation block if there was none.
-    // If this invariant was to be changed, we would need to allocate a new block here in the case where there was no continuation block ready.
-    RELEASE_ASSERT(inlineStackEntry.m_continuationBlock);
-    m_currentBlock = inlineStackEntry.m_continuationBlock;
+    // Most functions have at least one op_ret and thus set up the continuation block.
+    // In some rare cases, a function ends in op_unreachable, forcing us to allocate a new continuationBlock here.
+    // We must be careful to allocate it in the caller and not the top of the inline stack, since the callee is still on the stack at this point.
+    if (inlineStackEntry.m_continuationBlock)
+        m_currentBlock = inlineStackEntry.m_continuationBlock;
+    else
+        m_currentBlock = allocateTargetableBlock(inlineStackEntry.m_caller, m_currentIndex);
     ASSERT(!m_currentBlock->terminal());
 
     prepareToParseBlock();