Bug 133182 – REGRESSION(r154797): Debugger crashes when stepping over an uncaught exception

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

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.

[Mark Lam]

<rdar://problem/16520770>

[Mark Lam]

Created attachment 231902 [details]
the patch.

[Oliver Hunt]

Comment on attachment 231902 [details]
the patch.

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

> Source/JavaScriptCore/interpreter/Interpreter.cpp:455
> +        ASSERT(!callFrame->hadException());

Why will this assertion not fire?  Can't the debugger legitimately cause an unhanded exception?

[Mark Lam]

(In reply to comment #3)
> (From update of attachment 231902 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231902&action=review
> 
> > Source/JavaScriptCore/interpreter/Interpreter.cpp:455
> > +        ASSERT(!callFrame->hadException());
> 
> Why will this assertion not fire?  Can't the debugger legitimately cause an unhanded exception?

The inspector is supposed to catch and handle any exceptions thrown in the debugging session.  I don’t think there’s currently a way to introduce an exception where there is none.  If this changes, we can change this assert in the future.

[Joseph Pecoraro]

Comment on attachment 231902 [details]
the patch.

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

>>> Source/JavaScriptCore/interpreter/Interpreter.cpp:455
>>> +        ASSERT(!callFrame->hadException());
>> 
>> Why will this assertion not fire?  Can't the debugger legitimately cause an unhanded exception?
> 
> The inspector is supposed to catch and handle any exceptions thrown in the debugging session.  I don’t think there’s currently a way to introduce an exception where there is none.  If this changes, we can change this assert in the future.

If the Inspector executes anything on its own behalf (internal implementation details, breakpoint actions, probes, etc) we should be catching and clearing exception information. This assert sounds reasonable to me, and might bring to light a situation in the Inspector where we might not have been cleaning up after ourselves and probably should be.

[Mark Lam]

Thanks for the review.  Landed in r169221: <http://trac.webkit.org/r169221>.