[heap] Old-to-new pointer updates need atomic accessors.

This CL also introduces a NoBarrierAtomicValue with NoBarrier accessors.

BUG=chromium:648568

Review-Url: https://codereview.chromium.org/2408233004
Cr-Commit-Position: refs/heads/master@{#40213}

src/base/atomic-utils.h

@@ -51,6 +51,53 @@ class AtomicNumber {
   base::AtomicWord value_;
 };
 
+// This type uses no barrier accessors to change atomic word. Be careful with
+// data races.
+template <typename T>
+class NoBarrierAtomicValue {
+ public:
+  NoBarrierAtomicValue() : value_(0) {}
+
+  explicit NoBarrierAtomicValue(T initial)
+      : value_(cast_helper<T>::to_storage_type(initial)) {}
+
+  static NoBarrierAtomicValue* FromAddress(void* address) {
+    return reinterpret_cast<base::NoBarrierAtomicValue<T>*>(address);
+  }
+
+  V8_INLINE T Value() const {
+    return cast_helper<T>::to_return_type(base::NoBarrier_Load(&value_));
+  }
+
+  V8_INLINE void SetValue(T new_value) {
+    base::NoBarrier_Store(&value_, cast_helper<T>::to_storage_type(new_value));
+  }
+
+ private:
+  STATIC_ASSERT(sizeof(T) <= sizeof(base::AtomicWord));
+
+  template <typename S>
+  struct cast_helper {
+    static base::AtomicWord to_storage_type(S value) {
+      return static_cast<base::AtomicWord>(value);
+    }
+    static S to_return_type(base::AtomicWord value) {
+      return static_cast<S>(value);
+    }
+  };
+
+  template <typename S>
+  struct cast_helper<S*> {
+    static base::AtomicWord to_storage_type(S* value) {
+      return reinterpret_cast<base::AtomicWord>(value);
+    }
+    static S* to_return_type(base::AtomicWord value) {
+      return reinterpret_cast<S*>(value);
+    }
+  };
+
+  base::AtomicWord value_;
+};
 
 // Flag using T atomically. Also accepts void* as T.
 template <typename T>

src/heap/mark-compact.cc

@@ -3632,34 +3632,36 @@ class PointerUpdateJobTraits {
 
   static SlotCallbackResult CheckAndUpdateOldToNewSlot(Heap* heap,
                                                        Address slot_address) {
-    Object** slot = reinterpret_cast<Object**>(slot_address);
-    if (heap->InFromSpace(*slot)) {
-      HeapObject* heap_object = reinterpret_cast<HeapObject*>(*slot);
+    base::NoBarrierAtomicValue<Object*>* slot =
+        base::NoBarrierAtomicValue<Object*>::FromAddress(slot_address);
+    Object* slot_reference = slot->Value();
+    if (heap->InFromSpace(slot_reference)) {
+      HeapObject* heap_object = reinterpret_cast<HeapObject*>(slot_reference);
       DCHECK(heap_object->IsHeapObject());
       MapWord map_word = heap_object->map_word();
       // There could still be stale pointers in large object space, map space,
       // and old space for pages that have been promoted.
       if (map_word.IsForwardingAddress()) {
         // Update the corresponding slot.
-        *slot = map_word.ToForwardingAddress();
+        slot->SetValue(map_word.ToForwardingAddress());
       }
       // If the object was in from space before and is after executing the
       // callback in to space, the object is still live.
       // Unfortunately, we do not know about the slot. It could be in a
       // just freed free space object.
-      if (heap->InToSpace(*slot)) {
+      if (heap->InToSpace(slot->Value())) {
         return KEEP_SLOT;
       }
-    } else if (heap->InToSpace(*slot)) {
+    } else if (heap->InToSpace(slot_reference)) {
       // Slots can point to "to" space if the page has been moved, or if the
       // slot has been recorded multiple times in the remembered set. Since
       // there is no forwarding information present we need to check the
       // markbits to determine liveness.
-      if (Marking::IsBlack(
-              ObjectMarking::MarkBitFrom(reinterpret_cast<HeapObject*>(*slot))))
+      if (Marking::IsBlack(ObjectMarking::MarkBitFrom(
+              reinterpret_cast<HeapObject*>(slot_reference))))
         return KEEP_SLOT;
     } else {
-      DCHECK(!heap->InNewSpace(*slot));
+      DCHECK(!heap->InNewSpace(slot_reference));
     }
     return REMOVE_SLOT;
   }

test/unittests/base/atomic-utils-unittest.cc

@@ -125,6 +125,40 @@ TEST(AtomicValue, WithVoidStar) {
   EXPECT_EQ(&dummy, a.Value());
 }
 
+TEST(NoBarrierAtomicValue, Initial) {
+  NoBarrierAtomicValue<TestFlag> a(kA);
+  EXPECT_EQ(TestFlag::kA, a.Value());
+}
+
+TEST(NoBarrierAtomicValue, SetValue) {
+  NoBarrierAtomicValue<TestFlag> a(kB);
+  a.SetValue(kC);
+  EXPECT_EQ(TestFlag::kC, a.Value());
+}
+
+TEST(NoBarrierAtomicValue, WithVoidStar) {
+  NoBarrierAtomicValue<void*> a(nullptr);
+  NoBarrierAtomicValue<void*> dummy(nullptr);
+  EXPECT_EQ(nullptr, a.Value());
+  a.SetValue(&a);
+  EXPECT_EQ(&a, a.Value());
+}
+
+TEST(NoBarrierAtomicValue, Construction) {
+  NoBarrierAtomicValue<TestFlag> a(kA);
+  TestFlag b = kA;
+  NoBarrierAtomicValue<TestFlag>* ptr =
+      NoBarrierAtomicValue<TestFlag>::FromAddress(&b);
+  EXPECT_EQ(ptr->Value(), a.Value());
+}
+
+TEST(NoBarrierAtomicValue, ConstructionVoidStar) {
+  NoBarrierAtomicValue<void*> a(nullptr);
+  void* b = nullptr;
+  NoBarrierAtomicValue<void*>* ptr =
+      NoBarrierAtomicValue<void*>::FromAddress(&b);
+  EXPECT_EQ(ptr->Value(), a.Value());
+}
 
 namespace {