Title: [202710] trunk/Source/_javascript_Core
Revision
202710
Author
sbar...@apple.com
Date
2016-06-30 15:06:44 -0700 (Thu, 30 Jun 2016)

Log Message

get_by_id_with_this does not trigger a to_this in caller.
https://bugs.webkit.org/show_bug.cgi?id=159226

Reviewed by Keith Miller.

This is a bug if the caller is in sloppy mode and the callee is in strict
mode. This can't happen with ES6 classes because they're all in strict mode,
but it can happen with method syntax on an object literal. The caller must
to_this on |this| when it knows that it performs super property accesses.

* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* tests/stress/super-property-access-object-literal-to-this-2.js: Added.
(assert):
(test):
(let.o1.get foo):
(let.o2.a):
(let.o2.aa):
* tests/stress/super-property-access-object-literal-to-this.js: Added.
(assert):
(test):
(let.o1.get foo):
(let.o2.a):
(let.o2.aa):
(let.o2.b):
(let.o2.bb):
* tests/stress/super-property-access-to-this.js: Added.
(assert):
(test):
(Base.prototype.get foo):
(Base):
(Child.prototype.a):
(Child.prototype.b):
(Child):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (202709 => 202710)


--- trunk/Source/_javascript_Core/ChangeLog	2016-06-30 21:50:40 UTC (rev 202709)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-06-30 22:06:44 UTC (rev 202710)
@@ -1,5 +1,42 @@
 2016-06-30  Saam Barati  <sbar...@apple.com>
 
+        get_by_id_with_this does not trigger a to_this in caller.
+        https://bugs.webkit.org/show_bug.cgi?id=159226
+
+        Reviewed by Keith Miller.
+
+        This is a bug if the caller is in sloppy mode and the callee is in strict
+        mode. This can't happen with ES6 classes because they're all in strict mode,
+        but it can happen with method syntax on an object literal. The caller must
+        to_this on |this| when it knows that it performs super property accesses.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::BytecodeGenerator):
+        * tests/stress/super-property-access-object-literal-to-this-2.js: Added.
+        (assert):
+        (test):
+        (let.o1.get foo):
+        (let.o2.a):
+        (let.o2.aa):
+        * tests/stress/super-property-access-object-literal-to-this.js: Added.
+        (assert):
+        (test):
+        (let.o1.get foo):
+        (let.o2.a):
+        (let.o2.aa):
+        (let.o2.b):
+        (let.o2.bb):
+        * tests/stress/super-property-access-to-this.js: Added.
+        (assert):
+        (test):
+        (Base.prototype.get foo):
+        (Base):
+        (Child.prototype.a):
+        (Child.prototype.b):
+        (Child):
+
+2016-06-30  Saam Barati  <sbar...@apple.com>
+
         We need to to_this when an inner arrow function uses 'this'
         https://bugs.webkit.org/show_bug.cgi?id=159290
         <rdar://problem/27058322>

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (202709 => 202710)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2016-06-30 21:50:40 UTC (rev 202709)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2016-06-30 22:06:44 UTC (rev 202710)
@@ -549,14 +549,34 @@
                     emitMoveEmptyValue(&m_thisRegister);
                 else
                     emitCreateThis(&m_thisRegister);
-            } else if (constructorKind() != ConstructorKind::None) {
+            } else if (constructorKind() != ConstructorKind::None)
                 emitThrowTypeError("Cannot call a class constructor");
-            } else if (functionNode->usesThis() || codeBlock->usesEval() || isThisUsedInInnerArrowFunction()) {
-                m_codeBlock->addPropertyAccessInstruction(instructions().size());
-                emitOpcode(op_to_this);
-                instructions().append(kill(&m_thisRegister));
-                instructions().append(0);
-                instructions().append(0);
+            else {
+                bool shouldEmitToThis = false;
+                if (functionNode->usesThis() || codeBlock->usesEval() || m_scopeNode->doAnyInnerArrowFunctionsUseThis() || m_scopeNode->doAnyInnerArrowFunctionsUseEval())
+                    shouldEmitToThis = true;
+                else if ((functionNode->usesSuperProperty() || m_scopeNode->doAnyInnerArrowFunctionsUseSuperProperty()) && !codeBlock->isStrictMode()) {
+                    // We must emit to_this when we're not in strict mode because we
+                    // will convert |this| to an object, and that object may be passed
+                    // to a strict function as |this|. This is observable because that
+                    // strict function's to_this will just return the object.
+                    //
+                    // We don't need to emit this for strict-mode code because
+                    // strict-mode code may call another strict function, which will
+                    // to_this if it directly uses this; this is OK, because we defer
+                    // to_this until |this| is used directly. Strict-mode code might
+                    // also call a sloppy mode function, and that will to_this, which
+                    // will defer the conversion, again, until necessary.
+                    shouldEmitToThis = true;
+                }
+
+                if (shouldEmitToThis) {
+                    m_codeBlock->addPropertyAccessInstruction(instructions().size());
+                    emitOpcode(op_to_this);
+                    instructions().append(kill(&m_thisRegister));
+                    instructions().append(0);
+                    instructions().append(0);
+                }
             }
         }
         break;

Added: trunk/Source/_javascript_Core/tests/stress/super-property-access-object-literal-to-this-2.js (0 => 202710)


