Changeset 206459 in webkit


Ignore:
Timestamp:
Sep 27, 2016 1:32:13 PM (8 years ago)
Author:
mark.lam@apple.com
Message:

Add some needed CatchScopes in code that should not throw.
https://bugs.webkit.org/show_bug.cgi?id=162584

Reviewed by Keith Miller.

Re-landing minus the jsc.cpp and ExceptionHelpers.cpp changes. I'll address
those in a subsequent patch if the need manifests again in my testing.

  • API/JSObjectRef.cpp:

(JSObjectSetProperty):

  • This function already handles exceptions in its own way. We're honoring this contract and catching exceptions and passing it to the handler.
  • interpreter/Interpreter.cpp:

(JSC::notifyDebuggerOfUnwinding):

  • The debugger should not be throwing any exceptions.
  • profiler/ProfilerDatabase.cpp:

(JSC::Profiler::Database::save):

  • If an exception was thrown while saving the database, there's nothing we can really do about it anyway. Just fail nicely and return false. This is in line with existing error checking code in Database::save() that returns false if it's not able to open the file to save to.
  • runtime/JSModuleLoader.cpp:

(JSC::JSModuleLoader::finishCreation):

  • The existing code already RELEASE_ASSERT that no exception was thrown. Hence, it's appropriate to use a CatchScope here.
  • runtime/SamplingProfiler.cpp:

(JSC::SamplingProfiler::StackFrame::nameFromCallee):

  • The sampling profiler is doing a VMInquiry get here. It should never throw an exception. Hence, we'll just use a CatchScope and assert accordingly.
