Bug 93607 – Web Inspector: [JSC] Caught exception is treated as uncaught

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Clark]

Webkit Version: r125153

With the flag "All Uncaught Exception" ON, show Error Console. Load the attached file.

Actual: It paused in the "throw" line.

Expected: Since the "throw" is within a try...catch, it should not be treated as an uncaught exception.

This case is ok.===========================
try {
    throw new Error("x");
} catch (e) {

}

This case is bad: ===========================
try {
    function f() {
      throw new Error("x");
    }
    f();
} catch (e) {

}

[Deirdre Saoirse Moen]

rdar://problem/12101592

[Timothy Hatcher]

*** Bug 117215 has been marked as a duplicate of this bug. ***

[Joseph Pecoraro]

Yep, this looks like a JSC issue.

ScriptDebugServer::execution is getting called with "hasHandler" being false. We would expect this to be true, because there is an eventual exception handler up the stack. I suspect this is a regression that happened a while ago.


---

(lldb) b ScriptDebugServer::exception

(lldb) c
* thread #1: tid = 0x8da30a, 0x00000001124613f7 WebCore`WebCore::ScriptDebugServer::exception(this=0x00007f948310b130, debuggerCallFrame=0x00007fff51742118, sourceID=1, lineNumber=14, columnNumber=0, hasHandler=false) + 39 at ScriptDebugServer.cpp:529, queue = 'com.apple.main-thread, stop reason = breakpoint 2.1
    frame #0: 0x00000001124613f7 WebCore`WebCore::ScriptDebugServer::exception(this=0x00007f948310b130, debuggerCallFrame=0x00007fff51742118, sourceID=1, lineNumber=14, columnNumber=0, hasHandler=false) + 39 at ScriptDebugServer.cpp:529
   526 	
   527 	void ScriptDebugServer::exception(const DebuggerCallFrame& debuggerCallFrame, intptr_t sourceID, int lineNumber, int columnNumber, bool hasHandler)
   528 	{
-> 529 	    if (m_paused)
   530 	        return;
   531 	
   532 	    if (m_pauseOnExceptionsState == PauseOnAllExceptions || (m_pauseOnExceptionsState == PauseOnUncaughtExceptions && !hasHandler))

(lldb) p hasHandler
(bool) $8 = false

(lldb) bt
* thread #1: tid = 0x8da30a, 0x00000001124613f7 WebCore`WebCore::ScriptDebugServer::exception(this=0x00007f948310b130, debuggerCallFrame=0x00007fff51742118, sourceID=1, lineNumber=14, columnNumber=0, hasHandler=false) + 39 at ScriptDebugServer.cpp:529, queue = 'com.apple.main-thread, stop reason = breakpoint 2.1
    frame #0: 0x00000001124613f7 WebCore`WebCore::ScriptDebugServer::exception(this=0x00007f948310b130, debuggerCallFrame=0x00007fff51742118, sourceID=1, lineNumber=14, columnNumber=0, hasHandler=false) + 39 at ScriptDebugServer.cpp:529
    frame #1: 0x000000010fe92b3f JavaScriptCore`JSC::Interpreter::unwind(this=0x00007f9483464d50, callFrame=0x00007fff517421f0, exceptionValue=0x00007fff517421e8, bytecodeOffset=29) + 847 at Interpreter.cpp:633
    frame #2: 0x000000010feb359e JavaScriptCore`JSC::genericUnwind(vm=0x00007f9482821400, callFrame=0x0000000118f600c8, exceptionValue=JSValue at 0x00007fff517421e8, vPCIndex=29) + 110 at JITExceptions.cpp:73
    frame #3: 0x000000010ffc0e08 JavaScriptCore`doThrow(exec=0x0000000118f600c8, pc=0x00007f948303a970) + 136 at LLIntExceptions.cpp:51
    frame #4: 0x000000010ffc0d6d JavaScriptCore`JSC::LLInt::returnToThrow(exec=0x0000000118f600c8, pc=0x00007f948303a970) + 29 at LLIntExceptions.cpp:60
    frame #5: 0x000000010ffc89c5 JavaScriptCore`llint_slow_path_get_from_scope(exec=0x0000000118f600c8, pc=0x00007f948303a970) + 469 at LLIntSlowPaths.cpp:1295
    frame #6: 0x000000010ffce851 JavaScriptCore`llint_op_get_from_scope + 494

[Joseph Pecoraro]

Created attachment 211111 [details]
[TEST] Test Case

Here is a manual test case.

[Joseph Pecoraro]

Created attachment 211132 [details]
[PATCH] Proposed Fix

Check the entire call stack for exception handlers, instead of just the local call frame.

Added tests that cover this and other Inspector pause on exception states.

[Mark Lam]

Comment on attachment 211132 [details]
[PATCH] Proposed Fix

The Interpreter.cpp changes LGTM.

[Geoffrey Garen]

Comment on attachment 211132 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=211132&action=review

Thanks, Joe!

r=me, but please fix the logic error below, and add a test for it.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:582
> +        if (!codeBlock)
> +            return StackVisitor::Done;

I think you want StackVisitor::Continue here.

Consider this stack:
    js function with try/catch scope => Array.prototype.map => js function => throw
CodeBlock is null for host functions. Therefore, when you see Array.prototype.map, you'll return StackVisitor::Done and conclude that there's no handler. But there is a handler, in the function that called map.

[Paul Miller]

*** Bug 121010 has been marked as a duplicate of this bug. ***

[Joseph Pecoraro]

(In reply to comment #7)
> (From update of attachment 211132 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=211132&action=review
> 
> Thanks, Joe!
> 
> r=me, but please fix the logic error below, and add a test for it.
> 
> > Source/JavaScriptCore/interpreter/Interpreter.cpp:582
> > +        if (!codeBlock)
> > +            return StackVisitor::Done;
> 
> I think you want StackVisitor::Continue here.
> 
> Consider this stack:
>     js function with try/catch scope => Array.prototype.map => js function => throw
> CodeBlock is null for host functions. Therefore, when you see Array.prototype.map, you'll return StackVisitor::Done and conclude that there's no handler. But there is a handler, in the function that called map.

Good catch and thanks for the example! That indeed fixes a issue. It also uncovered a somewhat surprising double pause.

Given:

    function exceptionInHostFunction()
    {
        [1].map(function(x) {
            throw "exception in host function";
        });
    }

Setting the inspector to break on all exceptions, and evaluating: setTimeout(exceptionHostFunction, 0);

The inspector pauses twice:

  1. At the throw line. Continuing in Web Inspector goes to (2)
  2. At the [].map line. Continuing in Web Inspector no longer pauses.

I'll take a brief look at this. It exists separate from this issue/patch, but affects the tests I'm writing for this patch.

[Joseph Pecoraro]

(In reply to comment #9)
> I'll take a brief look at this. It exists separate from this issue/patch, but affects the tests I'm writing for this patch.

Filed: <https://webkit.org/b/121108> Web Inspector gets paused twice when there is an exception in host function

I added a workaround in these tests to expect an extra pause.

[Joseph Pecoraro]

Committed <http://trac.webkit.org/changeset/155471>.