Bug 202650

Summary: Throw the right exception upon memory exhaustion in Array::slice
Product: WebKit Reporter: Robin Morisset <rmorisset>
Component: JavaScriptCoreAssignee: Robin Morisset <rmorisset>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, ews-watchlist, glenn, jer.noble, keith_miller, mark.lam, msaboff, philipj, saam, sergio, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Robin Morisset 2019-10-07 13:22:50 PDT 
rdar://problem/55255615
Comment 1 Robin Morisset 2019-10-07 13:29:10 PDT 
Created attachment 380351 [details]
Patch
Comment 2 Robin Morisset 2019-10-07 20:35:03 PDT 
Created attachment 380387 [details]
Patch
Comment 3 Robin Morisset 2019-10-08 11:26:11 PDT 
Created attachment 380447 [details]
Patch

trivial rebase (exec -> callFrame)
Comment 4 Saam Barati 2019-10-08 13:14:32 PDT 
Comment on attachment 380447 [details]
Patch

Lots of red
Comment 5 Robin Morisset 2019-10-08 13:31:23 PDT 
(In reply to Saam Barati from comment #4)
> Comment on attachment 380447 [details]
> Patch
> 
> Lots of red

Seems like I accidentally resubmitted the old patch instead of the rebased one. I'll fix it imminently.
Comment 6 Robin Morisset 2019-10-08 13:58:52 PDT 
Created attachment 380462 [details]
Patch

Rebasing again, and attaching the right patch this time.
Comment 7 Saam Barati 2019-10-20 21:58:37 PDT 
Comment on attachment 380462 [details]
Patch

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

> Source/JavaScriptCore/runtime/ArrayBuffer.cpp:298
> +RefPtr<ArrayBuffer> ArrayBuffer::slice(double begin, double end) const

Do callers of these various functions check for exceptions?
Comment 8 Robin Morisset 2019-10-21 13:51:41 PDT 
(In reply to Saam Barati from comment #7)
> Comment on attachment 380462 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=380462&action=review
> 
> > Source/JavaScriptCore/runtime/ArrayBuffer.cpp:298
> > +RefPtr<ArrayBuffer> ArrayBuffer::slice(double begin, double end) const
> 
> Do callers of these various functions check for exceptions?

As far as I can tell, they are only called from arrayBufferProtoFuncSlice, which is installed as a normal JS function in JSArrayBufferPrototype.cpp at line 116:
JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->slice, arrayBufferProtoFuncSlice, static_cast<unsigned>(PropertyAttribute::DontEnum), 2);

So I believe it should be fine (and the fact that all bots are green is encouraging), but I may be wrong.
Comment 9 WebKit Commit Bot 2019-10-21 15:47:23 PDT 
Comment on attachment 380462 [details]
Patch

Clearing flags on attachment: 380462

Committed r251392: <https://trac.webkit.org/changeset/251392>
Comment 10 WebKit Commit Bot 2019-10-21 15:47:25 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2019-10-21 15:48:19 PDT 
<rdar://problem/56480013>
Comment 12 Yusuke Suzuki 2019-10-23 20:28:59 PDT 
Can you check WebCore side too? ArrayBuffer is also used in WebCore.
For example, StructuredClone.cpp is using slice() call.
And WebCore/fileapi/FileReaderLoader.cpp is also using it.

And I prefer renaming it to `trySlice` if it can fail.
Comment 13 Yusuke Suzuki 2020-03-15 04:29:10 PDT 
(In reply to Yusuke Suzuki from comment #12)
> Can you check WebCore side too? ArrayBuffer is also used in WebCore.
> For example, StructuredClone.cpp is using slice() call.
> And WebCore/fileapi/FileReaderLoader.cpp is also using it.
> 
> And I prefer renaming it to `trySlice` if it can fail.

I'll fix this issue. https://bugs.webkit.org/show_bug.cgi?id=209121