WebKit Bugzilla
Attachment 358776 Details for
Bug 193308
: [JSC] Global lexical bindings can shadow global variables if it is `configurable = true`
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[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:
Patch
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; > } >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193308
:
358776
|
358777
|
358811
|
358827
|
358845
|
358855
|
358872
|
358875
|
358877
|
358879
|
358912
|
358927
|
358929
|
358942