[typedarray] Fix crash when sorting SharedArrayBuffers

TypedArray#sort has a fast-path when the user does not provide a
comparison function. This fast-path utilizes std::sort which operates
directly on the raw data. Per spec, std::sort requires the "less than"
operation to be anti-symmetric and transitive.

When sorting SharedArrayBuffers (SAB) that are concurrently modified during
sorting, the "less than" operator stops being consistent as the
underlying data is constantly modified. This breaks some invariants
in std::sort resulting in infinite loops or straight out segfaults.

This CL fixes this by copying the data before sorting SABs and
writing the sorted result back.

Note: The added regression test is tailored for ASAN bots as a
normal build would need too many iterations to consistently crash.

R=neis@chromium.org, petermarshall@chromium.org

Bug: v8:9161
Change-Id: Ic089928652f75865bfdb11e7453806faa6ecb988
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1581641
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61004}

src/runtime/runtime-typedarray.cc

@@ -101,26 +101,43 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
 
-  CONVERT_ARG_HANDLE_CHECKED(Object, target_obj, 0);
-
-  Handle<JSTypedArray> array;
-  const char* method = "%TypedArray%.prototype.sort";
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, array, JSTypedArray::Validate(isolate, target_obj, method));
-
-  // This line can be removed when JSTypedArray::Validate throws
-  // if array.[[ViewedArrayBuffer]] is detached(v8:4648)
-  if (V8_UNLIKELY(array->WasDetached())) return *array;
+  // Validation is handled in the Torque builtin.
+  CONVERT_ARG_HANDLE_CHECKED(JSTypedArray, array, 0);
+  DCHECK(!array->WasDetached());
 
   size_t length = array->length();
   if (length <= 1) return *array;
 
   Handle<FixedTypedArrayBase> elements(
       FixedTypedArrayBase::cast(array->elements()), isolate);
+
+  // In case of a SAB, the data is copied into temporary memory, as
+  // std::sort might crash in case the underlying data is concurrently
+  // modified while sorting.
+  CHECK(array->buffer()->IsJSArrayBuffer());
+  Handle<JSArrayBuffer> buffer(JSArrayBuffer::cast(array->buffer()), isolate);
+  const bool copy_data = buffer->is_shared();
+
+  Handle<ByteArray> array_copy;
+  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*>(elements->DataPtr()), bytes);
+  }
+
+  DisallowHeapAllocation no_gc;
+
   switch (array->type()) {
 #define TYPED_ARRAY_SORT(Type, type, TYPE, ctype)                          \
   case kExternal##Type##Array: {                                           \
-    ctype* data = static_cast<ctype*>(elements->DataPtr());                \
+    ctype* data =                                                          \
+        copy_data                                                          \
+            ? reinterpret_cast<ctype*>(array_copy->GetDataStartAddress())  \
+            : static_cast<ctype*>(elements->DataPtr());                    \
     if (kExternal##Type##Array == kExternalFloat64Array ||                 \
         kExternal##Type##Array == kExternalFloat32Array) {                 \
       if (COMPRESS_POINTERS_BOOL && alignof(ctype) > kTaggedSize) {        \
@@ -146,6 +163,13 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
 #undef TYPED_ARRAY_SORT
   }
 
+  if (copy_data) {
+    DCHECK(!array_copy.is_null());
+    const size_t bytes = array->byte_length();
+    std::memcpy(static_cast<void*>(elements->DataPtr()),
+                static_cast<void*>(array_copy->GetDataStartAddress()), bytes);
+  }
+
   return *array;
 }
 

test/mjsunit/mjsunit.status

@@ -175,6 +175,7 @@
   'regress/regress-605470': [PASS, SLOW],
   'regress/regress-655573': [PASS, SLOW],
   'regress/regress-1200351': [PASS, SLOW],
+  'regress/regress-crbug-9161': [PASS, SLOW],
   'regress/wasm/regress-810973': [PASS, SLOW],
   'string-replace-gc': [PASS, SLOW],
   'wasm/asm-wasm-f32': [PASS, SLOW],
@@ -435,6 +436,7 @@
   'regress/regress-91010': [SKIP],
   'regress/regress-91013': [SKIP],
   'regress/regress-99167': [SKIP],
+  'regress/regress-crbug-9161': [SKIP],
 
   # BUG(v8:3457).
   'deserialize-reference': [PASS, FAIL],
@@ -578,6 +580,7 @@
   'big-object-literal': [SKIP],
   'compiler/alloc-number': [SKIP],
   'regress/regress-490': [SKIP],
+  'regress/regress-crbug-9161': [SKIP],
   'regress/regress-create-exception': [SKIP],
   'regress/regress-3247124': [SKIP],
 

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

@@ -0,0 +1,59 @@
+// 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.
+//
+// This test is a reproduction of a crash that happens when a TypedArray
+// backed by a SharedArrayBuffer is concurrently modified while sorting.
+// Segfaults would need a long time to trigger in normal builds, so this
+// reproduction is tailored to trigger on ASAN builds. On ASAN builds,
+// out-of-bounds accesses while sorting would result in an immediate failure.
+
+const lock = new Int32Array(new SharedArrayBuffer(4));
+
+const kIterations = 5000;
+const kLength = 2000;
+
+const kStageIndex = 0;
+const kStageInit = 0;
+const kStageRunning = 1;
+const kStageDone = 2;
+
+Atomics.store(lock, kStageIndex, kStageInit);
+
+function WaitUntil(expected) {
+  while (true) {
+    const value = Atomics.load(lock, kStageIndex);
+    if (value === expected) break;
+  }
+}
+
+const workerScript = `
+  onmessage = function([sab, lock]) {
+    const i32a = new Int32Array(sab);
+    Atomics.store(lock, ${kStageIndex}, ${kStageRunning});
+
+    for (let j = 1; j < ${kIterations}; ++j) {
+      for (let i = 0; i < i32a.length; ++i) {
+        i32a[i] = j;
+      }
+    }
+
+    postMessage("done");
+    Atomics.store(lock, ${kStageIndex}, ${kStageDone});
+  };`;
+
+const worker = new Worker(workerScript, {type: 'string'});
+
+const i32a = new Int32Array(
+  new SharedArrayBuffer(Int32Array.BYTES_PER_ELEMENT * kLength)
+);
+
+worker.postMessage([i32a.buffer, lock]);
+WaitUntil(kStageRunning);
+
+for (let i = 0; i < kIterations; ++i) {
+  i32a.sort();
+}
+
+WaitUntil(kStageDone);
+assertEquals(worker.getMessage(), "done");