From 1fcbb7314c6006dc67a6d5c3424df9c299c22830 Mon Sep 17 00:00:00 2001 From: "rmorisset@apple.com" Date: Wed, 3 Jan 2018 17:35:35 +0000 Subject: [PATCH] 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 --- JSTests/ChangeLog | 12 ++++++++++++ JSTests/stress/inlining-unreachable.js | 9 +++++++++ Source/JavaScriptCore/ChangeLog | 11 +++++++++++ Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp | 24 ++++++++++++++++-------- 4 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 JSTests/stress/inlining-unreachable.js diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog index ce707bb..6b2440c 100644 --- a/JSTests/ChangeLog +++ b/JSTests/ChangeLog @@ -1,3 +1,15 @@ +2018-01-03 Robin Morisset + + 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 Incorrect assertion inside AccessCase diff --git a/JSTests/stress/inlining-unreachable.js b/JSTests/stress/inlining-unreachable.js new file mode 100644 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 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,14 @@ +2018-01-03 Robin Morisset + + 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 Incorrect assertion inside AccessCase diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index ea3afa5..0f7f77a 100644 --- 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 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(); -- 1.8.3.1