--- trunk/Source/_javascript_Core/tests/stress/super-property-access-object-literal-to-this-2.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/super-property-access-object-literal-to-this-2.js	2016-06-30 22:06:44 UTC (rev 202710)
@@ -0,0 +1,58 @@
+function assert(b) {
+    if (!b)
+        throw new Error("Bad assertion")
+}
+
+function test(f, n = 1000) {
+    for (let i = 0; i < n; ++i)
+        f();
+}
+
+let o1 = {
+    get foo() {
+        return this;
+    }
+};
+
+let o2 = {
+    __proto__: o1,
+    a() {
+        "use strict";
+        return super.foo;
+    },
+
+    aa() {
+        "use strict";
+        let arr = () => super.foo;
+        return arr();
+    }
+};
+
+var A = o2.a;
+var AA = o2.aa;
+
+let globalObj = this;
+
+AA();
+
+test(function() {
+    let num = o2.a.call(25);
+    assert(typeof num === "object");
+    assert(num instanceof Number);
+
+    let str = o2.a.call("foo bar");
+    assert(typeof str === "object");
+    assert(str instanceof String);
+    assert(str == "foo bar");
+
+    let o = {};
+    assert(o2.a.call(o) === o);
+
+    assert(A() === globalObj);
+    assert(AA() === globalObj);
+
+    assert(o2.a.call(undefined) === globalObj);
+    assert(o2.a.call(null) === globalObj);
+    assert(o2.aa.call(undefined) === globalObj);
+    assert(o2.aa.call(null) === globalObj);
+});

Added: trunk/Source/_javascript_Core/tests/stress/super-property-access-object-literal-to-this.js (0 => 202710)


--- trunk/Source/_javascript_Core/tests/stress/super-property-access-object-literal-to-this.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/super-property-access-object-literal-to-this.js	2016-06-30 22:06:44 UTC (rev 202710)
@@ -0,0 +1,108 @@
+function assert(b) {
+    if (!b)
+        throw new Error("Bad assertion")
+}
+
+function test(f, n = 1000) {
+    for (let i = 0; i < n; ++i)
+        f();
+}
+
+let o1 = {
+    get foo() {
+        "use strict";
+        return this;
+    }
+};
+
+let o2 = {
+    __proto__: o1,
+    a() {
+        return super.foo;
+    },
+
+    aa() {
+        let arr = () => super.foo;
+        return arr();
+    },
+
+    b() {
+        "use strict";
+        return super.foo;
+    },
+
+    bb() {
+        "use strict";
+        let arr = () => super.foo;
+        return arr();
+    }
+};
+
+var A = o2.a;
+var AA = o2.aa;
+var B = o2.b;
+var BB = o2.b;
+
+let globalObj = this;
+
+test(function() {
+    let num = o2.a.call(25);
+    assert(typeof num === "object");
+    assert(num instanceof Number);
+
+    let str = o2.a.call("foo bar");
+    assert(typeof str === "object");
+    assert(str instanceof String);
+    assert(str == "foo bar");
+
+    let o = {};
+    assert(o2.a.call(o) === o);
+
+    assert(A() === globalObj);
+});
+
+test(function() {
+    let num = o2.aa.call(25);
+    assert(typeof num === "object");
+    assert(num instanceof Number);
+
+    let str = o2.aa.call("foo bar");
+    assert(typeof str === "object");
+    assert(str instanceof String);
+    assert(str == "foo bar");
+
+    let o = {};
+    assert(o2.aa.call(o) === o);
+
+    assert(AA() === globalObj);
+});
+
+test(function() {
+    let num = o2.b.call(25);
+    assert(typeof num === "number");
+    assert(num === 25);
+
+    let str = o2.b.call("foo bar");
+    assert(typeof str === "string");
+    assert(str === "foo bar");
+
+    let o = {};
+    assert(o2.b.call(o) === o);
+
+    assert(B() === undefined);
+});
+
+test(function() {
+    let num = o2.bb.call(25);
+    assert(typeof num === "number");
+    assert(num === 25);
+
+    let str = o2.bb.call("foo bar");
+    assert(typeof str === "string");
+    assert(str === "foo bar");
+
+    let o = {};
+    assert(o2.bb.call(o) === o);
+
+    assert(BB() === undefined);
+});

Added: trunk/Source/_javascript_Core/tests/stress/super-property-access-to-this.js (0 => 202710)


--- trunk/Source/_javascript_Core/tests/stress/super-property-access-to-this.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/super-property-access-to-this.js	2016-06-30 22:06:44 UTC (rev 202710)
@@ -0,0 +1,48 @@
+"use strict";
+function assert(b) {
+    if (!b)
+        throw new Error("Bad assertion")
+}
+
+function test(f, n = 1000) {
+    for (let i = 0; i < n; ++i)
+        f();
+}
+
+class Base {
+    get foo() { return this; }
+}
+
+class Child extends Base {
+    a() {
+        return super.foo;
+    }
+
+    b() {
+        let arr = () => super.foo;
+        return arr();
+    }
+};
+
+let A = Child.prototype.a;
+var AA = Child.prototype.a;
+this.AAA = Child.prototype.a;
+
+let globalObj = this;
+
+test(function() {
+    assert(Child.prototype.a.call("xyz") === "xyz");
+    let obj = {};
+    assert(Child.prototype.a.call(obj) === obj);
+    assert(Child.prototype.a.call(25) === 25);
+    assert(Child.prototype.a.call(globalObj) === globalObj);
+
+    assert(Child.prototype.b.call("xyz") === "xyz");
+    assert(Child.prototype.b.call(obj) === obj);
+    assert(Child.prototype.b.call(25) === 25);
+    assert(Child.prototype.b.call(globalObj) === globalObj);
+
+    assert(A() === undefined);
+    assert(AA() === undefined);
+    assert(AAA() === undefined);
+});
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to