From 3656b4656ed3b4c4f424f77e5b43fdebbd77b3c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9otime=20Grohens?= Date: Thu, 2 Aug 2018 16:41:58 +0200 Subject: [PATCH] [dataview] Fix too tight TNode type in DataView getters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL fixes a bug found by Clusterfuzz, in which the functions LoadDataViewByteOffset and -ByteLength incorrectly had a return type of TNode instead of TNode. This caused a CAST() call to fail when the requested byte offset or byte length did not fit inside a Smi, i.e. when the underlying ArrayBuffer of the DataView had a length longer than 2^30 on 32-bit platforms. The CL also includes a new test in mjsunit to test against this. Bug: chromium:869313 Change-Id: Ibb7d29bda5782a12c4b506c070bb03fef8c3ec70 Reviewed-on: https://chromium-review.googlesource.com/1158582 Commit-Queue: Théotime Grohens Reviewed-by: Tobias Tebbi Cr-Commit-Position: refs/heads/master@{#54900} --- src/builtins/base.tq | 25 +++++++++ src/builtins/builtins-data-view-gen.h | 4 +- src/builtins/data-view.tq | 54 ++++++++++++-------- test/mjsunit/regress/regress-crbug-869313.js | 15 ++++++ 4 files changed, 74 insertions(+), 24 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-869313.js diff --git a/src/builtins/base.tq b/src/builtins/base.tq index 53cf381d8f..054fe8e8d1 100644 --- a/src/builtins/base.tq +++ b/src/builtins/base.tq @@ -66,6 +66,8 @@ type RootListIndex generates 'TNode' constexpr 'Heap::RootListIndex'; type MessageTemplate constexpr 'MessageTemplate'; type HasPropertyLookupMode constexpr 'HasPropertyLookupMode'; +type ToIntegerTruncationMode constexpr 'ToIntegerTruncationMode'; + const NO_ELEMENTS: constexpr ElementsKind generates 'NO_ELEMENTS'; const PACKED_SMI_ELEMENTS: constexpr ElementsKind generates @@ -124,6 +126,9 @@ const kHasProperty: constexpr HasPropertyLookupMode generates 'kHasProperty'; const kMaxSafeInteger: constexpr float64 generates 'kMaxSafeInteger'; +const kTruncateMinusZero: constexpr ToIntegerTruncationMode generates +'ToIntegerTruncationMode::kTruncateMinusZero'; + const kNotTypedArray: constexpr MessageTemplate generates 'MessageTemplate::kNotTypedArray'; const kDetachedOperation: constexpr MessageTemplate generates @@ -165,6 +170,8 @@ extern macro Print(constexpr string, Object); extern macro Print(Object); extern macro DebugBreak(); extern macro ToInteger_Inline(Context, Object): Number; +extern macro ToInteger_Inline( + Context, Object, constexpr ToIntegerTruncationMode): Number; extern macro ToLength_Inline(Context, Object): Number; extern macro ToNumber_Inline(Context, Object): Number; extern macro ToString_Inline(Context, Object): String; @@ -359,9 +366,11 @@ extern macro SmiTag(intptr): Smi; extern macro SmiFromInt32(int32): Smi; extern macro SmiUntag(Smi): intptr; extern macro SmiToInt32(Smi): int32; +extern macro RoundIntPtrToFloat64(intptr): float64; extern macro LoadHeapNumberValue(HeapNumber): float64; extern macro ChangeFloat32ToFloat64(float32): float64; extern macro ChangeNumberToFloat64(Number): float64; +extern macro ChangeFloat64ToUintPtr(float64): uintptr; extern macro ChangeInt32ToIntPtr(int32): intptr; // Sign-extends. extern macro ChangeUint32ToWord(uint32): uintptr; // Doesn't sign-extend. @@ -494,6 +503,9 @@ macro convert(d: float64): A; convert(d: float64): Number { return AllocateHeapNumberWithValue(d); } +convert(d: float64): uintptr { + return ChangeFloat64ToUintPtr(d); +} macro convert(r: RawPtr): A; convert(r: RawPtr): uintptr { return Unsigned(r); @@ -732,3 +744,16 @@ macro ToBoolean(obj: Object): bool { return false; } } + +macro ToIndex(input: Object, context: Context): Number labels RangeError { + if (input == Undefined) { + return 0; + } + + let value: Number = ToInteger_Inline(context, input, kTruncateMinusZero); + if (value < 0 || value > kMaxSafeInteger) { + goto RangeError; + } + + return value; +} diff --git a/src/builtins/builtins-data-view-gen.h b/src/builtins/builtins-data-view-gen.h index cf4809efdd..6c755c4d08 100644 --- a/src/builtins/builtins-data-view-gen.h +++ b/src/builtins/builtins-data-view-gen.h @@ -17,11 +17,11 @@ class DataViewBuiltinsAssembler : public BaseBuiltinsFromDSLAssembler { explicit DataViewBuiltinsAssembler(compiler::CodeAssemblerState* state) : BaseBuiltinsFromDSLAssembler(state) {} - TNode LoadDataViewByteOffset(TNode data_view) { + TNode LoadDataViewByteOffset(TNode data_view) { return CAST(LoadObjectField(data_view, JSDataView::kByteOffsetOffset)); } - TNode LoadDataViewByteLength(TNode data_view) { + TNode LoadDataViewByteLength(TNode data_view) { return CAST(LoadObjectField(data_view, JSDataView::kByteLengthOffset)); } diff --git a/src/builtins/data-view.tq b/src/builtins/data-view.tq index 3200d5f30b..874c122995 100644 --- a/src/builtins/data-view.tq +++ b/src/builtins/data-view.tq @@ -4,12 +4,14 @@ module data_view { - extern operator '.buffer' macro LoadArrayBufferViewBuffer( - JSArrayBufferView): JSArrayBuffer; - extern operator '.byte_length' macro LoadDataViewByteLength(JSDataView): Smi; - extern operator '.byte_offset' macro LoadDataViewByteOffset(JSDataView): Smi; - extern operator '.backing_store' macro LoadArrayBufferBackingStore( - JSArrayBuffer): RawPtr; + extern operator '.buffer' + macro LoadArrayBufferViewBuffer(JSArrayBufferView): JSArrayBuffer; + extern operator '.byte_length' + macro LoadDataViewByteLength(JSDataView): Number; + extern operator '.byte_offset' + macro LoadDataViewByteOffset(JSDataView): Number; + extern operator '.backing_store' + macro LoadArrayBufferBackingStore(JSArrayBuffer): RawPtr; macro MakeDataViewGetterNameString(kind: constexpr ElementsKind): String { if constexpr (kind == UINT8_ELEMENTS) { @@ -87,7 +89,7 @@ module data_view { // ES6 section 24.2.4.2 get DataView.prototype.byteLength javascript builtin DataViewPrototypeGetByteLength( - context: Context, receiver: Object, ...arguments): Smi { + context: Context, receiver: Object, ...arguments): Number { let data_view: JSDataView = ValidateDataView( context, receiver, 'get DataView.prototype.byte_length'); if (WasNeutered(data_view)) { @@ -100,7 +102,7 @@ module data_view { // ES6 section 24.2.4.3 get DataView.prototype.byteOffset javascript builtin DataViewPrototypeGetByteOffset( - context: Context, receiver: Object, ...arguments): Smi { + context: Context, receiver: Object, ...arguments): Number { let data_view: JSDataView = ValidateDataView( context, receiver, 'get DataView.prototype.byte_offset'); if (WasNeutered(data_view)) { @@ -396,14 +398,13 @@ module data_view { let data_view: JSDataView = ValidateDataView( context, receiver, MakeDataViewGetterNameString(kind)); - let getIndexSmi: Smi; + let getIndex: Number; try { - getIndexSmi = ToSmiIndex(offset, context) otherwise RangeError; + getIndex = ToIndex(offset, context) otherwise RangeError; } label RangeError { ThrowRangeError(context, kInvalidDataViewAccessorOffset); } - let getIndex: intptr = convert(getIndexSmi); let littleEndian: bool = ToBoolean(requested_little_endian); let buffer: JSArrayBuffer = data_view.buffer; @@ -413,15 +414,20 @@ module data_view { MakeDataViewGetterNameString(kind)); } - let viewOffset: intptr = convert(data_view.byte_offset); - let viewSize: intptr = convert(data_view.byte_length); - let elementSize: intptr = DataViewElementSize(kind); + let viewOffset: Number = data_view.byte_offset; + let viewSize: Number = data_view.byte_length; + let elementSize: Number = DataViewElementSize(kind); if (getIndex + elementSize > viewSize) { ThrowRangeError(context, kInvalidDataViewAccessorOffset); } - let bufferIndex: intptr = getIndex + viewOffset; + let getIndexFloat: float64 = convert(getIndex); + let getIndexIntptr: intptr = Signed(convert(getIndexFloat)); + let viewOffsetFloat: float64 = convert(viewOffset); + let viewOffsetIntptr: intptr = Signed(convert(viewOffsetFloat)); + + let bufferIndex: intptr = getIndexIntptr + viewOffsetIntptr; if constexpr (kind == UINT8_ELEMENTS) { return LoadDataView8(buffer, bufferIndex, false); @@ -704,14 +710,13 @@ module data_view { let data_view: JSDataView = ValidateDataView( context, receiver, MakeDataViewSetterNameString(kind)); - let getIndexSmi: Smi; + let getIndex: Number; try { - getIndexSmi = ToSmiIndex(offset, context) otherwise RangeError; + getIndex = ToIndex(offset, context) otherwise RangeError; } label RangeError { ThrowRangeError(context, kInvalidDataViewAccessorOffset); } - let getIndex: intptr = convert(getIndexSmi); let littleEndian: bool = ToBoolean(requested_little_endian); let buffer: JSArrayBuffer = data_view.buffer; @@ -731,15 +736,20 @@ module data_view { MakeDataViewSetterNameString(kind)); } - let viewOffset: intptr = convert(data_view.byte_offset); - let viewSize: intptr = convert(data_view.byte_length); - let elementSize: intptr = DataViewElementSize(kind); + let viewOffset: Number = data_view.byte_offset; + let viewSize: Number = data_view.byte_length; + let elementSize: Number = DataViewElementSize(kind); if (getIndex + elementSize > viewSize) { ThrowRangeError(context, kInvalidDataViewAccessorOffset); } - let bufferIndex: intptr = getIndex + viewOffset; + let getIndexFloat: float64 = convert(getIndex); + let getIndexIntptr: intptr = Signed(convert(getIndexFloat)); + let viewOffsetFloat: float64 = convert(viewOffset); + let viewOffsetIntptr: intptr = Signed(convert(viewOffsetFloat)); + + let bufferIndex: intptr = getIndexIntptr + viewOffsetIntptr; if constexpr (kind == BIGUINT64_ELEMENTS || kind == BIGINT64_ELEMENTS) { StoreDataViewBigInt(buffer, bufferIndex, bigint_value, diff --git a/test/mjsunit/regress/regress-crbug-869313.js b/test/mjsunit/regress/regress-crbug-869313.js new file mode 100644 index 0000000000..1b4f9c5445 --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-869313.js @@ -0,0 +1,15 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +function f() { + try { + var a = new ArrayBuffer(1073741824); + var d = new DataView(a); + return d.getUint8() === 0; + } catch(e) { + return true; + } +} + +!f();