Skip to content

Commit e9447c3

Browse files
author
Mark Lam
committed
The parser should not emit a ApplyFunctionCallDotNode for Reflect.apply.
https://bugs.webkit.org/show_bug.cgi?id=190671 <rdar://problem/45201145> Reviewed by Saam Barati. The bytecode generator does not currently know how to inline Reflect.apply (see https://bugs.webkit.org/show_bug.cgi?id=190668). Hence, it's a waste of time to emit the ApplyFunctionCallDotNode since the function check against Function.apply that it will generate will always fail. Also fixed CallVariant::dump() to be able to handle dumping a non-executable callee. Reflect.apply used to trip this up. Any object with an apply property invoked as a function could also trip this up. This is now fixed. * bytecode/CallVariant.cpp: (JSC::CallVariant::dump const): * bytecompiler/NodesCodegen.cpp: (JSC::ApplyFunctionCallDotNode::emitBytecode): * parser/ASTBuilder.h: (JSC::ASTBuilder::makeFunctionCallNode): Canonical link: https://commits.webkit.org/205612@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237241 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 0ac5c10 commit e9447c3

4 files changed

Lines changed: 39 additions & 5 deletions

File tree

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,27 @@
1+
2018-10-17 Mark Lam <mark.lam@apple.com>
2+
3+
The parser should not emit a ApplyFunctionCallDotNode for Reflect.apply.
4+
https://bugs.webkit.org/show_bug.cgi?id=190671
5+
<rdar://problem/45201145>
6+
7+
Reviewed by Saam Barati.
8+
9+
The bytecode generator does not currently know how to inline Reflect.apply (see
10+
https://bugs.webkit.org/show_bug.cgi?id=190668). Hence, it's a waste of time to
11+
emit the ApplyFunctionCallDotNode since the function check against Function.apply
12+
that it will generate will always fail.
13+
14+
Also fixed CallVariant::dump() to be able to handle dumping a non-executable
15+
callee. Reflect.apply used to trip this up. Any object with an apply property
16+
invoked as a function could also trip this up. This is now fixed.
17+
18+
* bytecode/CallVariant.cpp:
19+
(JSC::CallVariant::dump const):
20+
* bytecompiler/NodesCodegen.cpp:
21+
(JSC::ApplyFunctionCallDotNode::emitBytecode):
22+
* parser/ASTBuilder.h:
23+
(JSC::ASTBuilder::makeFunctionCallNode):
24+
125
2018-10-17 Commit Queue <commit-queue@webkit.org>
226

327
Unreviewed, rolling out r237024.

‎Source/JavaScriptCore/bytecode/CallVariant.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,12 @@ void CallVariant::dump(PrintStream& out) const
8888
return;
8989
}
9090

91-
out.print("Executable: ", *executable());
91+
if (ExecutableBase* executable = this->executable()) {
92+
out.print("(Executable: ", *executable, ")");
93+
return;
94+
}
95+
96+
out.print("Non-executable callee: ", *nonExecutableCallee());
9297
}
9398

9499
CallVariantList variantListWithVariant(const CallVariantList& list, CallVariant variantToAdd)

‎Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1443,6 +1443,7 @@ RegisterID* ApplyFunctionCallDotNode::emitBytecode(BytecodeGenerator& generator,
14431443
generator.emitExpressionInfo(subexpressionDivot(), subexpressionStart(), subexpressionEnd());
14441444
if (emitCallCheck) {
14451445
makeFunction();
1446+
ASSERT(!m_base->isResolveNode() || static_cast<ResolveNode*>(m_base)->identifier() != "Reflect");
14461447
generator.emitJumpIfNotFunctionApply(function.get(), realCall.get());
14471448
}
14481449
if (mayBeCall) {

‎Source/JavaScriptCore/parser/ASTBuilder.h‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,12 +1367,16 @@ ExpressionNode* ASTBuilder::makeFunctionCallNode(const JSTokenLocation& location
13671367
}
13681368
ASSERT(func->isDotAccessorNode());
13691369
DotAccessorNode* dot = static_cast<DotAccessorNode*>(func);
1370-
FunctionCallDotNode* node;
1370+
FunctionCallDotNode* node = nullptr;
13711371
if (!previousBaseWasSuper && (dot->identifier() == m_vm->propertyNames->builtinNames().callPublicName() || dot->identifier() == m_vm->propertyNames->builtinNames().callPrivateName()))
13721372
node = new (m_parserArena) CallFunctionCallDotNode(location, dot->base(), dot->identifier(), args, divot, divotStart, divotEnd, callOrApplyChildDepth);
1373-
else if (!previousBaseWasSuper && (dot->identifier() == m_vm->propertyNames->builtinNames().applyPublicName() || dot->identifier() == m_vm->propertyNames->builtinNames().applyPrivateName()))
1374-
node = new (m_parserArena) ApplyFunctionCallDotNode(location, dot->base(), dot->identifier(), args, divot, divotStart, divotEnd, callOrApplyChildDepth);
1375-
else
1373+
else if (!previousBaseWasSuper && (dot->identifier() == m_vm->propertyNames->builtinNames().applyPublicName() || dot->identifier() == m_vm->propertyNames->builtinNames().applyPrivateName())) {
1374+
// FIXME: This check is only needed because we haven't taught the bytecode generator to inline
1375+
// Reflect.apply yet. See https://bugs.webkit.org/show_bug.cgi?id=190668.
1376+
if (!dot->base()->isResolveNode() || static_cast<ResolveNode*>(dot->base())->identifier() != "Reflect")
1377+
node = new (m_parserArena) ApplyFunctionCallDotNode(location, dot->base(), dot->identifier(), args, divot, divotStart, divotEnd, callOrApplyChildDepth);
1378+
}
1379+
if (!node)
13761380
node = new (m_parserArena) FunctionCallDotNode(location, dot->base(), dot->identifier(), args, divot, divotStart, divotEnd);
13771381
node->setSubexpressionInfo(dot->divot(), dot->divotEnd().offset);
13781382
return node;

0 commit comments

Comments
 (0)