| Summary: | [FTL] Support HasIndexedProperty for ArrayStorage and SlowPutArrayStorage | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||
| Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Yusuke Suzuki
2018-02-14 06:16:36 PST
Created attachment 333791 [details]
Patch
Comment on attachment 333791 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333791&action=review > Source/JavaScriptCore/ChangeLog:3 > + [FTL] Support HasIndexedProperty for ArrayStorage And SlowPutArrayStorage. > Source/JavaScriptCore/ChangeLog:8 > + This patch adds HasIndexedProperty for ArrayStorage in FTL. And SlowPutArrayStorage. > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9781 > + m_out.jump(checkHole); This needs to do a speculation check that it's not out of bounds. Seems like this should be testable via crashing on some OOB read. You also won't need a "checkHole" dedicated block if it's in bounds, you can just use the incoming block we're emitting code in. Comment on attachment 333791 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333791&action=review >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9781 >> + m_out.jump(checkHole); > > This needs to do a speculation check that it's not out of bounds. Seems like this should be testable via crashing on some OOB read. You also won't need a "checkHole" dedicated block if it's in bounds, you can just use the incoming block we're emitting code in. As is the same to GetByVal etc., HasIndexedProperty also has special lowering rule in SSA lowering phase, which emits CheckInBounds. We do not need to perform out-of-bounds speculation here since preceeding CheckInBounds does this. For ArrayStorage/SlowPutArrayStorage, we emit GetVectorLength and CheckInBounds instead of GetArrayLength and CheckInBounds. So it is correctly handled. For checkHole case, I'll update the patch to drop checkHole BB for InBounds case. Created attachment 334244 [details]
Patch
Comment on attachment 334244 [details]
Patch
r=me
Comment on attachment 334244 [details]
Patch
Thanks!
Comment on attachment 334244 [details] Patch Clearing flags on attachment: 334244 Committed r228943: <https://trac.webkit.org/changeset/228943> All reviewed patches have been landed. Closing bug. |