<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[206405] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/206405">206405</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2016-09-26 16:56:37 -0700 (Mon, 26 Sep 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Add some needed CatchScopes in code that should not throw.
https://bugs.webkit.org/show_bug.cgi?id=162584
Reviewed by Keith Miller.
* 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.
* jsc.cpp:
(runJSC):
- the buck stops here. There's no reason an exception should propagate past here.
* 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/ExceptionHelpers.cpp:
(JSC::createError):
- If we're not able to stringify the error value, then we'll just use the
provided message as the error string. It doesn't make sense to have the
Error factory throw an exception that shadows the intended exception that the
client probably wants to throw (assuming that that's why the client is creating
this Error object).
* 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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreAPIJSObjectRefcpp">trunk/Source/JavaScriptCore/API/JSObjectRef.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreinterpreterInterpretercpp">trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejsccpp">trunk/Source/JavaScriptCore/jsc.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreprofilerProfilerDatabasecpp">trunk/Source/JavaScriptCore/profiler/ProfilerDatabase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeExceptionHelperscpp">trunk/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSModuleLoadercpp">trunk/Source/JavaScriptCore/runtime/JSModuleLoader.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeSamplingProfilercpp">trunk/Source/JavaScriptCore/runtime/SamplingProfiler.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreAPIJSObjectRefcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/API/JSObjectRef.cpp (206404 => 206405)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/API/JSObjectRef.cpp 2016-09-26 23:51:17 UTC (rev 206404)
+++ trunk/Source/JavaScriptCore/API/JSObjectRef.cpp 2016-09-26 23:56:37 UTC (rev 206405)
</span><span class="lines">@@ -309,20 +309,24 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx"> ExecState* exec = toJS(ctx);
</span><del>- JSLockHolder locker(exec);
</del><ins>+ VM& vm = exec->vm();
+ JSLockHolder locker(vm);
+ auto scope = DECLARE_CATCH_SCOPE(vm);
</ins><span class="cx">
</span><span class="cx"> JSObject* jsObject = toJS(object);
</span><span class="cx"> Identifier name(propertyName->identifier(&exec->vm()));
</span><span class="cx"> JSValue jsValue = toJS(exec, value);
</span><span class="cx">
</span><del>- 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);
</del><ins>+ 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);
+ }
</ins><span class="cx"> }
</span><del>-
</del><span class="cx"> handleExceptionIfNeeded(exec, exception);
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (206404 => 206405)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-09-26 23:51:17 UTC (rev 206404)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-09-26 23:56:37 UTC (rev 206405)
</span><span class="lines">@@ -1,5 +1,50 @@
</span><span class="cx"> 2016-09-26 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><ins>+ Add some needed CatchScopes in code that should not throw.
+ https://bugs.webkit.org/show_bug.cgi?id=162584
+
+ Reviewed by Keith Miller.
+
+ * 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.
+
+ * jsc.cpp:
+ (runJSC):
+ - the buck stops here. There's no reason an exception should propagate past here.
+
+ * 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/ExceptionHelpers.cpp:
+ (JSC::createError):
+ - If we're not able to stringify the error value, then we'll just use the
+ provided message as the error string. It doesn't make sense to have the
+ Error factory throw an exception that shadows the intended exception that the
+ client probably wants to throw (assuming that that's why the client is creating
+ this Error object).
+
+ * 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-26 Mark Lam <mark.lam@apple.com>
+
</ins><span class="cx"> Exception unwinding code should use a CatchScope instead of a ThrowScope.
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=162583
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreinterpreterInterpretercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp (206404 => 206405)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp 2016-09-26 23:51:17 UTC (rev 206404)
+++ trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp 2016-09-26 23:56:37 UTC (rev 206405)
</span><span class="lines">@@ -583,7 +583,7 @@
</span><span class="cx"> ALWAYS_INLINE static void notifyDebuggerOfUnwinding(CallFrame* callFrame)
</span><span class="cx"> {
</span><span class="cx"> VM& vm = callFrame->vm();
</span><del>- auto throwScope = DECLARE_THROW_SCOPE(vm);
</del><ins>+ auto catchScope = DECLARE_CATCH_SCOPE(vm);
</ins><span class="cx"> if (Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger()) {
</span><span class="cx"> SuspendExceptionScope scope(&vm);
</span><span class="cx"> if (jsDynamicCast<JSFunction*>(callFrame->callee()))
</span><span class="lines">@@ -590,7 +590,7 @@
</span><span class="cx"> debugger->returnEvent(callFrame);
</span><span class="cx"> else
</span><span class="cx"> debugger->didExecuteProgram(callFrame);
</span><del>- ASSERT_UNUSED(throwScope, !throwScope.exception());
</del><ins>+ ASSERT_UNUSED(catchScope, !catchScope.exception());
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejsccpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jsc.cpp (206404 => 206405)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jsc.cpp 2016-09-26 23:51:17 UTC (rev 206404)
+++ trunk/Source/JavaScriptCore/jsc.cpp 2016-09-26 23:56:37 UTC (rev 206405)
</span><span class="lines">@@ -2539,6 +2539,7 @@
</span><span class="cx"> static int NEVER_INLINE runJSC(VM* vm, CommandLine options)
</span><span class="cx"> {
</span><span class="cx"> JSLockHolder locker(vm);
</span><ins>+ auto scope = DECLARE_CATCH_SCOPE(*vm);
</ins><span class="cx">
</span><span class="cx"> int result;
</span><span class="cx"> if (options.m_profile && !vm->m_perBytecodeProfiler)
</span><span class="lines">@@ -2545,6 +2546,8 @@
</span><span class="cx"> vm->m_perBytecodeProfiler = std::make_unique<Profiler::Database>(*vm);
</span><span class="cx">
</span><span class="cx"> GlobalObject* globalObject = GlobalObject::create(*vm, GlobalObject::createStructure(*vm, jsNull()), options.m_arguments);
</span><ins>+ RETURN_IF_EXCEPTION(scope, 3);
+
</ins><span class="cx"> bool success = runWithScripts(globalObject, options.m_scripts, options.m_uncaughtExceptionName, options.m_alwaysDumpUncaughtException, options.m_dump, options.m_module);
</span><span class="cx"> if (options.m_interactive && success)
</span><span class="cx"> runInteractive(globalObject);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreprofilerProfilerDatabasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/profiler/ProfilerDatabase.cpp (206404 => 206405)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/profiler/ProfilerDatabase.cpp 2016-09-26 23:51:17 UTC (rev 206404)
+++ trunk/Source/JavaScriptCore/profiler/ProfilerDatabase.cpp 2016-09-26 23:56:37 UTC (rev 206405)
</span><span class="lines">@@ -134,11 +134,17 @@
</span><span class="cx">
</span><span class="cx"> bool Database::save(const char* filename) const
</span><span class="cx"> {
</span><ins>+ auto scope = DECLARE_CATCH_SCOPE(m_vm);
</ins><span class="cx"> auto out = FilePrintStream::open(filename, "w");
</span><span class="cx"> if (!out)
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- out->print(toJSON());
</del><ins>+ String data = toJSON();
+ if (UNLIKELY(scope.exception())) {
+ scope.clearException();
+ return false;
+ }
+ out->print(data);
</ins><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeExceptionHelperscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp (206404 => 206405)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp 2016-09-26 23:51:17 UTC (rev 206404)
+++ trunk/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp 2016-09-26 23:56:37 UTC (rev 206405)
</span><span class="lines">@@ -236,7 +236,14 @@
</span><span class="cx">
</span><span class="cx"> JSObject* createError(ExecState* exec, JSValue value, const String& message, ErrorInstance::SourceAppender appender)
</span><span class="cx"> {
</span><ins>+ VM& vm = exec->vm();
+ auto scope = DECLARE_CATCH_SCOPE(vm);
+
</ins><span class="cx"> String errorMessage = makeString(errorDescriptionForValue(exec, value)->value(exec), ' ', message);
</span><ins>+ if (UNLIKELY(scope.exception())) {
+ scope.clearException();
+ errorMessage = message;
+ }
</ins><span class="cx"> JSObject* exception = createTypeError(exec, errorMessage, appender, runtimeTypeForValue(value));
</span><span class="cx"> ASSERT(exception->isErrorInstance());
</span><span class="cx"> return exception;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSModuleLoadercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSModuleLoader.cpp (206404 => 206405)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSModuleLoader.cpp 2016-09-26 23:51:17 UTC (rev 206404)
+++ trunk/Source/JavaScriptCore/runtime/JSModuleLoader.cpp 2016-09-26 23:56:37 UTC (rev 206405)
</span><span class="lines">@@ -57,9 +57,10 @@
</span><span class="cx">
</span><span class="cx"> void JSModuleLoader::finishCreation(ExecState* exec, VM& vm, JSGlobalObject* globalObject)
</span><span class="cx"> {
</span><ins>+ auto scope = DECLARE_CATCH_SCOPE(vm);
+
</ins><span class="cx"> Base::finishCreation(vm);
</span><span class="cx"> ASSERT(inherits(info()));
</span><del>- auto scope = DECLARE_THROW_SCOPE(vm);
</del><span class="cx"> JSMap* map = JSMap::create(exec, vm, globalObject->mapStructure());
</span><span class="cx"> RELEASE_ASSERT(!scope.exception());
</span><span class="cx"> putDirect(vm, Identifier::fromString(&vm, "registry"), map);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeSamplingProfilercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/SamplingProfiler.cpp (206404 => 206405)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/SamplingProfiler.cpp 2016-09-26 23:51:17 UTC (rev 206404)
+++ trunk/Source/JavaScriptCore/runtime/SamplingProfiler.cpp 2016-09-26 23:56:37 UTC (rev 206405)
</span><span class="lines">@@ -591,11 +591,14 @@
</span><span class="cx"> if (!callee)
</span><span class="cx"> return String();
</span><span class="cx">
</span><ins>+ auto scope = DECLARE_CATCH_SCOPE(vm);
</ins><span class="cx"> ExecState* exec = callee->globalObject()->globalExec();
</span><span class="cx"> auto getPropertyIfPureOperation = [&] (const Identifier& ident) -> String {
</span><span class="cx"> PropertySlot slot(callee, PropertySlot::InternalMethodType::VMInquiry);
</span><span class="cx"> PropertyName propertyName(ident);
</span><del>- if (callee->getPropertySlot(exec, propertyName, slot)) {
</del><ins>+ bool hasProperty = callee->getPropertySlot(exec, propertyName, slot);
+ ASSERT_UNUSED(scope, !scope.exception());
+ if (hasProperty) {
</ins><span class="cx"> if (slot.isValue()) {
</span><span class="cx"> JSValue nameValue = slot.getValue(exec, propertyName);
</span><span class="cx"> if (isJSString(nameValue))
</span></span></pre>
</div>
</div>
</body>
</html>