Various arguments optimisations in codegen fail to account for arguments being in lexical record
https://bugs.webkit.org/show_bug.cgi?id=137617
Reviewed by Michael Saboff.
Rework the way we track |arguments| references so that we don't try
to use the |arguments| reference on the stack if it's not safe.
To do this without nuking performance it was necessary to update
the parser to track modification of the |arguments| reference
itself.
* bytecode/CodeBlock.cpp:
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::willResolveToArguments):
(JSC::BytecodeGenerator::uncheckedLocalArgumentsRegister):
(JSC::BytecodeGenerator::emitCall):
(JSC::BytecodeGenerator::emitConstruct):
(JSC::BytecodeGenerator::emitEnumeration):
(JSC::BytecodeGenerator::uncheckedRegisterForArguments): Deleted.
* bytecompiler/BytecodeGenerator.h:
(JSC::BytecodeGenerator::hasSafeLocalArgumentsRegister):
* bytecompiler/NodesCodegen.cpp:
(JSC::BracketAccessorNode::emitBytecode):
(JSC::DotAccessorNode::emitBytecode):
(JSC::getArgumentByVal):
(JSC::CallFunctionCallDotNode::emitBytecode):
(JSC::ApplyFunctionCallDotNode::emitBytecode):
(JSC::ArrayPatternNode::emitDirectBinding):
* interpreter/StackVisitor.cpp:
(JSC::StackVisitor::Frame::existingArguments):
* parser/Nodes.h:
(JSC::ScopeNode::modifiesArguments):
* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseInner):
* parser/Parser.h:
(JSC::Scope::getCapturedVariables):
* parser/ParserModes.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@174821 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
index f569cdb..327b5d8 100644
--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
@@ -167,6 +167,7 @@
, m_lexicalEnvironmentRegister(0)
, m_emptyValueRegister(0)
, m_globalObjectRegister(0)
+ , m_localArgumentsRegister(0)
, m_finallyDepth(0)
, m_localScopeDepth(0)
, m_codeType(GlobalCode)
@@ -211,6 +212,7 @@
, m_lexicalEnvironmentRegister(0)
, m_emptyValueRegister(0)
, m_globalObjectRegister(0)
+ , m_localArgumentsRegister(0)
, m_finallyDepth(0)
, m_localScopeDepth(0)
, m_codeType(FunctionCode)
@@ -250,6 +252,7 @@
emitOpcode(op_create_lexical_environment);
instructions().append(m_lexicalEnvironmentRegister->index());
}
+ RegisterID* localArgumentsRegister = nullptr;
RegisterID* scratch = addVar();
m_symbolTable->setCaptureStart(virtualRegisterForLocal(m_codeBlock->m_numVars).offset());
@@ -257,6 +260,8 @@
RegisterID* unmodifiedArgumentsRegister = addVar(); // Anonymous, so it can't be modified by user code.
RegisterID* argumentsRegister = addVar(propertyNames().arguments, IsVariable, NotWatchable); // Can be changed by assigning to 'arguments'.
+ localArgumentsRegister = argumentsRegister;
+
// We can save a little space by hard-coding the knowledge that the two
// 'arguments' values are stored in consecutive registers, and storing
// only the index of the assignable one.
@@ -274,6 +279,15 @@
initializeCapturedVariable(argumentsRegister, propertyNames().arguments, argumentsRegister);
RegisterID* uncheckedArgumentsRegister = ®isterFor(JSC::unmodifiedArgumentsRegister(m_codeBlock->argumentsRegister()).offset());
initializeCapturedVariable(uncheckedArgumentsRegister, propertyNames().arguments, uncheckedArgumentsRegister);
+ if (functionBody->modifiesArguments()) {
+ emitOpcode(op_mov);
+ instructions().append(argumentsRegister->index());
+ instructions().append(addConstantValue(jsUndefined())->index());
+ emitOpcode(op_mov);
+ instructions().append(uncheckedArgumentsRegister->index());
+ instructions().append(addConstantValue(jsUndefined())->index());
+ localArgumentsRegister = nullptr;
+ }
}
}
}
@@ -386,6 +400,7 @@
int nextParameterIndex = CallFrame::thisArgumentOffset();
m_thisRegister.setIndex(nextParameterIndex++);
m_codeBlock->addParameter();
+
for (size_t i = 0; i < parameters.size(); ++i, ++nextParameterIndex) {
int index = nextParameterIndex;
auto pattern = parameters.at(i);
@@ -419,6 +434,7 @@
instructions().append(0);
instructions().append(0);
}
+ m_localArgumentsRegister = localArgumentsRegister;
}
BytecodeGenerator::BytecodeGenerator(VM& vm, EvalNode* evalNode, UnlinkedEvalCodeBlock* codeBlock, DebuggerMode debuggerMode, ProfilerMode profilerMode)
@@ -431,6 +447,7 @@
, m_lexicalEnvironmentRegister(0)
, m_emptyValueRegister(0)
, m_globalObjectRegister(0)
+ , m_localArgumentsRegister(0)
, m_finallyDepth(0)
, m_localScopeDepth(0)
, m_codeType(EvalCode)
@@ -545,19 +562,17 @@
if (entry.isNull())
return false;
- if (m_codeBlock->usesArguments() && m_codeType == FunctionCode)
+ if (m_codeBlock->usesArguments() && m_codeType == FunctionCode && m_localArgumentsRegister)
return true;
return false;
}
-RegisterID* BytecodeGenerator::uncheckedRegisterForArguments()
+RegisterID* BytecodeGenerator::uncheckedLocalArgumentsRegister()
{
ASSERT(willResolveToArguments(propertyNames().arguments));
-
- SymbolTableEntry entry = symbolTable().get(propertyNames().arguments.impl());
- ASSERT(!entry.isNull());
- return ®isterFor(entry.getIndex());
+ ASSERT(m_localArgumentsRegister);
+ return m_localArgumentsRegister;
}
RegisterID* BytecodeGenerator::createLazyRegisterIfNecessary(RegisterID* reg)
@@ -1828,7 +1843,7 @@
auto expression = static_cast<SpreadExpressionNode*>(n->m_expr)->expression();
RefPtr<RegisterID> argumentRegister;
if (expression->isResolveNode() && willResolveToArguments(static_cast<ResolveNode*>(expression)->identifier()) && !symbolTable().slowArguments())
- argumentRegister = uncheckedRegisterForArguments();
+ argumentRegister = uncheckedLocalArgumentsRegister();
else
argumentRegister = expression->emitBytecode(*this, callArguments.argumentRegister(0));
RefPtr<RegisterID> thisRegister = emitMove(newTemporary(), callArguments.thisRegister());
@@ -1970,7 +1985,7 @@
auto expression = static_cast<SpreadExpressionNode*>(n->m_expr)->expression();
RefPtr<RegisterID> argumentRegister;
if (expression->isResolveNode() && willResolveToArguments(static_cast<ResolveNode*>(expression)->identifier()) && !symbolTable().slowArguments())
- argumentRegister = uncheckedRegisterForArguments();
+ argumentRegister = uncheckedLocalArgumentsRegister();
else
argumentRegister = expression->emitBytecode(*this, callArguments.argumentRegister(0));
return emitConstructVarargs(dst, func, argumentRegister.get(), newTemporary(), 0, callArguments.profileHookRegister(), divot, divotStart, divotEnd);
@@ -2538,13 +2553,13 @@
emitJump(loopCondition.get());
emitLabel(loopStart.get());
emitLoopHint();
- emitGetArgumentByVal(value.get(), uncheckedRegisterForArguments(), index.get());
+ emitGetArgumentByVal(value.get(), uncheckedLocalArgumentsRegister(), index.get());
callBack(*this, value.get());
emitLabel(scope->continueTarget());
emitInc(index.get());
emitLabel(loopCondition.get());
- RefPtr<RegisterID> length = emitGetArgumentsLength(newTemporary(), uncheckedRegisterForArguments());
+ RefPtr<RegisterID> length = emitGetArgumentsLength(newTemporary(), uncheckedLocalArgumentsRegister());
emitJumpIfTrue(emitEqualityOp(op_less, newTemporary(), index.get(), length.get()), loopStart.get());
emitLabel(scope->breakTarget());
return;
diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
index f7b1481..46d590d 100644
--- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
+++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
@@ -277,7 +277,9 @@
void setIsNumericCompareFunction(bool isNumericCompareFunction);
bool willResolveToArguments(const Identifier&);
- RegisterID* uncheckedRegisterForArguments();
+
+ bool hasSafeLocalArgumentsRegister() { return m_localArgumentsRegister; }
+ RegisterID* uncheckedLocalArgumentsRegister();
bool isCaptured(int operand);
CaptureMode captureMode(int operand) { return isCaptured(operand) ? IsCaptured : NotCaptured; }
@@ -752,6 +754,8 @@
RegisterID* m_lexicalEnvironmentRegister;
RegisterID* m_emptyValueRegister;
RegisterID* m_globalObjectRegister;
+ RegisterID* m_localArgumentsRegister;
+
Vector<Identifier, 16> m_watchableVariables;
SegmentedVector<RegisterID, 32> m_constantPoolRegisters;
SegmentedVector<RegisterID, 32> m_calleeRegisters;
diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
index 4b4e5a7..b992bc62 100644
--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
@@ -386,7 +386,7 @@
&& !generator.symbolTable().slowArguments()) {
RegisterID* property = generator.emitNode(m_subscript);
generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
- return generator.emitGetArgumentByVal(generator.finalDestination(dst), generator.uncheckedRegisterForArguments(), property);
+ return generator.emitGetArgumentByVal(generator.finalDestination(dst), generator.uncheckedLocalArgumentsRegister(), property);
}
RefPtr<RegisterID> base = generator.emitNodeForLeftHandSide(m_base, m_subscriptHasAssignments, m_subscript->isPure(generator));
@@ -412,7 +412,7 @@
if (!generator.willResolveToArguments(resolveNode->identifier()))
goto nonArgumentsPath;
generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
- return generator.emitGetArgumentsLength(generator.finalDestination(dst), generator.uncheckedRegisterForArguments());
+ return generator.emitGetArgumentsLength(generator.finalDestination(dst), generator.uncheckedLocalArgumentsRegister());
}
nonArgumentsPath:
@@ -593,7 +593,7 @@
&& generator.willResolveToArguments(static_cast<ResolveNode*>(base)->identifier())
&& !generator.symbolTable().slowArguments()) {
generator.emitExpressionInfo(divot, divotStart, divotEnd);
- return generator.emitGetArgumentByVal(generator.finalDestination(dst), generator.uncheckedRegisterForArguments(), property);
+ return generator.emitGetArgumentByVal(generator.finalDestination(dst), generator.uncheckedLocalArgumentsRegister(), property);
}
return nullptr;
}
@@ -621,7 +621,7 @@
RefPtr<RegisterID> thisRegister = getArgumentByVal(generator, subject, generator.emitLoad(0, jsNumber(0)), 0, spread->divot(), spread->divotStart(), spread->divotEnd());
RefPtr<RegisterID> argumentsRegister;
if (thisRegister)
- argumentsRegister = generator.uncheckedRegisterForArguments();
+ argumentsRegister = generator.uncheckedLocalArgumentsRegister();
else {
argumentsRegister = generator.emitNode(subject);
generator.emitExpressionInfo(spread->divot(), spread->divotStart(), spread->divotEnd());
@@ -749,7 +749,7 @@
RefPtr<RegisterID> argsRegister;
ArgumentListNode* args = m_args->m_listNode->m_next;
if (args->m_expr->isResolveNode() && generator.willResolveToArguments(static_cast<ResolveNode*>(args->m_expr)->identifier()) && !generator.symbolTable().slowArguments())
- argsRegister = generator.uncheckedRegisterForArguments();
+ argsRegister = generator.uncheckedLocalArgumentsRegister();
else
argsRegister = generator.emitNode(args->m_expr);
@@ -2721,7 +2721,7 @@
{
if (rhs->isResolveNode()
&& generator.willResolveToArguments(static_cast<ResolveNode*>(rhs)->identifier())
- && !generator.symbolTable().slowArguments()) {
+ && generator.hasSafeLocalArgumentsRegister()&& !generator.symbolTable().slowArguments()) {
for (size_t i = 0; i < m_targetPatterns.size(); i++) {
auto target = m_targetPatterns[i];
if (!target)
@@ -2729,7 +2729,7 @@
RefPtr<RegisterID> temp = generator.newTemporary();
generator.emitLoad(temp.get(), jsNumber(i));
- generator.emitGetArgumentByVal(temp.get(), generator.uncheckedRegisterForArguments(), temp.get());
+ generator.emitGetArgumentByVal(temp.get(), generator.uncheckedLocalArgumentsRegister(), temp.get());
target->bindValue(generator, temp.get());
}
if (dst == generator.ignoredResult() || !dst)
