- Revision
- 206459
- Author
- [email protected]
- Date
- 2016-09-27 13:32:13 -0700 (Tue, 27 Sep 2016)
Log 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.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/API/JSObjectRef.cpp (206458 => 206459)
--- trunk/Source/_javascript_Core/API/JSObjectRef.cpp 2016-09-27 20:30:09 UTC (rev 206458)
+++ trunk/Source/_javascript_Core/API/JSObjectRef.cpp 2016-09-27 20:32:13 UTC (rev 206459)
@@ -309,20 +309,24 @@
return;
}
ExecState* exec = toJS(ctx);
- JSLockHolder locker(exec);
+ VM& vm = exec->vm();
+ JSLockHolder locker(vm);
+ auto scope = DECLARE_CATCH_SCOPE(vm);
JSObject* jsObject = toJS(object);
Identifier name(propertyName->identifier(&exec->vm()));
JSValue jsValue = toJS(exec, value);
- if (attributes && !jsObject->hasProperty(exec, name)) {
- PropertyDescriptor desc(jsValue, attributes);
- jsObject->methodTable()->defineOwnProperty(jsObject, exec, name, desc, false);
- } else {
- PutPropertySlot slot(jsObject);
- jsObject->methodTable()->put(jsObject, exec, name, jsValue, slot);
+ bool doesNotHaveProperty = attributes && !jsObject->hasProperty(exec, name);
+ if (LIKELY(!scope.exception())) {
+ if (doesNotHaveProperty) {
+ PropertyDescriptor desc(jsValue, attributes);
+ jsObject->methodTable()->defineOwnProperty(jsObject, exec, name, desc, false);
+ } else {
+ PutPropertySlot slot(jsObject);
+ jsObject->methodTable()->put(jsObject, exec, name, jsValue, slot);
+ }
}
-
handleExceptionIfNeeded(exec, exception);
}
Modified: trunk/Source/_javascript_Core/ChangeLog (206458 => 206459)
--- trunk/Source/_javascript_Core/ChangeLog 2016-09-27 20:30:09 UTC (rev 206458)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-09-27 20:32:13 UTC (rev 206459)
@@ -1,3 +1,39 @@
+2016-09-27 Mark Lam <[email protected]>
+
+ 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.
+
2016-09-27 Jer Noble <[email protected]>
Remove deprecated ENCRYPTED_MEDIA implementation.
Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (206458 => 206459)
--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2016-09-27 20:30:09 UTC (rev 206458)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2016-09-27 20:32:13 UTC (rev 206459)
@@ -583,7 +583,7 @@
ALWAYS_INLINE static void notifyDebuggerOfUnwinding(CallFrame* callFrame)
{
VM& vm = callFrame->vm();
- auto throwScope = DECLARE_THROW_SCOPE(vm);
+ auto catchScope = DECLARE_CATCH_SCOPE(vm);
if (Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger()) {
SuspendExceptionScope scope(&vm);
if (jsDynamicCast<JSFunction*>(callFrame->callee()))
@@ -590,7 +590,7 @@
debugger->returnEvent(callFrame);
else
debugger->didExecuteProgram(callFrame);
- ASSERT_UNUSED(throwScope, !throwScope.exception());
+ ASSERT_UNUSED(catchScope, !catchScope.exception());
}
}
Modified: trunk/Source/_javascript_Core/profiler/ProfilerDatabase.cpp (206458 => 206459)
--- trunk/Source/_javascript_Core/profiler/ProfilerDatabase.cpp 2016-09-27 20:30:09 UTC (rev 206458)
+++ trunk/Source/_javascript_Core/profiler/ProfilerDatabase.cpp 2016-09-27 20:32:13 UTC (rev 206459)
@@ -134,11 +134,17 @@
bool Database::save(const char* filename) const
{
+ auto scope = DECLARE_CATCH_SCOPE(m_vm);
auto out = FilePrintStream::open(filename, "w");
if (!out)
return false;
- out->print(toJSON());
+ String data = ""
+ if (UNLIKELY(scope.exception())) {
+ scope.clearException();
+ return false;
+ }
+ out->print(data);
return true;
}
Modified: trunk/Source/_javascript_Core/runtime/JSModuleLoader.cpp (206458 => 206459)
--- trunk/Source/_javascript_Core/runtime/JSModuleLoader.cpp 2016-09-27 20:30:09 UTC (rev 206458)
+++ trunk/Source/_javascript_Core/runtime/JSModuleLoader.cpp 2016-09-27 20:32:13 UTC (rev 206459)
@@ -57,9 +57,10 @@
void JSModuleLoader::finishCreation(ExecState* exec, VM& vm, JSGlobalObject* globalObject)
{
+ auto scope = DECLARE_CATCH_SCOPE(vm);
+
Base::finishCreation(vm);
ASSERT(inherits(info()));
- auto scope = DECLARE_THROW_SCOPE(vm);
JSMap* map = JSMap::create(exec, vm, globalObject->mapStructure());
RELEASE_ASSERT(!scope.exception());
putDirect(vm, Identifier::fromString(&vm, "registry"), map);
Modified: trunk/Source/_javascript_Core/runtime/SamplingProfiler.cpp (206458 => 206459)
--- trunk/Source/_javascript_Core/runtime/SamplingProfiler.cpp 2016-09-27 20:30:09 UTC (rev 206458)
+++ trunk/Source/_javascript_Core/runtime/SamplingProfiler.cpp 2016-09-27 20:32:13 UTC (rev 206459)
@@ -591,11 +591,14 @@
if (!callee)
return String();
+ auto scope = DECLARE_CATCH_SCOPE(vm);
ExecState* exec = callee->globalObject()->globalExec();
auto getPropertyIfPureOperation = [&] (const Identifier& ident) -> String {
PropertySlot slot(callee, PropertySlot::InternalMethodType::VMInquiry);
PropertyName propertyName(ident);
- if (callee->getPropertySlot(exec, propertyName, slot)) {
+ bool hasProperty = callee->getPropertySlot(exec, propertyName, slot);
+ ASSERT_UNUSED(scope, !scope.exception());
+ if (hasProperty) {
if (slot.isValue()) {
JSValue nameValue = slot.getValue(exec, propertyName);
if (isJSString(nameValue))
