Title: [202693] trunk/Source/_javascript_Core
- Revision
- 202693
- Author
- sbar...@apple.com
- Date
- 2016-06-30 11:46:23 -0700 (Thu, 30 Jun 2016)
Log Message
We need to to_this when an inner arrow function uses 'this'
https://bugs.webkit.org/show_bug.cgi?id=159290
<rdar://problem/27058322>
Reviewed by Geoffrey Garen.
We put the |this| value into the closure object when there
is an arrow function that uses |this|. However, an arrow function
using |this| wasn't causing the creator of the closure that
holds |this| to to_this its value before putting it in the
closure. That's a huge bug because it means some arrow functions
can capture the raw |this| value, which might be a JSLexicalEnvironment.
This patch fixes this by adding an easy to check to see if any
inner arrow functions use |this|, and if any do, it will to_this
the |this| value.
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* tests/stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js: Added.
(assert):
(obj):
(foo.capture):
(foo.wrapper.let.x.):
(foo2.capture):
(foo2.wrapper.let.x.):
(foo2.wrapper.bar):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (202692 => 202693)
--- trunk/Source/_javascript_Core/ChangeLog 2016-06-30 18:19:41 UTC (rev 202692)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-06-30 18:46:23 UTC (rev 202693)
@@ -1,3 +1,32 @@
+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>
+
+ Reviewed by Geoffrey Garen.
+
+ We put the |this| value into the closure object when there
+ is an arrow function that uses |this|. However, an arrow function
+ using |this| wasn't causing the creator of the closure that
+ holds |this| to to_this its value before putting it in the
+ closure. That's a huge bug because it means some arrow functions
+ can capture the raw |this| value, which might be a JSLexicalEnvironment.
+ This patch fixes this by adding an easy to check to see if any
+ inner arrow functions use |this|, and if any do, it will to_this
+ the |this| value.
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::BytecodeGenerator):
+ * tests/stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js: Added.
+ (assert):
+ (obj):
+ (foo.capture):
+ (foo.wrapper.let.x.):
+ (foo2.capture):
+ (foo2.wrapper.let.x.):
+ (foo2.wrapper.bar):
+
2016-06-29 Filip Pizlo <fpi...@apple.com>
Generators violate bytecode liveness validation
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (202692 => 202693)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2016-06-30 18:19:41 UTC (rev 202692)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2016-06-30 18:46:23 UTC (rev 202693)
@@ -551,7 +551,7 @@
emitCreateThis(&m_thisRegister);
} else if (constructorKind() != ConstructorKind::None) {
emitThrowTypeError("Cannot call a class constructor");
- } else if (functionNode->usesThis() || codeBlock->usesEval()) {
+ } else if (functionNode->usesThis() || codeBlock->usesEval() || isThisUsedInInnerArrowFunction()) {
m_codeBlock->addPropertyAccessInstruction(instructions().size());
emitOpcode(op_to_this);
instructions().append(kill(&m_thisRegister));
Added: trunk/Source/_javascript_Core/tests/stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js (0 => 202693)
--- trunk/Source/_javascript_Core/tests/stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js 2016-06-30 18:46:23 UTC (rev 202693)
@@ -0,0 +1,58 @@
+function assert(b) {
+ if (!b)
+ throw new Error("Bad assertion!")
+}
+
+function obj() {
+ return {};
+}
+noInline(obj);
+
+// This test makes sure that when wrapper() is called with the closure created in foo() as |this|
+// that we to_this the |this| that is a closure before the arrow function captures its value.
+// This crashes if there is a bug in debug builds.
+
+const globalThis = this;
+function foo() {
+ function capture() { return wrapper; }
+ function wrapper() {
+ let x = () => {
+ // This should not defineProperty on a JSLexicalEnvironment! That's a huge bug.
+ Object.defineProperty(this, "foo", {
+ get: function() { },
+ set: function() { }
+ });
+ assert(!("bar" in this));
+ assert(this === globalThis);
+ }
+
+ x();
+ }
+ wrapper();
+}
+foo();
+
+
+function foo2() {
+ function capture() { return wrapper; }
+ function wrapper() {
+ let x = () => {
+ // This should not defineProperty on a JSLexicalEnvironment! That's a huge bug.
+ Object.defineProperty(this, "foo", {
+ get: function() { },
+ set: function() { }
+ });
+ }
+
+ x();
+
+ function bar() {
+ with (obj()) {
+ assert;
+ }
+ }
+ bar();
+ }
+ wrapper();
+}
+foo2();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
