JavaScriptCore:

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

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 9ec94d8..2db0e17 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,5 +1,35 @@
 2008-09-23  Geoffrey Garen  <ggaren@apple.com>
 
 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:
         Try to fix the windows build.
 
         * VM/CTI.cpp:

diff --git a/JavaScriptCore/VM/Machine.cpp b/JavaScriptCore/VM/Machine.cpp
index 702f6c5..e6bd699 100644 (file)
--- a/JavaScriptCore/VM/Machine.cpp
+++ b/JavaScriptCore/VM/Machine.cpp
@@ -877,9 +877,11 @@ NEVER_INLINE Instruction* Machine::throwException(ExecState* exec, JSValue*& exc
 
 JSValue* Machine::execute(ProgramNode* programNode, ExecState* exec, ScopeChainNode* scopeChain, JSObject* thisObj, JSValue** exception)
 {
 
 JSValue* Machine::execute(ProgramNode* programNode, ExecState* exec, ScopeChainNode* scopeChain, JSObject* thisObj, JSValue** exception)
 {

+    ASSERT(!exec->hadException());
+

     if (m_reentryDepth >= MaxReentryDepth) {
         *exception = createStackOverflowError(exec);
     if (m_reentryDepth >= MaxReentryDepth) {
         *exception = createStackOverflowError(exec);

-        return 0;
+        return jsNull();

     }
 
     CodeBlock* codeBlock = &programNode->byteCode(scopeChain);
     }
 
     CodeBlock* codeBlock = &programNode->byteCode(scopeChain);


@@ -888,7 +890,7 @@ JSValue* Machine::execute(ProgramNode* programNode, ExecState* exec, ScopeChainN
     size_t newSize = oldSize + codeBlock->numParameters + RegisterFile::CallFrameHeaderSize + codeBlock->numCalleeRegisters;
     if (!m_registerFile.grow(newSize)) {
         *exception = createStackOverflowError(exec);
     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();
     }
 
     JSGlobalObject* lastGlobalObject = m_registerFile.globalObject();


@@ -932,9 +934,11 @@ JSValue* Machine::execute(ProgramNode* programNode, ExecState* exec, ScopeChainN
 
 JSValue* Machine::execute(FunctionBodyNode* functionBodyNode, ExecState* exec, JSFunction* function, JSObject* thisObj, const ArgList& args, ScopeChainNode* scopeChain, JSValue** exception)
 {
 
 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);
     if (m_reentryDepth >= MaxReentryDepth) {
         *exception = createStackOverflowError(exec);

-        return 0;
+        return jsNull();

     }
 
     size_t oldSize = m_registerFile.size();
     }
 
     size_t oldSize = m_registerFile.size();


@@ -942,7 +946,7 @@ JSValue* Machine::execute(FunctionBodyNode* functionBodyNode, ExecState* exec, J
 
     if (!m_registerFile.grow(oldSize + argc)) {
         *exception = createStackOverflowError(exec);
 
     if (!m_registerFile.grow(oldSize + argc)) {
         *exception = createStackOverflowError(exec);

-        return 0;
+        return jsNull();

     }
 
     Register* argv = m_registerFile.base() + oldSize;
     }
 
     Register* argv = m_registerFile.base() + oldSize;


@@ -957,7 +961,7 @@ JSValue* Machine::execute(FunctionBodyNode* functionBodyNode, ExecState* exec, J
     Register* r = slideRegisterWindowForCall(exec, newCodeBlock, &m_registerFile, m_registerFile.base(), argv, argc + RegisterFile::CallFrameHeaderSize, argc, *exception);
     if (UNLIKELY(*exception != 0)) {
         m_registerFile.shrink(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);
     }
     // a 0 codeBlock indicates a built-in caller
     initializeCallFrame(r, 0, 0, 0, argv, 0, argc, function);


@@ -992,9 +996,11 @@ JSValue* Machine::execute(EvalNode* evalNode, ExecState* exec, JSObject* thisObj
 
 JSValue* Machine::execute(EvalNode* evalNode, ExecState* exec, JSObject* thisObj, int registerOffset, ScopeChainNode* scopeChain, JSValue** exception)
 {
 
 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);
     if (m_reentryDepth >= MaxReentryDepth) {
         *exception = createStackOverflowError(exec);

-        return 0;
+        return jsNull();

     }
 
     EvalCodeBlock* codeBlock = &evalNode->byteCode(scopeChain);
     }
 
     EvalCodeBlock* codeBlock = &evalNode->byteCode(scopeChain);


@@ -1035,7 +1041,7 @@ JSValue* Machine::execute(EvalNode* evalNode, ExecState* exec, JSObject* thisObj
     size_t newSize = registerOffset + codeBlock->numCalleeRegisters;
     if (!m_registerFile.grow(newSize)) {
         *exception = createStackOverflowError(exec);
     size_t newSize = registerOffset + codeBlock->numCalleeRegisters;
     if (!m_registerFile.grow(newSize)) {
         *exception = createStackOverflowError(exec);

-        return 0;
+        return jsNull();

     }
 
     Register* r = m_registerFile.base() + registerOffset;
     }
 
     Register* r = m_registerFile.base() + registerOffset;

diff --git a/JavaScriptCore/kjs/StringPrototype.cpp b/JavaScriptCore/kjs/StringPrototype.cpp
index d006569..bea8629 100644 (file)
--- a/JavaScriptCore/kjs/StringPrototype.cpp
+++ b/JavaScriptCore/kjs/StringPrototype.cpp
@@ -257,6 +257,8 @@ JSValue* stringProtoFuncReplace(ExecState* exec, JSObject*, JSValue* thisValue,
                 args.append(sourceVal);
 
                 replacements.append(call(exec, replacement, callType, callData, exec->globalThisValue(), args)->toString(exec));
                 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));
 
             } else
                 replacements.append(substituteBackreferences(replacementString, source, ovector, reg));
 

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 7b44101..6cfb30d 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+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.
 2008-09-23  Beth Dakin  <bdakin@apple.com>
 
         Reviewed by Sam Weinig.

diff --git a/LayoutTests/fast/js/string-replace-exception-crash-expected.txt b/LayoutTests/fast/js/string-replace-exception-crash-expected.txt
new file mode 100644 (file)
index 0000000..8d71a01
--- /dev/null
+++ b/LayoutTests/fast/js/string-replace-exception-crash-expected.txt
@@ -0,0 +1,8 @@
+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.
+

diff --git a/LayoutTests/fast/js/string-replace-exception-crash.html b/LayoutTests/fast/js/string-replace-exception-crash.html
new file mode 100644 (file)
index 0000000..cf5e204
--- /dev/null
+++ b/LayoutTests/fast/js/string-replace-exception-crash.html
@@ -0,0 +1,59 @@
+<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>