Skip to content

V
V8
Commit a5941ac9 authored by Michael Achenbach's avatar Michael Achenbach Committed by Commit Bot
Browse files
Options

Revert "[typedarray] Fix crash when sorting SharedArrayBuffers" 

This reverts commit 3d846115.

Reason for revert: The test hangs flakily on windows:
https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/20612
https://ci.chromium.org/p/v8/builders/ci/V8%20Win32%20-%20nosnap%20-%20shared/33147
https://ci.chromium.org/p/v8/builders/ci/V8%20Win32%20-%20debug/19945

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}

TBR=neis@chromium.org,petermarshall@chromium.org,szuend@chromium.org

Change-Id: I046da3e4228bb1a8a3aa89d9c9d8de11875a9273
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:9161
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1583725Reviewed-by: 's avatarMichael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61007}
parent 56f9ed7c
Show whitespace changes
Inline Side-by-side
Showing with 11 additions and 97 deletions
src/runtime/runtime-typedarray.cc
View file @ a5941ac9
......@@ -101,43 +101,26 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
HandleScope scope(isolate);
DCHECK_EQ(1, args.length());
// Validation is handled in the Torque builtin.
CONVERT_ARG_HANDLE_CHECKED(JSTypedArray, array, 0);
DCHECK(!array->WasDetached());
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;
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 = \
copy_data \
? reinterpret_cast<ctype*>(array_copy->GetDataStartAddress()) \
: static_cast<ctype*>(elements->DataPtr()); \
ctype* data = static_cast<ctype*>(elements->DataPtr()); \
if (kExternal##Type##Array == kExternalFloat64Array || \
kExternal##Type##Array == kExternalFloat32Array) { \
if (COMPRESS_POINTERS_BOOL && alignof(ctype) > kTaggedSize) { \
......@@ -163,13 +146,6 @@ 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
View file @ a5941ac9
......@@ -175,7 +175,6 @@
'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],
......@@ -436,7 +435,6 @@
'regress/regress-91010': [SKIP],
'regress/regress-91013': [SKIP],
'regress/regress-99167': [SKIP],
'regress/regress-crbug-9161': [SKIP],
# BUG(v8:3457).
'deserialize-reference': [PASS, FAIL],
......@@ -580,7 +578,6 @@
'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 deleted 100644 → 0
View file @ 56f9ed7c
// 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