Changeset 240465 in webkit


Ignore:
Timestamp:
Jan 24, 2019 6:47:58 PM (5 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] ErrorConstructor should not have own IsoSubspace
​https://bugs.webkit.org/show_bug.cgi?id=193800

Reviewed by Saam Barati.

Similar to r240456, sizeof(ErrorConstructor) != sizeof(InternalFunction), and that is why we have
IsoSubspace errorConstructorSpace in VM. But it is allocated only one-per-JSGlobalObject, and it is
too costly to have IsoSubspace which allocates 16KB. Since stackTraceLimit information is per
JSGlobalObject information, we should have m_stackTraceLimit in JSGlobalObject instead and put
ErrorConstructor in InternalFunction's IsoSubspace. As r230813 (moving InternalFunction and subclasses
into IsoSubspaces) described,

"subclasses that are the same size as InternalFunction share its subspace. I did this because the subclasses
appear to just override methods, which are called dynamically via the structure or class of the object.
So, I don't see a type confusion risk if UAF is used to allocate one kind of InternalFunction over another."

Then, putting ErrorConstructor in InternalFunction IsoSubspace is fine since it meets the above condition.
This patch removes m_stackTraceLimit in ErrorConstructor, and drops IsoSubspace for errorConstructorSpace.
This reduces the memory usage.

  • interpreter/Interpreter.h:
  • runtime/Error.cpp:

(JSC::getStackTrace):

  • runtime/ErrorConstructor.cpp:

(JSC::ErrorConstructor::ErrorConstructor):
(JSC::ErrorConstructor::finishCreation):
(JSC::constructErrorConstructor):
(JSC::callErrorConstructor):
(JSC::ErrorConstructor::put):
(JSC::ErrorConstructor::deleteProperty):
(JSC::Interpreter::constructWithErrorConstructor): Deleted.
(JSC::Interpreter::callErrorConstructor): Deleted.

  • runtime/ErrorConstructor.h:
  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::JSGlobalObject):
(JSC::JSGlobalObject::init):
(JSC::JSGlobalObject::visitChildren):

  • runtime/JSGlobalObject.h:

(JSC::JSGlobalObject::stackTraceLimit const):
(JSC::JSGlobalObject::setStackTraceLimit):
(JSC::JSGlobalObject::errorConstructor const): Deleted.

  • runtime/VM.cpp:

(JSC::VM::VM):

  • runtime/VM.h:
