[webkit-reviews] review granted: [Bug 216062] [JSC] Add missing detached buffer errors for ArrayBuffer and DataView : [Attachment 407731] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Ross Kirsling
<ross.kirsling at sony.com>'s request for review:
Bug 216062: [JSC] Add missing detached buffer errors for ArrayBuffer and
DataView
https://bugs.webkit.org/show_bug.cgi?id=216062

Attachment 407731: Patch

https://bugs.webkit.org/attachment.cgi?id=407731&action=review




--- Comment #3 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 407731
  --> https://bugs.webkit.org/attachment.cgi?id=407731
Patch

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

r=me with DFG / FTL tests and `DataView.prototype.byteLength` handling in
`getOwnPropertySlot` revising.

> Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp:74
> +    auto* thisObject = jsDynamicCast<JSArrayBuffer*>(vm,
callFrame->thisValue());
> +    if (!thisObject || thisObject->isShared())
> +	   return throwVMTypeError(globalObject, scope, "Receiver must be an
ArrayBuffer"_s);
> +    if (thisObject->impl()->isNeutered())
> +	   return throwVMTypeError(globalObject, scope, "Buffer has already
been detached"_s);

Can you add a test for DFG / FTL?

> Source/JavaScriptCore/runtime/JSDataViewPrototype.cpp:142
> +    if (dataView->isNeutered())
> +	   return throwVMTypeError(globalObject, scope, "Underlying ArrayBuffer
has been detached from the view"_s);

Can you add a test which works in DFG / FTL? Like this.
Because DFG / FTL handles DataView Get / Set functions.

function test(dataView){ dataView.getXXX(...); }

for (...)
    test(dataView);
detach(dataView);
test(dataView);

> Source/JavaScriptCore/runtime/JSDataViewPrototype.cpp:197
> +    if (dataView->isNeutered())
> +	   return throwVMTypeError(globalObject, scope, "Underlying ArrayBuffer
has been detached from the view"_s);

Ditto.

> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h:57
> +    auto length = ViewClass::TypedArrayStorageType == TypeDataView ? 1 : 3;

We can just put it into the line below.
(jsNumber(ViewClass::TypedArrayStorageType == TypeDataView ? 1 : 3)).