Commit 9c5623c7 authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

Fix data race in array sorting

For copying the SharedArrayBuffer content, we cannot use a simple
{memcpy} because that produces data races with thread concurrently
modifying the content. Instead, use a custom {Relaxed_Memcpy} that uses
proper relaxed atomics. The implementation is slightly optimized to do
word-sized loads and stores where possible. If we still get performance
regressions, we can optimize it further in follow-up CLs.

R=ulan@chromium.org
CC=mlippautz@chromium.org

Bug: v8:11704, chromium:1205290
Change-Id: Ie34afc5c22ec5496c0fe822d55d4788031f06c54
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2874652
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74403}
parent 63b7afb4
......@@ -294,6 +294,32 @@ inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) {
}
#endif // defined(V8_HOST_ARCH_64_BIT)
inline void Relaxed_Memcpy(volatile Atomic8* dst, volatile const Atomic8* src,
size_t bytes) {
constexpr size_t kAtomicWordSize = sizeof(AtomicWord);
while (bytes > 0 &&
!IsAligned(reinterpret_cast<uintptr_t>(dst), kAtomicWordSize)) {
Relaxed_Store(dst++, Relaxed_Load(src++));
--bytes;
}
if (IsAligned(reinterpret_cast<uintptr_t>(src), kAtomicWordSize)) {
DCHECK(IsAligned(reinterpret_cast<uintptr_t>(dst), kAtomicWordSize));
while (bytes >= kAtomicWordSize) {
Relaxed_Store(
reinterpret_cast<volatile AtomicWord*>(dst),
Relaxed_Load(reinterpret_cast<const volatile AtomicWord*>(src)));
dst += kAtomicWordSize;
src += kAtomicWordSize;
bytes -= kAtomicWordSize;
}
}
while (bytes > 0) {
Relaxed_Store(dst++, Relaxed_Load(src++));
--bytes;
}
}
} // namespace base
} // namespace v8
......
......@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "src/base/atomicops.h"
#include "src/common/message-template.h"
#include "src/execution/arguments-inl.h"
#include "src/heap/factory.h"
......@@ -108,7 +109,8 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
offheap_copy.resize(bytes);
data_copy_ptr = &offheap_copy[0];
}
std::memcpy(data_copy_ptr, static_cast<void*>(array->DataPtr()), bytes);
base::Relaxed_Memcpy(static_cast<base::Atomic8*>(data_copy_ptr),
static_cast<base::Atomic8*>(array->DataPtr()), bytes);
}
DisallowGarbageCollection no_gc;
......@@ -147,7 +149,8 @@ RUNTIME_FUNCTION(Runtime_TypedArraySortFast) {
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()), data_copy_ptr, bytes);
base::Relaxed_Memcpy(static_cast<base::Atomic8*>(array->DataPtr()),
static_cast<base::Atomic8*>(data_copy_ptr), bytes);
}
return *array;
......
// Copyright 2021 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.
const sync_arr = new Int32Array(new SharedArrayBuffer(4));
function waitForWorker() {
while (Atomics.load(sync_arr) == 0) {}
}
function onmessage([sab, lock]) {
const i32a = new Int32Array(sab);
Atomics.store(lock, 0, 1);
for (let j = 1; j < 5000; ++j) {
for (let i = 0; i < i32a.length; ++i) {
i32a[i] = j;
}
}
}
const worker = new Worker(`onmessage = ${onmessage}`, {type: 'string'});
const arr = new Int32Array(new SharedArrayBuffer(Int32Array.BYTES_PER_ELEMENT * 2000));
worker.postMessage([arr.buffer, sync_arr]);
waitForWorker();
arr.sort();
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