Location:
trunk/Source/JavaScriptCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r240457 r240465  
     12019-01-24  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] ErrorConstructor should not have own IsoSubspace
     4        https://bugs.webkit.org/show_bug.cgi?id=193800
     5
     6        Reviewed by Saam Barati.
     7
     8        Similar to r240456, sizeof(ErrorConstructor) != sizeof(InternalFunction), and that is why we have
     9        IsoSubspace errorConstructorSpace in VM. But it is allocated only one-per-JSGlobalObject, and it is
     10        too costly to have IsoSubspace which allocates 16KB. Since stackTraceLimit information is per
     11        JSGlobalObject information, we should have m_stackTraceLimit in JSGlobalObject instead and put
     12        ErrorConstructor in InternalFunction's IsoSubspace. As r230813 (moving InternalFunction and subclasses
     13        into IsoSubspaces) described,
     14
     15            "subclasses that are the same size as InternalFunction share its subspace. I did this because the subclasses
     16            appear to just override methods, which are called dynamically via the structure or class of the object.
     17            So, I don't see a type confusion risk if UAF is used to allocate one kind of InternalFunction over another."
     18
     19        Then, putting ErrorConstructor in InternalFunction IsoSubspace is fine since it meets the above condition.
     20        This patch removes m_stackTraceLimit in ErrorConstructor, and drops IsoSubspace for errorConstructorSpace.
     21        This reduces the memory usage.
     22
     23        * interpreter/Interpreter.h:
     24        * runtime/Error.cpp:
     25        (JSC::getStackTrace):
     26        * runtime/ErrorConstructor.cpp:
     27        (JSC::ErrorConstructor::ErrorConstructor):
     28        (JSC::ErrorConstructor::finishCreation):
     29        (JSC::constructErrorConstructor):
     30        (JSC::callErrorConstructor):
     31        (JSC::ErrorConstructor::put):
     32        (JSC::ErrorConstructor::deleteProperty):
     33        (JSC::Interpreter::constructWithErrorConstructor): Deleted.
     34        (JSC::Interpreter::callErrorConstructor): Deleted.
     35        * runtime/ErrorConstructor.h:
     36        * runtime/JSGlobalObject.cpp:
     37        (JSC::JSGlobalObject::JSGlobalObject):
     38        (JSC::JSGlobalObject::init):
     39        (JSC::JSGlobalObject::visitChildren):
     40        * runtime/JSGlobalObject.h:
     41        (JSC::JSGlobalObject::stackTraceLimit const):
     42        (JSC::JSGlobalObject::setStackTraceLimit):
     43        (JSC::JSGlobalObject::errorConstructor const): Deleted.
     44        * runtime/VM.cpp:
     45        (JSC::VM::VM):
     46        * runtime/VM.h:
     47
    1482019-01-24  Joseph Pecoraro  <pecoraro@apple.com>
    249

  • trunk/Source/JavaScriptCore/interpreter/Interpreter.h

    r237547 r240465  
    118118        static String stackTraceAsString(VM&, const Vector<StackFrame>&);
    119119
    120         static EncodedJSValue JSC_HOST_CALL constructWithErrorConstructor(ExecState*);
    121         static EncodedJSValue JSC_HOST_CALL callErrorConstructor(ExecState*);
    122120        static EncodedJSValue JSC_HOST_CALL constructWithNativeErrorConstructor(ExecState*);
    123121        static EncodedJSValue JSC_HOST_CALL callNativeErrorConstructor(ExecState*);

  • trunk/Source/JavaScriptCore/runtime/Error.cpp

    r240246 r240465  
    162162{
    163163    JSGlobalObject* globalObject = obj->globalObject(vm);
    164     ErrorConstructor* errorConstructor = globalObject->errorConstructor();
    165     if (!errorConstructor->stackTraceLimit())
     164    if (!globalObject->stackTraceLimit())
    166165        return nullptr;
    167166
    168167    size_t framesToSkip = useCurrentFrame ? 0 : 1;
    169168    std::unique_ptr<Vector<StackFrame>> stackTrace = std::make_unique<Vector<StackFrame>>();
    170     vm.interpreter->getStackTrace(obj, *stackTrace, framesToSkip, errorConstructor->stackTraceLimit().value());
     169    vm.interpreter->getStackTrace(obj, *stackTrace, framesToSkip, globalObject->stackTraceLimit().value());
    171170    if (!stackTrace->isEmpty())
    172171        ASSERT_UNUSED(exec, exec == vm.topCallFrame || exec->isGlobalExec());

  • trunk/Source/JavaScriptCore/runtime/ErrorConstructor.cpp

    r236697 r240465  
    3434const ClassInfo ErrorConstructor::s_info = { "Function", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(ErrorConstructor) };
    3535
     36static EncodedJSValue JSC_HOST_CALL callErrorConstructor(ExecState*);
     37static EncodedJSValue JSC_HOST_CALL constructErrorConstructor(ExecState*);
     38
    3639ErrorConstructor::ErrorConstructor(VM& vm, Structure* structure)
    37     : InternalFunction(vm, structure, Interpreter::callErrorConstructor, Interpreter::constructWithErrorConstructor)
     40    : InternalFunction(vm, structure, callErrorConstructor, constructErrorConstructor)
    3841{
    3942}
    … …  
    4548    putDirectWithoutTransition(vm, vm.propertyNames->prototype, errorPrototype, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
    4649    putDirectWithoutTransition(vm, vm.propertyNames->length, jsNumber(1), PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly);
    47 
    48     unsigned defaultStackTraceLimit = Options::defaultErrorStackTraceLimit();
    49     m_stackTraceLimit = defaultStackTraceLimit;
    50     putDirectWithoutTransition(vm, vm.propertyNames->stackTraceLimit, jsNumber(defaultStackTraceLimit), static_cast<unsigned>(PropertyAttribute::None));
     50    putDirectWithoutTransition(vm, vm.propertyNames->stackTraceLimit, jsNumber(globalObject(vm)->stackTraceLimit().valueOr(Options::defaultErrorStackTraceLimit())), static_cast<unsigned>(PropertyAttribute::None));
    5151}
    5252
    5353// ECMA 15.9.3
    5454
    55 EncodedJSValue JSC_HOST_CALL Interpreter::constructWithErrorConstructor(ExecState* exec)
     55EncodedJSValue JSC_HOST_CALL constructErrorConstructor(ExecState* exec)
    5656{
    5757    VM& vm = exec->vm();
    … …  
    6363}
    6464
    65 EncodedJSValue JSC_HOST_CALL Interpreter::callErrorConstructor(ExecState* exec)
     65EncodedJSValue JSC_HOST_CALL callErrorConstructor(ExecState* exec)
    6666{
    6767    JSValue message = exec->argument(0);
    … …  
    8080            effectiveLimit = std::max(0., effectiveLimit);
    8181            effectiveLimit = std::min(effectiveLimit, static_cast<double>(std::numeric_limits<unsigned>::max()));
    82             thisObject->m_stackTraceLimit = static_cast<unsigned>(effectiveLimit);
     82            thisObject->globalObject(vm)->setStackTraceLimit(static_cast<unsigned>(effectiveLimit));
    8383        } else
    84             thisObject->m_stackTraceLimit = { };
     84            thisObject->globalObject(vm)->setStackTraceLimit(WTF::nullopt);
    8585    }
    8686
    … …  
    9494
    9595    if (propertyName == vm.propertyNames->stackTraceLimit)
    96         thisObject->m_stackTraceLimit = { };
     96        thisObject->globalObject(vm)->setStackTraceLimit(WTF::nullopt);
    9797
    9898    return Base::deleteProperty(thisObject, exec, propertyName);

  • trunk/Source/JavaScriptCore/runtime/ErrorConstructor.h

    r239427 r240465  
    3030class ErrorConstructor final : public InternalFunction {
    3131public:
    32     typedef InternalFunction Base;
    33 
    34     template<typename CellType>
    35     static IsoSubspace* subspaceFor(VM& vm)
    36     {
    37         return &vm.errorConstructorSpace;
    38     }
     32    using Base = InternalFunction;
    3933
    4034    static ErrorConstructor* create(VM& vm, Structure* structure, ErrorPrototype* errorPrototype, GetterSetter*)
    … …  
    5246    }
    5347
    54     Optional<unsigned> stackTraceLimit() const { return m_stackTraceLimit; }
    55 
    5648protected:
    5749    void finishCreation(VM&, ErrorPrototype*);
    … …  
    6355    ErrorConstructor(VM&, Structure*);
    6456
    65     Optional<unsigned> m_stackTraceLimit;
    6657};
    6758
     59static_assert(sizeof(ErrorConstructor) == sizeof(InternalFunction), "");
     60
    6861} // namespace JSC

  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

    r240456 r240465  
    361361    , m_numberToStringWatchpoint(IsWatched)
    362362    , m_runtimeFlags()
     363    , m_stackTraceLimit(Options::defaultErrorStackTraceLimit())
    363364    , m_globalObjectMethodTable(globalObjectMethodTable ? globalObjectMethodTable : &s_globalObjectMethodTable)
    364365{
    … …  
    701702#undef CREATE_CONSTRUCTOR_FOR_SIMPLE_TYPE
    702703
    703     m_errorConstructor.set(vm, this, errorConstructor);
    704704    m_promiseConstructor.set(vm, this, promiseConstructor);
    705705    m_internalPromiseConstructor.set(vm, this, internalPromiseConstructor);
    … …  
    879879        GlobalPropertyInfo(vm.propertyNames->builtinNames().importModulePrivateName(), privateFuncImportModule, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    880880        GlobalPropertyInfo(vm.propertyNames->builtinNames().enqueueJobPrivateName(), JSFunction::create(vm, this, 0, String(), enqueueJob), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    881         GlobalPropertyInfo(vm.propertyNames->builtinNames().ErrorPrivateName(), m_errorConstructor.get(), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
     881        GlobalPropertyInfo(vm.propertyNames->builtinNames().ErrorPrivateName(), errorConstructor, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    882882        GlobalPropertyInfo(vm.propertyNames->builtinNames().RangeErrorPrivateName(), m_rangeErrorConstructor.get(), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    883883        GlobalPropertyInfo(vm.propertyNames->builtinNames().TypeErrorPrivateName(), m_typeErrorConstructor.get(), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    … …  
    15491549    visitor.append(thisObject->m_stackOverflowFrameCallee);
    15501550    visitor.append(thisObject->m_regExpConstructor);
    1551     visitor.append(thisObject->m_errorConstructor);
    15521551    visitor.append(thisObject->m_nativeErrorPrototypeStructure);
    15531552    visitor.append(thisObject->m_nativeErrorStructure);

  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.h

    r240456 r240465  
    260260    WriteBarrier<JSCallee> m_stackOverflowFrameCallee;
    261261    WriteBarrier<RegExpConstructor> m_regExpConstructor;
    262     WriteBarrier<ErrorConstructor> m_errorConstructor;
    263262    WriteBarrier<Structure> m_nativeErrorPrototypeStructure;
    264263    WriteBarrier<Structure> m_nativeErrorStructure;
    … …  
    499498    RuntimeFlags m_runtimeFlags;
    500499    ConsoleClient* m_consoleClient { nullptr };
     500    Optional<unsigned> m_stackTraceLimit;
    501501
    502502#if !ASSERT_DISABLED
    … …  
    531531#endif
    532532
     533    Optional<unsigned> stackTraceLimit() const { return m_stackTraceLimit; }
     534    void setStackTraceLimit(Optional<unsigned> value) { m_stackTraceLimit = value; }
     535
    533536protected:
    534537    JS_EXPORT_PRIVATE explicit JSGlobalObject(VM&, Structure*, const GlobalObjectMethodTable* = nullptr);
    … …  
    574577    RegExpConstructor* regExpConstructor() const { return m_regExpConstructor.get(); }
    575578
    576     ErrorConstructor* errorConstructor() const { return m_errorConstructor.get(); }
    577579    ArrayConstructor* arrayConstructor() const { return m_arrayConstructor.get(); }
    578580    ObjectConstructor* objectConstructor() const { return m_objectConstructor.get(); }

  • trunk/Source/JavaScriptCore/runtime/VM.cpp

    r240456 r240465  
    298298    , callbackFunctionSpace ISO_SUBSPACE_INIT(heap, destructibleObjectHeapCellType.get(), JSCallbackFunction)
    299299    , customGetterSetterFunctionSpace ISO_SUBSPACE_INIT(heap, cellJSValueOOBHeapCellType.get(), JSCustomGetterSetterFunction)
    300     , errorConstructorSpace ISO_SUBSPACE_INIT(heap, destructibleObjectHeapCellType.get(), ErrorConstructor)
    301300    , executableToCodeBlockEdgeSpace ISO_SUBSPACE_INIT(heap, cellDangerousBitsHeapCellType.get(), ExecutableToCodeBlockEdge)
    302301    , functionSpace ISO_SUBSPACE_INIT(heap, cellJSValueOOBHeapCellType.get(), JSFunction)

  • trunk/Source/JavaScriptCore/runtime/VM.h

    r240456 r240465  
    373373    IsoSubspace callbackFunctionSpace;
    374374    IsoSubspace customGetterSetterFunctionSpace;
    375     IsoSubspace errorConstructorSpace;
    376375    IsoSubspace executableToCodeBlockEdgeSpace;
    377376    IsoSubspace functionSpace;
Note: See TracChangeset for help on using the changeset viewer.