Attachment 358855 Details for Bug 193308 – Patch

[patch] Patch

bug-193308-20190110172052.patch (text/plain), 39.23 KB, created by Yusuke Suzuki on 2019-01-10 17:20:53 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-01-10 17:20:53 PST

Size: 39.23 KB

patch

obsolete

>Subversion Revision: 239777
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 263008d077553db015276c61364b89198e8e3552..f48876910608c6acf8970d3d7bde3423bf2c11cb 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2019-01-10  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::finishCreation):
>+        (JSC::CodeBlock::ensureReferencedGlobalPropertyWatchpointSet):
>+        (JSC::CodeBlock::getReferencedGlobalPropertyWatchpointSetConcurrently):
>+        (JSC::CodeBlock::notifyLexicalBindingShadowing):
>+        * bytecode/CodeBlock.h:
>+        * bytecode/Watchpoint.h:
>+        (JSC::WatchpointSet::create):
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::parseBlock):
>+        * jit/JITPropertyAccess.cpp:
>+        (JSC::JIT::emit_op_resolve_scope):
>+        (JSC::JIT::emit_op_get_from_scope):
>+        (JSC::JIT::emit_op_put_to_scope):
>+        * runtime/JSGlobalObject.cpp:
>+        (JSC::JSGlobalObject::addStaticGlobals):
>+        (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..1e730d969ba9ad4fb1b1be9a8ea453a90dbe419d 100644
>--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>@@ -624,6 +624,13 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
>                 metadata.constantScope.set(vm, this, constantScope);
>             else
>                 metadata.globalObject = nullptr;
>+
>+#if ENABLE(DFG_JIT)
>+            // We create a watchpoint for GlobalProperty so that we can fire it once it is shadowed by lexical binding.
>+            // This jettisons DFG and FTL code depending on GlobalProperty and GlobalPropertyWithVarInjectionChecks resolve types.
>+            if (metadata.resolveType == GlobalProperty || metadata.resolveType == GlobalPropertyWithVarInjectionChecks)
>+                ensureReferencedGlobalPropertyWatchpointSet(ident);
>+#endif
>             break;
>         }
> 
>@@ -864,6 +871,22 @@ CodeBlock::~CodeBlock()
> #endif // ENABLE(JIT)
> }
> 
>+#if ENABLE(DFG_JIT)
>+WatchpointSet& CodeBlock::ensureReferencedGlobalPropertyWatchpointSet(const Identifier& ident)
>+{
>+    return m_referencedGlobalPropertyWatchpointSets.ensure(ident.impl(), [] {
>+        return WatchpointSet::create(IsWatched);
>+    }).iterator->value.get();
>+}
>+
>+WatchpointSet* CodeBlock::getReferencedGlobalPropertyWatchpointSetConcurrently(UniquedStringImpl* ident)
>+{
>+    // Actually, this m_referencedGlobalPropertyWatchpointSets is configured at finishCreation, and we do not add any watchpoint after that.
>+    // So, we can get the watchpoint set without locking.
>+    return m_referencedGlobalPropertyWatchpointSets.get(ident);
>+}
>+#endif
>+
> void CodeBlock::setConstantIdentifierSetRegisters(VM& vm, const Vector<ConstantIndentifierSetEntry>& constants)
> {
>     auto scope = DECLARE_THROW_SCOPE(vm);
>@@ -2663,6 +2686,83 @@ 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_UNUSED(throwScope, !throwScope.exception());
>+                    metadata.getPutInfo = GetPutInfo(bytecode.getPutInfo.resolveMode(), op.type, bytecode.getPutInfo.initializationMode());
>+                    metadata.structure.clear();
>+                    metadata.operand = op.operand;
>+                    ASSERT(metadata.getPutInfo.resolveType() == GlobalLexicalVar || metadata.getPutInfo.resolveType() == GlobalLexicalVarWithVarInjectionChecks);
>+                }
>+            }
>+            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_UNUSED(throwScope, !throwScope.exception());
>+                    metadata.getPutInfo = GetPutInfo(bytecode.getPutInfo.resolveMode(), op.type, bytecode.getPutInfo.initializationMode());
>+                    metadata.operand = op.operand;
>+                    // Dynamic happens if the op_put_to_scope performs the write attempt to the const lexical binding, and raises a TypeError.
>+                    ASSERT(metadata.getPutInfo.resolveType() == GlobalLexicalVar || metadata.getPutInfo.resolveType() == GlobalLexicalVarWithVarInjectionChecks || metadata.getPutInfo.resolveType() == Dynamic);
>+                }
>+            }
>+            break;
>+        }
>+
>+        case op_profile_type: {
>+            break;
>+        }
>+
>+        default:
>+            break;
>+        }
>+    }
>+
>+#if ENABLE(DFG_JIT)
>+    for (const auto& key : set) {
>+        if (auto result = m_referencedGlobalPropertyWatchpointSets.get(key.get()))
>+            result->fireAll(vm, "Lexical binding shadows the existing global properties");
>+    }
>+#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..2c652aa187b69164efc4cf089c1cc51e15d76f0c 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&);
> 
>@@ -728,6 +730,8 @@ class CodeBlock : public JSCell {
>     enum class OptimizeAction { None, ReoptimizeNow };
> #if ENABLE(DFG_JIT)
>     OptimizeAction updateOSRExitCounterAndCheckIfNeedToReoptimize(DFG::OSRExitState&);
>+    WatchpointSet* getReferencedGlobalPropertyWatchpointSetConcurrently(UniquedStringImpl*);
>+    WatchpointSet& ensureReferencedGlobalPropertyWatchpointSet(const Identifier&);
> #endif
> 
>     static ptrdiff_t offsetOfOSRExitCounter() { return OBJECT_OFFSETOF(CodeBlock, m_osrExitCounter); }
>@@ -991,6 +995,11 @@ class CodeBlock : public JSCell {
>     RefCountedArray<WriteBarrier<FunctionExecutable>> m_functionDecls;
>     RefCountedArray<WriteBarrier<FunctionExecutable>> m_functionExprs;
> 
>+#if ENABLE(DFG_JIT)
>+    using ReferencedGlobalPropertyWatchpointSets = HashMap<RefPtr<UniquedStringImpl>, Ref<WatchpointSet>, IdentifierRepHash>;
>+    ReferencedGlobalPropertyWatchpointSets m_referencedGlobalPropertyWatchpointSets;
>+#endif
>+
>     WriteBarrier<CodeBlock> m_alternative;
>     
>     BaselineExecutionCounter m_llintExecuteCounter;
>diff --git a/Source/JavaScriptCore/bytecode/Watchpoint.h b/Source/JavaScriptCore/bytecode/Watchpoint.h
>index 18d8c4ec0ad564ed6ef4aa48c13d4012a65da623..3720cc71ffd9c790b22fb3364aecd66ad7669eaa 100644
>--- a/Source/JavaScriptCore/bytecode/Watchpoint.h
>+++ b/Source/JavaScriptCore/bytecode/Watchpoint.h
>@@ -124,6 +124,11 @@ class WatchpointSet : public ThreadSafeRefCounted<WatchpointSet> {
>     // FIXME: In many cases, it would be amazing if this *did* fire the watchpoints. I suspect that
>     // this might be hard to get right, but still, it might be awesome.
>     JS_EXPORT_PRIVATE ~WatchpointSet(); // Note that this will not fire any of the watchpoints; if you need to know when a WatchpointSet dies then you need a separate mechanism for this.
>+
>+    static Ref<WatchpointSet> create(WatchpointState state)
>+    {
>+        return adoptRef(*new WatchpointSet(state));
>+    }
>     
>     // Fast way of getting the state, which only works from the main thread.
>     WatchpointState stateOnJSThread() const
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index a6d3128f755ce8748a249bebb91f990000ecb0f5..ed62af0d5d899edfb3767b076ffb075ace41a9d1 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -6140,28 +6140,64 @@ 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) {
>+            if (resolveType == GlobalProperty || resolveType == GlobalPropertyWithVarInjectionChecks) {
>+                unsigned identifierNumber = m_inlineStackTop->m_identifierRemap[bytecode.var];
>+                UniquedStringImpl* uid = m_graph.identifiers()[identifierNumber];
>+                if (auto* watchpointSet = m_inlineStackTop->m_profiledBlock->getReferencedGlobalPropertyWatchpointSetConcurrently(uid))
>+                    m_graph.watchpoints().addLazily(watchpointSet);
>+            }
>+
>+            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 +6206,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 +6217,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 +6263,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 +6281,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 +6431,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 +6457,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 +6486,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/jit/JITPropertyAccess.cpp b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
>index ad1d3dccc62ba5f7382702c2ee9e1e115d34f3d9..60d3cf50947972427a7da11f662b3aea74319064 100644
>--- a/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
>+++ b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
>@@ -766,9 +766,12 @@ void JIT::emit_op_resolve_scope(const Instruction* currentInstruction)
> 
>     auto emitCode = [&] (ResolveType resolveType) {
>         switch (resolveType) {
>+        // Here, we get constant value for GlobalProperty and GlobalPropertyWithVarInjectionChecks.
>+        // This is because UnresolvedProperty already has a resolveType check. If a resolveType is changed
>+        // to GlobalLexicalVar, we do not enter the code for GlobalProperty.
>         case GlobalProperty:
>-        case GlobalVar:
>         case GlobalPropertyWithVarInjectionChecks:
>+        case GlobalVar:
>         case GlobalVarWithVarInjectionChecks:
>         case GlobalLexicalVar:
>         case GlobalLexicalVarWithVarInjectionChecks: {
>@@ -798,6 +801,15 @@ void JIT::emit_op_resolve_scope(const Instruction* currentInstruction)
>     };
> 
>     switch (resolveType) {
>+    case GlobalProperty:
>+    case GlobalPropertyWithVarInjectionChecks: {
>+        // Since these GlobalProperty can be changed to GlobalLexicalVar, we should load the value from metadata.
>+        JSCell** constantScopeSlot = bitwise_cast<JSCell**>(&metadata.constantScope);
>+        emitVarInjectionCheck(needsVarInjectionChecks(resolveType));
>+        loadPtr(constantScopeSlot, regT0);
>+        emitPutVirtualRegister(dst);
>+        break;
>+    }
>     case UnresolvedProperty:
>     case UnresolvedPropertyWithVarInjectionChecks: {
>         JumpList skipToEnd;
>@@ -925,6 +937,22 @@ void JIT::emit_op_get_from_scope(const Instruction* currentInstruction)
>     };
> 
>     switch (resolveType) {
>+    case GlobalProperty:
>+    case GlobalPropertyWithVarInjectionChecks: {
>+        JumpList skipToEnd;
>+        load32(&metadata.getPutInfo, regT0);
>+        and32(TrustedImm32(GetPutInfo::typeBits), regT0); // Load ResolveType into T0
>+
>+        Jump isNotGlobalProperty = branch32(NotEqual, regT0, TrustedImm32(resolveType));
>+        emitCode(resolveType, false);
>+        skipToEnd.append(jump());
>+
>+        isNotGlobalProperty.link(this);
>+        emitCode(needsVarInjectionChecks(resolveType) ? GlobalLexicalVarWithVarInjectionChecks : GlobalLexicalVar, true);
>+
>+        skipToEnd.link(this);
>+        break;
>+    }
>     case UnresolvedProperty:
>     case UnresolvedPropertyWithVarInjectionChecks: {
>         JumpList skipToEnd;
>@@ -1064,6 +1092,22 @@ void JIT::emit_op_put_to_scope(const Instruction* currentInstruction)
>     };
> 
>     switch (resolveType) {
>+    case GlobalProperty:
>+    case GlobalPropertyWithVarInjectionChecks: {
>+        JumpList skipToEnd;
>+        load32(&metadata.getPutInfo, regT0);
>+        and32(TrustedImm32(GetPutInfo::typeBits), regT0); // Load ResolveType into T0
>+
>+        Jump isNotGlobalProperty = branch32(NotEqual, regT0, TrustedImm32(resolveType));
>+        emitCode(resolveType, false);
>+        skipToEnd.append(jump());
>+
>+        isNotGlobalProperty.link(this);
>+        emitCode(needsVarInjectionChecks(resolveType) ? GlobalLexicalVarWithVarInjectionChecks : GlobalLexicalVar, true);
>+
>+        skipToEnd.link(this);
>+        break;
>+    }
>     case UnresolvedProperty:
>     case UnresolvedPropertyWithVarInjectionChecks: {
>         JumpList skipToEnd;
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>index e86e82cfc6621c2a3493079bf8a429e64f58510d..c841d5e4fec36ea73bcb55dd8b6d4deae91c5915 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"
>@@ -1711,6 +1712,8 @@ void JSGlobalObject::addStaticGlobals(GlobalPropertyInfo* globals, int count)
> 
>     for (int i = 0; i < count; ++i) {
>         GlobalPropertyInfo& global = globals[i];
>+        // This `configurable = false` is necessary condition for static globals,
>+        // otherwise lexical bindings can change the result of GlobalVar queries too.
>         ASSERT(global.attributes & PropertyAttribute::DontDelete);
>         
>         WatchpointSet* watchpointSet = nullptr;
>@@ -1846,6 +1849,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..d6cc08d911c9b25ece5b33f9d486fa0a800e58b3 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; }
> 
>@@ -969,6 +971,8 @@ class JSGlobalObject : public JSSegmentedVariableObject {
> 
>     void setNeedsSiteSpecificQuirks(bool needQuirks) { m_needsSiteSpecificQuirks = needQuirks; }
> 
>+    WatchpointSet& ensureWatchpointSetReferencedProperty(const Identifier&);
>+
> private:
>     friend class LLIntOffsetsExtractor;
> 
>diff --git a/Source/JavaScriptCore/runtime/ProgramExecutable.cpp b/Source/JavaScriptCore/runtime/ProgramExecutable.cpp
>index 393b007aca8df1b4b01cf566243d6dfc759a9cfb..071bc57455644c3c9194205507ff9d79a1b8aec5 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,30 @@ 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
>+                // 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 GlobalProperty to GlobalLexicalVar.
>+                // 1. In LLInt, we always check metadata's resolveType. So rewritten instruction just works.
>+                // 2. In Baseline JIT, we check metadata's resolveType in GlobalProperty case so that we can notice once it is changed.
>+                // 3. In DFG and FTL, we watch the watchpoint and jettison once it is fired.
>+                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 +206,10 @@ JSObject* ProgramExecutable::initializeGlobalProperties(VM& vm, CallFrame* callF
>             RELEASE_ASSERT(offsetForAssert == offset);
>         }
>     }
>+
>+    if (!shadowedProperties.isEmpty())
>+        globalObject->notifyLexicalBindingShadowing(vm, WTFMove(shadowedProperties));
>+
>     return nullptr;
> }
> 
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index e5a31714c8cf048efb04058b479bda6c43494550..fff20a9b61d3db1c03bc22aac514f69b9d47db91 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,33 @@
>+2019-01-10  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!).
>+
>+        * stress/const-lexical-binding-shadow-existing-global-property.js: Added.
>+        (shouldThrow):
>+        (shouldBe):
>+        (foo):
>+        * stress/global-add-function-should-not-be-shadowed-by-lexical-bindings.js: Added.
>+        (shouldThrow):
>+        * stress/global-static-variables-should-not-be-shadowed-by-lexical-bindings.js: Added.
>+        (shouldThrow):
>+        * stress/let-lexical-binding-shadow-existing-global-property-baseline.js: Added.
>+        (shouldThrow):
>+        (shouldBe):
>+        (foo):
>+        * stress/let-lexical-binding-shadow-existing-global-property-tdz-baseline.js: Added.
>+        (shouldThrow):
>+        (shouldBe):
>+        (foo):
>+        (get shouldBe):
>+        (get shouldThrow):
>+        * stress/let-lexical-binding-shadow-existing-global-property.js: Added.
>+        (shouldThrow):
>+        (shouldBe):
>+        (foo):
>+
> 2019-01-08  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         Array.prototype.flat/flatMap have a minor bug in ArraySpeciesCreate
>diff --git a/JSTests/stress/const-lexical-binding-shadow-existing-global-property.js b/JSTests/stress/const-lexical-binding-shadow-existing-global-property.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..92b29de8860a9717bdfaa2c472257f2697b342f8
>--- /dev/null
>+++ b/JSTests/stress/const-lexical-binding-shadow-existing-global-property.js
>@@ -0,0 +1,33 @@
>+function shouldThrow(func, errorMessage) {
>+    var errorThrown = false;
>+    var error = null;
>+    try {
>+        func();
>+    } catch (e) {
>+        errorThrown = true;
>+        error = e;
>+    }
>+    if (!errorThrown)
>+        throw new Error('not thrown');
>+    if (String(error) !== errorMessage)
>+        throw new Error(`bad error: ${String(error)}`);
>+}
>+noInline(shouldThrow);
>+
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error(`bad value: ${String(actual)}`);
>+}
>+noInline(shouldBe);
>+
>+function foo() {
>+    bar = 4;
>+}
>+foo();
>+shouldBe(bar, 4);
>+$.evalScript('const bar = 3;');
>+shouldBe(bar, 3);
>+shouldThrow(() => {
>+    foo();
>+}, `TypeError: Attempted to assign to readonly property.`);
>+shouldBe(bar, 3);
>diff --git a/JSTests/stress/global-add-function-should-not-be-shadowed-by-lexical-bindings.js b/JSTests/stress/global-add-function-should-not-be-shadowed-by-lexical-bindings.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..9ebb839606be2156151b8c6c7bbb1e54be7d1c63
>--- /dev/null
>+++ b/JSTests/stress/global-add-function-should-not-be-shadowed-by-lexical-bindings.js
>@@ -0,0 +1,18 @@
>+function shouldThrow(func, errorMessage) {
>+    var errorThrown = false;
>+    var error = null;
>+    try {
>+        func();
>+    } catch (e) {
>+        errorThrown = true;
>+        error = e;
>+    }
>+    if (!errorThrown)
>+        throw new Error('not thrown');
>+    if (String(error) !== errorMessage)
>+        throw new Error(`bad error: ${String(error)}`);
>+}
>+
>+shouldThrow(() => {
>+    $.evalScript(`const shouldThrow = 42`);
>+}, `SyntaxError: Can't create duplicate variable that shadows a global property: 'shouldThrow'`);
>diff --git a/JSTests/stress/global-static-variables-should-not-be-shadowed-by-lexical-bindings.js b/JSTests/stress/global-static-variables-should-not-be-shadowed-by-lexical-bindings.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..307d3e8359d8433bbca0c4e3a65af5b0edf51d69
>--- /dev/null
>+++ b/JSTests/stress/global-static-variables-should-not-be-shadowed-by-lexical-bindings.js
>@@ -0,0 +1,18 @@
>+function shouldThrow(func, errorMessage) {
>+    var errorThrown = false;
>+    var error = null;
>+    try {
>+        func();
>+    } catch (e) {
>+        errorThrown = true;
>+        error = e;
>+    }
>+    if (!errorThrown)
>+        throw new Error('not thrown');
>+    if (String(error) !== errorMessage)
>+        throw new Error(`bad error: ${String(error)}`);
>+}
>+
>+shouldThrow(() => {
>+    $.evalScript(`const NaN = 42`);
>+}, `SyntaxError: Can't create duplicate variable that shadows a global property: 'NaN'`);
>diff --git a/JSTests/stress/let-lexical-binding-shadow-existing-global-property-baseline.js b/JSTests/stress/let-lexical-binding-shadow-existing-global-property-baseline.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..5d252d7924ed4f04c4eb71f338697079672a7c5e
>--- /dev/null
>+++ b/JSTests/stress/let-lexical-binding-shadow-existing-global-property-baseline.js
>@@ -0,0 +1,41 @@
>+//@ runDefault("--useDFGJIT=false", "--useConcurrentJIT=0")
>+function shouldThrow(func, errorMessage) {
>+    var errorThrown = false;
>+    var error = null;
>+    try {
>+        func();
>+    } catch (e) {
>+        errorThrown = true;
>+        error = e;
>+    }
>+    if (!errorThrown)
>+        throw new Error('not thrown');
>+    if (String(error) !== errorMessage)
>+        throw new Error(`bad error: ${String(error)}`);
>+}
>+noInline(shouldThrow);
>+
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error(`bad value: ${String(actual)}`);
>+}
>+noInline(shouldBe);
>+
>+function foo() {
>+    bar = 4;
>+}
>+function get() {
>+    return bar;
>+}
>+for (var i = 0; i < 100; ++i)
>+    foo();
>+for (var i = 0; i < 100; ++i)
>+    shouldBe(get(), 4);
>+
>+shouldBe(bar, 4);
>+$.evalScript('let bar = 3;');
>+shouldBe(bar, 3);
>+shouldBe(get(), 3);
>+foo();
>+shouldBe(bar, 4);
>+shouldBe(get(), 4);
>diff --git a/JSTests/stress/let-lexical-binding-shadow-existing-global-property-tdz-baseline.js b/JSTests/stress/let-lexical-binding-shadow-existing-global-property-tdz-baseline.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..e100b9a8682f8980029fa911c12aba5b95d8e501
>--- /dev/null
>+++ b/JSTests/stress/let-lexical-binding-shadow-existing-global-property-tdz-baseline.js
>@@ -0,0 +1,47 @@
>+//@ runDefault("--useDFGJIT=false", "--useConcurrentJIT=0")
>+function shouldThrow(func, errorMessage) {
>+    var errorThrown = false;
>+    var error = null;
>+    try {
>+        func();
>+    } catch (e) {
>+        errorThrown = true;
>+        error = e;
>+    }
>+    if (!errorThrown)
>+        throw new Error('not thrown');
>+    if (String(error) !== errorMessage)
>+        throw new Error(`bad error: ${String(error)}`);
>+}
>+noInline(shouldThrow);
>+
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error(`bad value: ${String(actual)}`);
>+}
>+noInline(shouldBe);
>+
>+function foo() {
>+    bar = 4;
>+}
>+function get() {
>+    return bar;
>+}
>+for (var i = 0; i < 1000; ++i)
>+    foo();
>+for (var i = 0; i < 1000; ++i)
>+    shouldBe(get(), 4);
>+
>+shouldBe(bar, 4);
>+shouldThrow(() => {
>+    $.evalScript('get(); let bar = 3;');
>+}, `ReferenceError: Cannot access uninitialized variable.`);
>+shouldThrow(() => {
>+    shouldBe(bar, 3);
>+}, `ReferenceError: Cannot access uninitialized variable.`);
>+shouldThrow(() => {
>+    shouldBe(get(), 3);
>+}, `ReferenceError: Cannot access uninitialized variable.`);
>+shouldThrow(() => {
>+    $.evalScript('bar;');
>+}, `ReferenceError: Cannot access uninitialized variable.`);
>diff --git a/JSTests/stress/let-lexical-binding-shadow-existing-global-property.js b/JSTests/stress/let-lexical-binding-shadow-existing-global-property.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..0ccfd587f6ae2495fb65bb147a5f46d21ba9b3c9
>--- /dev/null
>+++ b/JSTests/stress/let-lexical-binding-shadow-existing-global-property.js
>@@ -0,0 +1,31 @@
>+function shouldThrow(func, errorMessage) {
>+    var errorThrown = false;
>+    var error = null;
>+    try {
>+        func();
>+    } catch (e) {
>+        errorThrown = true;
>+        error = e;
>+    }
>+    if (!errorThrown)
>+        throw new Error('not thrown');
>+    if (String(error) !== errorMessage)
>+        throw new Error(`bad error: ${String(error)}`);
>+}
>+noInline(shouldThrow);
>+
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error(`bad value: ${String(actual)}`);
>+}
>+noInline(shouldBe);
>+
>+function foo() {
>+    bar = 4;
>+}
>+foo();
>+shouldBe(bar, 4);
>+$.evalScript('let bar = 3;');
>+shouldBe(bar, 3);
>+foo();
>+shouldBe(bar, 4);

Actions: View | Formatted Diff | Diff

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