Location:
trunk/Source/JavaScriptCore
Files:
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/API/JSObjectRef.cpp

    r206408 r206459  
    310310    }
    311311    ExecState* exec = toJS(ctx);
    312     JSLockHolder locker(exec);
     312    VM& vm = exec->vm();
     313    JSLockHolder locker(vm);
     314    auto scope = DECLARE_CATCH_SCOPE(vm);
    313315
    314316    JSObject* jsObject = toJS(object);
     
    316318    JSValue jsValue = toJS(exec, value);
    317319
    318     if (attributes && !jsObject->hasProperty(exec, name)) {
    319         PropertyDescriptor desc(jsValue, attributes);
    320         jsObject->methodTable()->defineOwnProperty(jsObject, exec, name, desc, false);
    321     } else {
    322         PutPropertySlot slot(jsObject);
    323         jsObject->methodTable()->put(jsObject, exec, name, jsValue, slot);
    324     }
    325 
     320    bool doesNotHaveProperty = attributes && !jsObject->hasProperty(exec, name);
     321    if (LIKELY(!scope.exception())) {
     322        if (doesNotHaveProperty) {
     323            PropertyDescriptor desc(jsValue, attributes);
     324            jsObject->methodTable()->defineOwnProperty(jsObject, exec, name, desc, false);
     325        } else {
     326            PutPropertySlot slot(jsObject);
     327            jsObject->methodTable()->put(jsObject, exec, name, jsValue, slot);
     328        }
     329    }
    326330    handleExceptionIfNeeded(exec, exception);
    327331}
  • trunk/Source/JavaScriptCore/ChangeLog

    r206440 r206459  
     12016-09-27  Mark Lam  <mark.lam@apple.com>
     2
     3        Add some needed CatchScopes in code that should not throw.
     4        https://bugs.webkit.org/show_bug.cgi?id=162584
     5
     6        Reviewed by Keith Miller.
     7
     8        Re-landing minus the jsc.cpp and ExceptionHelpers.cpp changes.  I'll address
     9        those in a subsequent patch if the need manifests again in my testing.
     10
     11        * API/JSObjectRef.cpp:
     12        (JSObjectSetProperty):
     13        - This function already handles exceptions in its own way.  We're honoring this
     14          contract and catching exceptions and passing it to the handler.
     15
     16        * interpreter/Interpreter.cpp:
     17        (JSC::notifyDebuggerOfUnwinding):
     18        - The debugger should not be throwing any exceptions.
     19
     20        * profiler/ProfilerDatabase.cpp:
     21        (JSC::Profiler::Database::save):
     22        - If an exception was thrown while saving the database, there's nothing we can
     23          really do about it anyway.  Just fail nicely and return false.  This is in line
     24          with existing error checking code in Database::save() that returns false if
     25          it's not able to open the file to save to.
     26
     27        * runtime/JSModuleLoader.cpp:
     28        (JSC::JSModuleLoader::finishCreation):
     29        - The existing code already RELEASE_ASSERT that no exception was thrown.
     30          Hence, it's appropriate to use a CatchScope here.
     31
     32        * runtime/SamplingProfiler.cpp:
     33        (JSC::SamplingProfiler::StackFrame::nameFromCallee):
     34        - The sampling profiler is doing a VMInquiry get here.  It should never throw an
     35          exception.  Hence, we'll just use a CatchScope and assert accordingly.
     36
    1372016-09-27  Jer Noble  <jer.noble@apple.com>
    238
  • trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

    r206408 r206459  
    584584{
    585585    VM& vm = callFrame->vm();
    586     auto throwScope = DECLARE_THROW_SCOPE(vm);
     586    auto catchScope = DECLARE_CATCH_SCOPE(vm);
    587587    if (Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger()) {
    588588        SuspendExceptionScope scope(&vm);
     
    591591        else
    592592            debugger->didExecuteProgram(callFrame);
    593         ASSERT_UNUSED(throwScope, !throwScope.exception());
     593        ASSERT_UNUSED(catchScope, !catchScope.exception());
    594594    }
    595595}
  • trunk/Source/JavaScriptCore/profiler/ProfilerDatabase.cpp

    r206408 r206459  
    135135bool Database::save(const char* filename) const
    136136{
     137    auto scope = DECLARE_CATCH_SCOPE(m_vm);
    137138    auto out = FilePrintStream::open(filename, "w");
    138139    if (!out)
    139140        return false;
    140141   
    141     out->print(toJSON());
     142    String data = toJSON();
     143    if (UNLIKELY(scope.exception())) {
     144        scope.clearException();
     145        return false;
     146    }
     147    out->print(data);
    142148    return true;
    143149}
  • trunk/Source/JavaScriptCore/runtime/JSModuleLoader.cpp

    r206408 r206459  
    5858void JSModuleLoader::finishCreation(ExecState* exec, VM& vm, JSGlobalObject* globalObject)
    5959{
     60    auto scope = DECLARE_CATCH_SCOPE(vm);
     61
    6062    Base::finishCreation(vm);
    6163    ASSERT(inherits(info()));
    62     auto scope = DECLARE_THROW_SCOPE(vm);
    6364    JSMap* map = JSMap::create(exec, vm, globalObject->mapStructure());
    6465    RELEASE_ASSERT(!scope.exception());
  • trunk/Source/JavaScriptCore/runtime/SamplingProfiler.cpp

    r206408 r206459  
    592592        return String();
    593593
     594    auto scope = DECLARE_CATCH_SCOPE(vm);
    594595    ExecState* exec = callee->globalObject()->globalExec();
    595596    auto getPropertyIfPureOperation = [&] (const Identifier& ident) -> String {
    596597        PropertySlot slot(callee, PropertySlot::InternalMethodType::VMInquiry);
    597598        PropertyName propertyName(ident);
    598         if (callee->getPropertySlot(exec, propertyName, slot)) {
     599        bool hasProperty = callee->getPropertySlot(exec, propertyName, slot);
     600        ASSERT_UNUSED(scope, !scope.exception());
     601        if (hasProperty) {
    599602            if (slot.isValue()) {
    600603                JSValue nameValue = slot.getValue(exec, propertyName);
Note: See TracChangeset for help on using the changeset viewer.