From c37c4e8adfd277097388bd7e27155ca3fe523983 Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Wed, 4 Aug 2021 11:05:45 +0200 Subject: [PATCH] [sab] Make TypedArray#set atomic for SABs Bug: chromium:1232620 Change-Id: Ie19fe8839966a1abb3d0a01fee1fb4b105fb6bf1 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3070702 Commit-Queue: Clemens Backes Auto-Submit: Leszek Swirski Reviewed-by: Clemens Backes Cr-Commit-Position: refs/heads/master@{#76085} --- src/builtins/builtins-typed-array-gen.cc | 11 +++++++++++ src/builtins/builtins-typed-array-gen.h | 3 +++ src/builtins/typed-array-set.tq | 7 ++++++- src/builtins/typed-array.tq | 2 ++ src/codegen/external-reference.cc | 7 +++++++ src/codegen/external-reference.h | 1 + test/mjsunit/regress/regress-1232620.js | 17 +++++++++++++++++ 7 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 test/mjsunit/regress/regress-1232620.js diff --git a/src/builtins/builtins-typed-array-gen.cc b/src/builtins/builtins-typed-array-gen.cc index 99ba35b6ec..a76650d052 100644 --- a/src/builtins/builtins-typed-array-gen.cc +++ b/src/builtins/builtins-typed-array-gen.cc @@ -270,6 +270,17 @@ void TypedArrayBuiltinsAssembler::CallCMemmove(TNode dest_ptr, std::make_pair(MachineType::UintPtr(), byte_length)); } +void TypedArrayBuiltinsAssembler::CallCRelaxedMemmove( + TNode dest_ptr, TNode src_ptr, + TNode byte_length) { + TNode memmove = + ExternalConstant(ExternalReference::relaxed_memmove_function()); + CallCFunction(memmove, MachineType::AnyTagged(), + std::make_pair(MachineType::Pointer(), dest_ptr), + std::make_pair(MachineType::Pointer(), src_ptr), + std::make_pair(MachineType::UintPtr(), byte_length)); +} + void TypedArrayBuiltinsAssembler::CallCMemcpy(TNode dest_ptr, TNode src_ptr, TNode byte_length) { diff --git a/src/builtins/builtins-typed-array-gen.h b/src/builtins/builtins-typed-array-gen.h index 0ec179ac9e..bb8a15ef02 100644 --- a/src/builtins/builtins-typed-array-gen.h +++ b/src/builtins/builtins-typed-array-gen.h @@ -52,6 +52,9 @@ class TypedArrayBuiltinsAssembler : public CodeStubAssembler { void CallCMemmove(TNode dest_ptr, TNode src_ptr, TNode byte_length); + void CallCRelaxedMemmove(TNode dest_ptr, TNode src_ptr, + TNode byte_length); + void CallCMemcpy(TNode dest_ptr, TNode src_ptr, TNode byte_length); diff --git a/src/builtins/typed-array-set.tq b/src/builtins/typed-array-set.tq index b5c9dcb261..f4d2a40f41 100644 --- a/src/builtins/typed-array-set.tq +++ b/src/builtins/typed-array-set.tq @@ -281,7 +281,12 @@ TypedArrayPrototypeSetTypedArray(implicit context: Context, receiver: JSAny)( // value, true, Unordered). // iii. Set srcByteIndex to srcByteIndex + 1. // iv. Set targetByteIndex to targetByteIndex + 1. - CallCMemmove(dstPtr, typedArray.data_ptr, countBytes); + if (IsSharedArrayBuffer(target.buffer)) { + // SABs need a relaxed memmove to preserve atomicity. + CallCRelaxedMemmove(dstPtr, typedArray.data_ptr, countBytes); + } else { + CallCMemmove(dstPtr, typedArray.data_ptr, countBytes); + } } label IfSlow deferred { // 22. If target.[[ContentType]] is not equal to // typedArray.[[ContentType]], throw a TypeError exception. diff --git a/src/builtins/typed-array.tq b/src/builtins/typed-array.tq index 2686005ba5..87bcb2fb59 100644 --- a/src/builtins/typed-array.tq +++ b/src/builtins/typed-array.tq @@ -65,6 +65,8 @@ extern macro TypedArrayBuiltinsAssembler::CallCMemset( RawPtr, intptr, uintptr): void; extern macro TypedArrayBuiltinsAssembler::CallCRelaxedMemcpy( RawPtr, RawPtr, uintptr): void; +extern macro TypedArrayBuiltinsAssembler::CallCRelaxedMemmove( + RawPtr, RawPtr, uintptr): void; extern macro GetTypedArrayBuffer(implicit context: Context)(JSTypedArray): JSArrayBuffer; extern macro TypedArrayBuiltinsAssembler::GetTypedArrayElementsInfo( diff --git a/src/codegen/external-reference.cc b/src/codegen/external-reference.cc index a3ad66827c..158e6add0c 100644 --- a/src/codegen/external-reference.cc +++ b/src/codegen/external-reference.cc @@ -813,6 +813,13 @@ void relaxed_memcpy(volatile base::Atomic8* dest, FUNCTION_REFERENCE(relaxed_memcpy_function, relaxed_memcpy) +void relaxed_memmove(volatile base::Atomic8* dest, + volatile const base::Atomic8* src, size_t n) { + base::Relaxed_Memmove(dest, src, n); +} + +FUNCTION_REFERENCE(relaxed_memmove_function, relaxed_memmove) + ExternalReference ExternalReference::printf_function() { return ExternalReference(Redirect(FUNCTION_ADDR(std::printf))); } diff --git a/src/codegen/external-reference.h b/src/codegen/external-reference.h index 1310ec80a4..dcbb1a41b1 100644 --- a/src/codegen/external-reference.h +++ b/src/codegen/external-reference.h @@ -175,6 +175,7 @@ class StatsCounter; V(libc_memmove_function, "libc_memmove") \ V(libc_memset_function, "libc_memset") \ V(relaxed_memcpy_function, "relaxed_memcpy") \ + V(relaxed_memmove_function, "relaxed_memmove") \ V(mod_two_doubles_operation, "mod_two_doubles") \ V(mutable_big_int_absolute_add_and_canonicalize_function, \ "MutableBigInt_AbsoluteAddAndCanonicalize") \ diff --git a/test/mjsunit/regress/regress-1232620.js b/test/mjsunit/regress/regress-1232620.js new file mode 100644 index 0000000000..3553eb7cc1 --- /dev/null +++ b/test/mjsunit/regress/regress-1232620.js @@ -0,0 +1,17 @@ +// 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. + +// Try to catch TSAN issues with access to SharedArrayBuffer. + +function onmessage([buf]) { + const arr = new Int32Array(buf); + for (let val = 1; val < 100; ++val) arr.fill(val); +} +const arr = new Int32Array(new SharedArrayBuffer(4)); +const worker = new Worker(`onmessage = ${onmessage}`, {type: 'string'}); +worker.postMessage([arr.buffer]); +// Wait until the worker starts filling the array. +while (Atomics.load(arr) == 0) { } +// Try setting a value on the shared array buffer that races with the fill. +arr.set(arr);