How we load new.target in arrow functions is broken
authorgskachkov@gmail.com <gskachkov@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Mar 2016 08:19:42 +0000 (08:19 +0000)
committergskachkov@gmail.com <gskachkov@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Mar 2016 08:19:42 +0000 (08:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155153

Reviewed by Saam Barati.

Fixed not correct approach of caching new.target. In current patch was added code feature
flag that shows that current function is using new.target, when generating byte code an arrow
function we are loading new.target value to its register from arrow function lexical environment.

* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::emitLoadNewTargetFromArrowFunctionLexicalEnvironment):
* bytecompiler/BytecodeGenerator.h:
(JSC::BytecodeGenerator::newTarget):
* parser/ASTBuilder.h:
(JSC::ASTBuilder::createNewTargetExpr):
(JSC::ASTBuilder::usesNewTarget):
* parser/Nodes.h:
(JSC::ScopeNode::usesNewTarget):
* parser/ParserModes.h:
* tests/stress/arrowfunction-lexical-bind-newtarget.js:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
Source/JavaScriptCore/parser/ASTBuilder.h
Source/JavaScriptCore/parser/Nodes.h
Source/JavaScriptCore/parser/ParserModes.h
Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-newtarget.js

index f7f1fa5..4a9ddf5 100644 (file)
@@ -1,3 +1,27 @@
+2016-03-08  Skachkov Oleksandr  <gskachkov@gmail.com>
+
+        How we load new.target in arrow functions is broken
+        https://bugs.webkit.org/show_bug.cgi?id=155153
+
+        Reviewed by Saam Barati.
+
+        Fixed not correct approach of caching new.target. In current patch was added code feature
+        flag that shows that current function is using new.target, when generating byte code an arrow 
+        function we are loading new.target value to its register from arrow function lexical environment. 
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::BytecodeGenerator):
+        (JSC::BytecodeGenerator::emitLoadNewTargetFromArrowFunctionLexicalEnvironment):
+        * bytecompiler/BytecodeGenerator.h:
+        (JSC::BytecodeGenerator::newTarget):
+        * parser/ASTBuilder.h:
+        (JSC::ASTBuilder::createNewTargetExpr):
+        (JSC::ASTBuilder::usesNewTarget):
+        * parser/Nodes.h:
+        (JSC::ScopeNode::usesNewTarget):
+        * parser/ParserModes.h:
+        * tests/stress/arrowfunction-lexical-bind-newtarget.js:
+
 2016-03-09  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Get a RemoteObject or ObjectPreview from HeapSnapshot Object Identifier
index 5ce15f8..ed1ba0c 100644 (file)
@@ -572,8 +572,13 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionNode* functionNode, Unlinke
     // Loading |this| inside an arrow function must be done after initializeDefaultParameterValuesAndSetupFunctionScopeStack()
     // because that function sets up the SymbolTable stack and emitLoadThisFromArrowFunctionLexicalEnvironment()
     // consults the SymbolTable stack
-    if (SourceParseMode::ArrowFunctionMode == parseMode && (functionNode->usesThis() || functionNode->usesSuperProperty()))
-        emitLoadThisFromArrowFunctionLexicalEnvironment();
+    if (SourceParseMode::ArrowFunctionMode == parseMode) {
+        if (functionNode->usesThis() || functionNode->usesSuperProperty())
+            emitLoadThisFromArrowFunctionLexicalEnvironment();
+    
+        if (m_scopeNode->usesNewTarget() || m_scopeNode->usesSuperCall())
+            emitLoadNewTargetFromArrowFunctionLexicalEnvironment();
+    }
     
     if (needsToUpdateArrowFunctionContext() && !codeBlock->isArrowFunction()) {
         initializeArrowFunctionContextScopeIfNeeded(functionSymbolTable);
@@ -4104,12 +4109,10 @@ void BytecodeGenerator::emitLoadThisFromArrowFunctionLexicalEnvironment()
     
 RegisterID* BytecodeGenerator::emitLoadNewTargetFromArrowFunctionLexicalEnvironment()
 {
-    m_isNewTargetLoadedInArrowFunction = true;
-
     Variable newTargetVar = variable(propertyNames().newTargetLocalPrivateName);
-    emitMove(m_newTargetRegister, emitGetFromScope(newTemporary(), emitLoadArrowFunctionLexicalEnvironment(propertyNames().newTargetLocalPrivateName), newTargetVar, ThrowIfNotFound));
+
+    return emitGetFromScope(m_newTargetRegister, emitLoadArrowFunctionLexicalEnvironment(propertyNames().newTargetLocalPrivateName), newTargetVar, ThrowIfNotFound);
     
-    return m_newTargetRegister;
 }
 
 RegisterID* BytecodeGenerator::emitLoadDerivedConstructorFromArrowFunctionLexicalEnvironment()
index 6155568..fef8d84 100644 (file)
@@ -303,8 +303,7 @@ namespace JSC {
         RegisterID* argumentsRegister() { return m_argumentsRegister; }
         RegisterID* newTarget()
         {
-            return !m_codeBlock->isArrowFunction() || m_isNewTargetLoadedInArrowFunction
-                ? m_newTargetRegister : emitLoadNewTargetFromArrowFunctionLexicalEnvironment();
+            return m_newTargetRegister;
         }
 
         RegisterID* scopeRegister() { return m_scopeRegister; }
@@ -938,7 +937,6 @@ namespace JSC {
         bool m_usesNonStrictEval { false };
         bool m_inTailPosition { false };
         bool m_needsToUpdateArrowFunctionContext;
-        bool m_isNewTargetLoadedInArrowFunction { false };
         DerivedContextType m_derivedContextType { DerivedContextType::None };
     };
 
index b2c6a9f..3ffc16a 100644 (file)
@@ -179,6 +179,7 @@ public:
     }
     ExpressionNode* createNewTargetExpr(const JSTokenLocation location)
     {
+        usesNewTarget();
         return new (m_parserArena) NewTargetNode(location);
     }
     ExpressionNode* createResolve(const JSTokenLocation& location, const Identifier& ident, const JSTextPosition& start, const JSTextPosition& end)
@@ -955,6 +956,7 @@ private:
         m_evalCount++;
         m_scope.m_features |= EvalFeature;
     }
