Bug 202650 - Throw the right exception upon memory exhaustion in Array::slice
Summary: Throw the right exception upon memory exhaustion in Array::slice
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Robin Morisset
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-07 13:22 PDT by Robin Morisset
Modified: 2020-03-15 04:29 PDT (History)
14 users (show)

See Also:

Attachments
Patch (5.18 KB, patch)
2019-10-07 13:29 PDT, Robin Morisset 		no flags Details | Formatted Diff | Diff
Patch (5.47 KB, patch)
2019-10-07 20:35 PDT, Robin Morisset 		no flags Details | Formatted Diff | Diff
Patch (5.47 KB, patch)
2019-10-08 11:26 PDT, Robin Morisset 		no flags Details | Formatted Diff | Diff
Patch (5.46 KB, patch)
2019-10-08 13:58 PDT, Robin Morisset 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) 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 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