Attachment 360046 Details for Bug 193774 – Patch

[patch] Patch

bug-193774-20190124162424.patch (text/plain), 16.28 KB, created by Yusuke Suzuki on 2019-01-24 16:24:25 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-01-24 16:24:25 PST

Size: 16.28 KB

patch

obsolete

>Subversion Revision: 240449
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 9ae276c0f84fa796457251aeb27ed8892cc0eff8..daf5aa7495f1ab2231b07e15796e3939ca841422 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,42 @@
>+2019-01-24  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] SharedArrayBufferConstructor and ArrayBufferConstructor should not have their own IsoSubspace
>+        https://bugs.webkit.org/show_bug.cgi?id=193774
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We put all the instances of InternalFunction and its subclasses in IsoSubspace to make safer from UAF.
>+        But since IsoSubspace requires the memory layout of instances is the same, we created different IsoSubspace
>+        for subclasses of InternalFunction if sizeof(subclass) != sizeof(InternalFunction). One example is
>+        ArrayBufferConstructor and SharedArrayBufferConstructor. But it is too costly to allocate 16KB page just
>+        for these two constructor instances. They are only two instances per JSGlobalObject.
>+
>+        This patch makes sizeof(ArrayBufferConstructor) == sizeof(InternalFunction) so that they can use IsoSubspace
>+        of InternalFunction. We introduce JSGenericArrayBufferConstructor, and it takes ArrayBufferSharingMode as
>+        its template parameter. We define JSArrayBufferConstructor as JSGenericArrayBufferConstructor<ArrayBufferSharingMode::Default>
>+        and JSSharedArrayBufferConstructor as JSGenericArrayBufferConstructor<ArrayBufferSharingMode::Shared> so that
>+        we do not need to hold ArrayBufferSharingMode in the field of the constructor. This change removes IsoSubspace
>+        for ArrayBufferConstructors, and reduces the memory usage.
>+
>+        * runtime/JSArrayBufferConstructor.cpp:
>+        (JSC::JSGenericArrayBufferConstructor<sharingMode>::JSGenericArrayBufferConstructor):
>+        (JSC::JSGenericArrayBufferConstructor<sharingMode>::finishCreation):
>+        (JSC::JSGenericArrayBufferConstructor<sharingMode>::constructArrayBuffer):
>+        (JSC::JSGenericArrayBufferConstructor<sharingMode>::createStructure):
>+        (JSC::JSGenericArrayBufferConstructor<sharingMode>::info):
>+        (JSC::JSArrayBufferConstructor::JSArrayBufferConstructor): Deleted.
>+        (JSC::JSArrayBufferConstructor::finishCreation): Deleted.
>+        (JSC::JSArrayBufferConstructor::create): Deleted.
>+        (JSC::JSArrayBufferConstructor::createStructure): Deleted.
>+        (JSC::constructArrayBuffer): Deleted.
>+        * runtime/JSArrayBufferConstructor.h:
>+        * runtime/JSGlobalObject.cpp:
>+        (JSC::JSGlobalObject::init):
>+        * runtime/JSGlobalObject.h:
>+        * runtime/VM.cpp:
>+        (JSC::VM::VM):
>+        * runtime/VM.h:
>+
> 2019-01-24  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         stress/const-semantics.js fails a dfg-eager / ftl-eager run with an ASAN release build.
>diff --git a/Source/JavaScriptCore/runtime/JSArrayBufferConstructor.cpp b/Source/JavaScriptCore/runtime/JSArrayBufferConstructor.cpp
>index 9d7422f84b963b29822452868c6f2dd5028b380b..e6e8e0d012576bf50c3590df0be2e22e737e0f8e 100644
>--- a/Source/JavaScriptCore/runtime/JSArrayBufferConstructor.cpp
>+++ b/Source/JavaScriptCore/runtime/JSArrayBufferConstructor.cpp
>@@ -38,60 +38,50 @@
> namespace JSC {
> 
> static EncodedJSValue JSC_HOST_CALL arrayBufferFuncIsView(ExecState*);
>+static EncodedJSValue JSC_HOST_CALL callArrayBuffer(ExecState*);
> 
>+template<>
> const ClassInfo JSArrayBufferConstructor::s_info = {
>     "Function", &Base::s_info, nullptr, nullptr,
>     CREATE_METHOD_TABLE(JSArrayBufferConstructor)
> };
> 
>-static EncodedJSValue JSC_HOST_CALL callArrayBuffer(ExecState*);
>-static EncodedJSValue JSC_HOST_CALL constructArrayBuffer(ExecState*);
>+template<>
>+const ClassInfo JSSharedArrayBufferConstructor::s_info = {
>+    "Function", &Base::s_info, nullptr, nullptr,
>+    CREATE_METHOD_TABLE(JSSharedArrayBufferConstructor)
>+};
> 
>-JSArrayBufferConstructor::JSArrayBufferConstructor(VM& vm, Structure* structure, ArrayBufferSharingMode sharingMode)
>-    : Base(vm, structure, callArrayBuffer, constructArrayBuffer)
>-    , m_sharingMode(sharingMode)
>+template<ArrayBufferSharingMode sharingMode>
>+JSGenericArrayBufferConstructor<sharingMode>::JSGenericArrayBufferConstructor(VM& vm, Structure* structure)
>+    : Base(vm, structure, callArrayBuffer, JSGenericArrayBufferConstructor<sharingMode>::constructArrayBuffer)
> {
> }
> 
>-void JSArrayBufferConstructor::finishCreation(VM& vm, JSArrayBufferPrototype* prototype, GetterSetter* speciesSymbol)
>+template<ArrayBufferSharingMode sharingMode>
>+void JSGenericArrayBufferConstructor<sharingMode>::finishCreation(VM& vm, JSArrayBufferPrototype* prototype, GetterSetter* speciesSymbol)
> {
>-    Base::finishCreation(vm, arrayBufferSharingModeName(m_sharingMode));
>+    Base::finishCreation(vm, arrayBufferSharingModeName(sharingMode));
>     putDirectWithoutTransition(vm, vm.propertyNames->prototype, prototype, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
>     putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(1), PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly);
>     putDirectNonIndexAccessor(vm, vm.propertyNames->speciesSymbol, speciesSymbol, PropertyAttribute::Accessor | PropertyAttribute::ReadOnly | PropertyAttribute::DontEnum);
> 
>-    if (m_sharingMode == ArrayBufferSharingMode::Default) {
>+    if (sharingMode == ArrayBufferSharingMode::Default) {
>         JSGlobalObject* globalObject = this->globalObject(vm);
>         JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->isView, arrayBufferFuncIsView, static_cast<unsigned>(PropertyAttribute::DontEnum), 1);
>         JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->builtinNames().isViewPrivateName(), arrayBufferFuncIsView, static_cast<unsigned>(PropertyAttribute::DontEnum), 1);
>     }
> }
> 
>-JSArrayBufferConstructor* JSArrayBufferConstructor::create(VM& vm, Structure* structure, JSArrayBufferPrototype* prototype, GetterSetter* speciesSymbol, ArrayBufferSharingMode sharingMode)
>-{
>-    JSArrayBufferConstructor* result =
>-        new (NotNull, allocateCell<JSArrayBufferConstructor>(vm.heap))
>-        JSArrayBufferConstructor(vm, structure, sharingMode);
>-    result->finishCreation(vm, prototype, speciesSymbol);
>-    return result;
>-}
>-
>-Structure* JSArrayBufferConstructor::createStructure(
>-    VM& vm, JSGlobalObject* globalObject, JSValue prototype)
>-{
>-    return Structure::create(
>-        vm, globalObject, prototype, TypeInfo(InternalFunctionType, StructureFlags), info());
>-}
>-
>-static EncodedJSValue JSC_HOST_CALL constructArrayBuffer(ExecState* exec)
>+template<ArrayBufferSharingMode sharingMode>
>+EncodedJSValue JSC_HOST_CALL JSGenericArrayBufferConstructor<sharingMode>::constructArrayBuffer(ExecState* exec)
> {
>     VM& vm = exec->vm();
>     auto scope = DECLARE_THROW_SCOPE(vm);
> 
>-    JSArrayBufferConstructor* constructor =
>-        jsCast<JSArrayBufferConstructor*>(exec->jsCallee());
>+    JSGenericArrayBufferConstructor* constructor = jsCast<JSGenericArrayBufferConstructor*>(exec->jsCallee());
> 
>-    Structure* arrayBufferStructure = InternalFunction::createSubclassStructure(exec, exec->newTarget(), constructor->globalObject(vm)->arrayBufferStructure(constructor->sharingMode()));
>+    Structure* arrayBufferStructure = InternalFunction::createSubclassStructure(exec, exec->newTarget(), constructor->globalObject(vm)->arrayBufferStructure(sharingMode));
>     RETURN_IF_EXCEPTION(scope, { });
> 
>     unsigned length;
>@@ -109,14 +99,26 @@ static EncodedJSValue JSC_HOST_CALL constructArrayBuffer(ExecState* exec)
>     if (!buffer)
>         return JSValue::encode(throwOutOfMemoryError(exec, scope));
>     
>-    if (constructor->sharingMode() == ArrayBufferSharingMode::Shared)
>+    if (sharingMode == ArrayBufferSharingMode::Shared)
>         buffer->makeShared();
>-    ASSERT(constructor->sharingMode() == buffer->sharingMode());
>+    ASSERT(sharingMode == buffer->sharingMode());
> 
>     JSArrayBuffer* result = JSArrayBuffer::create(vm, arrayBufferStructure, WTFMove(buffer));
>     return JSValue::encode(result);
> }
> 
>+template<ArrayBufferSharingMode sharingMode>
>+Structure* JSGenericArrayBufferConstructor<sharingMode>::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
>+{
>+    return Structure::create(vm, globalObject, prototype, TypeInfo(InternalFunctionType, StructureFlags), info());
>+}
>+
>+template<ArrayBufferSharingMode sharingMode>
>+const ClassInfo* JSGenericArrayBufferConstructor<sharingMode>::info()
>+{
>+    return &JSGenericArrayBufferConstructor<sharingMode>::s_info;
>+}
>+
> static EncodedJSValue JSC_HOST_CALL callArrayBuffer(ExecState* exec)
> {
>     VM& vm = exec->vm();
>@@ -131,7 +133,10 @@ EncodedJSValue JSC_HOST_CALL arrayBufferFuncIsView(ExecState* exec)
> {
>     return JSValue::encode(jsBoolean(jsDynamicCast<JSArrayBufferView*>(exec->vm(), exec->argument(0))));
> }
>-    
>+
>+// Instantiate JSGenericArrayBufferConstructors.
>+template class JSGenericArrayBufferConstructor<ArrayBufferSharingMode::Shared>;
>+template class JSGenericArrayBufferConstructor<ArrayBufferSharingMode::Default>;
> 
> } // namespace JSC
> 
>diff --git a/Source/JavaScriptCore/runtime/JSArrayBufferConstructor.h b/Source/JavaScriptCore/runtime/JSArrayBufferConstructor.h
>index 314b4038d0699fc2ebd8c2b169a10c850494267c..42f163fe38acc4f7cccc93e87be16962c9ccb34b 100644
>--- a/Source/JavaScriptCore/runtime/JSArrayBufferConstructor.h
>+++ b/Source/JavaScriptCore/runtime/JSArrayBufferConstructor.h
>@@ -33,31 +33,35 @@ namespace JSC {
> class JSArrayBufferPrototype;
> class GetterSetter;
> 
>-class JSArrayBufferConstructor final : public InternalFunction {
>+template<ArrayBufferSharingMode sharingMode>
>+class JSGenericArrayBufferConstructor final : public InternalFunction {
> public:
>-    typedef InternalFunction Base;
>-
>-    template<typename CellType>
>-    static IsoSubspace* subspaceFor(VM& vm)
>-    {
>-        return &vm.arrayBufferConstructorSpace;
>-    }
>+    using Base = InternalFunction;
> 
> protected:
>-    JSArrayBufferConstructor(VM&, Structure*, ArrayBufferSharingMode);
>+    JSGenericArrayBufferConstructor(VM&, Structure*);
>     void finishCreation(VM&, JSArrayBufferPrototype*, GetterSetter* speciesSymbol);
> 
>+    static EncodedJSValue JSC_HOST_CALL constructArrayBuffer(ExecState*);
>+
> public:
>-    static JSArrayBufferConstructor* create(VM&, Structure*, JSArrayBufferPrototype*, GetterSetter* speciesSymbol, ArrayBufferSharingMode);
>-    
>-    DECLARE_INFO;
>-    
>+    static JSGenericArrayBufferConstructor* create(VM& vm, Structure* structure, JSArrayBufferPrototype* prototype, GetterSetter* speciesSymbol)
>+    {
>+        JSGenericArrayBufferConstructor* result =
>+            new (NotNull, allocateCell<JSGenericArrayBufferConstructor>(vm.heap)) JSGenericArrayBufferConstructor(vm, structure);
>+        result->finishCreation(vm, prototype, speciesSymbol);
>+        return result;
>+    }
>+
>     static Structure* createStructure(VM&, JSGlobalObject*, JSValue prototype);
>     
>-    ArrayBufferSharingMode sharingMode() const { return m_sharingMode; }
>-
>-private:
>-    ArrayBufferSharingMode m_sharingMode;
>+    static const ClassInfo s_info; // This is never accessed directly, since that would break linkage on some compilers.
>+    static const ClassInfo* info();
> };
> 
>+using JSArrayBufferConstructor = JSGenericArrayBufferConstructor<ArrayBufferSharingMode::Default>;
>+using JSSharedArrayBufferConstructor = JSGenericArrayBufferConstructor<ArrayBufferSharingMode::Shared>;
>+static_assert(sizeof(JSArrayBufferConstructor::Base) == sizeof(JSArrayBufferConstructor), "");
>+static_assert(sizeof(JSSharedArrayBufferConstructor::Base) == sizeof(JSSharedArrayBufferConstructor), "");
>+
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>index 127c105f961149aa78a8e08d7767d144e354cda7..cce56a21bbc0366b0ff89743b51e5c5244527117 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>@@ -676,12 +676,12 @@ void JSGlobalObject::init(VM& vm)
>     
>     m_regExpConstructor.set(vm, this, RegExpConstructor::create(vm, RegExpConstructor::createStructure(vm, this, m_functionPrototype.get()), m_regExpPrototype.get(), m_speciesGetterSetter.get()));
>     
>-    JSArrayBufferConstructor* arrayBufferConstructor = JSArrayBufferConstructor::create(vm, JSArrayBufferConstructor::createStructure(vm, this, m_functionPrototype.get()), m_arrayBufferPrototype.get(), m_speciesGetterSetter.get(), ArrayBufferSharingMode::Default);
>+    JSArrayBufferConstructor* arrayBufferConstructor = JSArrayBufferConstructor::create(vm, JSArrayBufferConstructor::createStructure(vm, this, m_functionPrototype.get()), m_arrayBufferPrototype.get(), m_speciesGetterSetter.get());
>     m_arrayBufferPrototype->putDirectWithoutTransition(vm, vm.propertyNames->constructor, arrayBufferConstructor, static_cast<unsigned>(PropertyAttribute::DontEnum));
> 
> #if ENABLE(SHARED_ARRAY_BUFFER)
>-    JSArrayBufferConstructor* sharedArrayBufferConstructor = nullptr;
>-    sharedArrayBufferConstructor = JSArrayBufferConstructor::create(vm, JSArrayBufferConstructor::createStructure(vm, this, m_functionPrototype.get()), m_sharedArrayBufferPrototype.get(), m_speciesGetterSetter.get(), ArrayBufferSharingMode::Shared);
>+    JSSharedArrayBufferConstructor* sharedArrayBufferConstructor = nullptr;
>+    sharedArrayBufferConstructor = JSSharedArrayBufferConstructor::create(vm, JSSharedArrayBufferConstructor::createStructure(vm, this, m_functionPrototype.get()), m_sharedArrayBufferPrototype.get(), m_speciesGetterSetter.get());
>     m_sharedArrayBufferPrototype->putDirectWithoutTransition(vm, vm.propertyNames->constructor, sharedArrayBufferConstructor, static_cast<unsigned>(PropertyAttribute::DontEnum));
> 
>     AtomicsObject* atomicsObject = AtomicsObject::create(vm, this, AtomicsObject::createStructure(vm, this, m_objectPrototype.get()));
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>index 66ce16848b1c0d7109df3c86ac8e9fc1b44025a3..15f0637922ab4bfdcbe0c06163240d7c364c78cf 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObject.h
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>@@ -82,7 +82,6 @@ class IndirectEvalExecutable;
> class InputCursor;
> class IntlObject;
> class JSArrayBuffer;
>-class JSArrayBufferConstructor;
> class JSArrayBufferPrototype;
> class JSCallee;
> class JSGlobalObjectDebuggable;
>@@ -93,7 +92,6 @@ class JSPromise;
> class JSPromiseConstructor;
> class JSPromisePrototype;
> class JSSharedArrayBuffer;
>-class JSSharedArrayBufferConstructor;
> class JSSharedArrayBufferPrototype;
> class JSTypedArrayViewConstructor;
> class JSTypedArrayViewPrototype;
>diff --git a/Source/JavaScriptCore/runtime/VM.cpp b/Source/JavaScriptCore/runtime/VM.cpp
>index 6b5205157c2a3a59abd8ce3a7cad2bc18b2a8199..c087ca8d6952fd78c974a773214ac478773e7964 100644
>--- a/Source/JavaScriptCore/runtime/VM.cpp
>+++ b/Source/JavaScriptCore/runtime/VM.cpp
>@@ -292,7 +292,6 @@ VM::VM(VMType vmType, HeapType heapType)
>     , destructibleObjectSpace("JSDestructibleObject", heap, destructibleObjectHeapCellType.get(), fastMallocAllocator.get())
>     , eagerlySweptDestructibleObjectSpace("Eagerly Swept JSDestructibleObject", heap, destructibleObjectHeapCellType.get(), fastMallocAllocator.get())
>     , segmentedVariableObjectSpace("JSSegmentedVariableObjectSpace", heap, segmentedVariableObjectHeapCellType.get(), fastMallocAllocator.get())
>-    , arrayBufferConstructorSpace ISO_SUBSPACE_INIT(heap, destructibleObjectHeapCellType.get(), JSArrayBufferConstructor)
>     , asyncFunctionSpace ISO_SUBSPACE_INIT(heap, cellJSValueOOBHeapCellType.get(), JSAsyncFunction)
>     , asyncGeneratorFunctionSpace ISO_SUBSPACE_INIT(heap, cellJSValueOOBHeapCellType.get(), JSAsyncGeneratorFunction)
>     , boundFunctionSpace ISO_SUBSPACE_INIT(heap, cellJSValueOOBHeapCellType.get(), JSBoundFunction)
>diff --git a/Source/JavaScriptCore/runtime/VM.h b/Source/JavaScriptCore/runtime/VM.h
>index bf08d5e7527449c9a8811c8aaf64a1cc1c47921b..ee0b3a66dde1e74fb52596dd39e0929443337c19 100644
>--- a/Source/JavaScriptCore/runtime/VM.h
>+++ b/Source/JavaScriptCore/runtime/VM.h
>@@ -367,7 +367,6 @@ class VM : public ThreadSafeRefCounted<VM>, public DoublyLinkedListNode<VM> {
>     CompleteSubspace eagerlySweptDestructibleObjectSpace;
>     CompleteSubspace segmentedVariableObjectSpace;
>     
>-    IsoSubspace arrayBufferConstructorSpace;
>     IsoSubspace asyncFunctionSpace;
>     IsoSubspace asyncGeneratorFunctionSpace;
>     IsoSubspace boundFunctionSpace;

Flags:

mark.lam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193774: 360046