Commit ff3a26af authored by Simon Zünd's avatar Simon Zünd Committed by Commit Bot

Reland "[typedarray] Fix crash when sorting SharedArrayBuffers"

This is a reland of 3d846115

Reland changes mjsunit.status to skip the regression test on
all bots except ASAN.

Original change's description:
> [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}

Bug: v8:9161
Change-Id: Idffc3fbb5f28f4966c8f1ac6770d5b5d6003a7e7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1583726Reviewed-by: 's avatarPeter Marshall <petermarshall@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61011}
parent 71845472
...@@ -101,26 +101,43 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) { ...@@ -101,26 +101,43 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
HandleScope scope(isolate); HandleScope scope(isolate);
DCHECK_EQ(1, args.length()); DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(Object, target_obj, 0); // Validation is handled in the Torque builtin.
CONVERT_ARG_HANDLE_CHECKED(JSTypedArray, array, 0);
Handle<JSTypedArray> array; DCHECK(!array->WasDetached());
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;
size_t length = array->length(); size_t length = array->length();
if (length <= 1) return *array; if (length <= 1) return *array;
Handle<FixedTypedArrayBase> elements( Handle<FixedTypedArrayBase> elements(
FixedTypedArrayBase::cast(array->elements()), isolate); 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()) { switch (array->type()) {
#define TYPED_ARRAY_SORT(Type, type, TYPE, ctype) \ #define TYPED_ARRAY_SORT(Type, type, TYPE, ctype) \
case kExternal##Type##Array: { \ 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 || \ if (kExternal##Type##Array == kExternalFloat64Array || \
kExternal##Type##Array == kExternalFloat32Array) { \ kExternal##Type##Array == kExternalFloat32Array) { \
if (COMPRESS_POINTERS_BOOL && alignof(ctype) > kTaggedSize) { \ if (COMPRESS_POINTERS_BOOL && alignof(ctype) > kTaggedSize) { \
...@@ -146,6 +163,13 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) { ...@@ -146,6 +163,13 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
#undef TYPED_ARRAY_SORT #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; return *array;
} }
......
...@@ -235,6 +235,9 @@ ...@@ -235,6 +235,9 @@
# BUG(v8:8169) # BUG(v8:8169)
'external-backing-store-gc': [SKIP], 'external-backing-store-gc': [SKIP],
# Test is only enabled on ASAN. Takes too long on many other bots.
'regress/regress-crbug-9161': [SKIP],
}], # ALWAYS }], # ALWAYS
['novfp3 == True', { ['novfp3 == True', {
...@@ -515,6 +518,9 @@ ...@@ -515,6 +518,9 @@
# https://bugs.chromium.org/p/v8/issues/detail?id=7102 # https://bugs.chromium.org/p/v8/issues/detail?id=7102
# Flaky due to huge string allocation. # Flaky due to huge string allocation.
'regress/regress-748069': [SKIP], 'regress/regress-748069': [SKIP],
# Test is tailored for ASAN. Takes too long on many other bots.
'regress/regress-crbug-9161': [PASS, SLOW],
}], # 'asan == True' }], # 'asan == True'
############################################################################## ##############################################################################
......
// 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");
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