Bug 125328

Summary: JSC: Simplify interface between throw and catch handler
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, eflews.bot, fpizlo, ggaren, gyuyoung.kim, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
ggaren: review+, eflews.bot: commit-queue-
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 none

Description Michael Saboff 2013-12-05 18:58:56 PST
Currently the LLInt version of the throw handling (_llint_throw_from_slow_path_trampoline) and the JIT version (CCallHelpers::jumpToExceptionHandler()) put the contents of  VM.callFrameForThrow in regT0.  _llint_throw_from_slow_path_trampoline also sets the callFrame register to VM.topCallFrame.  The callFrame register is overwritten in the catch handler to the value in regT0 or popped of the stack in the uncaught case.

These exchanges should be simplified to have the catch handlers use VM.callFrameForThrow directly for setting callFrame register.
Comment 1 Michael Saboff 2013-12-05 19:18:49 PST
Created attachment 218562 [details]
Patch
Comment 2 Geoffrey Garen 2013-12-05 20:24:40 PST
Comment on attachment 218562 [details]
Patch

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

r=me

> Source/JavaScriptCore/jit/CCallHelpers.h:1578
>          move(TrustedImmPtr(vm()), GPRInfo::regT0);
>          loadPtr(Address(GPRInfo::regT0, VM::targetMachinePCForThrowOffset()), GPRInfo::regT1);

This should just be a load of an absolute address (VM::targetMachinePCForThrow). No need to put VM in a register.
Comment 3 EFL EWS Bot 2013-12-05 20:29:26 PST
Comment on attachment 218562 [details]
Patch

Attachment 218562 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/44958010
Comment 4 Build Bot 2013-12-05 21:21:37 PST
Comment on attachment 218562 [details]
Patch

Attachment 218562 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/45198001

New failing tests:
js/dom/JSON-parse.html
Comment 5 Build Bot 2013-12-05 21:21:39 PST
Created attachment 218569 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 6 Michael Saboff 2013-12-05 21:29:17 PST
Committed r160213: <http://trac.webkit.org/changeset/160213>
Comment 7 Michael Saboff 2013-12-05 21:40:56 PST
(In reply to comment #5)
> Created an attachment (id=218569) [details]
> Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2
> 
> The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
> Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

Didn't see this crash before landing.  Looks like the current callFrame in the catch block doesn't have a codeBlock.  Investigating further.
Comment 8 Michael Saboff 2013-12-05 21:41:19 PST
Working in a fix.
Comment 9 Michael Saboff 2013-12-05 22:08:03 PST
(In reply to comment #8)
> Working in a fix.

Fix tracked with https://bugs.webkit.org/show_bug.cgi?id=125335 - REGRESSION(r160213): Crash in js/dom/JSON-parse.html