2008-09-23 Geoffrey Garen <ggaren@apple.com>
Reviewed by Oliver Hunt.
Fixed https://bugs.webkit.org/show_bug.cgi?id=21038 | <rdar://problem/6240812>
Uncaught exceptions in regex replace callbacks crash webkit
This was a combination of two problems:
(1) the replace function would continue execution after an exception
had been thrown.
(2) In some cases, the Machine would return 0 in the case of an exception,
despite the fact that a few clients dereference the Machine's return
value without first checking for an exception.
* VM/Machine.cpp:
(JSC::Machine::execute):
^ Return jsNull() instead of 0 in the case of an exception, since some
clients depend on using our return value.
^ ASSERT that execution does not continue after an exception has been
thrown, to help catch problems like this in the future.
* kjs/StringPrototype.cpp:
(JSC::stringProtoFuncReplace):
^ Stop execution if an exception has been thrown.
LayoutTests:
2008-09-23 Geoffrey Garen <ggaren@apple.com>
Reviewed by Oliver Hunt.
Test for https://bugs.webkit.org/show_bug.cgi?id=21038
Uncaught exceptions in regex replace callbacks crash webkit
* fast/js/string-replace-exception-crash-expected.txt: Added.
* fast/js/string-replace-exception-crash.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@36842
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
2008-09-23 Geoffrey Garen <ggaren@apple.com>
+ Reviewed by Oliver Hunt.
+
+ Fixed https://bugs.webkit.org/show_bug.cgi?id=21038 | <rdar://problem/6240812>
+ Uncaught exceptions in regex replace callbacks crash webkit
+
+ This was a combination of two problems:
+
+ (1) the replace function would continue execution after an exception
+ had been thrown.
+
+ (2) In some cases, the Machine would return 0 in the case of an exception,
+ despite the fact that a few clients dereference the Machine's return
+ value without first checking for an exception.
+
+ * VM/Machine.cpp:
+ (JSC::Machine::execute):
+
+ ^ Return jsNull() instead of 0 in the case of an exception, since some
+ clients depend on using our return value.
+
+ ^ ASSERT that execution does not continue after an exception has been
+ thrown, to help catch problems like this in the future.
+
+ * kjs/StringPrototype.cpp:
+ (JSC::stringProtoFuncReplace):
+
+ ^ Stop execution if an exception has been thrown.
+
+2008-09-23 Geoffrey Garen <ggaren@apple.com>
+
Try to fix the windows build.
* VM/CTI.cpp:
JSValue* Machine::execute(ProgramNode* programNode, ExecState* exec, ScopeChainNode* scopeChain, JSObject* thisObj, JSValue** exception)
{
+ ASSERT(!exec->hadException());
+
if (m_reentryDepth >= MaxReentryDepth) {
*exception = createStackOverflowError(exec);
- return 0;
+ return jsNull();
}
CodeBlock* codeBlock = &programNode->byteCode(scopeChain);
size_t newSize = oldSize + codeBlock->numParameters + RegisterFile::CallFrameHeaderSize + codeBlock->numCalleeRegisters;
if (!m_registerFile.grow(newSize)) {
*exception = createStackOverflowError(exec);
- return 0;
+ return jsNull();
}
JSGlobalObject* lastGlobalObject = m_registerFile.globalObject();
JSValue* Machine::execute(FunctionBodyNode* functionBodyNode, ExecState* exec, JSFunction* function, JSObject* thisObj, const ArgList& args, ScopeChainNode* scopeChain, JSValue** exception)
{
+ ASSERT(!exec->hadException());
+
if (m_reentryDepth >= MaxReentryDepth) {
*exception = createStackOverflowError(exec);
- return 0;
+ return jsNull();
}
size_t oldSize = m_registerFile.size();
if (!m_registerFile.grow(oldSize + argc)) {
*exception = createStackOverflowError(exec);
- return 0;
+ return jsNull();
}
Register* argv = m_registerFile.base() + oldSize;
Register* r = slideRegisterWindowForCall(exec, newCodeBlock, &m_registerFile, m_registerFile.base(), argv, argc + RegisterFile::CallFrameHeaderSize, argc, *exception);
if (UNLIKELY(*exception != 0)) {
m_registerFile.shrink(oldSize);
- return 0;
+ return jsNull();
}
// a 0 codeBlock indicates a built-in caller
initializeCallFrame(r, 0, 0, 0, argv, 0, argc, function);
JSValue* Machine::execute(EvalNode* evalNode, ExecState* exec, JSObject* thisObj, int registerOffset, ScopeChainNode* scopeChain, JSValue** exception)
{
+ ASSERT(!exec->hadException());
+
if (m_reentryDepth >= MaxReentryDepth) {
*exception = createStackOverflowError(exec);
- return 0;
+ return jsNull();
}
EvalCodeBlock* codeBlock = &evalNode->byteCode(scopeChain);
size_t newSize = registerOffset + codeBlock->numCalleeRegisters;
if (!m_registerFile.grow(newSize)) {
*exception = createStackOverflowError(exec);
- return 0;
+ return jsNull();
}
Register* r = m_registerFile.base() + registerOffset;
args.append(sourceVal);
replacements.append(call(exec, replacement, callType, callData, exec->globalThisValue(), args)->toString(exec));
+ if (exec->hadException())
+ break;
} else
replacements.append(substituteBackreferences(replacementString, source, ovector, reg));
+2008-09-23 Geoffrey Garen <ggaren@apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Test for https://bugs.webkit.org/show_bug.cgi?id=21038
+ Uncaught exceptions in regex replace callbacks crash webkit
+
+ * fast/js/string-replace-exception-crash-expected.txt: Added.
+ * fast/js/string-replace-exception-crash.html: Added.
+
2008-09-23 Beth Dakin <bdakin@apple.com>
Reviewed by Sam Weinig.
--- /dev/null
+This page tests for a crash when throwing an exception from a callback provided to String.prototype.replace.
+
+If the test passes, you'll see a series of PASS messages below.
+
+PASS: You didn't crash.
+PASS: You didn't crash.
+PASS: String.prototype.replace did not continue executing after an exception was thrown.
+
--- /dev/null
+<p>This page tests for a crash when throwing an exception from a callback provided
+to String.prototype.replace.
+</p>
+
+<p>If the test passes, you'll see a series of PASS messages below.
+</p>
+
+<pre id="console"></pre>
+
+<script>
+function log(s)
+{
+ document.getElementById("console").appendChild(document.createTextNode(s + "\n"));
+}
+
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+// these should not crash
+
+try {
+ (function () {
+ "aa".replace(/a/g, function() {
+ var bogus;
+ bogus.property;
+ });
+ })();
+} catch(e) {
+ log ("PASS: You didn't crash.");
+}
+
+try {
+ (function () {
+ "aa".replace("a", function() {
+ ({})();
+ });
+ })();
+} catch(e) {
+ log ("PASS: You didn't crash.");
+}
+
+// this should not continue execution after an exception
+
+var message = "PASS: String.prototype.replace did not continue executing after an exception was thrown.";
+try {
+ (function () {
+ var count = 0;
+ "aa".replace(/a/g, function() {
+ if (++count > 1)
+ message = "FAIL: String.prototype.replace continued executing after an exception was thrown.";
+
+ var bogus;
+ bogus.property;
+ });
+ })();
+} catch(e) {
+ log (message);
+}
+</script>