Bug 202650 – Throw the right exception upon memory exhaustion in Array::slice

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Robin Morisset]

rdar://problem/55255615

[Robin Morisset]

Created attachment 380351 [details]
Patch

[Robin Morisset]

Created attachment 380387 [details]
Patch

[Robin Morisset]

Created attachment 380447 [details]
Patch

trivial rebase (exec -> callFrame)

[Saam Barati]

Comment on attachment 380447 [details]
Patch

Lots of red

[Robin Morisset]

(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.

[Robin Morisset]

Created attachment 380462 [details]
Patch

Rebasing again, and attaching the right patch this time.

[Saam Barati]

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?

[Robin Morisset]

(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.

[WebKit Commit Bot]

Comment on attachment 380462 [details]
Patch

Clearing flags on attachment: 380462

Committed r251392: <https://trac.webkit.org/changeset/251392>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56480013>

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

(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