Bug 125328 - JSC: Simplify interface between throw and catch handler
Summary: JSC: Simplify interface between throw and catch handler
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-05 18:58 PST by Michael Saboff
Modified: 2013-12-05 22:08 PST (History)
6 users (show)

See Also:

Attachments
Patch (6.83 KB, patch)
2013-12-05 19:18 PST, Michael Saboff 		ggaren: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 (465.25 KB, application/zip)
2013-12-05 21:21 PST, Build Bot 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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