[elements] Avoid racy data reads/writes

Instead of annotating those racy reads / writes to be ignore by TSan,
just use relaxed atomics. This makes us not rely on undefined behaviour,
and is unlikely to introduce noticeable overhead.

This removes the only uses of TSAN_ANNOTATE_IGNORE_WRITES_BEGIN and
friends, which allows us to remove the whole tsan.h header.

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

Bug: v8:11704
Change-Id: Ie6694c0ae5b40856b56fb97253ce626ec1f4c263
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2859957Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74321}

BUILD.gn

@@ -3003,7 +3003,6 @@ v8_header_set("v8_internal_headers") {
     "src/roots/roots.h",
     "src/runtime/runtime-utils.h",
     "src/runtime/runtime.h",
-    "src/sanitizer/tsan.h",
     "src/snapshot/code-serializer.h",
     "src/snapshot/context-deserializer.h",
     "src/snapshot/context-serializer.h",

src/objects/elements.cc

@@ -3003,25 +3003,23 @@ class TypedElementsAccessor
 
   static void SetImpl(ElementType* data_ptr, size_t entry, ElementType value) {
     // The JavaScript memory model allows for racy reads and writes to a
-    // SharedArrayBuffer's backing store. ThreadSanitizer will catch these
-    // racy accesses and warn about them, so we disable TSAN for these reads
-    // and writes using annotations.
-    //
-    // We don't use relaxed atomics here, as it is not a requirement of the
-    // JavaScript memory model to have tear-free reads of overlapping accesses,
-    // and using relaxed atomics may introduce overhead.
-    TSAN_ANNOTATE_IGNORE_WRITES_BEGIN;
+    // SharedArrayBuffer's backing store. Using relaxed atomics is not strictly
+    // required for JavaScript, but will avoid undefined behaviour in C++ and is
+    // unlikely to introduce noticable overhead.
     if (COMPRESS_POINTERS_BOOL && alignof(ElementType) > kTaggedSize) {
-      // TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
-      // fields (external pointers, doubles and BigInt data) are only
-      // kTaggedSize aligned so we have to use unaligned pointer friendly way of
-      // accessing them in order to avoid undefined behavior in C++ code.
-      base::WriteUnalignedValue<ElementType>(
-          reinterpret_cast<Address>(data_ptr + entry), value);
+      uint32_t words[2];
+      CHECK_EQ(sizeof(words), sizeof(value));
+      memcpy(words, &value, sizeof(value));
+      for (size_t word : {0, 1}) {
+        STATIC_ASSERT(sizeof(std::atomic<uint32_t>) == sizeof(uint32_t));
+        reinterpret_cast<std::atomic<uint32_t>*>(data_ptr + entry)[word].store(
+            words[word], std::memory_order_relaxed);
+      }
     } else {
-      data_ptr[entry] = value;
+      STATIC_ASSERT(sizeof(std::atomic<ElementType>) == sizeof(ElementType));
+      reinterpret_cast<std::atomic<ElementType>*>(data_ptr + entry)
+          ->store(value, std::memory_order_relaxed);
     }
-    TSAN_ANNOTATE_IGNORE_WRITES_END;
   }
 
   static Handle<Object> GetInternalImpl(Handle<JSObject> holder,
@@ -3042,26 +3040,25 @@ class TypedElementsAccessor
 
   static ElementType GetImpl(ElementType* data_ptr, size_t entry) {
     // The JavaScript memory model allows for racy reads and writes to a
-    // SharedArrayBuffer's backing store. ThreadSanitizer will catch these
-    // racy accesses and warn about them, so we disable TSAN for these reads
-    // and writes using annotations.
-    //
-    // We don't use relaxed atomics here, as it is not a requirement of the
-    // JavaScript memory model to have tear-free reads of overlapping accesses,
-    // and using relaxed atomics may introduce overhead.
-    TSAN_ANNOTATE_IGNORE_READS_BEGIN;
+    // SharedArrayBuffer's backing store. Using relaxed atomics is not strictly
+    // required for JavaScript, but will avoid undefined behaviour in C++ and is
+    // unlikely to introduce noticable overhead.
     ElementType result;
     if (COMPRESS_POINTERS_BOOL && alignof(ElementType) > kTaggedSize) {
-      // TODO(ishell, v8:8875): When pointer compression is enabled 8-byte size
-      // fields (external pointers, doubles and BigInt data) are only
-      // kTaggedSize aligned so we have to use unaligned pointer friendly way of
-      // accessing them in order to avoid undefined behavior in C++ code.
-      result = base::ReadUnalignedValue<ElementType>(
-          reinterpret_cast<Address>(data_ptr + entry));
+      uint32_t words[2];
+      for (size_t word : {0, 1}) {
+        STATIC_ASSERT(sizeof(std::atomic<uint32_t>) == sizeof(uint32_t));
+        words[word] =
+            reinterpret_cast<std::atomic<uint32_t>*>(data_ptr + entry)[word]
+                .load(std::memory_order_relaxed);
+      }
+      CHECK_EQ(sizeof(words), sizeof(result));
+      memcpy(&result, words, sizeof(result));
     } else {
-      result = data_ptr[entry];
+      STATIC_ASSERT(sizeof(std::atomic<ElementType>) == sizeof(ElementType));
+      result = reinterpret_cast<std::atomic<ElementType>*>(data_ptr + entry)
+                   ->load(std::memory_order_relaxed);
     }
-    TSAN_ANNOTATE_IGNORE_READS_END;
     return result;
   }
 

src/objects/fixed-array-inl.h

@@ -19,7 +19,6 @@
 #include "src/objects/oddball.h"
 #include "src/objects/slots.h"
 #include "src/roots/roots-inl.h"
-#include "src/sanitizer/tsan.h"
 
 // Has to be the last include (doesn't have include guards):
 #include "src/objects/object-macros.h"

src/objects/objects-inl.h

@@ -42,7 +42,6 @@
 #include "src/objects/tagged-impl-inl.h"
 #include "src/objects/tagged-index.h"
 #include "src/objects/templates.h"
-#include "src/sanitizer/tsan.h"
 
 // Has to be the last include (doesn't have include guards):
 #include "src/objects/object-macros.h"

src/sanitizer/tsan.h

-// Copyright 2017 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.
-
-#ifndef V8_SANITIZER_TSAN_H_
-#define V8_SANITIZER_TSAN_H_
-
-namespace v8 {
-namespace base {
-
-// This file contains annotations for ThreadSanitizer (TSan), a race detector.
-// See
-// https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interface_ann.cc
-
-#if THREAD_SANITIZER
-
-#define TSAN_ANNOTATE_IGNORE_READS_BEGIN \
-  v8::base::AnnotateIgnoreReadsBegin(__FILE__, __LINE__)
-#define TSAN_ANNOTATE_IGNORE_READS_END \
-  v8::base::AnnotateIgnoreReadsEnd(__FILE__, __LINE__)
-#define TSAN_ANNOTATE_IGNORE_WRITES_BEGIN \
-  v8::base::AnnotateIgnoreWritesBegin(__FILE__, __LINE__)
-#define TSAN_ANNOTATE_IGNORE_WRITES_END \
-  v8::base::AnnotateIgnoreWritesEnd(__FILE__, __LINE__)
-
-extern "C" {
-
-void AnnotateIgnoreReadsBegin(const char* file, int line);
-void AnnotateIgnoreReadsEnd(const char* file, int line);
-void AnnotateIgnoreWritesBegin(const char* file, int line);
-void AnnotateIgnoreWritesEnd(const char* file, int line);
-
-}  // extern "C"
-
-#else
-
-#define TSAN_ANNOTATE_IGNORE_READS_BEGIN ((void)0)
-#define TSAN_ANNOTATE_IGNORE_READS_END ((void)0)
-#define TSAN_ANNOTATE_IGNORE_WRITES_BEGIN ((void)0)
-#define TSAN_ANNOTATE_IGNORE_WRITES_END ((void)0)
-
-#endif
-
-}  // namespace base
-}  // namespace v8
-
-#endif  // V8_SANITIZER_TSAN_H_