REGRESSION(r154797): Debugger crashes when stepping over an uncaught exception.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 May 2014 21:40:21 +0000 (21:40 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 May 2014 21:40:21 +0000 (21:40 +0000)
<https://webkit.org/b/133182>

Reviewed by Oliver Hunt.

Source/JavaScriptCore:
Before r154797, we used to clear the VM exception before calling into the
debugger.  After r154797, we don't.  This patch will restore this clearing
of the exception before calling into the debugger.

Also added assertions after returning from calls into the debugger to
ensure that the debugger did not introduce any exceptions.

* interpreter/Interpreter.cpp:
(JSC::unwindCallFrame):
(JSC::Interpreter::unwind):
(JSC::Interpreter::debug):
- Fixed the assertion here.  Interpreter::debug() should never be called
  with a pending exception.  Debugger callbacks for exceptions should be
  handled by Interpreter::unwind() and Interpreter::unwindCallFrame().

LayoutTests:
* inspector-protocol/debugger/regress-133182-expected.txt: Added.
* inspector-protocol/debugger/regress-133182.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@169221 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector-protocol/debugger/regress-133182-expected.txt [new file with mode: 0644]
LayoutTests/inspector-protocol/debugger/regress-133182.html [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/interpreter/Interpreter.cpp

index 08ff17d..dfc4b58 100644 (file)
@@ -1,3 +1,13 @@
+2014-05-22  Mark Lam  <mark.lam@apple.com>
+
+        REGRESSION(r154797): Debugger crashes when stepping over an uncaught exception.
+        <https://webkit.org/b/133182>
+
+        Reviewed by Oliver Hunt.
+
+        * inspector-protocol/debugger/regress-133182-expected.txt: Added.
+        * inspector-protocol/debugger/regress-133182.html: Added.
+
 2014-05-22  Michał Pakuła vel Rutka  <m.pakula@samsung.com>
 
         Unreviewed EFL gardening
diff --git a/LayoutTests/inspector-protocol/debugger/regress-133182-expected.txt b/LayoutTests/inspector-protocol/debugger/regress-133182-expected.txt
new file mode 100644 (file)
index 0000000..cebaea0
--- /dev/null
@@ -0,0 +1,48 @@
+CONSOLE MESSAGE: line 48: [1] Testing statement '({}).a.b.c.d;'
+CONSOLE MESSAGE: line 49: [1] Paused and about to step
+CONSOLE MESSAGE: line 61: [1] Resumed
+CONSOLE MESSAGE: line 53: [1] Paused after stepping
+CONSOLE MESSAGE: line 61: [1] Resumed
+CONSOLE MESSAGE: line 1: TypeError: undefined is not an object (evaluating '({}).a.b')
+CONSOLE MESSAGE: line 48: [2] Testing statement 'exceptionBasic();'
+CONSOLE MESSAGE: line 49: [2] Paused and about to step
+CONSOLE MESSAGE: line 61: [2] Resumed
+CONSOLE MESSAGE: line 53: [2] Paused after stepping
+CONSOLE MESSAGE: line 61: [2] Resumed
+CONSOLE MESSAGE: line 3: TypeError: undefined is not an object (evaluating '({}).a.b')
+CONSOLE MESSAGE: line 48: [3] Testing statement 'exceptionDOM();'
+CONSOLE MESSAGE: line 49: [3] Paused and about to step
+CONSOLE MESSAGE: line 61: [3] Resumed
+CONSOLE MESSAGE: line 53: [3] Paused after stepping
+CONSOLE MESSAGE: line 61: [3] Resumed
+CONSOLE MESSAGE: line 8: NotFoundError: DOM Exception 8: An attempt was made to reference a Node in a context where it does not exist.
+CONSOLE MESSAGE: line 48: [4] Testing statement 'exceptionInHostFunction();'
+CONSOLE MESSAGE: line 49: [4] Paused and about to step
+CONSOLE MESSAGE: line 61: [4] Resumed
+CONSOLE MESSAGE: line 53: [4] Paused after stepping
+CONSOLE MESSAGE: line 61: [4] Resumed
+CONSOLE MESSAGE: line 24: exception in host function
+CONSOLE MESSAGE: line 48: [5] Testing statement 'throwString();'
+CONSOLE MESSAGE: line 49: [5] Paused and about to step
+CONSOLE MESSAGE: line 61: [5] Resumed
+CONSOLE MESSAGE: line 53: [5] Paused after stepping
+CONSOLE MESSAGE: line 61: [5] Resumed
+CONSOLE MESSAGE: line 13: exception string
+CONSOLE MESSAGE: line 48: [6] Testing statement 'throwParam({x:1});'
+CONSOLE MESSAGE: line 49: [6] Paused and about to step
+CONSOLE MESSAGE: line 61: [6] Resumed
+CONSOLE MESSAGE: line 53: [6] Paused after stepping
+CONSOLE MESSAGE: line 61: [6] Resumed
+CONSOLE MESSAGE: line 18: [object Object]
+CONSOLE MESSAGE: line 48: [7] Testing statement 'throwParam(new Error('error message'));'
+CONSOLE MESSAGE: line 49: [7] Paused and about to step
+CONSOLE MESSAGE: line 61: [7] Resumed
+CONSOLE MESSAGE: line 53: [7] Paused after stepping
+CONSOLE MESSAGE: line 61: [7] Resumed
+CONSOLE MESSAGE: line 18: Error: error message
+Regression test for https://bugs.webkit.org/show_bug.cgi?id=133182
+
+Stepping after breaking on uncaught exceptions should not crash
+
+PASS - paused for each uncaught exception
+
diff --git a/LayoutTests/inspector-protocol/debugger/regress-133182.html b/LayoutTests/inspector-protocol/debugger/regress-133182.html
new file mode 100644 (file)
index 0000000..6a39af9
--- /dev/null
@@ -0,0 +1,78 @@
+<html>
+<head>
+<script src="../../http/tests/inspector-protocol/resources/protocol-test.js"></script>
+<script src="resources/exception.js"></script>
+<script>
+function test()
+{
+    var expectPause = false;
+    var isStepping = false;
+
+    var testIndex = 0;
+    var statementsWithUncaughtExceptions = [
+        "({}).a.b.c.d;",
+        "exceptionBasic();",
+        "exceptionDOM();",
+        "exceptionInHostFunction();",
+        "throwString();",
+        "throwParam({x:1});",
+        "throwParam(new Error('error message'));"
+    ];
+
+    function triggerNextUncaughtException()
+    {
+
+        // Evaluate statement and expect to pause.
+        if (testIndex < statementsWithUncaughtExceptions.length) {
+            var statement = statementsWithUncaughtExceptions[testIndex++];
+            InspectorTest.sendCommand("Runtime.evaluate", {expression: "setTimeout(function() { " + statement + " }, 0);"});
+            return;
+        }
+
+        // Done evaluating statements to pause. Evaluate some more we do not expect to pause.
+        InspectorTest.log("PASS - paused for each uncaught exception");
+        InspectorTest.completeTest();
+    }
+
+
+    InspectorTest.sendCommand("Debugger.enable", {});
+    InspectorTest.sendCommand("Debugger.setPauseOnExceptions", {state: "uncaught"}, function(responseObject) {
+        InspectorTest.checkForError(responseObject);
+        expectPause = true;
+        triggerNextUncaughtException();
+    });
+
+    InspectorTest.eventHandler["Debugger.paused"] = function(messageObject)
+    {
+        if (!expectPause) {
+            InspectorTest.log("FAIL - debugger paused when we did not expect to");
+            InspectorTest.completeTest();
+            return;
+        }
+
+        if (!isStepping) {
+            console.log("[" + testIndex + "] Testing statement '" + statementsWithUncaughtExceptions[testIndex - 1] + "'");
+            console.log("[" + testIndex + "] Paused and about to step");
+            isStepping = true;
+            InspectorTest.sendCommand("Debugger.stepOver", {});
+        } else {
+            console.log("[" + testIndex + "] Paused after stepping");
+            isStepping = false;
+            InspectorTest.sendCommand("Debugger.resume", {});
+        }
+    }
+
+    InspectorTest.eventHandler["Debugger.resumed"] = function(messageObject)
+    {
+        console.log("[" + testIndex + "] Resumed");
+        if (!isStepping)
+            triggerNextUncaughtException();
+    }
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Regression test for https://bugs.webkit.org/show_bug.cgi?id=133182</p>
+<p>Stepping after breaking on uncaught exceptions should not crash</p>
+</body>
+</html>
index 33e9140..4671b50 100644 (file)
@@ -1,3 +1,25 @@
+2014-05-22  Mark Lam  <mark.lam@apple.com>
+
+        REGRESSION(r154797): Debugger crashes when stepping over an uncaught exception.
+        <https://webkit.org/b/133182>
+
+        Reviewed by Oliver Hunt.
+
+        Before r154797, we used to clear the VM exception before calling into the
+        debugger.  After r154797, we don't.  This patch will restore this clearing
+        of the exception before calling into the debugger.
+
+        Also added assertions after returning from calls into the debugger to
+        ensure that the debugger did not introduce any exceptions.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::unwindCallFrame):
+        (JSC::Interpreter::unwind):
+        (JSC::Interpreter::debug):
+        - Fixed the assertion here.  Interpreter::debug() should never be called
+          with a pending exception.  Debugger callbacks for exceptions should be
+          handled by Interpreter::unwind() and Interpreter::unwindCallFrame().
+
 2014-05-21  Filip Pizlo  <fpizlo@apple.com>
 
         Store barrier elision should run after DCE in both the DFG path and the FTL path
index a76c080..e83c681 100644 (file)
@@ -447,10 +447,12 @@ static bool unwindCallFrame(StackVisitor& visitor)
     JSScope* scope = callFrame->scope();
 
     if (Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger()) {
+        ClearExceptionScope scope(&callFrame->vm());
         if (callFrame->callee())
             debugger->returnEvent(callFrame);
         else
             debugger->didExecuteProgram(callFrame);
+        ASSERT(!callFrame->hadException());
     }
 
     JSValue activation;
@@ -700,9 +702,8 @@ NEVER_INLINE HandlerInfo* Interpreter::unwind(CallFrame*& callFrame, JSValue& ex
     if (exceptionValue.isEmpty() || (exceptionValue.isCell() && !exceptionValue.asCell()))
         exceptionValue = jsNull();
 
-    if (exceptionValue.isObject()) {
+    if (exceptionValue.isObject())
         isTermination = isTerminatedExecutionException(asObject(exceptionValue));
-    }
 
     ASSERT(callFrame->vm().exceptionStack().size());
 
@@ -726,6 +727,7 @@ NEVER_INLINE HandlerInfo* Interpreter::unwind(CallFrame*& callFrame, JSValue& ex
         }
 
         debugger->exception(callFrame, exceptionValue, hasHandler);
+        ASSERT(!callFrame->hadException());
     }
 
     // Calculate an exception handler vPC, unwinding call frames as necessary.
@@ -1225,28 +1227,31 @@ NEVER_INLINE void Interpreter::debug(CallFrame* callFrame, DebugHookID debugHook
     Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger();
     if (!debugger)
         return;
-    ASSERT(callFrame->codeBlock()->hasDebuggerRequests() || callFrame->hadException());
+
+    ASSERT(callFrame->codeBlock()->hasDebuggerRequests());
+    ASSERT(!callFrame->hadException());
 
     switch (debugHookID) {
         case DidEnterCallFrame:
             debugger->callEvent(callFrame);
-            return;
+            break;
         case WillLeaveCallFrame:
             debugger->returnEvent(callFrame);
-            return;
+            break;
         case WillExecuteStatement:
             debugger->atStatement(callFrame);
-            return;
+            break;
         case WillExecuteProgram:
             debugger->willExecuteProgram(callFrame);
-            return;
+            break;
         case DidExecuteProgram:
             debugger->didExecuteProgram(callFrame);
-            return;
+            break;
         case DidReachBreakpoint:
             debugger->didReachBreakpoint(callFrame);
-            return;
+            break;
     }
+    ASSERT(!callFrame->hadException());
 }    
 
 void Interpreter::enableSampler()