Diff
Modified: branches/safari-605-branch/JSTests/ChangeLog (227652 => 227653)
--- branches/safari-605-branch/JSTests/ChangeLog 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/JSTests/ChangeLog 2018-01-26 05:02:53 UTC (rev 227653)
@@ -1,3 +1,19 @@
+2018-01-25 Jason Marcell <[email protected]>
+
+ Cherry-pick r227410. rdar://problem/36873404
+
+ 2018-01-23 Robin Morisset <[email protected]>
+
+ Update the argument count in DFGByteCodeParser::handleRecursiveCall
+ https://bugs.webkit.org/show_bug.cgi?id=181739
+ <rdar://problem/36627662>
+
+ Reviewed by Saam Barati.
+
+ * stress/recursive-tail-call-with-different-argument-count.js: Added.
+ (foo):
+ (bar):
+
2018-01-24 Ryan Haddad <[email protected]>
JSC test gardening for <rdar://problem/36827460>.
Added: branches/safari-605-branch/JSTests/stress/recursive-tail-call-with-different-argument-count.js (0 => 227653)
--- branches/safari-605-branch/JSTests/stress/recursive-tail-call-with-different-argument-count.js (rev 0)
+++ branches/safari-605-branch/JSTests/stress/recursive-tail-call-with-different-argument-count.js 2018-01-26 05:02:53 UTC (rev 227653)
@@ -0,0 +1,25 @@
+"use strict";
+function foo(x, y)
+{
+ if (arguments.length >= 2)
+ return foo(x+y)
+ return x;
+}
+noInline(foo);
+
+function bar(x)
+{
+ if (arguments.length >= 2)
+ return bar(arguments[0] + arguments[1])
+ return x;
+}
+noInline(bar);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo(40, 2);
+ if (result !== 42)
+ throw "Wrong result for foo, expected 42, got " + result;
+ result = bar(40, 2);
+ if (result !== 42)
+ throw "Wrong result for bar, expected 42, got " + result;
+}
Modified: branches/safari-605-branch/Source/_javascript_Core/ChangeLog (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/ChangeLog 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/ChangeLog 2018-01-26 05:02:53 UTC (rev 227653)
@@ -1,3 +1,57 @@
+2018-01-25 Jason Marcell <[email protected]>
+
+ Cherry-pick r227410. rdar://problem/36873404
+
+ 2018-01-23 Robin Morisset <[email protected]>
+
+ Update the argument count in DFGByteCodeParser::handleRecursiveCall
+ https://bugs.webkit.org/show_bug.cgi?id=181739
+ <rdar://problem/36627662>
+
+ Reviewed by Saam Barati.
+
+ When calling a function, its number of arguments is set on the stack. When we turn a recursive tail call
+ into a jump, we should update that stack slot as there is no guarantee that the function was originally
+ called with the same number of arguments. Forgetting to do this is observable through 'arguments.length'.
+
+ It required adding a new DFG node: 'SetArgumentCountIncludingThis', that takes an unsigned int
+ as its first OpInfo field, and stores it to the stack at the right place.
+
+ We must be a bit careful in where we put this new node, as it ClobbersExit.
+ We must also fix DFGArgumentsEliminationPhase and DFGPutStackSinkingPhase as they assumed that any node that writes to the stack must write to either an argument or a local.
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGArgumentsEliminationPhase.cpp:
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleRecursiveTailCall):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGDoesGC.cpp:
+ (JSC::DFG::doesGC):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGMayExit.cpp:
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::argumentCountIncludingThis):
+ * dfg/DFGNodeType.h:
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ * dfg/DFGPutStackSinkingPhase.cpp:
+ * dfg/DFGSafeToExecute.h:
+ (JSC::DFG::safeToExecute):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileSetArgumentCountIncludingThis):
+ * dfg/DFGSpeculativeJIT.h:
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+ (JSC::FTL::DFG::LowerDFGToB3::compileSetArgumentCountIncludingThis):
+
2018-01-24 Jason Marcell <[email protected]>
Cherry-pick r227527. rdar://problem/36830339
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-01-26 05:02:53 UTC (rev 227653)
@@ -2334,6 +2334,9 @@
case GetArgumentCountIncludingThis:
forNode(node).setType(SpecInt32Only);
break;
+
+ case SetArgumentCountIncludingThis:
+ break;
case GetRestLength:
forNode(node).setType(SpecInt32Only);
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -484,7 +484,9 @@
}
ASSERT(!heap.payload().isTop());
VirtualRegister reg(heap.payload().value32());
- clobberedByThisBlock.operand(reg) = true;
+ // The register may not point to an argument or local, for example if we are looking at SetArgumentCountIncludingThis.
+ if (!reg.isHeader())
+ clobberedByThisBlock.operand(reg) = true;
},
NoOpClobberize());
}
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -1396,6 +1396,8 @@
flushForTerminal();
// We must set the arguments to the right values
+ if (!stackEntry->m_inlineCallFrame)
+ addToGraph(SetArgumentCountIncludingThis, OpInfo(argumentCountIncludingThis));
int argIndex = 0;
for (; argIndex < argumentCountIncludingThis; ++argIndex) {
Node* value = get(virtualRegisterForArgument(argIndex, registerOffset));
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGClobberize.h (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGClobberize.h 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGClobberize.h 2018-01-26 05:02:53 UTC (rev 227653)
@@ -706,6 +706,10 @@
def(HeapLocation(StackPayloadLoc, AbstractHeap(Stack, CallFrameSlot::argumentCount)), LazyNode(node));
return;
+ case SetArgumentCountIncludingThis:
+ write(AbstractHeap(Stack, CallFrameSlot::argumentCount));
+ return;
+
case GetRestLength:
read(Stack);
return;
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGDoesGC.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -52,6 +52,7 @@
case IdentityWithProfile:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case GetRestLength:
case GetLocal:
case SetLocal:
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -2090,6 +2090,7 @@
case GetLocal:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case GetRestLength:
case GetArgument:
case Flush:
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGMayExit.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGMayExit.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGMayExit.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -74,6 +74,7 @@
case GetStack:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case GetRestLength:
case GetScope:
case PhantomLocal:
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGNode.h (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGNode.h 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGNode.h 2018-01-26 05:02:53 UTC (rev 227653)
@@ -960,6 +960,12 @@
ASSERT(hasStackAccessData());
return m_opInfo.as<StackAccessData*>();
}
+
+ unsigned argumentCountIncludingThis()
+ {
+ ASSERT(op() == SetArgumentCountIncludingThis);
+ return m_opInfo.as<unsigned>();
+ }
bool hasPhi()
{
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGNodeType.h (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGNodeType.h 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGNodeType.h 2018-01-26 05:02:53 UTC (rev 227653)
@@ -55,6 +55,7 @@
macro(CreateThis, NodeResultJS) /* Note this is not MustGenerate since we're returning it anyway. */ \
macro(GetCallee, NodeResultJS) \
macro(GetArgumentCountIncludingThis, NodeResultInt32) \
+ macro(SetArgumentCountIncludingThis, NodeMustGenerate) \
\
/* Nodes for local variable access. These nodes are linked together using Phi nodes. */\
/* Any two nodes that are part of the same Phi graph will share the same */\
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -766,6 +766,9 @@
break;
}
+ case SetArgumentCountIncludingThis:
+ break;
+
case MapHash:
setPrediction(SpecInt32Only);
break;
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -121,6 +121,8 @@
};
auto writeHandler = [&] (VirtualRegister operand) {
+ if (operand.isHeader())
+ return;
RELEASE_ASSERT(node->op() == PutStack || node->op() == LoadVarargs || node->op() == ForwardVarargs);
writes.append(operand);
};
@@ -288,6 +290,8 @@
};
auto writeHandler = [&] (VirtualRegister operand) {
+ if (operand.isHeader())
+ return;
RELEASE_ASSERT(node->op() == LoadVarargs || node->op() == ForwardVarargs);
deferred.operand(operand) = DeadFlush;
};
@@ -501,6 +505,8 @@
};
auto writeHandler = [&] (VirtualRegister operand) {
+ if (operand.isHeader())
+ return;
// LoadVarargs and ForwardVarargs are unconditional writes to the stack
// locations they claim to write to. They do not read from the stack
// locations they write to. This makes those stack locations dead right
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSafeToExecute.h (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-01-26 05:02:53 UTC (rev 227653)
@@ -172,6 +172,7 @@
case CreateThis:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case GetRestLength:
case GetLocal:
case SetLocal:
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -11276,6 +11276,12 @@
int32Result(result.gpr(), node);
}
+void SpeculativeJIT::compileSetArgumentCountIncludingThis(Node* node)
+{
+ m_jit.store32(TrustedImm32(node->argumentCountIncludingThis()), JITCompiler::payloadFor(CallFrameSlot::argumentCount));
+ noResult(node);
+}
+
void SpeculativeJIT::compileStrCat(Node* node)
{
JSValueOperand op1(this, node->child1(), ManualOperandSpeculation);
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2018-01-26 05:02:53 UTC (rev 227653)
@@ -3103,6 +3103,7 @@
void compileGetSetter(Node*);
void compileGetCallee(Node*);
void compileGetArgumentCountIncludingThis(Node*);
+ void compileSetArgumentCountIncludingThis(Node*);
void compileStrCat(Node*);
void compileNewArrayWithSize(Node*);
void compileNewTypedArray(Node*);
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -3798,6 +3798,10 @@
compileGetArgumentCountIncludingThis(node);
break;
}
+
+ case SetArgumentCountIncludingThis:
+ compileSetArgumentCountIncludingThis(node);
+ break;
case GetScope:
compileGetScope(node);
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -4006,6 +4006,10 @@
break;
}
+ case SetArgumentCountIncludingThis:
+ compileSetArgumentCountIncludingThis(node);
+ break;
+
case GetRestLength: {
compileGetRestLength(node);
break;
Modified: branches/safari-605-branch/Source/_javascript_Core/ftl/FTLCapabilities.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -180,6 +180,7 @@
case GetScope:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case ToNumber:
case ToString:
case ToObject:
Modified: branches/safari-605-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (227652 => 227653)
--- branches/safari-605-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-01-26 04:58:33 UTC (rev 227652)
+++ branches/safari-605-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-01-26 05:02:53 UTC (rev 227653)
@@ -918,6 +918,9 @@
case GetArgumentCountIncludingThis:
compileGetArgumentCountIncludingThis();
break;
+ case SetArgumentCountIncludingThis:
+ compileSetArgumentCountIncludingThis();
+ break;
case GetScope:
compileGetScope();
break;
@@ -6352,6 +6355,11 @@
{
setInt32(m_out.load32(payloadFor(CallFrameSlot::argumentCount)));
}
+
+ void compileSetArgumentCountIncludingThis()
+ {
+ m_out.store32(m_out.constInt32(m_node->argumentCountIncludingThis()), payloadFor(CallFrameSlot::argumentCount));
+ }
void compileGetScope()
{
