[builtins] Fix sorting of huge shared TypedArrays

isheludko · Commit Bot · commit bc9e4675d0dc · 2019-11-14T21:03:08.000Z
Bug: v8:4153, chromium:1024099 Change-Id: Ia7a53c710ad2e2abcfa6fbc4ea1b2229b8690308 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1914564 Commit-Queue: Igor Sheludko <ishell@chromium.org> Reviewed-by: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#64969}
diff --git a/src/builtins/typed-array-sort.tq b/src/builtins/typed-array-sort.tq
@@ -104,17 +104,17 @@ namespace typed_array {
     const array: JSTypedArray =
         ValidateTypedArray(context, obj, kBuiltinNameSort);
 
-    // Default sorting is done in C++ using std::sort
-    if (comparefnObj == Undefined) {
-      return TypedArraySortFast(context, obj);
-    }
-
     // 4. Let len be obj.[[ArrayLength]].
     const len: uintptr = array.length;
 
     // Arrays of length 1 or less are considered sorted.
     if (len < 2) return array;
 
+    // Default sorting is done in C++ using std::sort
+    if (comparefnObj == Undefined) {
+      return TypedArraySortFast(context, obj);
+    }
+
     const comparefn: Callable =
         Cast<Callable>(comparefnObj) otherwise unreachable;
     const accessor: TypedArrayAccessor =
diff --git a/src/runtime/runtime-typedarray.cc b/src/runtime/runtime-typedarray.cc
@@ -85,7 +85,7 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
   DCHECK(!array->WasDetached());
 
   size_t length = array->length();
-  if (length <= 1) return *array;
+  DCHECK_LT(1, length);
 
   // In case of a SAB, the data is copied into temporary memory, as
   // std::sort might crash in case the underlying data is concurrently
@@ -95,25 +95,29 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
   const bool copy_data = buffer->is_shared();
 
   Handle<ByteArray> array_copy;
+  std::vector<uint8_t> offheap_copy;
+  void* data_copy_ptr = nullptr;
   if (copy_data) {
     const size_t bytes = array->byte_length();
-    // TODO(szuend): Re-check this approach once support for larger typed
-    //               arrays has landed.
-    CHECK_LE(bytes, INT_MAX);
-    array_copy = isolate->factory()->NewByteArray(static_cast<int>(bytes));
-    std::memcpy(static_cast<void*>(array_copy->GetDataStartAddress()),
-                static_cast<void*>(array->DataPtr()), bytes);
+    if (bytes <= static_cast<unsigned>(
+                     ByteArray::LengthFor(kMaxRegularHeapObjectSize))) {
+      array_copy = isolate->factory()->NewByteArray(static_cast<int>(bytes));
+      data_copy_ptr = array_copy->GetDataStartAddress();
+    } else {
+      // Allocate copy in C++ heap.
+      offheap_copy.resize(bytes);
+      data_copy_ptr = &offheap_copy[0];
+    }
+    std::memcpy(data_copy_ptr, static_cast<void*>(array->DataPtr()), bytes);
   }
 
   DisallowHeapAllocation no_gc;
 
   switch (array->type()) {
 #define TYPED_ARRAY_SORT(Type, type, TYPE, ctype)                          \
   case kExternal##Type##Array: {                                           \
-    ctype* data =                                                          \
-        copy_data                                                          \
-            ? reinterpret_cast<ctype*>(array_copy->GetDataStartAddress())  \
-            : static_cast<ctype*>(array->DataPtr());                       \
+    ctype* data = copy_data ? reinterpret_cast<ctype*>(data_copy_ptr)      \
+                            : static_cast<ctype*>(array->DataPtr());       \
     if (kExternal##Type##Array == kExternalFloat64Array ||                 \
         kExternal##Type##Array == kExternalFloat32Array) {                 \
       if (COMPRESS_POINTERS_BOOL && alignof(ctype) > kTaggedSize) {        \
@@ -140,10 +144,10 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
   }
 
   if (copy_data) {
-    DCHECK(!array_copy.is_null());
+    DCHECK_NOT_NULL(data_copy_ptr);
+    DCHECK_NE(array_copy.is_null(), offheap_copy.empty());
     const size_t bytes = array->byte_length();
-    std::memcpy(static_cast<void*>(array->DataPtr()),
-                static_cast<void*>(array_copy->GetDataStartAddress()), bytes);
+    std::memcpy(static_cast<void*>(array->DataPtr()), data_copy_ptr, bytes);
   }
 
   return *array;
diff --git a/test/mjsunit/regress/regress-crbug-1024099.js b/test/mjsunit/regress/regress-crbug-1024099.js
@@ -0,0 +1,16 @@
+// Copyright 2019 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.
+
+let len = 0x20000;
+let ar = new Int32Array(new SharedArrayBuffer(Int32Array.BYTES_PER_ELEMENT * len));
+ar[7] = -13;
+ar[0x1673] = -42;
+ar[0x1f875] = -153;
+
+ar.sort();
+
+assertEquals(ar[0], -153);
+assertEquals(ar[1], -42);
+assertEquals(ar[2], -13);
+assertEquals(ar[3], 0);
