- Revision
- 194340
- Author
- [email protected]
- Date
- 2015-12-21 12:40:04 -0800 (Mon, 21 Dec 2015)
Log Message
Unexpected exception assigning to this._property inside arrow function
https://bugs.webkit.org/show_bug.cgi?id=152028
Patch by Skachkov Oleksandr <[email protected]> on 2015-12-21
Reviewed by Saam Barati.
Source/_javascript_Core:
The issue appeared in case if in arrow function created base-level lexical envioronment, and in this case
|this| value was loaded from wrong scope. The problem was that loading of the |this| happened too early when
compiling bytecode because the bytecode generators's scope stack wasn't in sync with runtime scope stack.
To fix issue loading of |this| was moved after initializeDefaultParameterValuesAndSetupFunctionScopeStack
in BytecodeGenerator.cpp
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* tests/stress/arrowfunction-lexical-bind-this-2.js:
LayoutTests:
Added new test cases for arrow function, to force create lexical env in body of function.
* js/arrowfunction-lexical-bind-this-expected.txt:
* js/script-tests/arrowfunction-lexical-bind-this.js:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (194339 => 194340)
--- trunk/LayoutTests/ChangeLog 2015-12-21 20:26:54 UTC (rev 194339)
+++ trunk/LayoutTests/ChangeLog 2015-12-21 20:40:04 UTC (rev 194340)
@@ -1,3 +1,15 @@
+2015-12-21 Skachkov Oleksandr <[email protected]>
+
+ Unexpected exception assigning to this._property inside arrow function
+ https://bugs.webkit.org/show_bug.cgi?id=152028
+
+ Reviewed by Saam Barati.
+
+ Added new test cases for arrow function, to force create lexical env in body of function.
+
+ * js/arrowfunction-lexical-bind-this-expected.txt:
+ * js/script-tests/arrowfunction-lexical-bind-this.js:
+
2015-12-21 Ryan Haddad <[email protected]>
Marking inspector/debugger/command-line-api-exception-nested-catch.html as a flaky timeout on mac-wk1
Modified: trunk/LayoutTests/js/arrowfunction-lexical-bind-this-expected.txt (194339 => 194340)
--- trunk/LayoutTests/js/arrowfunction-lexical-bind-this-expected.txt 2015-12-21 20:26:54 UTC (rev 194339)
+++ trunk/LayoutTests/js/arrowfunction-lexical-bind-this-expected.txt 2015-12-21 20:40:04 UTC (rev 194340)
@@ -16,6 +16,9 @@
PASS real.borrow()() === real is true
PASS arrowFunction('-this') is "right-this"
PASS hostObj.func('-this') is "right-this"
+PASS arrowWithEval.func() is "new-value"
+PASS fooObject.arr() is internal_value_1
+PASS fooObject._id is internal_value_2
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/js/script-tests/arrowfunction-lexical-bind-this.js (194339 => 194340)
--- trunk/LayoutTests/js/script-tests/arrowfunction-lexical-bind-this.js 2015-12-21 20:26:54 UTC (rev 194339)
+++ trunk/LayoutTests/js/script-tests/arrowfunction-lexical-bind-this.js 2015-12-21 20:40:04 UTC (rev 194340)
@@ -1,10 +1,10 @@
description('Tests for ES6 arrow function lexical bind of this');
function Dog(name) {
- this.name = name;
- this.getName = () => eval("this.name");
- this.getNameHard = () => eval("(() => this.name)()");
- this.getNameNesting = () => () => () => this.name;
+ this.name = name;
+ this.getName = () => eval("this.name");
+ this.getNameHard = () => eval("(() => this.name)()");
+ this.getNameNesting = () => () => () => this.name;
}
var d = new Dog("Max");
@@ -13,10 +13,10 @@
shouldBe('d.getNameNesting()()()', 'd.name');
var obj = {
- name : 'objCode',
- method : function () {
- return (value) => this.name + "-name-" + value;
- }
+ name : 'objCode',
+ method : function () {
+ return (value) => this.name + "-name-" + value;
+ }
};
shouldBe("obj.method()('correct')", "'objCode-name-correct'");
@@ -24,11 +24,11 @@
shouldBe("obj.method()('correct')", "'newObjCode-name-correct'");
var deepObj = {
- name : 'wrongObjCode',
- internalObject: {
- name :'internalObject',
- method: function () { return (value) => this.name + "-name-" + value; }
- }
+ name : 'wrongObjCode',
+ internalObject: {
+ name :'internalObject',
+ method: function () { return (value) => this.name + "-name-" + value; }
+ }
};
shouldBe("deepObj.internalObject.method()('correct')", "'internalObject-name-correct'");
@@ -37,7 +37,7 @@
shouldBe("deepObj.internalObject.method()('correct')", "'newInternalObject-name-correct'");
var functionConstructor = function () {
- this.func = () => this;
+ this.func = () => this;
};
var instance = new functionConstructor();
@@ -45,9 +45,9 @@
shouldBe("instance.func() === instance", "true");
var ownerObj = {
- method : function () {
- return () => this;
- }
+ method : function () {
+ return () => this;
+ }
};
shouldBe("ownerObj.method()() === ownerObj", "true");
@@ -64,4 +64,38 @@
shouldBe("arrowFunction('-this')", '"right-this"');
shouldBe("hostObj.func('-this')", '"right-this"');
+var functionConstructorWithEval = function () {
+ this._id = 'old-value';
+ this.func = () => {
+ var f;
+ eval('10==10');
+ this._id = 'new-value';
+ return this._id;
+ };
+};
+
+var arrowWithEval = new functionConstructorWithEval();
+
+shouldBe("arrowWithEval.func()", '"new-value"');
+
+var internal_value_1 = 123;
+var internal_value_2 = '1234';
+
+function foo() {
+ let arr = () => {
+ var x = internal_value_1;
+ function bas() {
+ return x;
+ };
+ this._id = internal_value_2;
+ return bas();
+ };
+ this.arr = arr;
+};
+
+var fooObject = new foo();
+
+shouldBe("fooObject.arr()", 'internal_value_1');
+shouldBe("fooObject._id", 'internal_value_2');
+
var successfullyParsed = true;
Modified: trunk/Source/_javascript_Core/ChangeLog (194339 => 194340)
--- trunk/Source/_javascript_Core/ChangeLog 2015-12-21 20:26:54 UTC (rev 194339)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-12-21 20:40:04 UTC (rev 194340)
@@ -1,3 +1,20 @@
+2015-12-21 Skachkov Oleksandr <[email protected]>
+
+ Unexpected exception assigning to this._property inside arrow function
+ https://bugs.webkit.org/show_bug.cgi?id=152028
+
+ Reviewed by Saam Barati.
+
+ The issue appeared in case if in arrow function created base-level lexical envioronment, and in this case
+ |this| value was loaded from wrong scope. The problem was that loading of the |this| happened too early when
+ compiling bytecode because the bytecode generators's scope stack wasn't in sync with runtime scope stack.
+ To fix issue loading of |this| was moved after initializeDefaultParameterValuesAndSetupFunctionScopeStack
+ in BytecodeGenerator.cpp
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::BytecodeGenerator):
+ * tests/stress/arrowfunction-lexical-bind-this-2.js:
+
2015-12-21 Filip Pizlo <[email protected]>
FTL B3 should do vararg calls
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (194339 => 194340)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2015-12-21 20:26:54 UTC (rev 194339)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2015-12-21 20:40:04 UTC (rev 194340)
@@ -521,12 +521,6 @@
m_newTargetRegister = addVar();
switch (parseMode) {
- case SourceParseMode::ArrowFunctionMode: {
- if (functionNode->usesThis())
- emitLoadThisFromArrowFunctionLexicalEnvironment();
- break;
- }
-
case SourceParseMode::GeneratorWrapperFunctionMode: {
m_generatorRegister = addVar();
@@ -550,20 +544,22 @@
}
default: {
- if (isConstructor()) {
- emitMove(m_newTargetRegister, &m_thisRegister);
- if (constructorKind() == ConstructorKind::Derived)
- emitMoveEmptyValue(&m_thisRegister);
- else
- emitCreateThis(&m_thisRegister);
- } else if (constructorKind() != ConstructorKind::None) {
- emitThrowTypeError("Cannot call a class constructor");
- } else if (functionNode->usesThis() || codeBlock->usesEval()) {
- m_codeBlock->addPropertyAccessInstruction(instructions().size());
- emitOpcode(op_to_this);
- instructions().append(kill(&m_thisRegister));
- instructions().append(0);
- instructions().append(0);
+ if (SourceParseMode::ArrowFunctionMode != parseMode) {
+ if (isConstructor()) {
+ emitMove(m_newTargetRegister, &m_thisRegister);
+ if (constructorKind() == ConstructorKind::Derived)
+ emitMoveEmptyValue(&m_thisRegister);
+ else
+ emitCreateThis(&m_thisRegister);
+ } else if (constructorKind() != ConstructorKind::None) {
+ emitThrowTypeError("Cannot call a class constructor");
+ } else if (functionNode->usesThis() || codeBlock->usesEval()) {
+ m_codeBlock->addPropertyAccessInstruction(instructions().size());
+ emitOpcode(op_to_this);
+ instructions().append(kill(&m_thisRegister));
+ instructions().append(0);
+ instructions().append(0);
+ }
}
break;
}
@@ -573,6 +569,12 @@
// because a function's default parameter ExpressionNodes will use temporary registers.
m_TDZStack.append(std::make_pair(*parentScopeTDZVariables, false));
initializeDefaultParameterValuesAndSetupFunctionScopeStack(parameters, functionNode, functionSymbolTable, symbolTableConstantIndex, captures);
+
+ // 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 (parseMode == SourceParseMode::ArrowFunctionMode && functionNode->usesThis())
+ emitLoadThisFromArrowFunctionLexicalEnvironment();
if (needsToUpdateArrowFunctionContext() && !codeBlock->isArrowFunction()) {
initializeArrowFunctionContextScopeIfNeeded(functionSymbolTable);
Modified: trunk/Source/_javascript_Core/tests/stress/arrowfunction-lexical-bind-this-2.js (194339 => 194340)
--- trunk/Source/_javascript_Core/tests/stress/arrowfunction-lexical-bind-this-2.js 2015-12-21 20:26:54 UTC (rev 194339)
+++ trunk/Source/_javascript_Core/tests/stress/arrowfunction-lexical-bind-this-2.js 2015-12-21 20:40:04 UTC (rev 194340)
@@ -1,11 +1,11 @@
var testCase = function (actual, expected, message) {
- if (actual !== expected) {
- throw message + ". Expected '" + expected + "', but was '" + actual + "'";
- }
+ if (actual !== expected) {
+ throw message + ". Expected '" + expected + "', but was '" + actual + "'";
+ }
};
var functionConstructor = function () {
- this.func = () => this;
+ this.func = () => this;
};
var instance = new functionConstructor();
@@ -13,9 +13,9 @@
testCase(instance.func() === instance, true, "Error: this is not lexically binded inside of the arrow function #2");
var obj = {
- method: function () {
- return () => this;
- }
+ method: function () {
+ return () => this;
+ }
};
noInline(obj.method);
@@ -37,3 +37,40 @@
for (var i=0; i < 10000; i++) {
testCase(real.borrow()() === real, true, "Error: this is not lexically binded inside of the arrow function #5");
}
+
+// Force create the lexical env inside of arrow function
+
+var functionConstructorWithEval = function () {
+ this._id = 'old-value';
+ this.func = () => {
+ var f;
+ eval('10==10');
+ this._id = 'new-value';
+ return this._id;
+ };
+};
+
+var arrowWithEval = new functionConstructorWithEval();
+
+for (var i=0; i < 10000; i++) {
+ testCase(arrowWithEval.func() === 'new-value', true, "Error: this is not lexically binded inside of the arrow function #6");
+}
+
+function foo() {
+ let arr = () => {
+ var x = 123;
+ function bas() {
+ return x;
+ };
+ this._id = '12345';
+ return bas();
+ };
+ this.arr = arr;
+};
+
+var fooObject = new foo();
+
+for (var i=0; i < 10000; i++) {
+ testCase(fooObject.arr() === 123, true, "Error: this is not lexically binded inside of the arrow function #7");
+ testCase(fooObject._id === '12345', true, "Error: this is not lexically binded inside of the arrow function #8");
+}
