WebKit Bugzilla
Attachment 360694 Details for
Bug 194082
: [JSC] Symbol should be in destructibleCellSpace
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-194082-20190130232227.patch (text/plain), 14.53 KB, created by
Yusuke Suzuki
on 2019-01-30 23:22:27 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Yusuke Suzuki
Created:
2019-01-30 23:22:27 PST
Size:
14.53 KB
patch
obsolete
>Subversion Revision: 240759 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index aeeabc82b18046ae1c614422d45dfda790b7d8d2..39bf91795f23790cb082eae36b94b13625daaff9 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,34 @@ >+2019-01-30 Yusuke Suzuki <ysuzuki@apple.com> >+ >+ [JSC] Symbol should be in destructibleCellSpace >+ https://bugs.webkit.org/show_bug.cgi?id=194082 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Because Symbol's member was not poisoned, we changed the subspace for Symbol from destructibleCellSpace >+ to cellJSValueOOBSpace. But the problem is cellJSValueOOBSpace is a space for cells which are not >+ destructible. As a result, Symbol::destroy is never called, and SymbolImpl is leaked. This patch poisons >+ the member of Symbol and make Symbol's space destructibleCellSpace again. >+ >+ * dfg/DFGOperations.cpp: >+ * dfg/DFGSpeculativeJIT64.cpp: >+ (JSC::DFG::SpeculativeJIT::compile): >+ * ftl/FTLLowerDFGToB3.cpp: >+ (JSC::FTL::DFG::LowerDFGToB3::compileHasOwnProperty): >+ * runtime/JSCJSValueInlines.h: >+ (JSC::JSValue::toPropertyKey const): >+ * runtime/JSCPoison.h: >+ * runtime/JSFunction.cpp: >+ (JSC::JSFunction::setFunctionName): >+ * runtime/Symbol.cpp: >+ (JSC::Symbol::Symbol): >+ (JSC::Symbol::finishCreation): >+ (JSC::Symbol::descriptiveString const): >+ (JSC::Symbol::description const): >+ * runtime/Symbol.h: >+ * runtime/SymbolConstructor.cpp: >+ (JSC::symbolConstructorKeyFor): >+ > 2019-01-30 Michael Catanzaro <mcatanzaro@igalia.com> > > Unreviewed, fix GCC build after r240730 >diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp >index 22750b7997af9c9a6b0eea3a5acfced1458c7f2e..afede9d4772d7c2d72ff39a04bf3e2df714aefae 100644 >--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp >+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp >@@ -782,7 +782,7 @@ EncodedJSValue JIT_OPERATION operationGetByValObjectSymbol(ExecState* exec, JSCe > VM& vm = exec->vm(); > NativeCallFrameTracer tracer(&vm, exec); > >- return JSValue::encode(getByValObject(exec, vm, asObject(base), asSymbol(symbol)->privateName())); >+ return JSValue::encode(getByValObject(exec, vm, asObject(base), Identifier::fromUid(&vm, &asSymbol(symbol)->symbolImpl()))); > } > > void JIT_OPERATION operationPutByValStrict(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedProperty, EncodedJSValue encodedValue) >@@ -838,7 +838,7 @@ void JIT_OPERATION operationPutByValCellSymbolStrict(ExecState* exec, JSCell* ce > VM& vm = exec->vm(); > NativeCallFrameTracer tracer(&vm, exec); > >- putByValCellInternal<true, false>(exec, vm, cell, asSymbol(symbol)->privateName(), JSValue::decode(encodedValue)); >+ putByValCellInternal<true, false>(exec, vm, cell, Identifier::fromUid(&vm, &asSymbol(symbol)->symbolImpl()), JSValue::decode(encodedValue)); > } > > void JIT_OPERATION operationPutByValCellSymbolNonStrict(ExecState* exec, JSCell* cell, JSCell* symbol, EncodedJSValue encodedValue) >@@ -846,7 +846,7 @@ void JIT_OPERATION operationPutByValCellSymbolNonStrict(ExecState* exec, JSCell* > VM& vm = exec->vm(); > NativeCallFrameTracer tracer(&vm, exec); > >- putByValCellInternal<false, false>(exec, vm, cell, asSymbol(symbol)->privateName(), JSValue::decode(encodedValue)); >+ putByValCellInternal<false, false>(exec, vm, cell, Identifier::fromUid(&vm, &asSymbol(symbol)->symbolImpl()), JSValue::decode(encodedValue)); > } > > void JIT_OPERATION operationPutByValBeyondArrayBoundsStrict(ExecState* exec, JSObject* object, int32_t index, EncodedJSValue encodedValue) >@@ -998,7 +998,7 @@ void JIT_OPERATION operationPutByValDirectCellSymbolStrict(ExecState* exec, JSCe > VM& vm = exec->vm(); > NativeCallFrameTracer tracer(&vm, exec); > >- putByValCellInternal<true, true>(exec, vm, cell, asSymbol(symbol)->privateName(), JSValue::decode(encodedValue)); >+ putByValCellInternal<true, true>(exec, vm, cell, Identifier::fromUid(&vm, &asSymbol(symbol)->symbolImpl()), JSValue::decode(encodedValue)); > } > > void JIT_OPERATION operationPutByValDirectCellSymbolNonStrict(ExecState* exec, JSCell* cell, JSCell* symbol, EncodedJSValue encodedValue) >@@ -1006,7 +1006,7 @@ void JIT_OPERATION operationPutByValDirectCellSymbolNonStrict(ExecState* exec, J > VM& vm = exec->vm(); > NativeCallFrameTracer tracer(&vm, exec); > >- putByValCellInternal<false, true>(exec, vm, cell, asSymbol(symbol)->privateName(), JSValue::decode(encodedValue)); >+ putByValCellInternal<false, true>(exec, vm, cell, Identifier::fromUid(&vm, &asSymbol(symbol)->symbolImpl()), JSValue::decode(encodedValue)); > } > > void JIT_OPERATION operationPutByValDirectBeyondArrayBoundsStrict(ExecState* exec, JSObject* object, int32_t index, EncodedJSValue encodedValue) >@@ -1558,7 +1558,7 @@ void JIT_OPERATION operationDefineDataPropertySymbol(ExecState* exec, JSObject* > { > VM& vm = exec->vm(); > NativeCallFrameTracer tracer(&vm, exec); >- defineDataProperty(exec, vm, base, Identifier::fromUid(property->privateName()), JSValue::decode(encodedValue), attributes); >+ defineDataProperty(exec, vm, base, Identifier::fromUid(&vm, &property->symbolImpl()), JSValue::decode(encodedValue), attributes); > } > > ALWAYS_INLINE static void defineAccessorProperty(ExecState* exec, VM& vm, JSObject* base, const Identifier& propertyName, JSObject* getter, JSObject* setter, int32_t attributes) >@@ -1604,7 +1604,7 @@ void JIT_OPERATION operationDefineAccessorPropertySymbol(ExecState* exec, JSObje > { > VM& vm = exec->vm(); > NativeCallFrameTracer tracer(&vm, exec); >- defineAccessorProperty(exec, vm, base, Identifier::fromUid(property->privateName()), getter, setter, attributes); >+ defineAccessorProperty(exec, vm, base, Identifier::fromUid(&vm, &property->symbolImpl()), getter, setter, attributes); > } > > char* JIT_OPERATION operationNewArray(ExecState* exec, Structure* arrayStructure, void* buffer, size_t size) >diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp >index 7f9e381b755d264ca75e3e7dcaca80c8741dde66..37b1ffc2662adc15022952ef85152b4be7b77694 100644 >--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp >+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp >@@ -4407,6 +4407,7 @@ void SpeculativeJIT::compile(Node* node) > case SymbolUse: { > speculateSymbol(node->child2(), keyGPR); > m_jit.loadPtr(MacroAssembler::Address(keyGPR, Symbol::offsetOfSymbolImpl()), implGPR); >+ m_jit.xorPtr(JITCompiler::TrustedImmPtr(SymbolPoison::key()), implGPR); > break; > } > case StringUse: { >@@ -4431,6 +4432,7 @@ void SpeculativeJIT::compile(Node* node) > isNotString.link(&m_jit); > slowPath.append(m_jit.branchIfNotSymbol(keyGPR)); > m_jit.loadPtr(MacroAssembler::Address(keyGPR, Symbol::offsetOfSymbolImpl()), implGPR); >+ m_jit.xorPtr(JITCompiler::TrustedImmPtr(SymbolPoison::key()), implGPR); > > hasUniquedImpl.link(&m_jit); > break; >diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp >index 4f3a5a054b58b145c70c8a4a6a4af6cc0f5a0807..c7442d4f1b9a71abfebf952900058715a2122993 100644 >--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp >+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp >@@ -10117,7 +10117,7 @@ class LowerDFGToB3 { > } > case SymbolUse: { > keyAsValue = lowSymbol(m_node->child2()); >- uniquedStringImpl = m_out.loadPtr(keyAsValue, m_heaps.Symbol_symbolImpl); >+ uniquedStringImpl = m_out.bitXor(m_out.loadPtr(keyAsValue, m_heaps.Symbol_symbolImpl), m_out.constInt64(SymbolPoison::key())); > lastNext = m_out.insertNewBlocksBefore(slowCase); > break; > } >@@ -10148,7 +10148,7 @@ class LowerDFGToB3 { > m_out.branch(isSymbol(keyAsValue), unsure(isSymbolCase), unsure(slowCase)); > > m_out.appendTo(isSymbolCase, hasUniquedStringImpl); >- ValueFromBlock symbolResult = m_out.anchor(m_out.loadPtr(keyAsValue, m_heaps.Symbol_symbolImpl)); >+ ValueFromBlock symbolResult = m_out.anchor(m_out.bitXor(m_out.loadPtr(keyAsValue, m_heaps.Symbol_symbolImpl), m_out.constInt64(SymbolPoison::key()))); > m_out.jump(hasUniquedStringImpl); > > m_out.appendTo(hasUniquedStringImpl, slowCase); >diff --git a/Source/JavaScriptCore/runtime/JSCJSValueInlines.h b/Source/JavaScriptCore/runtime/JSCJSValueInlines.h >index 6cf3a223c2dabecd28c429bf0725415328d24d1a..412b073dab2f052f0890ed2a27199d1fcdedf0c0 100644 >--- a/Source/JavaScriptCore/runtime/JSCJSValueInlines.h >+++ b/Source/JavaScriptCore/runtime/JSCJSValueInlines.h >@@ -652,7 +652,7 @@ ALWAYS_INLINE Identifier JSValue::toPropertyKey(ExecState* exec) const > JSValue primitive = toPrimitive(exec, PreferString); > RETURN_IF_EXCEPTION(scope, vm.propertyNames->emptyIdentifier); > if (primitive.isSymbol()) >- RELEASE_AND_RETURN(scope, Identifier::fromUid(asSymbol(primitive)->privateName())); >+ RELEASE_AND_RETURN(scope, Identifier::fromUid(&vm, &asSymbol(primitive)->symbolImpl())); > > RELEASE_AND_RETURN(scope, primitive.toString(exec)->toIdentifier(exec)); > } >diff --git a/Source/JavaScriptCore/runtime/JSCPoison.h b/Source/JavaScriptCore/runtime/JSCPoison.h >index 1db4ac6d5463019d9fc6fa11883107c851da840d..ced826d875b7cee4fbe1625b13d3bef41ff42211 100644 >--- a/Source/JavaScriptCore/runtime/JSCPoison.h >+++ b/Source/JavaScriptCore/runtime/JSCPoison.h >@@ -51,6 +51,7 @@ namespace JSC { > v(NativeCode) \ > v(ScopedArguments) \ > v(StructureTransitionTable) \ >+ v(Symbol) \ > v(UnlinkedSourceCode) \ > v(WebAssemblyFunctionBase) \ > v(WebAssemblyModuleRecord) \ >diff --git a/Source/JavaScriptCore/runtime/JSFunction.cpp b/Source/JavaScriptCore/runtime/JSFunction.cpp >index 50568e846b6a7f0388c9ea469df9edfc77504ab3..94b639e8386c5970db03cc2af94f90badfaecd72 100644 >--- a/Source/JavaScriptCore/runtime/JSFunction.cpp >+++ b/Source/JavaScriptCore/runtime/JSFunction.cpp >@@ -669,7 +669,7 @@ void JSFunction::setFunctionName(ExecState* exec, JSValue value) > ASSERT(jsExecutable()->ecmaName().isNull()); > String name; > if (value.isSymbol()) { >- SymbolImpl& uid = asSymbol(value)->privateName().uid(); >+ SymbolImpl& uid = asSymbol(value)->symbolImpl(); > if (uid.isNullSymbol()) > name = emptyString(); > else >diff --git a/Source/JavaScriptCore/runtime/Symbol.cpp b/Source/JavaScriptCore/runtime/Symbol.cpp >index d83e31d1029c3ea29931c1de23b00bc4ecf6c109..380dabac19bf2366cf48ed73c21995af9f312f0d 100644 >--- a/Source/JavaScriptCore/runtime/Symbol.cpp >+++ b/Source/JavaScriptCore/runtime/Symbol.cpp >@@ -37,19 +37,19 @@ const ClassInfo Symbol::s_info = { "symbol", nullptr, nullptr, nullptr, CREATE_M > > Symbol::Symbol(VM& vm) > : Base(vm, vm.symbolStructure.get()) >- , m_privateName() >+ , m_symbolImpl(SymbolImpl::createNullSymbol()) > { > } > >-Symbol::Symbol(VM& vm, const String& string) >+Symbol::Symbol(VM& vm, const String& description) > : Base(vm, vm.symbolStructure.get()) >- , m_privateName(PrivateName::Description, string) >+ , m_symbolImpl(SymbolImpl::create(*description.impl())) > { > } > > Symbol::Symbol(VM& vm, SymbolImpl& uid) > : Base(vm, vm.symbolStructure.get()) >- , m_privateName(uid) >+ , m_symbolImpl(uid) > { > } > >@@ -58,7 +58,7 @@ void Symbol::finishCreation(VM& vm) > Base::finishCreation(vm); > ASSERT(inherits(vm, info())); > >- vm.symbolImplToSymbolMap.set(&m_privateName.uid(), this); >+ vm.symbolImplToSymbolMap.set(m_symbolImpl.ptr(), this); > } > > inline SymbolObject* SymbolObject::create(VM& vm, JSGlobalObject* globalObject, Symbol* symbol) >@@ -100,12 +100,12 @@ void Symbol::destroy(JSCell* cell) > > String Symbol::descriptiveString() const > { >- return makeString("Symbol(", String(privateName().uid()), ')'); >+ return makeString("Symbol(", String(symbolImpl()), ')'); > } > > String Symbol::description() const > { >- auto& uid = privateName().uid(); >+ auto& uid = symbolImpl(); > return uid.isNullSymbol() ? String() : uid; > } > >diff --git a/Source/JavaScriptCore/runtime/Symbol.h b/Source/JavaScriptCore/runtime/Symbol.h >index 71023e61abf00d44edcc851ba92264369710ecc6..403a3c17bf621ff8ec458fa24382c4ef564070cf 100644 >--- a/Source/JavaScriptCore/runtime/Symbol.h >+++ b/Source/JavaScriptCore/runtime/Symbol.h >@@ -26,8 +26,9 @@ > > #pragma once > >+#include "JSCPoison.h" > #include "JSString.h" >-#include "PrivateName.h" >+#include <wtf/text/SymbolImpl.h> > > namespace JSC { > >@@ -36,12 +37,6 @@ class Symbol final : public JSCell { > typedef JSCell Base; > static const unsigned StructureFlags = Base::StructureFlags | StructureIsImmortal | OverridesToThis; > >- template<typename> >- static CompleteSubspace* subspaceFor(VM& vm) >- { >- return &vm.cellJSValueOOBSpace; >- } >- > DECLARE_EXPORT_INFO; > > static const bool needsDestruction = true; >@@ -55,7 +50,7 @@ class Symbol final : public JSCell { > static Symbol* create(ExecState*, JSString* description); > JS_EXPORT_PRIVATE static Symbol* create(VM&, SymbolImpl& uid); > >- const PrivateName& privateName() const { return m_privateName; } >+ SymbolImpl& symbolImpl() const { return m_symbolImpl.get(); } > String descriptiveString() const; > String description() const; > >@@ -66,8 +61,7 @@ class Symbol final : public JSCell { > > static ptrdiff_t offsetOfSymbolImpl() > { >- // PrivateName is just a Ref<SymbolImpl> which can just be used as a SymbolImpl*. >- return OBJECT_OFFSETOF(Symbol, m_privateName); >+ return OBJECT_OFFSETOF(Symbol, m_symbolImpl); > } > > protected: >@@ -79,7 +73,7 @@ class Symbol final : public JSCell { > > void finishCreation(VM&); > >- PrivateName m_privateName; >+ PoisonedRef<SymbolPoison, SymbolImpl> m_symbolImpl; > }; > > Symbol* asSymbol(JSValue); >diff --git a/Source/JavaScriptCore/runtime/SymbolConstructor.cpp b/Source/JavaScriptCore/runtime/SymbolConstructor.cpp >index ac6fe851f5154f5f0d1ad8009054cd64fdad2a90..999d5655b98c62315fbb305e345b3a2e0de18c26 100644 >--- a/Source/JavaScriptCore/runtime/SymbolConstructor.cpp >+++ b/Source/JavaScriptCore/runtime/SymbolConstructor.cpp >@@ -109,7 +109,7 @@ EncodedJSValue JSC_HOST_CALL symbolConstructorKeyFor(ExecState* exec) > if (!symbolValue.isSymbol()) > return JSValue::encode(throwTypeError(exec, scope, SymbolKeyForTypeError)); > >- SymbolImpl& uid = asSymbol(symbolValue)->privateName().uid(); >+ SymbolImpl& uid = asSymbol(symbolValue)->symbolImpl(); > if (!uid.symbolRegistry()) > return JSValue::encode(jsUndefined()); >
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
Flags:
saam
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194082
: 360694 |
360695
|
360696