Attachment 358776 Details for Bug 193308 – Patch

[patch] Patch

bug-193308-20190109204537.patch (text/plain), 21.13 KB, created by Yusuke Suzuki on 2019-01-09 20:45:37 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-01-09 20:45:37 PST

Size: 21.13 KB

patch

obsolete

>Subversion Revision: 239777
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 263008d077553db015276c61364b89198e8e3552..dd41e037c44f660ea470870d1a7f8c4e2fe367a0 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-01-09  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        [JSC] Global lexical bindings can shadow global variables if it is `configurable = true`
>+        https://bugs.webkit.org/show_bug.cgi?id=193308
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * bytecode/CodeBlock.cpp:
>+        (JSC::CodeBlock::notifyLexicalBindingShadowing):
>+        * bytecode/CodeBlock.h:
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::parseBlock):
>+        * runtime/JSGlobalObject.cpp:
>+        (JSC::JSGlobalObject::notifyLexicalBindingShadowing):
>+        * runtime/JSGlobalObject.h:
>+        * runtime/ProgramExecutable.cpp:
>+        (JSC::hasRestrictedGlobalProperty):
>+        (JSC::ProgramExecutable::initializeGlobalProperties):
>+
> 2019-01-08  Keith Miller  <keith_miller@apple.com>
> 
>         builtins should be able to use async/await
>diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>index 86db8693f60e773b61ba37358ca3ee937dc0f35d..0f8735cc06a6209ba6eb9a5352a5673e98d3193a 100644
>--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>@@ -2663,6 +2663,74 @@ void CodeBlock::tallyFrequentExitSites()
> }
> #endif // ENABLE(DFG_JIT)
> 
>+void CodeBlock::notifyLexicalBindingShadowing(VM& vm, const IdentifierSet& set)
>+{
>+    JSGlobalObject* globalObject = m_globalObject.get();
>+    auto throwScope = DECLARE_THROW_SCOPE(vm);
>+
>+    ConcurrentJSLocker locker(m_lock);
>+
>+    for (const auto& instruction : *m_instructions) {
>+        OpcodeID opcodeID = instruction->opcodeID();
>+        switch (opcodeID) {
>+        case op_resolve_scope: {
>+            auto bytecode = instruction->as<OpResolveScope>();
>+            auto& metadata = bytecode.metadata(this);
>+            if (metadata.resolveType == GlobalProperty || metadata.resolveType == GlobalPropertyWithVarInjectionChecks) {
>+                const Identifier& ident = identifier(bytecode.var);
>+                if (set.contains(ident.impl())) {
>+                    metadata.resolveType = metadata.resolveType == GlobalProperty ? GlobalLexicalVar : GlobalLexicalVarWithVarInjectionChecks;
>+                    metadata.constantScope.set(vm, this, JSScope::constantScopeForCodeBlock(metadata.resolveType, this));
>+                    metadata.localScopeDepth = 0; // For GlobalLexicalVar and GlobalLexicalVarWithVarInjectionChecks, depth is useless.
>+                }
>+            }
>+            break;
>+        }
>+
>+        case op_get_from_scope: {
>+            auto bytecode = instruction->as<OpGetFromScope>();
>+            auto& metadata = bytecode.metadata(this);
>+            if (metadata.getPutInfo.resolveType() == GlobalProperty || metadata.getPutInfo.resolveType() == GlobalPropertyWithVarInjectionChecks) {
>+                const Identifier& ident = identifier(bytecode.var);
>+                if (set.contains(ident.impl())) {
>+                    ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), bytecode.localScopeDepth, globalObject->globalScope(), ident, Get, bytecode.getPutInfo.resolveType(), InitializationMode::NotInitialization);
>+                    EXCEPTION_ASSERT(!throwScope.exception());
>+                    metadata.getPutInfo = GetPutInfo(bytecode.getPutInfo.resolveMode(), op.type, bytecode.getPutInfo.initializationMode());
>+                    metadata.structure.clear();
>+                    metadata.operand = op.operand;
>+                }
>+            }
>+            break;
>+        }
>+
>+        case op_put_to_scope: {
>+            auto bytecode = instruction->as<OpPutToScope>();
>+            auto& metadata = bytecode.metadata(this);
>+            if (metadata.getPutInfo.resolveType() == GlobalProperty || metadata.getPutInfo.resolveType() == GlobalPropertyWithVarInjectionChecks) {
>+                const Identifier& ident = identifier(bytecode.var);
>+                if (set.contains(ident.impl())) {
>+                    metadata.watchpointSet = nullptr;
>+                    ResolveOp op = JSScope::abstractResolve(m_globalObject->globalExec(), bytecode.symbolTableOrScopeDepth, globalObject->globalScope(), ident, Put, bytecode.getPutInfo.resolveType(), bytecode.getPutInfo.initializationMode());
>+                    EXCEPTION_ASSERT(!throwScope.exception());
>+                    metadata.getPutInfo = GetPutInfo(bytecode.getPutInfo.resolveMode(), op.type, bytecode.getPutInfo.initializationMode());
>+                    metadata.operand = op.operand;
>+                }
>+            }
>+            break;
>+        }
>+
>+        case op_profile_type: {
>+            break;
>+        }
>+
>+        default:
>+            break;
>+        }
>+    }
>+#if ENABLE(DFG_JIT)
>+#endif // ENABLE(DFG_JIT)
>+}
>+
> #if ENABLE(VERBOSE_VALUE_PROFILE)
> void CodeBlock::dumpValueProfiles()
> {
>diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h
>index 6c766a8ab229db0f683e66c06b7d5c50b8e81217..cd5c4f13d8a204852b3796822c7cba8e05db7332 100644
>--- a/Source/JavaScriptCore/bytecode/CodeBlock.h
>+++ b/Source/JavaScriptCore/bytecode/CodeBlock.h
>@@ -195,6 +195,8 @@ class CodeBlock : public JSCell {
>     void visitChildren(SlotVisitor&);
>     void finalizeUnconditionally(VM&);
> 
>+    void notifyLexicalBindingShadowing(VM&, const IdentifierSet&);
>+
>     void dumpSource();
>     void dumpSource(PrintStream&);
> 
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index a6d3128f755ce8748a249bebb91f990000ecb0f5..16bad39fc07b04fdf66401b506ce0953d9efaf63 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -6140,28 +6140,57 @@ void ByteCodeParser::parseBlock(unsigned limit)
>         case op_resolve_scope: {
>             auto bytecode = currentInstruction->as<OpResolveScope>();
>             auto& metadata = bytecode.metadata(codeBlock);
>-            unsigned depth = metadata.localScopeDepth;
> 
>-            if (needsDynamicLookup(metadata.resolveType, op_resolve_scope)) {
>+            ResolveType resolveType;
>+            unsigned depth;
>+            JSScope* constantScope = nullptr;
>+            JSCell* lexicalEnvironment = nullptr;
>+            SymbolTable* symbolTable = nullptr;
>+            {
>+                ConcurrentJSLocker locker(m_inlineStackTop->m_profiledBlock->m_lock);
>+                resolveType = metadata.resolveType;
>+                depth = metadata.localScopeDepth;
>+                switch (resolveType) {
>+                case GlobalProperty:
>+                case GlobalVar:
>+                case GlobalPropertyWithVarInjectionChecks:
>+                case GlobalVarWithVarInjectionChecks:
>+                case GlobalLexicalVar:
>+                case GlobalLexicalVarWithVarInjectionChecks:
>+                    constantScope = metadata.constantScope.get();
>+                    break;
>+                case ModuleVar:
>+                    lexicalEnvironment = metadata.lexicalEnvironment.get();
>+                    break;
>+                case LocalClosureVar:
>+                case ClosureVar:
>+                case ClosureVarWithVarInjectionChecks:
>+                    symbolTable = metadata.symbolTable.get();
>+                    break;
>+                default:
>+                    break;
>+                }
>+            }
>+
>+            if (needsDynamicLookup(resolveType, op_resolve_scope)) {
>                 unsigned identifierNumber = m_inlineStackTop->m_identifierRemap[bytecode.var];
>                 set(bytecode.dst, addToGraph(ResolveScope, OpInfo(identifierNumber), get(bytecode.scope)));
>                 NEXT_OPCODE(op_resolve_scope);
>             }
> 
>             // get_from_scope and put_to_scope depend on this watchpoint forcing OSR exit, so they don't add their own watchpoints.
>-            if (needsVarInjectionChecks(metadata.resolveType))
>+            if (needsVarInjectionChecks(resolveType))
>                 m_graph.watchpoints().addLazily(m_inlineStackTop->m_codeBlock->globalObject()->varInjectionWatchpoint());
> 
>-            switch (metadata.resolveType) {
>+            switch (resolveType) {
>             case GlobalProperty:
>             case GlobalVar:
>             case GlobalPropertyWithVarInjectionChecks:
>             case GlobalVarWithVarInjectionChecks:
>             case GlobalLexicalVar:
>             case GlobalLexicalVarWithVarInjectionChecks: {
>-                JSScope* constantScope = JSScope::constantScopeForCodeBlock(metadata.resolveType, m_inlineStackTop->m_codeBlock);
>                 RELEASE_ASSERT(constantScope);
>-                RELEASE_ASSERT(metadata.constantScope.get() == constantScope);
>+                RELEASE_ASSERT(constantScope == JSScope::constantScopeForCodeBlock(resolveType, m_inlineStackTop->m_codeBlock));
>                 set(bytecode.dst, weakJSConstant(constantScope));
>                 addToGraph(Phantom, get(bytecode.scope));
>                 break;
>@@ -6170,7 +6199,7 @@ void ByteCodeParser::parseBlock(unsigned limit)
>                 // Since the value of the "scope" virtual register is not used in LLInt / baseline op_resolve_scope with ModuleVar,
>                 // we need not to keep it alive by the Phantom node.
>                 // Module environment is already strongly referenced by the CodeBlock.
>-                set(bytecode.dst, weakJSConstant(metadata.lexicalEnvironment.get()));
>+                set(bytecode.dst, weakJSConstant(lexicalEnvironment));
>                 break;
>             }
>             case LocalClosureVar:
>@@ -6181,7 +6210,7 @@ void ByteCodeParser::parseBlock(unsigned limit)
>                 
>                 // We have various forms of constant folding here. This is necessary to avoid
>                 // spurious recompiles in dead-but-foldable code.
>-                if (SymbolTable* symbolTable = metadata.symbolTable.get()) {
>+                if (symbolTable) {
>                     InferredValue* singleton = symbolTable->singletonScope();
>                     if (JSValue value = singleton->inferredValue()) {
>                         m_graph.watchpoints().addLazily(singleton);
>@@ -6227,13 +6256,16 @@ void ByteCodeParser::parseBlock(unsigned limit)
>             auto& metadata = bytecode.metadata(codeBlock);
>             unsigned identifierNumber = m_inlineStackTop->m_identifierRemap[bytecode.var];
>             UniquedStringImpl* uid = m_graph.identifiers()[identifierNumber];
>-            ResolveType resolveType = metadata.getPutInfo.resolveType();
> 
>+            ResolveType resolveType;
>+            GetPutInfo getPutInfo(0);
>             Structure* structure = 0;
>             WatchpointSet* watchpoints = 0;
>             uintptr_t operand;
>             {
>                 ConcurrentJSLocker locker(m_inlineStackTop->m_profiledBlock->m_lock);
>+                getPutInfo = metadata.getPutInfo;
>+                resolveType = getPutInfo.resolveType();
>                 if (resolveType == GlobalVar || resolveType == GlobalVarWithVarInjectionChecks || resolveType == GlobalLexicalVar || resolveType == GlobalLexicalVarWithVarInjectionChecks)
>                     watchpoints = metadata.watchpointSet;
>                 else if (resolveType != UnresolvedProperty && resolveType != UnresolvedPropertyWithVarInjectionChecks)
>@@ -6242,7 +6274,7 @@ void ByteCodeParser::parseBlock(unsigned limit)
>             }
> 
>             if (needsDynamicLookup(resolveType, op_get_from_scope)) {
>-                uint64_t opInfo1 = makeDynamicVarOpInfo(identifierNumber, metadata.getPutInfo.operand());
>+                uint64_t opInfo1 = makeDynamicVarOpInfo(identifierNumber, getPutInfo.operand());
>                 SpeculatedType prediction = getPrediction();
>                 set(bytecode.dst,
>                     addToGraph(GetDynamicVar, OpInfo(opInfo1), OpInfo(prediction), get(bytecode.scope)));
>@@ -6392,18 +6424,21 @@ void ByteCodeParser::parseBlock(unsigned limit)
>             unsigned identifierNumber = bytecode.var;
>             if (identifierNumber != UINT_MAX)
>                 identifierNumber = m_inlineStackTop->m_identifierRemap[identifierNumber];
>-            ResolveType resolveType = metadata.getPutInfo.resolveType();
>             UniquedStringImpl* uid;
>             if (identifierNumber != UINT_MAX)
>                 uid = m_graph.identifiers()[identifierNumber];
>             else
>                 uid = nullptr;
>             
>+            ResolveType resolveType;
>+            GetPutInfo getPutInfo(0);
>             Structure* structure = nullptr;
>             WatchpointSet* watchpoints = nullptr;
>             uintptr_t operand;
>             {
>                 ConcurrentJSLocker locker(m_inlineStackTop->m_profiledBlock->m_lock);
>+                getPutInfo = metadata.getPutInfo;
>+                resolveType = getPutInfo.resolveType();
>                 if (resolveType == GlobalVar || resolveType == GlobalVarWithVarInjectionChecks || resolveType == LocalClosureVar || resolveType == GlobalLexicalVar || resolveType == GlobalLexicalVarWithVarInjectionChecks)
>                     watchpoints = metadata.watchpointSet;
>                 else if (resolveType != UnresolvedProperty && resolveType != UnresolvedPropertyWithVarInjectionChecks)
>@@ -6415,7 +6450,7 @@ void ByteCodeParser::parseBlock(unsigned limit)
> 
>             if (needsDynamicLookup(resolveType, op_put_to_scope)) {
>                 ASSERT(identifierNumber != UINT_MAX);
>-                uint64_t opInfo1 = makeDynamicVarOpInfo(identifierNumber, metadata.getPutInfo.operand());
>+                uint64_t opInfo1 = makeDynamicVarOpInfo(identifierNumber, getPutInfo.operand());
>                 addToGraph(PutDynamicVar, OpInfo(opInfo1), OpInfo(), get(bytecode.scope), get(bytecode.value));
>                 NEXT_OPCODE(op_put_to_scope);
>             }
>@@ -6444,7 +6479,7 @@ void ByteCodeParser::parseBlock(unsigned limit)
>             case GlobalLexicalVarWithVarInjectionChecks:
>             case GlobalVar:
>             case GlobalVarWithVarInjectionChecks: {
>-                if (!isInitialization(metadata.getPutInfo.initializationMode()) && (resolveType == GlobalLexicalVar || resolveType == GlobalLexicalVarWithVarInjectionChecks)) {
>+                if (!isInitialization(getPutInfo.initializationMode()) && (resolveType == GlobalLexicalVar || resolveType == GlobalLexicalVarWithVarInjectionChecks)) {
>                     SpeculatedType prediction = SpecEmpty;
>                     Node* value = addToGraph(GetGlobalLexicalVariable, OpInfo(operand), OpInfo(prediction));
>                     addToGraph(CheckNotEmpty, value);
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>index e86e82cfc6621c2a3493079bf8a429e64f58510d..b4d09f344ca0c7737e42e74346947ff2f50728b3 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>@@ -50,6 +50,7 @@
> #include "CatchScope.h"
> #include "ClonedArguments.h"
> #include "CodeBlock.h"
>+#include "CodeBlockSetInlines.h"
> #include "CodeCache.h"
> #include "ConsoleObject.h"
> #include "DateConstructor.h"
>@@ -1846,6 +1847,15 @@ const HashSet<String>& JSGlobalObject::intlPluralRulesAvailableLocales()
> }
> #endif // ENABLE(INTL)
> 
>+void JSGlobalObject::notifyLexicalBindingShadowing(VM& vm, const IdentifierSet& set)
>+{
>+    vm.heap.codeBlockSet().iterate([&] (CodeBlock* codeBlock) {
>+        if (codeBlock->globalObject() != this)
>+            return;
>+        codeBlock->notifyLexicalBindingShadowing(vm, set);
>+    });
>+}
>+
> void JSGlobalObject::queueMicrotask(Ref<Microtask>&& task)
> {
>     if (globalObjectMethodTable()->queueTaskToEventLoop) {
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>index ef597fb7883f0aaf00b229f6628881469c043027..2fe2fb731df922eba20104314621d93504032a6a 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObject.h
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>@@ -741,6 +741,8 @@ class JSGlobalObject : public JSSegmentedVariableObject {
>     const HashSet<String>& intlPluralRulesAvailableLocales();
> #endif // ENABLE(INTL)
> 
>+    void notifyLexicalBindingShadowing(VM&, const IdentifierSet&);
>+
>     void setConsoleClient(ConsoleClient* consoleClient) { m_consoleClient = consoleClient; }
>     ConsoleClient* consoleClient() const { return m_consoleClient; }
> 
>diff --git a/Source/JavaScriptCore/runtime/ProgramExecutable.cpp b/Source/JavaScriptCore/runtime/ProgramExecutable.cpp
>index 393b007aca8df1b4b01cf566243d6dfc759a9cfb..eabfea657ecb475daeacef2f88faa2153ae3da1f 100644
>--- a/Source/JavaScriptCore/runtime/ProgramExecutable.cpp
>+++ b/Source/JavaScriptCore/runtime/ProgramExecutable.cpp
>@@ -59,14 +59,19 @@ void ProgramExecutable::destroy(JSCell* cell)
> }
> 
> // http://www.ecma-international.org/ecma-262/6.0/index.html#sec-hasrestrictedglobalproperty
>-static bool hasRestrictedGlobalProperty(ExecState* exec, JSGlobalObject* globalObject, PropertyName propertyName)
>+enum class GlobalPropertyLookUpStatus {
>+    NotFound,
>+    Configurable,
>+    NonConfigurable,
>+};
>+static GlobalPropertyLookUpStatus hasRestrictedGlobalProperty(ExecState* exec, JSGlobalObject* globalObject, PropertyName propertyName)
> {
>     PropertyDescriptor descriptor;
>     if (!globalObject->getOwnPropertyDescriptor(exec, propertyName, descriptor))
>-        return false;
>+        return GlobalPropertyLookUpStatus::NotFound;
>     if (descriptor.configurable())
>-        return false;
>-    return true;
>+        return GlobalPropertyLookUpStatus::Configurable;
>+    return GlobalPropertyLookUpStatus::NonConfigurable;
> }
> 
> JSObject* ProgramExecutable::initializeGlobalProperties(VM& vm, CallFrame* callFrame, JSScope* scope)
>@@ -102,6 +107,7 @@ JSObject* ProgramExecutable::initializeGlobalProperties(VM& vm, CallFrame* callF
>     JSGlobalLexicalEnvironment* globalLexicalEnvironment = globalObject->globalLexicalEnvironment();
>     const VariableEnvironment& variableDeclarations = unlinkedCodeBlock->variableDeclarations();
>     const VariableEnvironment& lexicalDeclarations = unlinkedCodeBlock->lexicalDeclarations();
>+    IdentifierSet shadowedProperties;
>     // The ES6 spec says that no vars/global properties/let/const can be duplicated in the global scope.
>     // This carried out section 15.1.8 of the ES6 spec: http://www.ecma-international.org/ecma-262/6.0/index.html#sec-globaldeclarationinstantiation
>     {
>@@ -112,16 +118,28 @@ JSObject* ProgramExecutable::initializeGlobalProperties(VM& vm, CallFrame* callF
>                 return createSyntaxError(exec, makeString("Can't create duplicate variable: '", String(entry.key.get()), "'"));
>         }
> 
>-        // Check if any new "let"/"const"/"class" will shadow any pre-existing global property names, or "var"/"let"/"const" variables.
>+        // Check if any new "let"/"const"/"class" will shadow any pre-existing global property names (with configurable = false), or "var"/"let"/"const" variables.
>         // It's an error to introduce a shadow.
>         for (auto& entry : lexicalDeclarations) {
>             // The ES6 spec says that RestrictedGlobalProperty can't be shadowed.
>-            bool hasProperty = hasRestrictedGlobalProperty(exec, globalObject, entry.key.get());
>+            GlobalPropertyLookUpStatus status = hasRestrictedGlobalProperty(exec, globalObject, entry.key.get());
>             RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
>-            if (hasProperty)
>+            switch (status) {
>+            case GlobalPropertyLookUpStatus::NonConfigurable:
>                 return createSyntaxError(exec, makeString("Can't create duplicate variable that shadows a global property: '", String(entry.key.get()), "'"));
>+            case GlobalPropertyLookUpStatus::Configurable:
>+                // Lexical bindings can shadow global properties if the given property's attribute is configurable.
>+                // https://tc39.github.io/ecma262/#sec-globaldeclarationinstantiation step 5-c, `hasRestrictedGlobal` becomes false
>+                // if the given global property is configurable. However we may emit GlobalProperty look up in bytecodes already and
>+                // it may cache the value for the global scope. To make it invalid, we iterate all the CodeBlocks and rewrite the instruction to convert
>+                // 1. We perform structure transition on JSGlobalObject to make the cache invalidated.
>+                shadowedProperties.add(entry.key.get());
>+                break;
>+            case GlobalPropertyLookUpStatus::NotFound:
>+                break;
>+            }
> 
>-            hasProperty = globalLexicalEnvironment->hasProperty(exec, entry.key.get());
>+            bool hasProperty = globalLexicalEnvironment->hasProperty(exec, entry.key.get());
>             RETURN_IF_EXCEPTION(throwScope, throwScope.exception());
>             if (hasProperty) {
>                 if (UNLIKELY(entry.value.isConst() && !vm.globalConstRedeclarationShouldThrow() && !isStrictMode())) {
>@@ -186,6 +204,10 @@ JSObject* ProgramExecutable::initializeGlobalProperties(VM& vm, CallFrame* callF
>             RELEASE_ASSERT(offsetForAssert == offset);
>         }
>     }
>+
>+    if (!shadowedProperties.isEmpty())
>+        globalObject->notifyLexicalBindingShadowing(vm, WTFMove(shadowedProperties));
>+
>     return nullptr;
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 193308: 358776 | 358777 | 358811 | 358827 | 358845 | 358855 | 358872 | 358875 | 358877 | 358879 | 358912 | 358927 | 358929 | 358942