Commit 532ba437 authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

[arraybuffer] Add checked ByteLength getter

V8 changes the size field of ArrayBuffers to {size_t}. Therefore an
ArrayBuffer from V8 can overflow the size field of a blink::ArrayBuffer.
With this CL we change the {ByteLength} getter of blink::ArrayBuffer to
two new getters: {ByteLengthAsUnsigned} and {ByteLengthAsSizeT}.
{ByteLengthAsUnsigned} includes an overflow check and is used for now
for most existing accesses to {ByteLength}. Eventually we would like to
change all calls to {ByteLengthAsSizeT}, so that eventually we can
remove {ByteLengthAsUnsigned} and rename {ByteLengthAsSizeT} to
{ByteLength} again.

R=haraken@chromium.org

Bug: v8:4153
Change-Id: I1af532b8ef525b04e8e7f39147f4ab87f8e29082
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1888831
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713816}
parent 9dc67ba7
......@@ -72,7 +72,10 @@ class CORE_EXPORT ArrayBuffer : public RefCounted<ArrayBuffer> {
inline const void* DataShared() const;
inline void* DataMaybeShared();
inline const void* DataMaybeShared() const;
inline unsigned ByteLength() const;
inline size_t ByteLengthAsSizeT() const;
// This function is deprecated and should not be used. Use {ByteLengthAsSizeT}
// instead.
inline unsigned ByteLengthAsUnsigned() const;
// Creates a new ArrayBuffer object with copy of bytes in this object
// ranging from |begin| up to but not including |end|.
......@@ -124,7 +127,7 @@ scoped_refptr<ArrayBuffer> ArrayBuffer::Create(ArrayBuffer* other) {
// TODO(binji): support creating a SharedArrayBuffer by copying another
// ArrayBuffer?
DCHECK(!other->IsShared());
return ArrayBuffer::Create(other->Data(), other->ByteLength());
return ArrayBuffer::Create(other->Data(), other->ByteLengthAsSizeT());
}
scoped_refptr<ArrayBuffer> ArrayBuffer::Create(const void* source,
......@@ -238,7 +241,15 @@ const void* ArrayBuffer::DataMaybeShared() const {
return contents_.DataMaybeShared();
}
unsigned ArrayBuffer::ByteLength() const {
size_t ArrayBuffer::ByteLengthAsSizeT() const {
return contents_.DataLength();
}
// This function is deprecated and should not be used. Use {ByteLengthAsSizeT}
// instead.
unsigned ArrayBuffer::ByteLengthAsUnsigned() const {
CHECK_LE(contents_.DataLength(),
static_cast<size_t>(std::numeric_limits<unsigned>::max()));
// TODO(dtapuska): Revisit this cast. ArrayBufferContents
// uses size_t for storing data. Whereas ArrayBuffer IDL is
// only uint32_t based.
......@@ -254,7 +265,7 @@ scoped_refptr<ArrayBuffer> ArrayBuffer::Slice(unsigned begin,
}
unsigned ArrayBuffer::ClampIndex(unsigned index) const {
return index < ByteLength() ? index : ByteLength();
return index < ByteLengthAsUnsigned() ? index : ByteLengthAsUnsigned();
}
} // namespace blink
......
......@@ -90,10 +90,10 @@ class CORE_EXPORT ArrayBufferView : public RefCounted<ArrayBufferView> {
return false;
if (sizeof(T) > 1 && byte_offset % sizeof(T))
return false;
if (byte_offset > buffer->ByteLength())
if (byte_offset > buffer->ByteLengthAsUnsigned())
return false;
unsigned remaining_elements =
static_cast<unsigned>((buffer->ByteLength() - byte_offset) / sizeof(T));
unsigned remaining_elements = static_cast<unsigned>(
(buffer->ByteLengthAsUnsigned() - byte_offset) / sizeof(T));
if (num_elements > remaining_elements)
return false;
return true;
......
......@@ -47,7 +47,8 @@ unsigned ArrayPiece::ByteLength() const {
void ArrayPiece::InitWithArrayBuffer(ArrayBuffer* buffer) {
if (buffer) {
InitWithData(buffer->Data(), SafeCast<unsigned>(buffer->ByteLength()));
InitWithData(buffer->Data(),
SafeCast<unsigned>(buffer->ByteLengthAsUnsigned()));
is_detached_ = buffer->IsDetached();
} else {
InitNull();
......
......@@ -41,8 +41,8 @@ bool DOMArrayBuffer::Transfer(v8::Isolate* isolate,
ArrayBufferContents& result) {
DOMArrayBuffer* to_transfer = this;
if (!IsDetachable(isolate)) {
to_transfer =
DOMArrayBuffer::Create(Buffer()->Data(), Buffer()->ByteLength());
to_transfer = DOMArrayBuffer::Create(Buffer()->Data(),
Buffer()->ByteLengthAsUnsigned());
}
if (!to_transfer->Buffer()->Transfer(result))
......
......@@ -21,7 +21,7 @@ class CORE_EXPORT DOMArrayBufferBase : public ScriptWrappable {
const void* Data() const { return Buffer()->Data(); }
void* Data() { return Buffer()->Data(); }
unsigned ByteLength() const { return Buffer()->ByteLength(); }
unsigned ByteLength() const { return Buffer()->ByteLengthAsUnsigned(); }
bool IsDetached() const { return Buffer()->IsDetached(); }
bool IsShared() const { return Buffer()->IsShared(); }
......
......@@ -20,7 +20,7 @@ class DataView final : public ArrayBufferView {
unsigned byte_length) {
base::CheckedNumeric<uint32_t> checked_max = byte_offset;
checked_max += byte_length;
CHECK_LE(checked_max.ValueOrDie(), buffer->ByteLength());
CHECK_LE(checked_max.ValueOrDie(), buffer->ByteLengthAsUnsigned());
return base::AdoptRef(new DataView(buffer, byte_offset, byte_length));
}
......
......@@ -107,7 +107,7 @@ bool ConvertBufferSource(const ArrayBufferOrArrayBufferView& buffer_source,
}
vector->Append(static_cast<uint8_t*>(array_buffer->Data()),
array_buffer->ByteLength());
array_buffer->ByteLengthAsUnsigned());
} else {
ArrayBufferView* view = buffer_source.GetAsArrayBufferView().View()->View();
if (!view->Buffer() || view->Buffer()->IsDetached()) {
......
......@@ -210,6 +210,8 @@ crbug.com/730267 virtual/gpu-rasterization/images/yuv-decode-eligible/color-prof
crbug.com/730267 virtual/gpu-rasterization-disable-yuv/images/yuv-decode-eligible/color-profile-layer.html [ Pass Timeout Failure ]
crbug.com/730267 virtual/gpu-rasterization-disable-yuv/images/yuv-decode-eligible/color-profile-layer-filter.html [ Pass Timeout Failure ]
crbug.com/1022460 [ Win ] storage/indexeddb/key-type-array.html [ Pass Crash ]
# Flaky virtual/threaded/fast/scrolling tests
crbug.com/841567 virtual/threaded-prefer-compositing/fast/scrolling/absolute-position-behind-scrollbar.html [ Failure Pass ]
crbug.com/841567 virtual/threaded-prefer-compositing/fast/scrolling/fixed-position-behind-scrollbar.html [ Failure Pass ]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment