Title: [264809] trunk
Revision
264809
Author
ysuz...@apple.com
Date
2020-07-23 20:53:46 -0700 (Thu, 23 Jul 2020)

Log Message

[JSC] Arrow function |this| resolution should not be trapped by with-scope
https://bugs.webkit.org/show_bug.cgi?id=214716
<rdar://problem/65980639>

Reviewed by Darin Adler.

JSTests:

* stress/proxy-trap-this.js: Added.
(shouldNotThrow):
(const.handler.has):
(test):

Source/_javascript_Core:

We were using usual "this" named variable in lexical-environment to load and store arrow-function's |this|.
But since this looks normal variable, it can be trapped by "with" scope's object while it should not be.
We use thisPrivateName instead to avoid this behavior since Proxy does not trap private names.

* builtins/BuiltinNames.h:
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::initializeArrowFunctionContextScopeIfNeeded):
(JSC::BytecodeGenerator::variable):
(JSC::BytecodeGenerator::createVariable):
(JSC::BytecodeGenerator::emitLoadThisFromArrowFunctionLexicalEnvironment):
(JSC::BytecodeGenerator::emitPutThisToArrowFunctionContextScope):
* bytecompiler/NodesCodegen.cpp:
(JSC::HasOwnPropertyFunctionCallDotNode::emitBytecode):
(JSC::ForInNode::emitBytecode):
* runtime/CommonIdentifiers.cpp:
(JSC::CommonIdentifiers::CommonIdentifiers):
* runtime/CommonIdentifiers.h:

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (264808 => 264809)


--- trunk/JSTests/ChangeLog	2020-07-24 02:28:04 UTC (rev 264808)
+++ trunk/JSTests/ChangeLog	2020-07-24 03:53:46 UTC (rev 264809)
@@ -1,5 +1,18 @@
 2020-07-23  Yusuke Suzuki  <ysuz...@apple.com>
 
+        [JSC] Arrow function |this| resolution should not be trapped by with-scope
+        https://bugs.webkit.org/show_bug.cgi?id=214716
+        <rdar://problem/65980639>
+
+        Reviewed by Darin Adler.
+
+        * stress/proxy-trap-this.js: Added.
+        (shouldNotThrow):
+        (const.handler.has):
+        (test):
+
+2020-07-23  Yusuke Suzuki  <ysuz...@apple.com>
+
         [JSC] BigInt can be `false` in boolean context in DFG AI
         https://bugs.webkit.org/show_bug.cgi?id=214678
         <rdar://problem/65894751>

Added: trunk/JSTests/stress/proxy-trap-this.js (0 => 264809)


--- trunk/JSTests/stress/proxy-trap-this.js	                        (rev 0)
+++ trunk/JSTests/stress/proxy-trap-this.js	2020-07-24 03:53:46 UTC (rev 264809)
@@ -0,0 +1,26 @@
+function shouldNotThrow(func) {
+    func();
+}
+
+const handler = {
+    has() {
+        throw new Error("bad");
+    },
+    get() {
+        throw new Error("bad");
+    }
+};
+
+const proxy = new Proxy({}, handler);
+
+function test() {
+    with (proxy) {
+        const func = () => {
+            return this;
+        };
+        return func();
+    }
+}
+
+for (let i = 0; i < 5000; i++)
+    shouldNotThrow(() => test());

Modified: trunk/Source/_javascript_Core/ChangeLog (264808 => 264809)


--- trunk/Source/_javascript_Core/ChangeLog	2020-07-24 02:28:04 UTC (rev 264808)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-07-24 03:53:46 UTC (rev 264809)
@@ -1,5 +1,31 @@
 2020-07-23  Yusuke Suzuki  <ysuz...@apple.com>
 