+    void usesNewTarget() { m_scope.m_features |= NewTargetFeature; }
     ExpressionNode* createIntegerLikeNumber(const JSTokenLocation& location, double d)
     {
         return new (m_parserArena) IntegerNode(location, d);
index 7a39b2d..852ee86 100644 (file)
@@ -1591,6 +1591,7 @@ namespace JSC {
         bool usesThis() const { return m_features & ThisFeature; }
         bool usesSuperCall() const { return m_features & SuperCallFeature; }
         bool usesSuperProperty() const { return m_features & SuperPropertyFeature; }
+        bool usesNewTarget() const { return m_features & NewTargetFeature; }
         bool needsActivation() const { return (hasCapturedVariables()) || (m_features & (EvalFeature | WithFeature)); }
         bool hasCapturedVariables() const { return m_varDeclarations.hasCapturedVariables(); }
         bool captures(UniquedStringImpl* uid) { return m_varDeclarations.captures(uid); }
index 08213c0..320ce9f 100644 (file)
@@ -160,9 +160,10 @@ const CodeFeatures ArrowFunctionFeature =        1 << 8;
 const CodeFeatures ArrowFunctionContextFeature = 1 << 9;
 const CodeFeatures SuperCallFeature =            1 << 10;
 const CodeFeatures SuperPropertyFeature =        1 << 11;
+const CodeFeatures NewTargetFeature =            1 << 12;
 
 const CodeFeatures AllFeatures = EvalFeature | ArgumentsFeature | WithFeature | ThisFeature | StrictModeFeature | ShadowsArgumentsFeature | ModifiedParameterFeature | ArrowFunctionFeature | ArrowFunctionContextFeature |
-    SuperCallFeature | SuperPropertyFeature;
+    SuperCallFeature | SuperPropertyFeature | NewTargetFeature;
 
 typedef uint8_t InnerArrowFunctionCodeFeatures;
     
index 3976260..032664d 100644 (file)
@@ -12,12 +12,33 @@ noInline(getTarget)
 
 for (var i=0; i < 1000; i++) {
     var undefinedTarget = getTarget()();
-    testCase(undefinedTarget, undefined, "Error: new.target is not lexically binded inside of the arrow function #1");
+    testCase(undefinedTarget, undefined, "Error: new.target is not lexically binded inside of the arrow function #1.0");
 }
 
 for (var i = 0; i < 1000; i++) {
     var newTarget = new getTarget()();
-    testCase(newTarget, getTarget, "Error: new.target is not lexically binded inside of the arrow function #2");
+    testCase(newTarget, getTarget, "Error: new.target is not lexically binded inside of the arrow function #2.0");
+}
+
+function getTargetWithBlock(name) {
+    return x => {
+        if (false)
+            return new.target;
+        else
+            return new.target;
+    }
+}
+
+noInline(getTargetWithBlock);
+
+for (var i=0; i < 1000; i++) {
+    var undefinedTarget = getTargetWithBlock()();
+    testCase(undefinedTarget, undefined, "Error: new.target is not lexically binded inside of the arrow function #1.1");
+}
+
+for (var i = 0; i < 1000; i++) {
+    var newTarget = new getTargetWithBlock()();
+    testCase(newTarget, getTargetWithBlock, "Error: new.target is not lexically binded inside of the arrow function #2.1");
 }
 
 var passed = false;
@@ -45,6 +66,7 @@ for (var i = 0; i < 1000; i++) {
     testCase(passed, true, "Error: new.target is not lexically binded inside of the arrow function in constructor #3");
 }
 
+// newTargetLocal - is hidden variable that emited for arrow function
 var C = class C extends A {
     constructor(tryToAccessToVarInArrow) {
         var f = () => {
@@ -66,7 +88,7 @@ var tryToCreateClass = function (val) {
         new C(val);
     }
     catch (e) {
-        result = e instanceof ReferenceError; 
+        result = e instanceof ReferenceError;
     }
 
     return result;
@@ -90,3 +112,29 @@ for (var i = 0; i < 1000; i++) {
     var undefinedTarget = getTargetBlockScope()()
     testCase(undefinedTarget, undefined, "Error: new.target is not lexically binded inside of the arrow function #4");
 }
+
+class D {
+    getNewTarget() {
+        var arr = () => {
+            if (false) {
+                return new.target;
+            } else {
+                return new.target;
+            }
+        }
+        return arr();
+    }
+};
+
+class E extends D {
+    getParentNewTarget() {
+        return super.getNewTarget();
+    }
+}
+
+var e = new E();
+
+for (var i = 0; i < 1000; i++) {
+    var parentNewTarget = e.getParentNewTarget();
+    testCase(parentNewTarget, undefined, "Error: new.target is not lexically binded inside of the arrow function #5");
+}