RESOLVED FIXED 133182
REGRESSION(r154797): Debugger crashes when stepping over an uncaught exception 
https://bugs.webkit.org/show_bug.cgi?id=133182
Summary REGRESSION(r154797): Debugger crashes when stepping over an uncaught exception
Mark Lam
Reported 2014-05-22 08:49:06 PDT
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.
Attachments
the patch. (10.28 KB, patch)
2014-05-22 12:11 PDT, Mark Lam 		oliver: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2014-05-22 08:49:50 PDT
<rdar://problem/16520770>
Mark Lam
Comment 2 2014-05-22 12:11:23 PDT
Created attachment 231902 [details] the patch.
Oliver Hunt
Comment 3 2014-05-22 14:30:13 PDT
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
Comment 4 2014-05-22 14:32:32 PDT
(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 5 2014-05-22 14:37:17 PDT
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
Comment 6 2014-05-22 14:41:37 PDT
Thanks for the review. Landed in r169221: <http://trac.webkit.org/r169221>.
Note You need to log in before you can comment on or make changes to this bug.