+        [JSC] Arrow function |this| resolution should not be trapped by with-scope
+        https://bugs.webkit.org/show_bug.cgi?id=214716
+        <rdar://problem/65980639>
+
+        Reviewed by Darin Adler.
+
+        We were using usual "this" named variable in lexical-environment to load and store arrow-function's |this|.
+        But since this looks normal variable, it can be trapped by "with" scope's object while it should not be.
+        We use thisPrivateName instead to avoid this behavior since Proxy does not trap private names.
+
+        * builtins/BuiltinNames.h:
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::initializeArrowFunctionContextScopeIfNeeded):
+        (JSC::BytecodeGenerator::variable):
+        (JSC::BytecodeGenerator::createVariable):
+        (JSC::BytecodeGenerator::emitLoadThisFromArrowFunctionLexicalEnvironment):
+        (JSC::BytecodeGenerator::emitPutThisToArrowFunctionContextScope):
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::HasOwnPropertyFunctionCallDotNode::emitBytecode):
+        (JSC::ForInNode::emitBytecode):
+        * runtime/CommonIdentifiers.cpp:
+        (JSC::CommonIdentifiers::CommonIdentifiers):
+        * runtime/CommonIdentifiers.h:
+
+2020-07-23  Yusuke Suzuki  <ysuz...@apple.com>
+
         [JSC] FTL OSR entry should store boxed |this|
         https://bugs.webkit.org/show_bug.cgi?id=214675
         <rdar://problem/65474072>

Modified: trunk/Source/_javascript_Core/builtins/BuiltinNames.h (264808 => 264809)


--- trunk/Source/_javascript_Core/builtins/BuiltinNames.h	2020-07-24 02:28:04 UTC (rev 264808)
+++ trunk/Source/_javascript_Core/builtins/BuiltinNames.h	2020-07-24 03:53:46 UTC (rev 264809)
@@ -112,6 +112,7 @@
     macro(asyncGeneratorQueueItemNext) \
     macro(dateTimeFormat) \
     macro(intlSubstituteValue) \
