Bug 197732

Summary: [JSC] Add CodeOffset, 32bit offset in JIT memory pool
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: NEW    
Severity: Normal CC: ews-watchlist
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews114 for mac-highsierra
none
Patch
none
Patch none

Yusuke Suzuki
Reported 2019-05-09 00:15:50 PDT
We can just hold the offset in JIT code pool.
Attachments
Patch (58.29 KB, patch)
2019-05-14 01:06 PDT, Yusuke Suzuki
no flags
Archive of layout-test-results from ews114 for mac-highsierra (494.15 KB, application/zip)
2019-05-14 02:20 PDT, EWS Watchlist
no flags
Patch (58.63 KB, patch)
2019-05-14 16:07 PDT, Yusuke Suzuki
no flags
Patch (58.68 KB, patch)
2019-05-14 19:22 PDT, Yusuke Suzuki
no flags
Yusuke Suzuki
Comment 1 2019-05-09 00:18:40 PDT
(In reply to Yusuke Suzuki from comment #0) > We can just hold the offset in JIT code pool. We should have different types for (1) JIT code pointer and (2) C runtime pointer. And making (1) 32bit.
Yusuke Suzuki
Comment 2 2019-05-14 01:06:08 PDT
EWS Watchlist
Comment 3 2019-05-14 01:10:34 PDT
Attachment 369825 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/CodeOffset.h:47: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/JavaScriptCore/jit/CodeOffset.h:52: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/JavaScriptCore/jit/CodeOffset.h:54: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 3 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.
EWS Watchlist
Comment 4 2019-05-14 02:20:36 PDT
Comment on attachment 369825 [details] Patch Attachment 369825 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12185748 Number of test failures exceeded the failure limit.
EWS Watchlist
Comment 5 2019-05-14 02:20:37 PDT
Created attachment 369829 [details] Archive of layout-test-results from ews114 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6
Yusuke Suzuki
Comment 6 2019-05-14 16:07:19 PDT
EWS Watchlist
Comment 7 2019-05-14 16:09:16 PDT
Attachment 369906 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/CodeOffset.h:47: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/JavaScriptCore/jit/CodeOffset.h:52: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/JavaScriptCore/jit/CodeOffset.h:54: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 3 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 8 2019-05-14 19:22:21 PDT
EWS Watchlist
Comment 9 2019-05-14 19:26:34 PDT
Attachment 369919 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/CodeOffset.h:47: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/JavaScriptCore/jit/CodeOffset.h:52: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] ERROR: Source/JavaScriptCore/jit/CodeOffset.h:54: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 3 in 24 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 10 2019-05-18 17:58:55 PDT
I think this should be definitely done. https://bugs.webkit.org/show_bug.cgi?id=186422 shows that Bag takes much memory. 6. WebKit BagNode_0x117c78000 30.0M 16.6M 16.6M 0K 195956 16.0M 575K 4% 30 This Bag is used by Watchpoint and JITIC things. And JITIC things include these CodeLocationLabel, and each takes 8bytes. If we use CodeOffset instead, it becomes 4bytes. Furthermore, CodeOffset thing ensures that repatch only happens within 32bit range, this is further safer than having a pointer which can be read/write later. I think the current patch is WIP. We should limit the source of CodeOffset and threading it in various places.
Note You need to log in before you can comment on or make changes to this bug.