The parser should not emit a ApplyFunctionCallDotNode for Reflect.apply.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Oct 2018 23:56:56 +0000 (23:56 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Oct 2018 23:56:56 +0000 (23:56 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237241 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CallVariant.cpp
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
Source/JavaScriptCore/parser/ASTBuilder.h

index 918c5ea..e79c0fe 100644 (file)
@@ -1,3 +1,27 @@
+2018-10-17  Mark Lam  <mark.lam@apple.com>
+
+        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):
+
 2018-10-17  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r237024.
index 3666a13..d20ea93 100644 (file)
@@ -88,7 +88,12 @@ void CallVariant::dump(PrintStream& out) const
         return;
     }
     
-    out.print("Executable: ", *executable());
+    if (ExecutableBase* executable = this->executable()) {
+        out.print("(Executable: ", *executable, ")");
+        return;
+    }
+
+    out.print("Non-executable callee: ", *nonExecutableCallee());
 }
 
 CallVariantList variantListWithVariant(const CallVariantList& list, CallVariant variantToAdd)
index 0437497..497b00d 100644 (file)
@@ -1443,6 +1443,7 @@ RegisterID* ApplyFunctionCallDotNode::emitBytecode(BytecodeGenerator& generator,
     generator.emitExpressionInfo(subexpressionDivot(), subexpressionStart(), subexpressionEnd());
     if (emitCallCheck) {
         makeFunction();
+        ASSERT(!m_base->isResolveNode() || static_cast<ResolveNode*>(m_base)->identifier() != "Reflect");
         generator.emitJumpIfNotFunctionApply(function.get(), realCall.get());
     }
     if (mayBeCall) {
index 8101900..622d291 100644 (file)
@@ -1367,12 +1367,16 @@ ExpressionNode* ASTBuilder::makeFunctionCallNode(const JSTokenLocation& location
     }
     ASSERT(func->isDotAccessorNode());
     DotAccessorNode* dot = static_cast<DotAccessorNode*>(func);
-    FunctionCallDotNode* node;
+    FunctionCallDotNode* node = nullptr;
     if (!previousBaseWasSuper && (dot->identifier() == m_vm->propertyNames->builtinNames().callPublicName() || dot->identifier() == m_vm->propertyNames->builtinNames().callPrivateName()))
         node = new (m_parserArena) CallFunctionCallDotNode(location, dot->base(), dot->identifier(), args, divot, divotStart, divotEnd, callOrApplyChildDepth);
-    else if (!previousBaseWasSuper && (dot->identifier() == m_vm->propertyNames->builtinNames().applyPublicName() || dot->identifier() == m_vm->propertyNames->builtinNames().applyPrivateName()))
-        node = new (m_parserArena) ApplyFunctionCallDotNode(location, dot->base(), dot->identifier(), args, divot, divotStart, divotEnd, callOrApplyChildDepth);
-    else
+    else if (!previousBaseWasSuper && (dot->identifier() == m_vm->propertyNames->builtinNames().applyPublicName() || dot->identifier() == m_vm->propertyNames->builtinNames().applyPrivateName())) {
+        // FIXME: This check is only needed because we haven't taught the bytecode generator to inline
+        // Reflect.apply yet. See https://bugs.webkit.org/show_bug.cgi?id=190668.
+        if (!dot->base()->isResolveNode() || static_cast<ResolveNode*>(dot->base())->identifier() != "Reflect")
+            node = new (m_parserArena) ApplyFunctionCallDotNode(location, dot->base(), dot->identifier(), args, divot, divotStart, divotEnd, callOrApplyChildDepth);
+    }
+    if (!node)
         node = new (m_parserArena) FunctionCallDotNode(location, dot->base(), dot->identifier(), args, divot, divotStart, divotEnd);
     node->setSubexpressionInfo(dot->divot(), dot->divotEnd().offset);
     return node;