Modified: trunk/JSTests/ChangeLog (226810 => 226811)
--- trunk/JSTests/ChangeLog 2018-01-11 22:45:37 UTC (rev 226810)
+++ trunk/JSTests/ChangeLog 2018-01-11 23:21:18 UTC (rev 226811)
@@ -1,5 +1,15 @@
2018-01-11 Saam Barati <sbar...@apple.com>
+ When inserting Unreachable in byte code parser we need to flush all the right things
+ https://bugs.webkit.org/show_bug.cgi?id=181509
+ <rdar://problem/36423110>
+
+ Reviewed by Mark Lam.
+
+ * stress/proper-flushing-when-we-insert-unreachable-after-force-exit-in-bytecode-parser.js: Added.
+
+2018-01-11 Saam Barati <sbar...@apple.com>
+
JITMathIC code in the FTL is wrong when code gets duplicated
https://bugs.webkit.org/show_bug.cgi?id=181525
<rdar://problem/36351993>
Added: trunk/JSTests/stress/proper-flushing-when-we-insert-unreachable-after-force-exit-in-bytecode-parser.js (0 => 226811)
--- trunk/JSTests/stress/proper-flushing-when-we-insert-unreachable-after-force-exit-in-bytecode-parser.js (rev 0)
+++ trunk/JSTests/stress/proper-flushing-when-we-insert-unreachable-after-force-exit-in-bytecode-parser.js 2018-01-11 23:21:18 UTC (rev 226811)
@@ -0,0 +1,27 @@
+function test(b, f) {
+ if (b)
+ return f(b);
+}
+noInline(test);
+
+function throwError(b) {
+ if (b) {
+ try {
+ throw new Error;
+ } catch(e) { }
+ }
+ return 2;
+}
+noInline(throwError);
+
+function makeFoo() {
+ return function foo(b) {
+ throwError(b);
+ OSRExit();
+ }
+}
+
+let foos = [makeFoo(), makeFoo()];
+for (let i = 0; i < 10000; ++i) {
+ test(!!(i%2), foos[((Math.random() * 100) | 0) % foos.length]);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (226810 => 226811)
--- trunk/Source/_javascript_Core/ChangeLog 2018-01-11 22:45:37 UTC (rev 226810)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-01-11 23:21:18 UTC (rev 226811)
@@ -1,5 +1,31 @@
2018-01-11 Saam Barati <sbar...@apple.com>
+ When inserting Unreachable in byte code parser we need to flush all the right things
+ https://bugs.webkit.org/show_bug.cgi?id=181509
+ <rdar://problem/36423110>
+
+ Reviewed by Mark Lam.
+
+ I added code in r226655 that had its own mechanism for preserving liveness when
+ inserting Unreachable nodes after ForceOSRExit. There are two ways to preserve
+ liveness: PhantomLocal and Flush. Certain values *must* be flushed to the stack.
+ I got some of these values wrong, which was leading to a crash when recovering the
+ callee value from an inlined frame. Instead of making the same mistake and repeating
+ similar code again, this patch refactors this logic to be shared with the other
+ liveness preservation code in the DFG bytecode parser. This is what I should have
+ done in my initial patch.
+
+ * bytecode/InlineCallFrame.h:
+ (JSC::remapOperand):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::flushImpl):
+ (JSC::DFG::flushForTerminalImpl):
+ (JSC::DFG::ByteCodeParser::flush):
+ (JSC::DFG::ByteCodeParser::flushForTerminal):
+ (JSC::DFG::ByteCodeParser::parse):
+
+2018-01-11 Saam Barati <sbar...@apple.com>
+
JITMathIC code in the FTL is wrong when code gets duplicated
https://bugs.webkit.org/show_bug.cgi?id=181525
<rdar://problem/36351993>
Modified: trunk/Source/_javascript_Core/bytecode/InlineCallFrame.h (226810 => 226811)
--- trunk/Source/_javascript_Core/bytecode/InlineCallFrame.h 2018-01-11 22:45:37 UTC (rev 226810)
+++ trunk/Source/_javascript_Core/bytecode/InlineCallFrame.h 2018-01-11 23:21:18 UTC (rev 226811)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -257,6 +257,13 @@
}
}
+ALWAYS_INLINE VirtualRegister remapOperand(InlineCallFrame* inlineCallFrame, VirtualRegister reg)
+{
+ if (inlineCallFrame)
+ return VirtualRegister(reg.offset() + inlineCallFrame->stackOffset);
+ return reg;
+}
+
} // namespace JSC
namespace WTF {
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (226810 => 226811)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-11 22:45:37 UTC (rev 226810)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-11 23:21:18 UTC (rev 226811)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -82,6 +82,51 @@
dataLog(__VA_ARGS__); \
} while (false)
+template <typename F1, typename F2>
+static ALWAYS_INLINE void flushImpl(Graph& graph, InlineCallFrame* inlineCallFrame, const F1& addFlushDirect, const F2& addPhantomLocalDirect)
+{
+ int numArguments;
+ if (inlineCallFrame) {
+ ASSERT(!graph.hasDebuggerEnabled());
+ numArguments = inlineCallFrame->argumentsWithFixup.size();
+ if (inlineCallFrame->isClosureCall)
+ addFlushDirect(remapOperand(inlineCallFrame, VirtualRegister(CallFrameSlot::callee)));
+ if (inlineCallFrame->isVarargs())
+ addFlushDirect(remapOperand(inlineCallFrame, VirtualRegister(CallFrameSlot::argumentCount)));
+ } else
+ numArguments = graph.baselineCodeBlockFor(inlineCallFrame)->numParameters();
+
+ for (unsigned argument = numArguments; argument-- > 1;)
+ addFlushDirect(remapOperand(inlineCallFrame, virtualRegisterForArgument(argument)));
+
+ if (!inlineCallFrame && graph.needsFlushedThis())
+ addFlushDirect(remapOperand(inlineCallFrame, virtualRegisterForArgument(0)));
+ else
+ addPhantomLocalDirect(remapOperand(inlineCallFrame, virtualRegisterForArgument(0)));
+
+ if (graph.needsScopeRegister())
+ addFlushDirect(graph.m_codeBlock->scopeRegister());
+}
+
+template <typename F1, typename F2>
+static ALWAYS_INLINE void flushForTerminalImpl(Graph& graph, CodeOrigin origin, const F1& addFlushDirect, const F2& addPhantomLocalDirect)
+{
+ origin.walkUpInlineStack([&] (CodeOrigin origin) {
+ unsigned bytecodeIndex = origin.bytecodeIndex;
+ InlineCallFrame* inlineCallFrame = origin.inlineCallFrame;
+ flushImpl(graph, inlineCallFrame, addFlushDirect, addPhantomLocalDirect);
+
+ CodeBlock* codeBlock = graph.baselineCodeBlockFor(inlineCallFrame);
+ FullBytecodeLiveness& fullLiveness = graph.livenessFor(codeBlock);
+ const FastBitVector& livenessAtBytecode = fullLiveness.getLiveness(bytecodeIndex);
+
+ for (unsigned local = codeBlock->m_numCalleeLocals; local--;) {
+ if (livenessAtBytecode[local])
+ addPhantomLocalDirect(remapOperand(inlineCallFrame, virtualRegisterForLocal(local)));
+ }
+ });
+}
+
// === ByteCodeParser ===
//
// This class is used to compile the dataflow graph from a CodeBlock.
@@ -561,55 +606,16 @@
void flush(InlineStackEntry* inlineStackEntry)
{
- int numArguments;
- if (InlineCallFrame* inlineCallFrame = inlineStackEntry->m_inlineCallFrame) {
- ASSERT(!m_hasDebuggerEnabled);
- numArguments = inlineCallFrame->argumentsWithFixup.size();
- if (inlineCallFrame->isClosureCall)
- flushDirect(inlineStackEntry->remapOperand(VirtualRegister(CallFrameSlot::callee)));
- if (inlineCallFrame->isVarargs())
- flushDirect(inlineStackEntry->remapOperand(VirtualRegister(CallFrameSlot::argumentCount)));
- } else
- numArguments = inlineStackEntry->m_codeBlock->numParameters();
- for (unsigned argument = numArguments; argument-- > 1;)
- flushDirect(inlineStackEntry->remapOperand(virtualRegisterForArgument(argument)));
- if (!inlineStackEntry->m_inlineCallFrame && m_graph.needsFlushedThis())
- flushDirect(virtualRegisterForArgument(0));
- else
- phantomLocalDirect(virtualRegisterForArgument(0));
-
- if (m_graph.needsScopeRegister())
- flushDirect(m_codeBlock->scopeRegister());
+ auto addFlushDirect = [&] (VirtualRegister reg) { flushDirect(reg); };
+ auto addPhantomLocalDirect = [&] (VirtualRegister reg) { phantomLocalDirect(reg); };
+ flushImpl(m_graph, inlineStackEntry->m_inlineCallFrame, addFlushDirect, addPhantomLocalDirect);
}
void flushForTerminal()
{
- CodeOrigin origin = currentCodeOrigin();
- unsigned bytecodeIndex = origin.bytecodeIndex;
-
- for (InlineStackEntry* inlineStackEntry = m_inlineStackTop; inlineStackEntry; inlineStackEntry = inlineStackEntry->m_caller) {
- flush(inlineStackEntry);
-
- ASSERT(origin.inlineCallFrame == inlineStackEntry->m_inlineCallFrame);
- InlineCallFrame* inlineCallFrame = inlineStackEntry->m_inlineCallFrame;
- CodeBlock* codeBlock = m_graph.baselineCodeBlockFor(inlineCallFrame);
- FullBytecodeLiveness& fullLiveness = m_graph.livenessFor(codeBlock);
- const FastBitVector& livenessAtBytecode = fullLiveness.getLiveness(bytecodeIndex);
-
- for (unsigned local = codeBlock->m_numCalleeLocals; local--;) {
- if (livenessAtBytecode[local]) {
- VirtualRegister reg = virtualRegisterForLocal(local);
- if (inlineCallFrame)
- reg = inlineStackEntry->remapOperand(reg);
- phantomLocalDirect(reg);
- }
- }
-
- if (inlineCallFrame) {
- bytecodeIndex = inlineCallFrame->directCaller.bytecodeIndex;
- origin = inlineCallFrame->directCaller;
- }
- }
+ auto addFlushDirect = [&] (VirtualRegister reg) { flushDirect(reg); };
+ auto addPhantomLocalDirect = [&] (VirtualRegister reg) { phantomLocalDirect(reg); };
+ flushForTerminalImpl(m_graph, currentCodeOrigin(), addFlushDirect, addPhantomLocalDirect);
}
void flushForReturn()
@@ -6606,19 +6612,20 @@
block->resize(nodeIndex + 1);
insertionSet.insertNode(block->size(), SpecNone, ExitOK, endOrigin);
- m_graph.forAllLiveInBytecode(endOrigin.semantic, [&] (VirtualRegister operand) {
+
+ auto insertLivenessPreservingOp = [&] (NodeType op, VirtualRegister operand) {
VariableAccessData* variable = mapping.operand(operand);
- if (!variable)
+ if (!variable) {
variable = newVariableAccessData(operand);
-
- auto op = PhantomLocal;
- if ((m_graph.needsScopeRegister() && operand == m_codeBlock->scopeRegister())
- || (operand.isArgument() && (operand != virtualRegisterForArgument(0) || m_graph.needsFlushedThis()))) {
- op = Flush;
+ mapping.operand(operand) = variable;
}
insertionSet.insertNode(block->size(), SpecNone, op, endOrigin, OpInfo(variable));
- });
+ };
+ auto addFlushDirect = [&] (VirtualRegister operand) { insertLivenessPreservingOp(Flush, operand); };
+ auto addPhantomLocalDirect = [&] (VirtualRegister operand) { insertLivenessPreservingOp(PhantomLocal, operand); };
+ flushForTerminalImpl(m_graph, endOrigin.semantic, addFlushDirect, addPhantomLocalDirect);
+
insertionSet.insertNode(block->size(), SpecNone, Unreachable, endOrigin);
insertionSet.execute(block);
break;
