[dataview] Fix too tight TNode type in DataView getters

This CL fixes a bug found by Clusterfuzz, in which the functions
LoadDataViewByteOffset and -ByteLength incorrectly had a return
type of TNode<Smi> instead of TNode<Number>.

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 <theotime@google.com>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54900}

src/builtins/base.tq

@@ -66,6 +66,8 @@ type RootListIndex generates 'TNode<Int32T>' 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<A : type>(d: float64): A;
 convert<Number>(d: float64): Number {
   return AllocateHeapNumberWithValue(d);
 }
+convert<uintptr>(d: float64): uintptr {
+  return ChangeFloat64ToUintPtr(d);
+}
 macro convert<A : type>(r: RawPtr): A;
 convert<uintptr>(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;
+}

src/builtins/builtins-data-view-gen.h

@@ -17,11 +17,11 @@ class DataViewBuiltinsAssembler : public BaseBuiltinsFromDSLAssembler {
   explicit DataViewBuiltinsAssembler(compiler::CodeAssemblerState* state)
       : BaseBuiltinsFromDSLAssembler(state) {}
 
-  TNode<Smi> LoadDataViewByteOffset(TNode<JSDataView> data_view) {
+  TNode<Number> LoadDataViewByteOffset(TNode<JSDataView> data_view) {
     return CAST(LoadObjectField(data_view, JSDataView::kByteOffsetOffset));
   }
 
-  TNode<Smi> LoadDataViewByteLength(TNode<JSDataView> data_view) {
+  TNode<Number> LoadDataViewByteLength(TNode<JSDataView> data_view) {
     return CAST(LoadObjectField(data_view, JSDataView::kByteLengthOffset));
   }
 

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<intptr>(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<intptr>(data_view.byte_offset);
-    let viewSize: intptr = convert<intptr>(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<float64>(getIndex);
+    let getIndexIntptr: intptr = Signed(convert<uintptr>(getIndexFloat));
+    let viewOffsetFloat: float64 = convert<float64>(viewOffset);
+    let viewOffsetIntptr: intptr = Signed(convert<uintptr>(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<intptr>(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<intptr>(data_view.byte_offset);
-    let viewSize: intptr = convert<intptr>(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<float64>(getIndex);
+    let getIndexIntptr: intptr = Signed(convert<uintptr>(getIndexFloat));
+    let viewOffsetFloat: float64 = convert<float64>(viewOffset);
+    let viewOffsetIntptr: intptr = Signed(convert<uintptr>(viewOffsetFloat));
+
+    let bufferIndex: intptr = getIndexIntptr + viewOffsetIntptr;
 
     if constexpr (kind == BIGUINT64_ELEMENTS || kind == BIGINT64_ELEMENTS) {
       StoreDataViewBigInt(buffer, bufferIndex, bigint_value,

test/mjsunit/regress/regress-crbug-869313.js

+// 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();