Diff
Modified: trunk/JSTests/ChangeLog (227409 => 227410)
--- trunk/JSTests/ChangeLog 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/JSTests/ChangeLog 2018-01-23 14:28:10 UTC (rev 227410)
@@ -1,3 +1,15 @@
+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-22 Michael Saboff <[email protected]>
DFG abstract interpreter needs to properly model effects of some Math ops
Added: trunk/JSTests/stress/recursive-tail-call-with-different-argument-count.js (0 => 227410)
--- trunk/JSTests/stress/recursive-tail-call-with-different-argument-count.js (rev 0)
+++ trunk/JSTests/stress/recursive-tail-call-with-different-argument-count.js 2018-01-23 14:28:10 UTC (rev 227410)
@@ -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: trunk/Source/_javascript_Core/ChangeLog (227409 => 227410)
--- trunk/Source/_javascript_Core/ChangeLog 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-01-23 14:28:10 UTC (rev 227410)
@@ -1,3 +1,53 @@
+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-22 Michael Saboff <[email protected]>
DFG abstract interpreter needs to properly model effects of some Math ops
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-01-23 14:28:10 UTC (rev 227410)
@@ -2345,6 +2345,9 @@
case GetArgumentCountIncludingThis:
forNode(node).setType(SpecInt32Only);
break;
+
+ case SetArgumentCountIncludingThis:
+ break;
case GetRestLength:
forNode(node).setType(SpecInt32Only);
Modified: trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -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: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -1403,6 +1403,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: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h 2018-01-23 14:28:10 UTC (rev 227410)
@@ -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: trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -52,6 +52,7 @@
case IdentityWithProfile:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case GetRestLength:
case GetLocal:
case SetLocal:
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -2099,6 +2099,7 @@
case GetLocal:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case GetRestLength:
case GetArgument:
case Flush:
Modified: trunk/Source/_javascript_Core/dfg/DFGMayExit.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGMayExit.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGMayExit.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -74,6 +74,7 @@
case GetStack:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case GetRestLength:
case GetScope:
case PhantomLocal:
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2018-01-23 14:28:10 UTC (rev 227410)
@@ -971,6 +971,12 @@
ASSERT(hasStackAccessData());
return m_opInfo.as<StackAccessData*>();
}
+
+ unsigned argumentCountIncludingThis()
+ {
+ ASSERT(op() == SetArgumentCountIncludingThis);
+ return m_opInfo.as<unsigned>();
+ }
bool hasPhi()
{
Modified: trunk/Source/_javascript_Core/dfg/DFGNodeType.h (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2018-01-23 14:28:10 UTC (rev 227410)
@@ -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: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -768,6 +768,9 @@
break;
}
+ case SetArgumentCountIncludingThis:
+ break;
+
case MapHash:
setPrediction(SpecInt32Only);
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGPutStackSinkingPhase.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -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: trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-01-23 14:28:10 UTC (rev 227410)
@@ -172,6 +172,7 @@
case CreateThis:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case GetRestLength:
case GetLocal:
case SetLocal:
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -11324,6 +11324,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: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2018-01-23 14:28:10 UTC (rev 227410)
@@ -3125,6 +3125,7 @@
void compileGetSetter(Node*);
void compileGetCallee(Node*);
void compileGetArgumentCountIncludingThis(Node*);
+ void compileSetArgumentCountIncludingThis(Node*);
void compileStrCat(Node*);
void compileNewArrayWithSize(Node*);
void compileNewTypedArray(Node*);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -3808,6 +3808,10 @@
compileGetArgumentCountIncludingThis(node);
break;
}
+
+ case SetArgumentCountIncludingThis:
+ compileSetArgumentCountIncludingThis(node);
+ break;
case GetScope:
compileGetScope(node);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -4016,6 +4016,10 @@
break;
}
+ case SetArgumentCountIncludingThis:
+ compileSetArgumentCountIncludingThis(node);
+ break;
+
case GetRestLength: {
compileGetRestLength(node);
break;
Modified: trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -180,6 +180,7 @@
case GetScope:
case GetCallee:
case GetArgumentCountIncludingThis:
+ case SetArgumentCountIncludingThis:
case ToNumber:
case ToString:
case ToObject:
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (227409 => 227410)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-01-23 12:07:42 UTC (rev 227409)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-01-23 14:28:10 UTC (rev 227410)
@@ -918,6 +918,9 @@
case GetArgumentCountIncludingThis:
compileGetArgumentCountIncludingThis();
break;
+ case SetArgumentCountIncludingThis:
+ compileSetArgumentCountIncludingThis();
+ break;
case GetScope:
compileGetScope();
break;
@@ -6343,6 +6346,11 @@
{
setInt32(m_out.load32(payloadFor(CallFrameSlot::argumentCount)));
}
+
+ void compileSetArgumentCountIncludingThis()
+ {
+ m_out.store32(m_out.constInt32(m_node->argumentCountIncludingThis()), payloadFor(CallFrameSlot::argumentCount));
+ }
void compileGetScope()
{
