Bug 214914 – Update some JSArrayBufferView comments and add some assertions.

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

[Mark Lam]

...

[Mark Lam]

Created attachment 405444 [details]
proposed patch.

[Darin Adler]

Comment on attachment 405444 [details]
proposed patch.

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

> Source/JavaScriptCore/runtime/ArrayBuffer.cpp:213
> +    ASSERT(!Gigacage::isEnabled() || (Gigacage::contains(data) && Gigacage::contains(bitwise_cast<const uint8_t*>(data) + byteLength - 1)));

Converting from const void* to const uint8_t* requires only a static_cast, not a bitwise_cast. We should reserve bitwise_cast for the cases where it’s needed.

> Source/JavaScriptCore/runtime/JSArrayBufferView.cpp:51
> +    ASSERT(!Gigacage::isEnabled() || (Gigacage::contains(vector) && Gigacage::contains(bitwise_cast<const uint8_t*>(vector) + length - 1)));

Ditto. Also, I suggest casting to uint8_t* for brevity rather than adding const. That can still be passed to a function that expects const uint8_t.

[Mark Lam]

Comment on attachment 405444 [details]
proposed patch.

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

Thanks for the review.

>> Source/JavaScriptCore/runtime/ArrayBuffer.cpp:213
>> +    ASSERT(!Gigacage::isEnabled() || (Gigacage::contains(data) && Gigacage::contains(bitwise_cast<const uint8_t*>(data) + byteLength - 1)));
> 
> Converting from const void* to const uint8_t* requires only a static_cast, not a bitwise_cast. We should reserve bitwise_cast for the cases where it’s needed.

I'll switch static_cast.  const is still needed.  Otherwise Clang will complain about: error: static_cast from 'const void *' to 'uint8_t *' (aka 'unsigned char *') casts away qualifiers.

>> Source/JavaScriptCore/runtime/JSArrayBufferView.cpp:51
>> +    ASSERT(!Gigacage::isEnabled() || (Gigacage::contains(vector) && Gigacage::contains(bitwise_cast<const uint8_t*>(vector) + length - 1)));
> 
> Ditto. Also, I suggest casting to uint8_t* for brevity rather than adding const. That can still be passed to a function that expects const uint8_t.

Ditto.

[Mark Lam]

Landed in r265045: <http://trac.webkit.org/r265045>.

[Radar WebKit Bug Importer]

<rdar://problem/66278689>