+    macro(this) \
     macro(thisTimeValue) \
     macro(newTargetLocal) \
     macro(derivedConstructor) \

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (264808 => 264809)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2020-07-24 02:28:04 UTC (rev 264808)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2020-07-24 03:53:46 UTC (rev 264809)
@@ -1160,7 +1160,7 @@
         
         if (isThisUsedInInnerArrowFunction()) {
             offset = functionSymbolTable->takeNextScopeOffset(NoLockingNecessary);
-            functionSymbolTable->set(NoLockingNecessary, propertyNames().thisIdentifier.impl(), SymbolTableEntry(VarOffset(offset)));
+            functionSymbolTable->set(NoLockingNecessary, propertyNames().builtinNames().thisPrivateName().impl(), SymbolTableEntry(VarOffset(offset)));
         }
 
         if (m_codeType == FunctionCode && isNewTargetUsedInInnerArrowFunction()) {
@@ -1179,7 +1179,7 @@
     VariableEnvironment environment;
 
     if (isThisUsedInInnerArrowFunction()) {
-        auto addResult = environment.add(propertyNames().thisIdentifier);
+        auto addResult = environment.add(propertyNames().builtinNames().thisPrivateName());
         addResult.iterator->value.setIsCaptured();
         addResult.iterator->value.setIsLet();
     }
@@ -2240,7 +2240,7 @@
 
 Variable BytecodeGenerator::variable(const Identifier& property, ThisResolutionType thisResolutionType)
 {
-    if (property == propertyNames().thisIdentifier && thisResolutionType == ThisResolutionType::Local)
+    if (property == propertyNames().builtinNames().thisPrivateName() && thisResolutionType == ThisResolutionType::Local)
         return Variable(property, VarOffset(thisRegister()->virtualRegister()), thisRegister(), static_cast<unsigned>(PropertyAttribute::ReadOnly), Variable::SpecialVariable, 0, false);
     
     // We can optimize lookups if the lexical variable is found before a "with" or "catch"
@@ -2306,7 +2306,7 @@
 void BytecodeGenerator::createVariable(
     const Identifier& property, VarKind varKind, SymbolTable* symbolTable, ExistingVariableMode existingVariableMode)
 {
-    ASSERT(property != propertyNames().thisIdentifier);
+    ASSERT(property != propertyNames().builtinNames().thisPrivateName());
     SymbolTableEntry entry = symbolTable->get(NoLockingNecessary, property.impl());
     
     if (!entry.isNull()) {
@@ -4462,7 +4462,7 @@
 
 void BytecodeGenerator::emitLoadThisFromArrowFunctionLexicalEnvironment()
 {
-    emitGetFromScope(thisRegister(), emitLoadArrowFunctionLexicalEnvironment(propertyNames().thisIdentifier), variable(propertyNames().thisIdentifier, ThisResolutionType::Scoped), DoNotThrowIfNotFound);
+    emitGetFromScope(thisRegister(), emitLoadArrowFunctionLexicalEnvironment(propertyNames().builtinNames().thisPrivateName()), variable(propertyNames().builtinNames().thisPrivateName(), ThisResolutionType::Scoped), DoNotThrowIfNotFound);
 }
     
 RegisterID* BytecodeGenerator::emitLoadNewTargetFromArrowFunctionLexicalEnvironment()
@@ -4549,8 +4549,8 @@
     if (isThisUsedInInnerArrowFunction() || (m_scopeNode->usesSuperCall() && m_codeType == EvalCode)) {
         ASSERT(isDerivedConstructorContext() || m_arrowFunctionContextLexicalEnvironmentRegister != nullptr);
 
-        Variable thisVar = variable(propertyNames().thisIdentifier, ThisResolutionType::Scoped);
-        RegisterID* scope = isDerivedConstructorContext() ? emitLoadArrowFunctionLexicalEnvironment(propertyNames().thisIdentifier) : m_arrowFunctionContextLexicalEnvironmentRegister;
+        Variable thisVar = variable(propertyNames().builtinNames().thisPrivateName(), ThisResolutionType::Scoped);
+        RegisterID* scope = isDerivedConstructorContext() ? emitLoadArrowFunctionLexicalEnvironment(propertyNames().builtinNames().thisPrivateName()) : m_arrowFunctionContextLexicalEnvironmentRegister;
     
         emitPutToScope(scope, thisVar, thisRegister(), ThrowIfNotFound, InitializationMode::NotInitialization);
     }

Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (264808 => 264809)


--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2020-07-24 02:28:04 UTC (rev 264808)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2020-07-24 03:53:46 UTC (rev 264809)
@@ -1955,7 +1955,7 @@
         if (m_base->isThisNode()) {
             // After generator.ensureThis (which must be invoked in |base|'s materialization), we can ensure that |this| is in local this-register.
             ASSERT(base);
-            return generator.variable(generator.propertyNames().thisIdentifier, ThisResolutionType::Local) == structureContext->baseVariable().value();
+            return generator.variable(generator.propertyNames().builtinNames().thisPrivateName(), ThisResolutionType::Local) == structureContext->baseVariable().value();
         }
         return false;
     };
@@ -3894,7 +3894,7 @@
     else if (m_expr->isThisNode()) {
         // After generator.ensureThis (which must be invoked in |base|'s materialization), we can ensure that |this| is in local this-register.
         ASSERT(base);
-        baseVariable = generator.variable(generator.propertyNames().thisIdentifier, ThisResolutionType::Local);
+        baseVariable = generator.variable(generator.propertyNames().builtinNames().thisPrivateName(), ThisResolutionType::Local);
     }
 
     // Pause at the assignment _expression_ for each for..in iteration.

Modified: trunk/Source/_javascript_Core/runtime/CommonIdentifiers.cpp (264808 => 264809)


--- trunk/Source/_javascript_Core/runtime/CommonIdentifiers.cpp	2020-07-24 02:28:04 UTC (rev 264808)
+++ trunk/Source/_javascript_Core/runtime/CommonIdentifiers.cpp	2020-07-24 03:53:46 UTC (rev 264809)
@@ -36,7 +36,6 @@
     : nullIdentifier()
     , emptyIdentifier(Identifier::EmptyIdentifier)
     , underscoreProto(Identifier::fromString(vm, "__proto__"))
-    , thisIdentifier(Identifier::fromString(vm, "this"))
     , useStrictIdentifier(Identifier::fromString(vm, "use strict"))
     , timesIdentifier(Identifier::fromString(vm, "*"))
     , m_builtinNames(makeUnique<BuiltinNames>(vm, this))

Modified: trunk/Source/_javascript_Core/runtime/CommonIdentifiers.h (264808 => 264809)


--- trunk/Source/_javascript_Core/runtime/CommonIdentifiers.h	2020-07-24 02:28:04 UTC (rev 264808)
+++ trunk/Source/_javascript_Core/runtime/CommonIdentifiers.h	2020-07-24 03:53:46 UTC (rev 264809)
@@ -305,7 +305,6 @@
         const Identifier nullIdentifier;
         const Identifier emptyIdentifier;
         const Identifier underscoreProto;
-        const Identifier thisIdentifier;
         const Identifier useStrictIdentifier;
         const Identifier timesIdentifier;
     private:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to